Merge pull request #3000 from onflow/sainati/type-name-default-argu

Prevent use of non-attachment type names in default arguments
onflow · Jan 10, 2024 · 03ecaca · 03ecaca
2 parents 36ac22d + 70b49f2
commit 03ecaca
Show file tree

Hide file tree

Showing 3 changed files with 164 additions and 20 deletions.
diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go
@@ -1992,9 +1992,13 @@ func (checker *Checker) checkDefaultDestroyParamExpressionKind(
 			break
 		}
 
+		idTypeVariable := checker.typeActivations.Find(identifier)
+
 		// if it's an attachment, then it's also okay
-		if checker.typeActivations.Find(identifier) != nil {
-			break
+		if idTypeVariable != nil {
+			if compositeType, isComposite := idTypeVariable.Type.(*CompositeType); isComposite && compositeType.Kind == common.CompositeKindAttachment {
+				break
+			}
 		}
 		checker.report(&DefaultDestroyInvalidArgumentError{
 			Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg),

diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go
@@ -784,9 +784,10 @@ func TestCheckDefaultEventParamChecking(t *testing.T) {
 				event ResourceDestroyed(name: Type = R)
 			}
         `)
-		errs := RequireCheckerErrors(t, err, 2)
+		errs := RequireCheckerErrors(t, err, 3)
 		require.IsType(t, &sema.TypeMismatchError{}, errs[0])
 		require.IsType(t, &sema.DefaultDestroyInvalidParameterError{}, errs[1])
+		require.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[2])
 	})
 
 	t.Run("address expr", func(t *testing.T) {
@@ -1368,6 +1369,7 @@ func TestCheckDefaultEventParamChecking(t *testing.T) {
 		t.Parallel()
 
 		_, err := ParseAndCheck(t, `
+
 			access(all) resource R {
 				access(all) var i: Int
 				access(all) init() {
@@ -1377,6 +1379,7 @@ func TestCheckDefaultEventParamChecking(t *testing.T) {
 			access(all) var globalArray: @[R] <- [<- create R()]
 			access(all) var base: &R = &globalArray[0] // strategically named variable
 			access(all) var dummy: @R <- globalArray.removeLast() // invalidate the ref
+
 			access(all) resource IndestructibleTroll {
 				// Reference the fake "base" variable
 				access(all) event ResourceDestroyed( x: Int? = base.i)
@@ -1391,4 +1394,84 @@ func TestCheckDefaultEventParamChecking(t *testing.T) {
 		require.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0])
 		require.Equal(t, errs[0].(*sema.DefaultDestroyInvalidArgumentError).Kind, sema.InvalidIdentifier)
 	})
+
+	t.Run("non-attachment type name", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+			access(all) contract Contract {
+				access(all) var i: Int
+				access(all) init() { self.i = 1}
+			}
+			access(all) resource IndestructibleTroll {
+				access(all) event ResourceDestroyed( x: Int? = Contract.i)
+			}
+			access(all) fun main() {
+				var troll <- create IndestructibleTroll()
+				destroy troll
+			}
+        `)
+
+		errs := RequireCheckerErrors(t, err, 1)
+		require.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0])
+		require.Equal(t, errs[0].(*sema.DefaultDestroyInvalidArgumentError).Kind, sema.InvalidIdentifier)
+	})
+
+	t.Run("base contract type name", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+			access(all) contract base {
+				access(all) var i: Int
+				access(all) init() { self.i = 1}
+			}
+			access(all) resource IndestructibleTroll {
+				access(all) event ResourceDestroyed( x: Int? = base.i)
+			}
+			access(all) fun main() {
+				var troll <- create IndestructibleTroll()
+				destroy troll
+			}
+        `)
+
+		errs := RequireCheckerErrors(t, err, 1)
+		require.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0])
+		require.Equal(t, errs[0].(*sema.DefaultDestroyInvalidArgumentError).Kind, sema.InvalidIdentifier)
+	})
+
+	t.Run("attachment same name as variable", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+			access(all) attachment varName for IndestructibleTroll {
+				access(all) var i: Int
+				access(all) init() { self.i = 1}
+			}
+
+			access(all) resource R {
+				access(all) var i: Int
+				access(all) init() {
+					self.i = 123
+				}
+			}
+			access(all) var globalArray: @[R] <- [<- create R()]
+			access(all) var varName: &R = &globalArray[0] // strategically named variable
+			access(all) var dummy: @R <- globalArray.removeLast() // invalidate the ref
+
+			access(all) resource IndestructibleTroll {
+				access(all) event ResourceDestroyed( x: Int? = varName.i)
+			}
+			access(all) fun main() {
+				var troll <- create IndestructibleTroll()
+				destroy troll
+			}
+        `)
+
+		errs := RequireCheckerErrors(t, err, 2)
+		require.IsType(t, &sema.RedeclarationError{}, errs[0])
+		require.IsType(t, &sema.NotDeclaredMemberError{}, errs[1])
+	})
 }
diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go
@@ -2503,9 +2503,13 @@ func TestInterpreterDefaultDestroyEventBaseShadowing(t *testing.T) {
 
 	t.Parallel()
 
-	var events []*interpreter.CompositeValue
+	t.Run("non-attachment type name", func(t *testing.T) {
 
-	inter, err := parseCheckAndInterpretWithOptions(t, `
+		t.Parallel()
+
+		var events []*interpreter.CompositeValue
+
+		inter, err := parseCheckAndInterpretWithOptions(t, `
 		access(all) resource R {
 			access(all) var i: Int
 			access(all) init() {
@@ -2533,26 +2537,79 @@ func TestInterpreterDefaultDestroyEventBaseShadowing(t *testing.T) {
 			destroy trollAttachment
 		}	
         `, ParseCheckAndInterpretOptions{
-		CheckerConfig: &sema.Config{
-			AttachmentsEnabled: true,
-		},
-		Config: &interpreter.Config{
-			OnEventEmitted: func(inter *interpreter.Interpreter, locationRange interpreter.LocationRange, event *interpreter.CompositeValue, eventType *sema.CompositeType) error {
-				events = append(events, event)
-				return nil
+			CheckerConfig: &sema.Config{
+				AttachmentsEnabled: true,
 			},
-		},
+			Config: &interpreter.Config{
+				OnEventEmitted: func(inter *interpreter.Interpreter, locationRange interpreter.LocationRange, event *interpreter.CompositeValue, eventType *sema.CompositeType) error {
+					events = append(events, event)
+					return nil
+				},
+			},
+		})
+
+		require.NoError(t, err)
+		_, err = inter.Invoke("test")
+		require.NoError(t, err)
+
+		require.Len(t, events, 1)
+		require.Equal(t, "TrollAttachment.ResourceDestroyed", events[0].QualifiedIdentifier)
+
+		// should be 1, not 123
+		require.Equal(t, interpreter.NewIntValueFromInt64(nil, 1), events[0].GetField(inter, interpreter.EmptyLocationRange, "x"))
 	})
 
