[v1.3] Port internal fixes

ast/visitor.go

@@ -30,7 +30,7 @@ type Element interface {
 
 type StatementDeclarationVisitor[T any] interface {
 	VisitVariableDeclaration(*VariableDeclaration) T
-	VisitFunctionDeclaration(*FunctionDeclaration) T
+	VisitFunctionDeclaration(declaration *FunctionDeclaration, isStatement bool) T
 	VisitSpecialFunctionDeclaration(*SpecialFunctionDeclaration) T
 	VisitCompositeDeclaration(*CompositeDeclaration) T
 	VisitAttachmentDeclaration(*AttachmentDeclaration) T
@@ -68,7 +68,7 @@ func AcceptDeclaration[T any](declaration Declaration, visitor DeclarationVisito
 		return visitor.VisitVariableDeclaration(declaration.(*VariableDeclaration))
 
 	case ElementTypeFunctionDeclaration:
-		return visitor.VisitFunctionDeclaration(declaration.(*FunctionDeclaration))
+		return visitor.VisitFunctionDeclaration(declaration.(*FunctionDeclaration), false)
 
 	case ElementTypeSpecialFunctionDeclaration:
 		return visitor.VisitSpecialFunctionDeclaration(declaration.(*SpecialFunctionDeclaration))
@@ -151,7 +151,7 @@ func AcceptStatement[T any](statement Statement, visitor StatementVisitor[T]) (_
 		return visitor.VisitVariableDeclaration(statement.(*VariableDeclaration))
 
 	case ElementTypeFunctionDeclaration:
-		return visitor.VisitFunctionDeclaration(statement.(*FunctionDeclaration))
+		return visitor.VisitFunctionDeclaration(statement.(*FunctionDeclaration), true)
 
 	case ElementTypeSpecialFunctionDeclaration:
 		return visitor.VisitSpecialFunctionDeclaration(statement.(*SpecialFunctionDeclaration))

interpreter/interface_test.go

@@ -1107,6 +1107,102 @@ func TestInterpretInterfaceFunctionConditionsInheritance(t *testing.T) {
 		assert.Equal(t, "\"hello\"", logs[0])
 	})
 
+	t.Run("default and conditions in parent, more conditions in child", func(t *testing.T) {
+
+		t.Parallel()
+
+		var logs []string
+
+		logFunction := stdlib.NewStandardLibraryStaticFunction(
+			"log",
+			&sema.FunctionType{
+				Parameters: []sema.Parameter{
+					{
+						Label:          sema.ArgumentLabelNotRequired,
+						Identifier:     "value",
+						TypeAnnotation: sema.NewTypeAnnotation(sema.AnyStructType),
+					},
+				},
+				ReturnTypeAnnotation: sema.NewTypeAnnotation(
+					sema.VoidType,
+				),
+				Purity: sema.FunctionPurityView,
+			},
+			``,
+			func(invocation interpreter.Invocation) interpreter.Value {
+				message := invocation.Arguments[0].MeteredString(
+					invocation.Interpreter,
+					interpreter.SeenReferences{},
+					invocation.LocationRange,
+				)
+				logs = append(logs, message)
+				return interpreter.Void
+			},
+		)
+
+		baseValueActivation := sema.NewVariableActivation(sema.BaseValueActivation)
+		baseValueActivation.DeclareValue(logFunction)
+
+		baseActivation := activations.NewActivation(nil, interpreter.BaseActivation)
+		interpreter.Declare(baseActivation, logFunction)
+
+		code := `
+            struct interface Foo {
+                fun test() {
+                    pre {
+                         printMessage("invoked Foo.test() pre-condition")
+                    }
+                    post {
+                         printMessage("invoked Foo.test() post-condition")
+                    }
+                    printMessage("invoked Foo.test()")
+                }
+            }
+
+            struct Test: Foo {
+            }
+
+            view fun printMessage(_ msg: String): Bool {
+                log(msg)
+                return true
+            }
+
+            fun main() {
+               Test().test()
+            }
+        `
+
+		inter, err := parseCheckAndInterpretWithOptions(
+			t,
+			code,
+			ParseCheckAndInterpretOptions{
+				Config: &interpreter.Config{
+					BaseActivationHandler: func(_ common.Location) *interpreter.VariableActivation {
+						return baseActivation
+					},
+				},
+				CheckerConfig: &sema.Config{
+					BaseValueActivationHandler: func(_ common.Location) *sema.VariableActivation {
+						return baseValueActivation
+					},
+				},
+				HandleCheckerError: nil,
+			},
+		)
+		require.NoError(t, err)
+
+		_, err = inter.Invoke("main")
+		require.NoError(t, err)
+
+		require.Equal(
+			t,
+			[]string{
+				`"invoked Foo.test() pre-condition"`,
+				`"invoked Foo.test()"`,
+				`"invoked Foo.test() post-condition"`,
+			}, logs,
+		)
+	})
 }
 
 func TestInterpretNestedInterfaceCast(t *testing.T) {

interpreter/interpreter.go

@@ -703,10 +703,10 @@ func (interpreter *Interpreter) VisitProgram(program *ast.Program) {
 }
 
 func (interpreter *Interpreter) VisitSpecialFunctionDeclaration(declaration *ast.SpecialFunctionDeclaration) StatementResult {
-	return interpreter.VisitFunctionDeclaration(declaration.FunctionDeclaration)
+	return interpreter.VisitFunctionDeclaration(declaration.FunctionDeclaration, false)
 }
 
-func (interpreter *Interpreter) VisitFunctionDeclaration(declaration *ast.FunctionDeclaration) StatementResult {
+func (interpreter *Interpreter) VisitFunctionDeclaration(declaration *ast.FunctionDeclaration, isStatement bool) StatementResult {
 
 	identifier := declaration.Identifier.Identifier
 
@@ -718,6 +718,33 @@ func (interpreter *Interpreter) VisitFunctionDeclaration(declaration *ast.Functi
 	// lexical scope: variables in functions are bound to what is visible at declaration time
 	lexicalScope := interpreter.activations.CurrentOrNew()
 
+	if isStatement {
+
+		// This function declaration is an inner function.
+		//
+		// Variables which are declared after this function declaration
+		// should not be visible or even overwrite the variables captured by the closure
+		/// (e.g. through shadowing).
+		//
+		// For example:
+		//
+		//     fun foo(a: Int): Int {
+		//         fun bar(): Int {
+		//             return a
+		//             //     ^ should refer to the `a` parameter of `foo`,
+		//             //     not to the `a` variable declared after `bar`
+		//         }
+		//         let a = 2
+		//         return bar()
+		//     }
+		//
+		// As variable declarations mutate the current activation in place,
+		// push a new activation, so that the mutations are not performed
+		// on the captured activation.
+
+		interpreter.activations.PushNewWithCurrent()
+	}
+
 	// make the function itself available inside the function
 	lexicalScope.Set(identifier, variable)
 
@@ -1297,6 +1324,8 @@ func (declarationInterpreter *Interpreter) declareNonEnumCompositeValue(
 		}
 	}
 
+	config := declarationInterpreter.SharedState.Config
+
 	wrapFunctions := func(ty *sema.InterfaceType, code WrapperCode) {
 
 		// Wrap initializer
@@ -1315,6 +1344,16 @@ func (declarationInterpreter *Interpreter) declareNonEnumCompositeValue(
 		// the order does not matter.
 
 		for name, functionWrapper := range code.FunctionWrappers { //nolint:maprange
+			// If there's a default implementation, then skip explicitly/separately
+			// running the conditions of that functions.
+			// Because the conditions also get executed when the default implementation is executed.
+			// This works because:
+			// 	- `code.Functions` only contains default implementations.
+			//	- There is always only one default implementation (cannot override by other interfaces).
+			if code.Functions.Contains(name) {
+				continue
+			}
+
 			fn, ok := functions.Get(name)
 			// If there is a wrapper, there MUST be a body.
 			if !ok {
@@ -1354,8 +1393,6 @@ func (declarationInterpreter *Interpreter) declareNonEnumCompositeValue(
 
 	qualifiedIdentifier := compositeType.QualifiedIdentifier()
 
-	config := declarationInterpreter.SharedState.Config
-
 	constructorType := compositeType.ConstructorFunctionType()
 
 	constructorGenerator := func(address common.Address) *HostFunctionValue {
@@ -2457,7 +2494,11 @@ func (interpreter *Interpreter) functionConditionsWrapper(
 	lexicalScope *VariableActivation,
 ) FunctionWrapper {
 
-	if declaration.FunctionBlock == nil {
+	if declaration.FunctionBlock == nil ||
+		declaration.FunctionBlock.HasStatements() {
+		// If there's a default implementation (i.e: has statements),
+		// then skip explicitly/separately running the conditions of that functions.
+		// Because the conditions also get executed when the default implementation is executed.
 		return nil
 	}
 

interpreter/interpreter_expression.go

@@ -1315,9 +1315,31 @@ func (interpreter *Interpreter) visitEntries(entries []ast.DictionaryEntry) []Di
 
 func (interpreter *Interpreter) VisitFunctionExpression(expression *ast.FunctionExpression) Value {
 
-	// lexical scope: variables in functions are bound to what is visible at declaration time
+	// lexical scope: variables in functions are bound to what is visible at declaration time.
 	lexicalScope := interpreter.activations.CurrentOrNew()
 
+	// Variables which are declared after this function declaration
+	// should not be visible or even overwrite the variables captured by the closure
+	/// (e.g. through shadowing).
+	//
+	// For example:
+	//
+	//     fun foo(a: Int): Int {
+	//         let bar = fun(): Int {
+	//             return a
+	//             //     ^ should refer to the `a` parameter of `foo`,
+	//             //     not to the `a` variable declared after `bar`
+	//         }
+	//         let a = 2
+	//         return bar()
+	//     }
+	//
+	// As variable declarations mutate the current activation in place,
+	// push a new activation, so that the mutations are not performed
+	// on the captured activation.
+
+	interpreter.activations.PushNewWithCurrent()
+
 	functionType := interpreter.Program.Elaboration.FunctionExpressionFunctionType(expression)
 
 	var preConditions []ast.Condition

interpreter/misc_test.go

@@ -6864,6 +6864,108 @@ func TestInterpretClosure(t *testing.T) {
 	)
 }
 
+func TestInterpretClosureScopingFunctionExpression(t *testing.T) {
+	t.Parallel()
+
+	inter := parseCheckAndInterpret(t, `
+        fun test(a: Int): Int {
+            let bar = fun(): Int {
+                return a
+            }
+            let a = 2
+            return bar()
+        }
+    `)
+
+	actual, err := inter.Invoke("test",
+		interpreter.NewUnmeteredIntValueFromInt64(1),
+	)
+	require.NoError(t, err)
+
+	AssertValuesEqual(
+		t,
+		inter,
+		interpreter.NewUnmeteredIntValueFromInt64(1),
+		actual,
+	)
+}
+
+func TestInterpretClosureScopingInnerFunction(t *testing.T) {
+	t.Parallel()
+
+	inter := parseCheckAndInterpret(t, `
+        fun test(a: Int): Int {
+            fun bar(): Int {
+                return a
+            }
+            let a = 2
+            return bar()
+        }
+    `)
+
+	value, err := inter.Invoke("test",
+		interpreter.NewUnmeteredIntValueFromInt64(1),
+	)
+	require.NoError(t, err)
+
+	AssertValuesEqual(
+		t,
+		inter,
+		interpreter.NewUnmeteredIntValueFromInt64(1),
+		value,
+	)
+}
+
+func TestInterpretAssignmentAfterClosureFunctionExpression(t *testing.T) {
+	t.Parallel()
+
+	inter := parseCheckAndInterpret(t, `
+        fun test(): Int {
+            var a = 1
+            let bar = fun(): Int {
+                return a
+            }
+            a = 2
+            return bar()
+        }
+    `)
+
+	value, err := inter.Invoke("test")
+	require.NoError(t, err)
+
+	AssertValuesEqual(
+		t,
+		inter,
+		interpreter.NewUnmeteredIntValueFromInt64(2),
+		value,
+	)
+}
+
+func TestInterpretAssignmentAfterClosureInnerFunction(t *testing.T) {
+	t.Parallel()
+
+	inter := parseCheckAndInterpret(t, `
+        fun test(): Int {
+            var a = 1
+            fun bar(): Int {
+                return a
+            }
+            a = 2
+            return bar()
+        }
+    `)
+
+	value, err := inter.Invoke("test")
+	require.NoError(t, err)
+
+	AssertValuesEqual(
+		t,
+		inter,
+		interpreter.NewUnmeteredIntValueFromInt64(2),
+		value,
+	)
+}
+
 // TestInterpretCompositeFunctionInvocationFromImportingProgram checks
 // that member functions of imported composites can be invoked from an importing program.
 // See https://github.com/dapperlabs/flow-go/issues/838