Merge pull request #1561 from onflow/bastian/fix-member-mutation-check

Handle missing member when checking mutability of member

runtime/sema/check_assignment.go

@@ -207,17 +207,17 @@ func (checker *Checker) visitIdentifierExpressionAssignment(
 }
 
 func (checker *Checker) visitIndexExpressionAssignment(
-	target *ast.IndexExpression,
+	indexExpression *ast.IndexExpression,
 ) (elementType Type) {
 
-	elementType = checker.visitIndexExpression(target, true)
+	elementType = checker.visitIndexExpression(indexExpression, true)
 
-	targetExpression := target.TargetExpression
-	switch targetExpression := targetExpression.(type) {
-	case *ast.MemberExpression:
-		// calls to this method are cached, so this performs no computation
+	if targetExpression, ok := indexExpression.TargetExpression.(*ast.MemberExpression); ok {
+		// visitMember caches its result, so visiting the target expression again,
+		// after it had been previously visited by visiting the outer index expression,
+		// performs no computation
 		_, member, _ := checker.visitMember(targetExpression)
-		if !checker.isMutatableMember(member) {
+		if member != nil && !checker.isMutatableMember(member) {
 			checker.report(
 				&ExternalMutationError{
 					Name:            member.Identifier.Identifier,

runtime/sema/check_member_expression.go

@@ -132,7 +132,7 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT
 		return accessedType, member, isOptional
 	}
 
-	// If the the access is to a member of `self` and a resource,
+	// If the access is to a member of `self` and a resource,
 	// its use must be recorded/checked, so that it isn't used after it was invalidated
 
 	accessedSelfMember := checker.accessedSelfMember(expression)
@@ -163,19 +163,22 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT
 		}
 		targetRange := ast.NewRangeFromPositioned(expression.Expression)
 		member = resolver.Resolve(identifier, targetRange, checker.report)
-		switch targetExpression := accessedExpression.(type) {
-		case *ast.MemberExpression:
-			// calls to this method are cached, so this performs no computation
-			_, m, _ := checker.visitMember(targetExpression)
-			if !checker.isMutatableMember(m) && resolver.Mutating {
-				checker.report(
-					&ExternalMutationError{
-						Name:            m.Identifier.Identifier,
-						DeclarationKind: m.DeclarationKind,
-						Range:           ast.NewRangeFromPositioned(targetRange),
-						ContainerType:   m.ContainerType,
-					},
-				)
+		if resolver.Mutating {
+			if targetExpression, ok := accessedExpression.(*ast.MemberExpression); ok {
+				// visitMember caches its result, so visiting the target expression again,
+				// after it had been previously visited to get the resolver,
+				// performs no computation
+				_, subMember, _ := checker.visitMember(targetExpression)
+				if subMember != nil && !checker.isMutatableMember(subMember) {
+					checker.report(
+						&ExternalMutationError{
+							Name:            subMember.Identifier.Identifier,
+							DeclarationKind: subMember.DeclarationKind,
+							Range:           ast.NewRangeFromPositioned(targetRange),
+							ContainerType:   subMember.ContainerType,
+						},
+					)
+				}
 			}
 		}
 	}

runtime/tests/checker/indexing_test.go

@@ -131,6 +131,24 @@ func TestCheckArrayIndexingAssignmentWithInteger(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestCheckInvalidIndexAssignmentMissingMember(t *testing.T) {
+
+	t.Parallel()
+
+	_, err := ParseAndCheck(t, `
+	  struct S {}
+
+      fun test() {
+	      let s = S()
+	      s.x[0] = 0
+	  }
+    `)
+
+	errs := ExpectCheckerErrors(t, err, 1)
+
+	assert.IsType(t, &sema.NotDeclaredMemberError{}, errs[0])
+}
+
 func TestCheckInvalidArrayIndexingAssignmentWithWrongType(t *testing.T) {
 
 	t.Parallel()