-	require.NoError(t, err)
-	_, err = inter.Invoke("test")
-	require.NoError(t, err)
+	t.Run("base contract name", func(t *testing.T) {
 
-	require.Len(t, events, 1)
-	require.Equal(t, "TrollAttachment.ResourceDestroyed", events[0].QualifiedIdentifier)
+		t.Parallel()
 
-	// should be 1, not 123
-	require.Equal(t, interpreter.NewIntValueFromInt64(nil, 1), events[0].GetField(inter, interpreter.EmptyLocationRange, "x"))
+		var events []*interpreter.CompositeValue
+
+		inter, err := parseCheckAndInterpretWithOptions(t, `
+		access(all) contract base {
+			access(all) var i: Int
+			access(all) init() { self.i = 1}
+		}
+
+		access(all) attachment TrollAttachment for IndestructibleTroll {
+			access(all) event ResourceDestroyed( x: Int = base.i)
+		}
+		
+		access(all) resource IndestructibleTroll {
+			let i: Int
+			init() {
+				self.i = 1
+			}
+		}
+
+		access(all) fun test() {
+			var troll <- create IndestructibleTroll()
+			var trollAttachment <- attach TrollAttachment() to <-troll
+			destroy trollAttachment
+		}	
+        `, ParseCheckAndInterpretOptions{
+			CheckerConfig: &sema.Config{
+				AttachmentsEnabled: true,
+			},
+			Config: &interpreter.Config{
+				OnEventEmitted: func(inter *interpreter.Interpreter, locationRange interpreter.LocationRange, event *interpreter.CompositeValue, eventType *sema.CompositeType) error {
+					events = append(events, event)
+					return nil
+				},
+				ContractValueHandler: makeContractValueHandler(nil, nil, nil),
+			},
+		})
+
+		require.NoError(t, err)
+		_, err = inter.Invoke("test")
+		require.NoError(t, err)
+
+		require.Len(t, events, 1)
+		require.Equal(t, "TrollAttachment.ResourceDestroyed", events[0].QualifiedIdentifier)
+
+		// should be 1, not 123
+		require.Equal(t, interpreter.NewIntValueFromInt64(nil, 1), events[0].GetField(inter, interpreter.EmptyLocationRange, "x"))
+	})
 }
 
 func TestInterpretDefaultDestroyEventArgumentScoping(t *testing.T) {