Merge pull request #3332 from onflow/supun/port-fixes-v0.42

Port bug fixes to v0.42

runtime/interpreter/value.go

@@ -18170,21 +18170,25 @@ func (v *DictionaryValue) SetKey(
 		locationRange,
 	)
 
+	var existingValue Value
 	switch value := value.(type) {
 	case *SomeValue:
 		innerValue := value.InnerValue(interpreter, locationRange)
-		existingValue := v.Insert(interpreter, locationRange, keyValue, innerValue)
-		interpreter.checkResourceLoss(existingValue, locationRange)
+		existingValue = v.Insert(interpreter, locationRange, keyValue, innerValue)
 
 	case NilValue:
-		_ = v.Remove(interpreter, locationRange, keyValue)
+		existingValue = v.Remove(interpreter, locationRange, keyValue)
 
 	case placeholderValue:
 		// NO-OP
 
 	default:
 		panic(errors.NewUnreachableError())
 	}
+
+	if existingValue != nil {
+		interpreter.checkResourceLoss(existingValue, locationRange)
+	}
 }
 
 func (v *DictionaryValue) String() string {

runtime/sema/check_assignment.go

@@ -124,6 +124,34 @@ func (checker *Checker) checkAssignment(
 	return
 }
 
+func (checker *Checker) rootOfAccessChain(target ast.Expression) (baseVariable *Variable, accessChain []Type) {
+	var inAccessChain = true
+
+	// seek the variable expression (if it exists) at the base of the access chain
+	for inAccessChain {
+		switch targetExp := target.(type) {
+		case *ast.IdentifierExpression:
+			baseVariable = checker.valueActivations.Find(targetExp.Identifier.Identifier)
+			if baseVariable != nil {
+				accessChain = append(accessChain, baseVariable.Type)
+			}
+			inAccessChain = false
+		case *ast.IndexExpression:
+			target = targetExp.TargetExpression
+			elementType := checker.Elaboration.IndexExpressionTypes(targetExp).IndexedType.ElementType(true)
+			accessChain = append(accessChain, elementType)
+		case *ast.MemberExpression:
+			target = targetExp.Expression
+			memberType, _, _ := checker.visitMember(targetExp)
+			accessChain = append(accessChain, memberType)
+		default:
+			inAccessChain = false
+		}
+	}
+
+	return
+}
+
 func (checker *Checker) accessedSelfMember(expression ast.Expression) *Member {
 	memberExpression, isMemberExpression := expression.(*ast.MemberExpression)
 	if !isMemberExpression {
@@ -199,19 +227,23 @@ func (checker *Checker) visitAssignmentValueType(
 func (checker *Checker) visitIdentifierExpressionAssignment(
 	target *ast.IdentifierExpression,
 ) (targetType Type) {
-	identifier := target.Identifier.Identifier
+	identifier := target.Identifier
 
 	// check identifier was declared before
 	variable := checker.findAndCheckValueVariable(target, true)
 	if variable == nil {
 		return InvalidType
 	}
 
+	if variable.Type.IsResourceType() {
+		checker.checkResourceVariableCapturingInFunction(variable, identifier)
+	}
+
 	// check identifier is not a constant
 	if variable.IsConstant {
 		checker.report(
 			&AssignmentToConstantError{
-				Name:  identifier,
+				Name:  identifier.Identifier,
 				Range: ast.NewRangeFromPositioned(checker.memoryGauge, target),
 			},
 		)
@@ -358,7 +390,26 @@ func (checker *Checker) visitMemberExpressionAssignment(
 		reportAssignmentToConstant()
 	}
 
-	return member.TypeAnnotation.Type
+	memberType = member.TypeAnnotation.Type
+
+	if memberType.IsResourceType() {
+		// if the member is a resource, check that it is not captured in a function,
+		// based off the activation depth of the root of the access chain, i.e. `a` in `a.b.c`
+		// we only want to make this check for transactions, as they are the only "resource-like" types
+		// (that can contain resources and must destroy them in their `execute` blocks), that are themselves
+		// not checked by the capturing logic, since they are not themselves resources.
+		baseVariable, _ := checker.rootOfAccessChain(target)
+
+		if baseVariable == nil {
+			return
+		}
+
+		if _, isTransaction := baseVariable.Type.(*TransactionType); isTransaction {
+			checker.checkResourceVariableCapturingInFunction(baseVariable, member.Identifier)
+		}
+	}
+
+	return
 }
 
 func IsValidAssignmentTargetExpression(expression ast.Expression) bool {

runtime/tests/checker/access_test.go

@@ -1884,6 +1884,7 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi
     `)
 	require.NoError(t, err)
 
+	// these capture x and y because they are created in a different file
 	_, err = ParseAndCheckWithOptions(t,
 		`
            import x, y, createR from "imported"
@@ -1907,7 +1908,7 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi
 		},
 	)
 
-	errs := RequireCheckerErrors(t, err, 5)
+	errs := RequireCheckerErrors(t, err, 7)
 
 	require.IsType(t, &sema.InvalidAccessError{}, errs[0])
 	assert.Equal(t,
@@ -1917,18 +1918,22 @@ func TestCheckAccessImportGlobalValueVariableDeclarationWithSecondValue(t *testi
 
 	require.IsType(t, &sema.ResourceCapturingError{}, errs[1])
 
-	require.IsType(t, &sema.AssignmentToConstantError{}, errs[2])
+	require.IsType(t, &sema.ResourceCapturingError{}, errs[2])
+
+	require.IsType(t, &sema.AssignmentToConstantError{}, errs[3])
 	assert.Equal(t,
 		"x",
-		errs[2].(*sema.AssignmentToConstantError).Name,
+		errs[3].(*sema.AssignmentToConstantError).Name,
 	)
 
-	require.IsType(t, &sema.ResourceCapturingError{}, errs[3])
+	require.IsType(t, &sema.ResourceCapturingError{}, errs[4])
+
+	require.IsType(t, &sema.ResourceCapturingError{}, errs[5])
 
-	require.IsType(t, &sema.AssignmentToConstantError{}, errs[4])
+	require.IsType(t, &sema.AssignmentToConstantError{}, errs[6])
 	assert.Equal(t,
 		"y",
-		errs[4].(*sema.AssignmentToConstantError).Name,
+		errs[6].(*sema.AssignmentToConstantError).Name,
 	)
 }
 

runtime/tests/checker/resources_test.go

@@ -9792,3 +9792,147 @@ func TestCheckBoundFunctionToResourceInAssignment(t *testing.T) {
 		assert.IsType(t, &sema.ResourceMethodBindingError{}, errs[0])
 	})
 }
+
+func TestCheckInvalidResourceCaptureOnLeft(t *testing.T) {
+
+	t.Parallel()
+
+	_, err := ParseAndCheck(t, `
+      fun test() {
+          var x: @AnyResource? <- nil
+          fun () {
+              x <-! []
+          }
+          destroy x
+      }
+    `)
+	errs := RequireCheckerErrors(t, err, 1)
+
+	assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
+}
+
+func TestCheckInvalidNestedResourceCapture(t *testing.T) {
+
+	t.Parallel()
+
+	t.Run("on right", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            transaction {
+                var x: @AnyResource?
+                prepare() {
+                   self.x <- nil
+                }
+                execute {
+                    fun() {
+                        let y <- self.x
+                        destroy y
+                    }
+                }
+            }
+      `)
+		require.NoError(t, err)
+	})
+
+	t.Run("resource field on right", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            resource R  {
+                var x: @AnyResource?
+                init() {
+                   self.x <- nil
+                }
+                fun foo() {
+                    fun() {
+                        let y <- self.x <- nil
+                        destroy y
+                    }
+                }
+                destroy() {
+                    destroy self.x
+                }
+            }
+      `)
+		errs := RequireCheckerErrors(t, err, 1)
+
+		assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
+	})
+
+	t.Run("on left", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            transaction {
+                var x: @AnyResource?
+                prepare() {
+                   self.x <- nil
+                }
+                execute {
+                    fun() {
+                        self.x <-! nil
+                    }
+                    destroy self.x
+                }
+            }
+    `)
+		errs := RequireCheckerErrors(t, err, 1)
+
+		assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
+	})
+
+	t.Run("on left method scope", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            transaction {
+                var x: @AnyResource?
+                prepare() {
+                   self.x <- nil
+                }
+                execute {
+                    self.x <-! nil
+                    destroy self.x
+                }
+            }
+    `)
+		require.NoError(t, err)
+	})
+
+	t.Run("contract self variable on left", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            contract C {
+                var x: @AnyResource?
+                init() {
+                   self.x <- nil
+                }
+                fun foo() { 
+                    fun() {
+                        self.x <-! nil
+                    }
+                }
+            }
+    `)
+		require.NoError(t, err)
+	})
+
+	t.Run("contract self variable on left method scope", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            contract C {
+                var x: @AnyResource?
+                init() {
+                   self.x <- nil
+                }
+                fun foo() {
+                    self.x <-! nil
+                }
+            }
+    `)
+		require.NoError(t, err)
+	})
+}

