wip

crates/analyzer/src/context.rs

@@ -223,7 +223,7 @@ pub enum NamedThing {
     },
     // SelfType // when/if we add a `Self` type keyword
     Variable {
-        name: String,
+        name: SmolStr,
         typ: Result<FixedSize, TypeError>,
         mutability: BindingMutability,
         span: Span,

crates/analyzer/src/db/queries/functions.rs

@@ -5,7 +5,9 @@ use crate::namespace::items::{
     Class, DepGraph, DepGraphWrapper, DepLocality, FunctionId, Item, TypeDef,
 };
 use crate::namespace::scopes::{BlockScope, BlockScopeType, FunctionScope, ItemScope};
-use crate::namespace::types::{self, Contract, CtxDecl, FixedSize, SelfDecl, Struct, Type};
+use crate::namespace::types::{
+    self, Contract, CtxDecl, FixedSize, SelfDecl, SelfDeclKind, Struct, Type,
+};
 use crate::traversal::functions::traverse_statements;
 use crate::traversal::types::type_desc;
 use fe_common::diagnostics::Label;
@@ -56,7 +58,10 @@ pub fn function_signature(
                         "not allowed in functions defined outside of a contract or struct",
                     );
                 } else {
-                    self_decl = Some(if mut_.is_some() { SelfDecl::MutRef } else { SelfDecl::Ref });
+                    self_decl = Some(SelfDecl {
+                        kind: if mut_.is_some() { SelfDeclKind::MutRef } else { SelfDeclKind::Ref },
+                        span: arg.span,
+                    });
                     if index != 0 {
                         scope.error(
                             "`self` is not the first parameter",

crates/analyzer/src/namespace/scopes.rs

@@ -115,6 +115,8 @@ impl<'a> AnalyzerContext for ItemScope<'a> {
     fn resolve_name(&self, name: &str, span: Span) -> Result<Option<NamedThing>, IncompleteItem> {
         let item = self.module.resolve_name(self.db, name)?;
 
+        // TODO: do this somewhere else, so name resolution can be attempted
+        // without emitting an error
         if let Some(item) = item {
             check_item_visibility(self, item, span);
         }
@@ -332,7 +334,7 @@ impl<'a> AnalyzerContext for FunctionScope<'a> {
                     .expect("found param type but not span");
 
                 NamedThing::Variable {
-                    name: name.to_string(),
+                    name: name.into(),
                     typ: param.typ.clone(),
                     mutability: if param.is_mut {
                         BindingMutability::Mutable
@@ -417,7 +419,7 @@ impl AnalyzerContext for BlockScope<'_, '_> {
             self.variable_defs
                 .get(name)
                 .map(|(typ, mutability, span)| NamedThing::Variable {
-                    name: name.to_string(),
+                    name: name.into(),
                     typ: Ok(typ.clone()),
                     mutability: *mutability,
                     span: *span,

crates/analyzer/src/namespace/types.rs

@@ -162,8 +162,13 @@ pub struct FunctionSignature {
     pub return_type: Result<FixedSize, TypeError>,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Ord, Eq, Hash)]
-pub enum SelfDecl {
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub struct SelfDecl {
+    pub kind: SelfDeclKind,
+    pub span: Span,
+}
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub enum SelfDeclKind {
     Ref,
     MutRef,
     // Value

crates/analyzer/src/traversal/assignments.rs

@@ -6,7 +6,8 @@ use crate::traversal::expressions;
 use crate::traversal::utils::add_bin_operations_errors;
 use fe_common::diagnostics::Label;
 use fe_parser::ast as fe;
-use fe_parser::node::Node;
+use fe_parser::node::{Node, Span};
+use smol_str::SmolStr;
 
 /// Gather context information for assignments and check for type errors.
 ///
@@ -70,14 +71,31 @@ pub fn assign(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(), F
     Ok(())
 }
 
+fn find_name_def(scope: &BlockScope, expr: &Node<fe::Expr>) -> Option<(SmolStr, Span)> {
+    match &expr.kind {
+        fe::Expr::Attribute { value, .. } | fe::Expr::Subscript { value, .. } => {
+            find_name_def(scope, &value)
+        }
+        fe::Expr::Name(name) => {
+            let thing = scope.resolve_name(&name, expr.span).ok()??;
+            thing.name_span(scope.db()).map(|span| (name.clone(), span))
+        }
+        _ => None,
+    }
+}
+
 fn check_assign_target(scope: &mut BlockScope, target: &Node<fe::Expr>) -> Result<(), FatalError> {
     use fe::Expr::*;
 
     match &target.kind {
         Attribute { value: container, .. } | Subscript { value: container, .. } => {
             if !scope.expr_is_mutable(&container) {
-                scope.fancy_error("immutable", // XXX better error
-                                  vec![Label::primary(container.span, "")],
+                let mut labels = vec![Label::primary(container.span, "not mutable")];
+                if let Some((name, span)) = find_name_def(scope, container) {
+                    labels.push(Label::secondary(span, &format!("consider changing this to be mutable: `mut {}`", name)));
+                }
+                scope.fancy_error(&format!("cannot modify `{}`, as it is not mutable", &container.kind),
+                                  labels,
                                   vec![]);
             }
             // XXX

crates/analyzer/src/traversal/expressions.rs

@@ -6,7 +6,8 @@ use crate::errors::{FatalError, IndexingError, NotFixedSize};
 use crate::namespace::items::{Class, FunctionId, Item, TypeDef};
 use crate::namespace::scopes::BlockScopeType;
 use crate::namespace::types::{
-    Array, Base, Contract, FeString, Integer, SelfDecl, Struct, Tuple, Type, TypeDowncast, U256,
+    Array, Base, Contract, FeString, Integer, SelfDecl, SelfDeclKind, Struct, Tuple, Type,
+    TypeDowncast, U256,
 };
 use crate::operations;
 use crate::traversal::call_args::{validate_arg_count, validate_named_args};
@@ -65,16 +66,14 @@ pub fn expr_list(
     expected_type: Option<&Array>,
 ) -> Result<ExpressionAttributes, FatalError> {
     if elts.is_empty() {
-        return Ok(ExpressionAttributes {
-            typ: Type::Array(Array {
+        return Ok(ExpressionAttributes::new(
+            Type::Array(Array {
                 size: 0,
                 inner: expected_type.map_or(Base::Unit, |arr| arr.inner),
             }),
-            location: Location::Memory,
-            move_location: None,
-            const_value: None,
-            mutable: false, // XXX
-        });
+            Location::Memory,
+            true,
+        ));
     }
 
     let inner_type = if let Some(expected) = expected_type {
@@ -135,13 +134,11 @@ pub fn expr_list(
         inner: inner_type,
     };
 
-    Ok(ExpressionAttributes {
-        typ: Type::Array(array_typ),
-        location: Location::Memory,
-        move_location: None,
-        const_value: None,
-        mutable: false, // XXX
-    })
+    Ok(ExpressionAttributes::new(
+        Type::Array(array_typ),
+        Location::Memory,
+        true,
+    ))
 }
 
 /// Gather context information for expressions and check for type errors.
@@ -331,7 +328,13 @@ fn expr_named_thing(
                         },
                     );
                 }
-                let mutable = decl == Some(SelfDecl::MutRef);
+                let mutable = matches!(
+                    decl,
+                    Some(SelfDecl {
+                        kind: SelfDeclKind::MutRef,
+                        ..
+                    })
+                );
                 let (typ, location) = match class {
                     Class::Struct(id) => (
                         Type::Struct(Struct::from_id(id, context.db())),
@@ -1413,13 +1416,30 @@ fn expr_call_method(
                             )],
                         );
                     }
-                    Some(SelfDecl::MutRef) => {
-                        if context.parent_function().self_decl(context.db()) == Some(SelfDecl::Ref)
+                    Some(SelfDecl {
+                        kind: SelfDeclKind::MutRef,
+                        ..
+                    }) => {
+                        if let Some(SelfDecl {
+                            kind: SelfDeclKind::Ref,
+                            span,
+                        }) = context.parent_function().self_decl(context.db())
                         {
                             context.fancy_error(
-                                // XXX better error message
-                                "bad call to mut self fn",
-                                vec![Label::primary(field.span, "function requires mut self")],
+                                &format!(
+                                    "function `{}` requires `mut self`",
+                                    method.name(context.db())
+                                ),
+                                vec![
+                                    Label::primary(field.span, "requires `mut self`"),
+                                    Label::secondary(target.span, "not mutable"),
+                                    Label::secondary(
+                                        span,
+                                        &format!(
+                                            "consider changing this to be mutable: `mut self`"
+                                        ),
+                                    ),
+                                ],
                                 vec![],
                             );
                         }
@@ -1515,7 +1535,14 @@ fn expr_call_method(
             // This should be improved when we add explicit references.
             let self_decl = method.self_decl(context.db());
             let mutable = !return_type.is_base()
-                && !(matches!(class, Class::Struct(_)) && self_decl == Some(SelfDecl::Ref));
+                && !(matches!(class, Class::Struct(_))
+                    && matches!(
+                        self_decl,
+                        Some(SelfDecl {
+                            kind: SelfDeclKind::Ref,
+                            ..
+                        })
+                    ));
 
             return Ok((
                 ExpressionAttributes::new(return_type.into(), location, mutable),