Merge pull request #2651 from onflow/supun/port-internal-130

[v0.39] Remove spurious resource loss errors (internal #130)

runtime/sema/check_return_statement.go

@@ -25,8 +25,13 @@ func (checker *Checker) VisitReturnStatement(statement *ast.ReturnStatement) (_
 
 	defer func() {
 		// NOTE: check for resource loss before declaring the function
-		// as having definitely returned
-		checker.checkResourceLossForFunction()
+		// as having definitely returned.
+		//
+		// Check all variables declared *inside* of the function.
+		// The function activation's value activation depth is where the *function* is declared ("parent scope"),
+		// and two value activation scopes are defined for the function itself: for the parameters and the body.
+
+		checker.checkResourceLoss(functionActivation.ValueActivationDepth + 1)
 		functionActivation.ReturnInfo.MaybeReturned = true
 		functionActivation.ReturnInfo.DefinitelyReturned = true
 	}()
@@ -73,9 +78,3 @@ func (checker *Checker) VisitReturnStatement(statement *ast.ReturnStatement) (_
 
 	return
 }
-
-func (checker *Checker) checkResourceLossForFunction() {
-	functionValueActivationDepth :=
-		checker.functionActivations.Current().ValueActivationDepth
-	checker.checkResourceLoss(functionValueActivationDepth)
-}

runtime/sema/variable_activations.go

@@ -253,7 +253,11 @@ func (a *VariableActivations) Find(name string) *Variable {
 
 // Depth returns the depth (size) of the activation stack.
 func (a *VariableActivations) Depth() int {
-	return len(a.activations)
+	current := a.Current()
+	if current == nil {
+		return 0
+	}
+	return current.Depth
 }
 
 type variableDeclaration struct {

runtime/tests/checker/function_test.go

@@ -380,10 +380,9 @@ func TestCheckInvalidResourceCapturingJustMemberAccess(t *testing.T) {
       let test = makeKittyIdGetter()
     `)
 
-	errs := RequireCheckerErrors(t, err, 2)
+	errs := RequireCheckerErrors(t, err, 1)
 
 	assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
-	assert.IsType(t, &sema.ResourceLossError{}, errs[1])
 }
 
 func TestCheckInvalidFunctionWithResult(t *testing.T) {

runtime/tests/checker/resources_test.go

@@ -3982,11 +3982,10 @@ func TestCheckInvalidResourceDictionaryKeysForeach(t *testing.T) {
         }
     `)
 
-	errs := RequireCheckerErrors(t, err, 3)
+	errs := RequireCheckerErrors(t, err, 2)
 
 	assert.IsType(t, &sema.InvalidDictionaryKeyTypeError{}, errs[0])
 	assert.IsType(t, &sema.InvalidResourceDictionaryMemberError{}, errs[1])
-	assert.IsType(t, &sema.ResourceLossError{}, errs[2])
 }
 
 func TestCheckInvalidResourceLossAfterMoveThroughDictionaryIndexing(t *testing.T) {
@@ -9303,3 +9302,59 @@ func TestCheckInvalidUnreachableResourceInvalidation(t *testing.T) {
 
 	assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
 }
+
+func TestCheckResourceWithFunction(t *testing.T) {
+
+	t.Parallel()
+
+	t.Run("without return statement", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+
+          fun test() {
+              let x: @AnyResource? <- nil
+
+              fun () {}
+
+              destroy x
+          }
+        `)
+		require.NoError(t, err)
+	})
+
+	t.Run("with return statement", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+
+          fun test() {
+              let x: @AnyResource? <- nil
+
+              fun (): Bool {
+                  return true
+              }
+
+              destroy x
+          }
+        `)
+		require.NoError(t, err)
+	})
+}
+
+func TestCheckInvalidResourceDestructionInFunction(t *testing.T) {
+	t.Parallel()
+
+	_, err := ParseAndCheck(t, `
+      fun test() {
+          let x: @AnyResource? <- nil
+
+          fun () {
+              destroy x
+          }
+      }
+    `)
+	errs := RequireCheckerErrors(t, err, 1)
+
+	assert.IsType(t, &sema.ResourceCapturingError{}, errs[0])
+}