runtime/tests/checker/transactions_test.go

@@ -477,3 +477,34 @@ func TestCheckInvalidTransactionSelfMoveIntoDictionaryLiteral(t *testing.T) {
 
 	assert.IsType(t, &sema.InvalidMoveError{}, errs[0])
 }
+
+func TestCheckInvalidTransactionResourceLoss(t *testing.T) {
+
+	t.Parallel()
+
+	_, err := ParseAndCheck(t, `
+	access(all) resource R{}
+    transaction {
+		var r: @R?
+	  
+		prepare() {
+			  self.r <- nil
+		}
+	  
+		execute {
+			let writeback = fun() {
+				self.r <-! create R()
+			}
+	  
+			var x <- self.r
+	  
+			destroy x
+			writeback()
+		}
+	  }	  
+   `)
+
+	errs := RequireCheckerErrors(t, err, 1)
+
+	assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
+}

runtime/tests/interpreter/resources_test.go

@@ -3488,7 +3488,11 @@ func TestInterpretResourceLoss(t *testing.T) {
 
 	t.Parallel()
 
-	inter, _, err := parseCheckAndInterpretWithLogs(t, `
+	t.Run("in callback", func(t *testing.T) {
+
+		t.Parallel()
+
+		inter := parseCheckAndInterpret(t, `
         access(all) resource R {
             access(all) let id: String
 
@@ -3536,9 +3540,32 @@ func TestInterpretResourceLoss(t *testing.T) {
            destroy rl
         }
     `)
-	require.NoError(t, err)
 
-	_, err = inter.Invoke("main")
-	RequireError(t, err)
-	require.ErrorAs(t, err, &interpreter.ResourceLossError{})
+		_, err := inter.Invoke("main")
+		RequireError(t, err)
+		require.ErrorAs(t, err, &interpreter.ResourceLossError{})
+	})
+
+	t.Run("force nil assignment", func(t *testing.T) {
+
+		t.Parallel()
+
+		inter := parseCheckAndInterpret(t, `
+            access(all) resource R {}
+
+            access(all) fun loseResource(_ victim: @R) {
+                var dict <- { 0: <- victim}
+                dict[0] <-! nil
+                destroy dict
+            }
+
+            access(all) fun main() {
+                loseResource(<- create R())
+            }
+        `)
+
+		_, err := inter.Invoke("main")
+		RequireError(t, err)
+		require.ErrorAs(t, err, &interpreter.ResourceLossError{})
+	})
 }