Merge pull request #2868 from onflow/supun/resource-ref-assignment

[stable-cadence] Fix field assignment via references
onflow · Oct 12, 2023 · cdde6c6 · cdde6c6
2 parents 544aed4 + dc9ce06
commit cdde6c6
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 9 deletions.
diff --git a/runtime/sema/check_assignment.go b/runtime/sema/check_assignment.go
@@ -163,7 +163,7 @@ func (checker *Checker) enforceViewAssignment(assignment ast.Statement, target a
 			accessChain = append(accessChain, elementType)
 		case *ast.MemberExpression:
 			target = targetExp.Expression
-			memberType, _, _, _ := checker.visitMember(targetExp)
+			memberType, _, _, _ := checker.visitMember(targetExp, true)
 			accessChain = append(accessChain, memberType)
 		default:
 			inAccessChain = false
@@ -352,7 +352,7 @@ func (checker *Checker) visitMemberExpressionAssignment(
 	target *ast.MemberExpression,
 ) (memberType Type) {
 
-	_, memberType, member, isOptional := checker.visitMember(target)
+	_, memberType, member, isOptional := checker.visitMember(target, true)
 
 	if member == nil {
 		return InvalidType

diff --git a/runtime/sema/check_expression.go b/runtime/sema/check_expression.go
@@ -321,7 +321,7 @@ func (checker *Checker) visitIndexExpression(
 		//   2) is container-typed,
 		// then the element type should also be a reference.
 		returnReference := false
-		if !isAssignment && shouldReturnReference(valueIndexedType, elementType) {
+		if shouldReturnReference(valueIndexedType, elementType, isAssignment) {
 			// For index expressions, element are un-authorized.
 			elementType = checker.getReferenceType(elementType, false, UnauthorizedAccess)
 

diff --git a/runtime/sema/check_invocation_expression.go b/runtime/sema/check_invocation_expression.go
@@ -299,7 +299,7 @@ func (checker *Checker) checkMemberInvocationArgumentLabels(
 	invocationExpression *ast.InvocationExpression,
 	memberExpression *ast.MemberExpression,
 ) {
-	_, _, member, _ := checker.visitMember(memberExpression)
+	_, _, member, _ := checker.visitMember(memberExpression, false)
 
 	if member == nil || len(member.ArgumentLabels) == 0 {
 		return

diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go
@@ -25,7 +25,7 @@ import (
 
 // NOTE: only called if the member expression is *not* an assignment
 func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) Type {
-	accessedType, memberType, member, isOptional := checker.visitMember(expression)
+	accessedType, memberType, member, isOptional := checker.visitMember(expression, false)
 
 	if !accessedType.IsInvalidType() {
 		memberAccessType := accessedType
@@ -111,7 +111,11 @@ func (checker *Checker) getReferenceType(typ Type, substituteAuthorization bool,
 	return NewReferenceType(checker.memoryGauge, auth, typ)
 }
 
-func shouldReturnReference(parentType, memberType Type) bool {
+func shouldReturnReference(parentType, memberType Type, isAssignment bool) bool {
+	if isAssignment {
+		return false
+	}
+
 	if _, isReference := referenceType(parentType); !isReference {
 		return false
 	}
@@ -125,7 +129,12 @@ func referenceType(typ Type) (*ReferenceType, bool) {
 	return refType, isReference
 }
 
-func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedType Type, resultingType Type, member *Member, isOptional bool) {
+func (checker *Checker) visitMember(expression *ast.MemberExpression, isAssignment bool) (
+	accessedType Type,
+	resultingType Type,
+	member *Member,
+	isOptional bool,
+) {
 	memberInfo, ok := checker.Elaboration.MemberExpressionMemberAccessInfo(expression)
 	if ok {
 		return memberInfo.AccessedType, memberInfo.ResultingType, memberInfo.Member, memberInfo.IsOptional
@@ -367,7 +376,7 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT
 	// i.e: `accessedSelfMember == nil`
 
 	if accessedSelfMember == nil &&
-		shouldReturnReference(accessedType, resultingType) &&
+		shouldReturnReference(accessedType, resultingType, isAssignment) &&
 		member.DeclarationKind == common.DeclarationKindField {
 
 		// Get a reference to the type

diff --git a/runtime/tests/checker/account_test.go b/runtime/tests/checker/account_test.go
@@ -980,10 +980,11 @@ func TestCheckAccountContractsNames(t *testing.T) {
           }
         `)
 
-		errors := RequireCheckerErrors(t, err, 2)
+		errors := RequireCheckerErrors(t, err, 3)
 
 		assert.IsType(t, &sema.InvalidAssignmentAccessError{}, errors[0])
 		assert.IsType(t, &sema.AssignmentToConstantMemberError{}, errors[1])
+		assert.IsType(t, &sema.NonReferenceTypeReferenceError{}, errors[2])
 	})
 }
 

diff --git a/runtime/tests/checker/reference_test.go b/runtime/tests/checker/reference_test.go
@@ -2998,3 +2998,82 @@ func TestCheckReferenceCreationWithInvalidType(t *testing.T) {
 		require.ErrorAs(t, errs[0], &nonReferenceTypeReferenceError)
 	})
 }
+
+func TestCheckResourceReferenceFieldNilAssignment(t *testing.T) {
+	t.Parallel()
+
+	_, err := ParseAndCheck(t, `
+        access(all) resource Outer {
+            access(all) var inner : @Inner?
+
+            init(_ v: @Inner){
+                self.inner <- v
+                var outerRef = &self as &Outer
+                outerRef.inner = nil
+            }
+
+            destroy(){
+                destroy self.inner
+            }
+        }
+
+        access(all) resource Inner {}
+
+        fun main() {
+            let inner <- create Inner()
+            let outer <- create Outer(<- inner)
+            destroy outer
+        }
+    `)
+
+	errors := RequireCheckerErrors(t, err, 2)
+	require.IsType(t, &sema.IncorrectTransferOperationError{}, errors[0])
+	require.IsType(t, &sema.InvalidResourceAssignmentError{}, errors[1])
+}
+
+func TestCheckResourceReferenceIndexNilAssignment(t *testing.T) {
+	t.Parallel()
+
+	t.Run("one level", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            access(all) resource Foo {}
+
+            fun main() {
+                let array: @[Foo?] <- [<- create Foo()]
+                let arrayRef = &array as auth(Mutate) &[Foo?]
+
+                arrayRef[0] = nil
+
+                destroy array
+            }
+        `)
+
+		errors := RequireCheckerErrors(t, err, 2)
+		require.IsType(t, &sema.IncorrectTransferOperationError{}, errors[0])
+		require.IsType(t, &sema.InvalidResourceAssignmentError{}, errors[1])
+	})
+
+	t.Run("nested", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            access(all) resource Foo {}
+
+            fun main() {
+                let array: @[[Foo?]] <- [<- [<- create Foo()]]
+                let arrayRef = &array as auth(Mutate) &[[Foo?]]
+
+                arrayRef[0][0] = nil
+
+                destroy array
+            }
+        `)
+
+		errors := RequireCheckerErrors(t, err, 3)
+		require.IsType(t, &sema.UnauthorizedReferenceAssignmentError{}, errors[0])
+		require.IsType(t, &sema.IncorrectTransferOperationError{}, errors[1])
+		require.IsType(t, &sema.InvalidResourceAssignmentError{}, errors[2])
+	})
+}