From d09b578ea359a6e807129f9dde12975b3c6e703c Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 21 Sep 2023 12:59:52 -0400 Subject: [PATCH 01/15] partial checking --- runtime/sema/check_composite_declaration.go | 36 +++++- runtime/sema/errors.go | 21 ++++ runtime/sema/type.go | 2 + runtime/tests/checker/events_test.go | 129 ++++++++++++++++++++ 4 files changed, 184 insertions(+), 4 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 14f9581f66..c8cab9d8d5 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -428,10 +428,23 @@ func (checker *Checker) declareNestedDeclarations( firstNestedCompositeDeclaration := nestedCompositeDeclarations[0] - reportInvalidNesting( - firstNestedCompositeDeclaration.DeclarationKind(), - firstNestedCompositeDeclaration.Identifier, - ) + // we want to permit this nesting under 2 conditions: the container is a resource declaration, + // and this nested declaration is a default destroy event + + if firstNestedCompositeDeclaration.IsResourceDestructionDefaultEvent() { + if len(nestedCompositeDeclarations) > 1 { + firstNestedCompositeDeclaration = nestedCompositeDeclarations[1] + reportInvalidNesting( + firstNestedCompositeDeclaration.DeclarationKind(), + firstNestedCompositeDeclaration.Identifier, + ) + } + } else { + reportInvalidNesting( + firstNestedCompositeDeclaration.DeclarationKind(), + firstNestedCompositeDeclaration.Identifier, + ) + } } else if len(nestedInterfaceDeclarations) > 0 { @@ -479,6 +492,13 @@ func (checker *Checker) declareNestedDeclarations( nestedDeclarationKind common.DeclarationKind, identifier ast.Identifier, ) { + if nestedCompositeKind == common.CompositeKindEvent && identifier.Identifier == ast.ResourceDestructionDefaultEventName { + checker.report(&DefaultDestroyEventInNonResourceError{ + Kind: containerDeclarationKind.Name(), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, identifier), + }) + } + if containerDeclarationKind.IsInterfaceDeclaration() && !nestedDeclarationKind.IsInterfaceDeclaration() { switch nestedCompositeKind { case common.CompositeKindEvent: @@ -811,6 +831,14 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( nestedCompositeDeclarationVariable := checker.valueActivations.Find(identifier.Identifier) + if identifier.Identifier == ast.ResourceDestructionDefaultEventName && compositeKind == common.CompositeKindResource { + // Find the default event's type declaration + defaultEventType := + checker.typeActivations.Find(identifier.Identifier) + compositeType.DefaultDestroyEvent = defaultEventType.Type.(*CompositeType) + return + } + declarationMembers.Set( nestedCompositeDeclarationVariable.Identifier, &Member{ diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index cdb52ed9d4..0409456e32 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -4636,3 +4636,24 @@ func (e *InvalidAttachmentEntitlementError) StartPosition() ast.Position { func (e *InvalidAttachmentEntitlementError) EndPosition(common.MemoryGauge) ast.Position { return e.Pos } + +// DefaultDestroyEventInNonResourceError + +type DefaultDestroyEventInNonResourceError struct { + Kind string + ast.Range +} + +var _ SemanticError = &DefaultDestroyEventInNonResourceError{} +var _ errors.UserError = &DefaultDestroyEventInNonResourceError{} + +func (*DefaultDestroyEventInNonResourceError) isSemanticError() {} + +func (*DefaultDestroyEventInNonResourceError) IsUserError() {} + +func (e *DefaultDestroyEventInNonResourceError) Error() string { + return fmt.Sprintf( + "cannot declare default destruction event in %s", + e.Kind, + ) +} diff --git a/runtime/sema/type.go b/runtime/sema/type.go index 4292b0e8dc..00e9f9533b 100644 --- a/runtime/sema/type.go +++ b/runtime/sema/type.go @@ -4264,6 +4264,8 @@ type CompositeType struct { RequiredEntitlements *EntitlementOrderedSet AttachmentEntitlementAccess *EntitlementMapAccess + DefaultDestroyEvent *CompositeType + cachedIdentifiers *struct { TypeID TypeID QualifiedIdentifier string diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index 62d6428648..4b244ce042 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -481,3 +481,132 @@ func TestCheckDeclareEventInInterface(t *testing.T) { }) } + +func TestCheckDefaultEventDeclaration(t *testing.T) { + + t.Parallel() + + t.Run("empty", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed() + } + `) + require.NoError(t, err) + + }) + + t.Run("allowed in resource interface", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource interface R { + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + }) + + t.Run("fail in struct", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + struct R { + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + }) + + t.Run("fail in struct interface", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + struct R { + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + }) + + t.Run("fail in contract", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + contract R { + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + }) + + t.Run("fail in contract interface", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + contract interface R { + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + }) + + t.Run("allowed in resource attachment", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + attachment A for AnyResource { + event ResourceDestroyed() + } + `) + require.NoError(t, err) + }) + + t.Run("not allowed in struct attachment", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + attachment A for AnyStruct { + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + }) + + t.Run("nested declarations after first disallowed", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed() + event OtherEvent() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.InvalidNestedDeclarationError{}, errs[0]) + }) +} From 68467372485b124bd3d5539ce9a3e9a3ef528ae4 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 21 Sep 2023 13:38:34 -0400 Subject: [PATCH 02/15] support for declarations --- runtime/sema/check_composite_declaration.go | 16 ++++++++-------- runtime/sema/check_interface_declaration.go | 17 ++++++++++++++++- runtime/sema/type.go | 3 +++ runtime/tests/checker/events_test.go | 19 ++++++++++++++----- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index c8cab9d8d5..dfc5e94bbc 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -267,6 +267,13 @@ func (checker *Checker) visitCompositeLikeDeclaration(declaration ast.CompositeL ) } + if !compositeType.IsResourceType() && compositeType.DefaultDestroyEvent != nil { + checker.report(&DefaultDestroyEventInNonResourceError{ + Kind: declaration.DeclarationKind().Name(), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, declaration), + }) + } + // NOTE: visit entitlements, then interfaces, then composites // DON'T use `nestedDeclarations`, because of non-deterministic order @@ -492,13 +499,6 @@ func (checker *Checker) declareNestedDeclarations( nestedDeclarationKind common.DeclarationKind, identifier ast.Identifier, ) { - if nestedCompositeKind == common.CompositeKindEvent && identifier.Identifier == ast.ResourceDestructionDefaultEventName { - checker.report(&DefaultDestroyEventInNonResourceError{ - Kind: containerDeclarationKind.Name(), - Range: ast.NewRangeFromPositioned(checker.memoryGauge, identifier), - }) - } - if containerDeclarationKind.IsInterfaceDeclaration() && !nestedDeclarationKind.IsInterfaceDeclaration() { switch nestedCompositeKind { case common.CompositeKindEvent: @@ -831,7 +831,7 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( nestedCompositeDeclarationVariable := checker.valueActivations.Find(identifier.Identifier) - if identifier.Identifier == ast.ResourceDestructionDefaultEventName && compositeKind == common.CompositeKindResource { + if identifier.Identifier == ast.ResourceDestructionDefaultEventName { // Find the default event's type declaration defaultEventType := checker.typeActivations.Find(identifier.Identifier) diff --git a/runtime/sema/check_interface_declaration.go b/runtime/sema/check_interface_declaration.go index 4f084ad608..e8edf27e56 100644 --- a/runtime/sema/check_interface_declaration.go +++ b/runtime/sema/check_interface_declaration.go @@ -134,6 +134,13 @@ func (checker *Checker) VisitInterfaceDeclaration(declaration *ast.InterfaceDecl fieldPositionGetter, ) + if !interfaceType.IsResourceType() && interfaceType.DefaultDestroyEvent != nil { + checker.report(&DefaultDestroyEventInNonResourceError{ + Kind: declaration.DeclarationKind().Name(), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, declaration), + }) + } + // NOTE: visit entitlements, then interfaces, then composites // DON'T use `nestedDeclarations`, because of non-deterministic order @@ -432,7 +439,15 @@ func (checker *Checker) declareInterfaceMembersAndValue(declaration *ast.Interfa for _, nestedCompositeDeclaration := range declaration.Members.Composites() { if nestedCompositeDeclaration.Kind() == common.CompositeKindEvent { - checker.declareNestedEvent(nestedCompositeDeclaration, eventMembers, interfaceType) + if nestedCompositeDeclaration.IsResourceDestructionDefaultEvent() { + // Find the value declaration + nestedEvent := + checker.typeActivations.Find(nestedCompositeDeclaration.Identifier.Identifier) + interfaceType.DefaultDestroyEvent = nestedEvent.Type.(*CompositeType) + // interfaceType.DefaultDestroyEvent = + } else { + checker.declareNestedEvent(nestedCompositeDeclaration, eventMembers, interfaceType) + } } } })() diff --git a/runtime/sema/type.go b/runtime/sema/type.go index 00e9f9533b..8f54a2e8bd 100644 --- a/runtime/sema/type.go +++ b/runtime/sema/type.go @@ -4620,6 +4620,7 @@ func (t *CompositeType) InterfaceType() *InterfaceType { InitializerPurity: t.ConstructorPurity, containerType: t.containerType, NestedTypes: t.NestedTypes, + DefaultDestroyEvent: t.DefaultDestroyEvent, } } @@ -5050,6 +5051,8 @@ type InterfaceType struct { effectiveInterfaceConformances []Conformance effectiveInterfaceConformanceSet *InterfaceSet supportedEntitlements *EntitlementOrderedSet + + DefaultDestroyEvent *CompositeType } var _ Type = &InterfaceType{} diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index 4b244ce042..c6fe46065d 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -490,27 +490,36 @@ func TestCheckDefaultEventDeclaration(t *testing.T) { t.Parallel() - _, err := ParseAndCheck(t, ` + checker, err := ParseAndCheck(t, ` resource R { event ResourceDestroyed() } `) require.NoError(t, err) + variable, exists := checker.Elaboration.GetGlobalType("R") + require.True(t, exists) + + require.IsType(t, variable.Type, &sema.CompositeType{}) + require.Equal(t, variable.Type.(*sema.CompositeType).DefaultDestroyEvent.Identifier, "ResourceDestroyed") }) t.Run("allowed in resource interface", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheck(t, ` + checker, err := ParseAndCheck(t, ` resource interface R { event ResourceDestroyed() } `) - errs := RequireCheckerErrors(t, err, 1) + require.NoError(t, err) - assert.IsType(t, &sema.DefaultDestroyEventInNonResourceError{}, errs[0]) + variable, exists := checker.Elaboration.GetGlobalType("R") + require.True(t, exists) + + require.IsType(t, variable.Type, &sema.InterfaceType{}) + require.Equal(t, variable.Type.(*sema.InterfaceType).DefaultDestroyEvent.Identifier, "ResourceDestroyed") }) t.Run("fail in struct", func(t *testing.T) { @@ -532,7 +541,7 @@ func TestCheckDefaultEventDeclaration(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` - struct R { + struct interface R { event ResourceDestroyed() } `) From 60c1d1ee93fe84d335d2cd7585656ccc2e8d6a16 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 22 Sep 2023 09:47:27 -0400 Subject: [PATCH 03/15] more progress on checking --- runtime/sema/check_composite_declaration.go | 28 ++++- runtime/sema/check_interface_declaration.go | 2 + runtime/sema/checker.go | 11 +- runtime/sema/errors.go | 35 ++++++ runtime/sema/type.go | 7 +- runtime/tests/checker/events_test.go | 117 ++++++++++++++++++++ 6 files changed, 194 insertions(+), 6 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index dfc5e94bbc..33395d3085 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -835,7 +835,9 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( // Find the default event's type declaration defaultEventType := checker.typeActivations.Find(identifier.Identifier) - compositeType.DefaultDestroyEvent = defaultEventType.Type.(*CompositeType) + defaultEventComposite := defaultEventType.Type.(*CompositeType) + checker.checkDefaultDestroyEvent(defaultEventComposite, nestedCompositeDeclaration, compositeType) + compositeType.DefaultDestroyEvent = defaultEventComposite return } @@ -2007,6 +2009,30 @@ func (checker *Checker) enumMembersAndOrigins( return } +func (checker *Checker) checkDefaultDestroyEvent( + eventType *CompositeType, + eventDeclaration ast.CompositeLikeDeclaration, + containterType ContainerType, +) { + // default events must have default arguments for all their parameters; this is enforced in the parser + // we want to check that these arguments are all either literals or field accesses + + checkParamTypeValid := func(paramType Type) { + if !IsSubType(paramType, StringType) && + !IsSubType(paramType, NumberType) && + !IsSubType(paramType, BoolType) { + checker.report(&DefaultDestroyInvalidParameterError{ + ParamType: paramType, + Range: ast.NewRangeFromPositioned(checker.memoryGauge, eventDeclaration), + }) + } + } + + for _, param := range eventType.ConstructorParameters { + checkParamTypeValid(param.TypeAnnotation.Type) + } +} + func (checker *Checker) checkInitializers( initializers []*ast.SpecialFunctionDeclaration, fields []*ast.FieldDeclaration, diff --git a/runtime/sema/check_interface_declaration.go b/runtime/sema/check_interface_declaration.go index e8edf27e56..d35ecadc07 100644 --- a/runtime/sema/check_interface_declaration.go +++ b/runtime/sema/check_interface_declaration.go @@ -443,6 +443,8 @@ func (checker *Checker) declareInterfaceMembersAndValue(declaration *ast.Interfa // Find the value declaration nestedEvent := checker.typeActivations.Find(nestedCompositeDeclaration.Identifier.Identifier) + defaultEventComposite := nestedEvent.Type.(*CompositeType) + checker.checkDefaultDestroyEvent(defaultEventComposite, nestedCompositeDeclaration, interfaceType) interfaceType.DefaultDestroyEvent = nestedEvent.Type.(*CompositeType) // interfaceType.DefaultDestroyEvent = } else { diff --git a/runtime/sema/checker.go b/runtime/sema/checker.go index d206d15469..ab8a2a50b3 100644 --- a/runtime/sema/checker.go +++ b/runtime/sema/checker.go @@ -1288,12 +1288,19 @@ func (checker *Checker) parameters(parameterList *ast.ParameterList) []Parameter for i, parameter := range parameterList.Parameters { convertedParameterType := checker.ConvertType(parameter.TypeAnnotation.Type) + var convertedParameterDefaultArgument Type = nil + if parameter.DefaultArgument != nil { + // default arg expressions must be subtypes of the type annotation + convertedParameterDefaultArgument = checker.VisitExpression(parameter.DefaultArgument, convertedParameterType) + } + // NOTE: copying resource annotation from source type annotation as-is, // so a potential error is properly reported parameters[i] = Parameter{ - Label: parameter.Label, - Identifier: parameter.Identifier.Identifier, + Label: parameter.Label, + Identifier: parameter.Identifier.Identifier, + DefaultArgument: convertedParameterDefaultArgument, TypeAnnotation: TypeAnnotation{ IsResource: parameter.TypeAnnotation.IsResource, Type: convertedParameterType, diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index 0409456e32..8b3df1823b 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -4657,3 +4657,38 @@ func (e *DefaultDestroyEventInNonResourceError) Error() string { e.Kind, ) } + +// DefaultDestroyInvalidArgumentError + +type DefaultDestroyInvalidArgumentError struct { + ast.Range +} + +var _ SemanticError = &DefaultDestroyInvalidArgumentError{} +var _ errors.UserError = &DefaultDestroyInvalidArgumentError{} + +func (*DefaultDestroyInvalidArgumentError) isSemanticError() {} + +func (*DefaultDestroyInvalidArgumentError) IsUserError() {} + +func (e *DefaultDestroyInvalidArgumentError) Error() string { + return "default destroy event arguments must be literals or member access expressions on `self`" +} + +// DefaultDestroyInvalidArgumentError + +type DefaultDestroyInvalidParameterError struct { + ParamType Type + ast.Range +} + +var _ SemanticError = &DefaultDestroyInvalidParameterError{} +var _ errors.UserError = &DefaultDestroyInvalidParameterError{} + +func (*DefaultDestroyInvalidParameterError) isSemanticError() {} + +func (*DefaultDestroyInvalidParameterError) IsUserError() {} + +func (e *DefaultDestroyInvalidParameterError) Error() string { + return fmt.Sprintf("`%s` is not a valid parameter type for a default destroy event", e.ParamType.QualifiedString()) +} diff --git a/runtime/sema/type.go b/runtime/sema/type.go index 8f54a2e8bd..408fc27a97 100644 --- a/runtime/sema/type.go +++ b/runtime/sema/type.go @@ -2922,9 +2922,10 @@ func formatParameter(spaces bool, label, identifier, typeAnnotation string) stri } type Parameter struct { - TypeAnnotation TypeAnnotation - Label string - Identifier string + TypeAnnotation TypeAnnotation + DefaultArgument Type + Label string + Identifier string } func (p Parameter) String() string { diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index c6fe46065d..f41c65f8fd 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -618,4 +618,121 @@ func TestCheckDefaultEventDeclaration(t *testing.T) { assert.IsType(t, &sema.InvalidNestedDeclarationError{}, errs[0]) }) + + t.Run("cannot declare two default events", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed() + event ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 4) + + assert.IsType(t, &sema.InvalidNestedDeclarationError{}, errs[0]) + assert.IsType(t, &sema.RedeclarationError{}, errs[1]) + assert.IsType(t, &sema.RedeclarationError{}, errs[2]) + assert.IsType(t, &sema.RedeclarationError{}, errs[3]) + }) +} + +func TestCheckDefaultEventParamChecking(t *testing.T) { + + t.Parallel() + + t.Run("basic", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: String = "foo") + } + `) + require.NoError(t, err) + }) + + t.Run("3 param", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: String = "foo", id: UInt16 = 4, condition: Bool = true) + } + `) + require.NoError(t, err) + }) + + t.Run("type error", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: Int = "foo") + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + }) + + t.Run("field", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + let field : String + event ResourceDestroyed(name: String = self.field) + + init() { + self.field = "" + } + } + `) + require.NoError(t, err) + }) + + t.Run("field type mismatch", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + let field : Int + event ResourceDestroyed(name: String = self.field) + + init() { + self.field = 3 + } + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + }) + + t.Run("array field", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + let field : [Int] + event ResourceDestroyed(name: [Int] = self.field) + + init() { + self.field = [3] + } + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyInvalidParameterError{}, errs[0]) + }) + } From 2b1ef907b4fa30b820f42f5f80f5218b6b0d9d16 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 22 Sep 2023 11:32:19 -0400 Subject: [PATCH 04/15] fix checking order so variables are properly in scope --- runtime/sema/check_composite_declaration.go | 36 ++++++++++++++++----- runtime/sema/check_interface_declaration.go | 6 ++-- runtime/sema/checker.go | 11 ++----- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 33395d3085..ffcf746160 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -290,6 +290,10 @@ func (checker *Checker) visitCompositeLikeDeclaration(declaration ast.CompositeL } for _, nestedComposite := range members.Composites() { + if compositeType.DefaultDestroyEvent != nil { + // we enforce elsewhere that each composite can have only one default destroy event + checker.checkDefaultDestroyEvent(compositeType.DefaultDestroyEvent, nestedComposite, compositeType, declaration) + } ast.AcceptDeclaration[struct{}](nestedComposite, checker) } @@ -836,7 +840,6 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( defaultEventType := checker.typeActivations.Find(identifier.Identifier) defaultEventComposite := defaultEventType.Type.(*CompositeType) - checker.checkDefaultDestroyEvent(defaultEventComposite, nestedCompositeDeclaration, compositeType) compositeType.DefaultDestroyEvent = defaultEventComposite return } @@ -2012,24 +2015,41 @@ func (checker *Checker) enumMembersAndOrigins( func (checker *Checker) checkDefaultDestroyEvent( eventType *CompositeType, eventDeclaration ast.CompositeLikeDeclaration, - containterType ContainerType, + containerType ContainerType, + containerDeclaration ast.Declaration, ) { - // default events must have default arguments for all their parameters; this is enforced in the parser - // we want to check that these arguments are all either literals or field accesses - checkParamTypeValid := func(paramType Type) { + // an event definition always has one "constructor" function in its declaration list + members := eventDeclaration.DeclarationMembers() + functions := members.SpecialFunctions() + constructorFunctionParameters := functions[0].FunctionDeclaration.ParameterList.Parameters + + for index, param := range eventType.ConstructorParameters { + paramType := param.TypeAnnotation.Type + paramDefaultArgument := constructorFunctionParameters[index] + + // make `self` and `base` available when checking default arguments so the fields of the composite are available + checker.declareSelfValue(containerType, containerDeclaration.DeclarationDocString()) + if compositeContainer, isComposite := containerType.(*CompositeType); isComposite && compositeContainer.Kind == common.CompositeKindAttachment { + checker.declareBaseValue( + compositeContainer.baseType, + compositeContainer, + compositeContainer.baseTypeDocString) + } + checker.VisitExpression(paramDefaultArgument.DefaultArgument, paramType) + + // default events must have default arguments for all their parameters; this is enforced in the parser + // we want to check that these arguments are all either literals or field accesses, and have primitive types if !IsSubType(paramType, StringType) && !IsSubType(paramType, NumberType) && + !IsSubType(paramType, TheAddressType) && !IsSubType(paramType, BoolType) { checker.report(&DefaultDestroyInvalidParameterError{ ParamType: paramType, Range: ast.NewRangeFromPositioned(checker.memoryGauge, eventDeclaration), }) } - } - for _, param := range eventType.ConstructorParameters { - checkParamTypeValid(param.TypeAnnotation.Type) } } diff --git a/runtime/sema/check_interface_declaration.go b/runtime/sema/check_interface_declaration.go index d35ecadc07..bf8a172c04 100644 --- a/runtime/sema/check_interface_declaration.go +++ b/runtime/sema/check_interface_declaration.go @@ -158,6 +158,9 @@ func (checker *Checker) VisitInterfaceDeclaration(declaration *ast.InterfaceDecl if nestedComposite.Kind() == common.CompositeKindEvent { checker.visitCompositeLikeDeclaration(nestedComposite) } + if interfaceType.DefaultDestroyEvent != nil { + checker.checkDefaultDestroyEvent(interfaceType.DefaultDestroyEvent, nestedComposite, interfaceType, declaration) + } } return @@ -444,8 +447,7 @@ func (checker *Checker) declareInterfaceMembersAndValue(declaration *ast.Interfa nestedEvent := checker.typeActivations.Find(nestedCompositeDeclaration.Identifier.Identifier) defaultEventComposite := nestedEvent.Type.(*CompositeType) - checker.checkDefaultDestroyEvent(defaultEventComposite, nestedCompositeDeclaration, interfaceType) - interfaceType.DefaultDestroyEvent = nestedEvent.Type.(*CompositeType) + interfaceType.DefaultDestroyEvent = defaultEventComposite // interfaceType.DefaultDestroyEvent = } else { checker.declareNestedEvent(nestedCompositeDeclaration, eventMembers, interfaceType) diff --git a/runtime/sema/checker.go b/runtime/sema/checker.go index ab8a2a50b3..d206d15469 100644 --- a/runtime/sema/checker.go +++ b/runtime/sema/checker.go @@ -1288,19 +1288,12 @@ func (checker *Checker) parameters(parameterList *ast.ParameterList) []Parameter for i, parameter := range parameterList.Parameters { convertedParameterType := checker.ConvertType(parameter.TypeAnnotation.Type) - var convertedParameterDefaultArgument Type = nil - if parameter.DefaultArgument != nil { - // default arg expressions must be subtypes of the type annotation - convertedParameterDefaultArgument = checker.VisitExpression(parameter.DefaultArgument, convertedParameterType) - } - // NOTE: copying resource annotation from source type annotation as-is, // so a potential error is properly reported parameters[i] = Parameter{ - Label: parameter.Label, - Identifier: parameter.Identifier.Identifier, - DefaultArgument: convertedParameterDefaultArgument, + Label: parameter.Label, + Identifier: parameter.Identifier.Identifier, TypeAnnotation: TypeAnnotation{ IsResource: parameter.TypeAnnotation.IsResource, Type: convertedParameterType, From 26b5adeac0910933b6c7bbad83cd3098a2e9799b Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 22 Sep 2023 12:06:32 -0400 Subject: [PATCH 05/15] check only certain expressions are allowed --- runtime/sema/check_composite_declaration.go | 52 +++- runtime/tests/checker/events_test.go | 270 +++++++++++++++++++- 2 files changed, 313 insertions(+), 9 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index ffcf746160..c4f2d7e8a2 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -2024,9 +2024,44 @@ func (checker *Checker) checkDefaultDestroyEvent( functions := members.SpecialFunctions() constructorFunctionParameters := functions[0].FunctionDeclaration.ParameterList.Parameters + var checkParamExpressionKind func(ast.Expression) + checkParamExpressionKind = func(arg ast.Expression) { + switch arg := arg.(type) { + case *ast.StringExpression, + *ast.BoolExpression, + *ast.NilExpression, + *ast.IntegerExpression, + *ast.FixedPointExpression, + *ast.IdentifierExpression, + *ast.PathExpression: + break + case *ast.MemberExpression: + checkParamExpressionKind(arg.Expression) + case *ast.IndexExpression: + checkParamExpressionKind(arg.TargetExpression) + checkParamExpressionKind(arg.IndexingExpression) + + // indexing expressions on arrays can fail, and must be disallowed, but + // indexing expressions on dicts, or composites (for attachments) will return `nil` and thus never fail + targetExprType := checker.Elaboration.IndexExpressionTypes(arg).IndexedType + switch targetExprType.(type) { + case *VariableSizedType, *ConstantSizedType: + checker.report(&DefaultDestroyInvalidArgumentError{ + ast.NewRangeFromPositioned(checker.memoryGauge, arg), + }) + } + + default: + checker.report(&DefaultDestroyInvalidArgumentError{ + ast.NewRangeFromPositioned(checker.memoryGauge, arg), + }) + } + } + for index, param := range eventType.ConstructorParameters { paramType := param.TypeAnnotation.Type - paramDefaultArgument := constructorFunctionParameters[index] + paramExpr := constructorFunctionParameters[index] + paramDefaultArgument := paramExpr.DefaultArgument // make `self` and `base` available when checking default arguments so the fields of the composite are available checker.declareSelfValue(containerType, containerDeclaration.DeclarationDocString()) @@ -2036,20 +2071,23 @@ func (checker *Checker) checkDefaultDestroyEvent( compositeContainer, compositeContainer.baseTypeDocString) } - checker.VisitExpression(paramDefaultArgument.DefaultArgument, paramType) + checker.VisitExpression(paramDefaultArgument, paramType) + unwrappedParamType := UnwrapOptionalType(paramType) // default events must have default arguments for all their parameters; this is enforced in the parser // we want to check that these arguments are all either literals or field accesses, and have primitive types - if !IsSubType(paramType, StringType) && - !IsSubType(paramType, NumberType) && - !IsSubType(paramType, TheAddressType) && - !IsSubType(paramType, BoolType) { + if !IsSubType(unwrappedParamType, StringType) && + !IsSubType(unwrappedParamType, NumberType) && + !IsSubType(unwrappedParamType, TheAddressType) && + !IsSubType(unwrappedParamType, BoolType) { checker.report(&DefaultDestroyInvalidParameterError{ ParamType: paramType, - Range: ast.NewRangeFromPositioned(checker.memoryGauge, eventDeclaration), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, paramExpr), }) } + checkParamExpressionKind(paramDefaultArgument) + } } diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index f41c65f8fd..226501d514 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -660,7 +660,7 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { _, err := ParseAndCheck(t, ` resource R { - event ResourceDestroyed(name: String = "foo", id: UInt16 = 4, condition: Bool = true) + event ResourceDestroyed(name: String = "foo", id: UInt16? = 4, condition: Bool = true) } `) require.NoError(t, err) @@ -697,15 +697,69 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { require.NoError(t, err) }) + t.Run("address", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + let field : Address + event ResourceDestroyed(name: Address? = self.field) + + init() { + self.field = 0x1 + } + } + `) + require.NoError(t, err) + }) + + t.Run("nil", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: Address? = nil) + } + `) + require.NoError(t, err) + }) + + t.Run("address expr", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: Address = 0x1) + } + `) + require.NoError(t, err) + }) + + t.Run("float", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: UFix64 = 0.0034) + } + `) + require.NoError(t, err) + }) + t.Run("field type mismatch", func(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` resource R { - let field : Int event ResourceDestroyed(name: String = self.field) + let field : Int + init() { self.field = 3 } @@ -735,4 +789,216 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { assert.IsType(t, &sema.DefaultDestroyInvalidParameterError{}, errs[0]) }) + t.Run("function call", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: Int = self.fn()) + + fun fn(): Int { + return 3 + } + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("external field", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + let s: S = S() + + resource R { + event ResourceDestroyed(name: UFix64 = s.field) + } + + struct S { + let field : UFix64 + init() { + self.field = 0.0034 + } + } + `) + require.NoError(t, err) + }) + + t.Run("double nested field", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + let s: S + event ResourceDestroyed(name: UFix64 = self.s.field) + init() { + self.s = S() + } + } + + struct S { + let field : UFix64 + init() { + self.field = 0.0034 + } + } + `) + require.NoError(t, err) + }) + + t.Run("function call member", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + fun getS(): S { + return S() + } + + resource R { + event ResourceDestroyed(name: UFix64 = getS().field) + } + + struct S { + let field : UFix64 + init() { + self.field = 0.0034 + } + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("method call member", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + fun getS(): S { + return S() + } + event ResourceDestroyed(name: UFix64 = self.getS().field) + } + + struct S { + let field : UFix64 + init() { + self.field = 0.0034 + } + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("array index expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + var arr : [String] = [] + + resource R { + event ResourceDestroyed(name: String? = arr[0]) + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("dict index expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + var arr : {Int: String} = {} + + resource R { + event ResourceDestroyed(name: String? = arr[0]) + } + `) + require.NoError(t, err) + }) + + t.Run("function call dict index expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + fun get(): {Int: String} { + return {} + } + + resource R { + event ResourceDestroyed(name: String? = get()[0]) + } + `) + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("function call dict indexed expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + var dict : {Int: String} = {} + + resource R { + event ResourceDestroyed(name: String? = dict[0+1]) + } + `) + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("attachment index expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + attachment A for R { + let name: String + init() { + self.name = "foo" + } + } + + resource R { + event ResourceDestroyed(name: String? = self[A]?.name) + } + `) + require.NoError(t, err) + }) + + t.Run("attachment with base", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + attachment A for R { + event ResourceDestroyed(name: Int = base.field) + } + + resource R { + let field : Int + + init() { + self.field = 3 + } + } + `) + require.NoError(t, err) + }) + } From ff166ec8e0cd2f511e4eb5ac10df19d5759af3bf Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 22 Sep 2023 12:16:14 -0400 Subject: [PATCH 06/15] amend error message --- runtime/sema/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index 8b3df1823b..366799ea5b 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -4672,7 +4672,7 @@ func (*DefaultDestroyInvalidArgumentError) isSemanticError() {} func (*DefaultDestroyInvalidArgumentError) IsUserError() {} func (e *DefaultDestroyInvalidArgumentError) Error() string { - return "default destroy event arguments must be literals or member access expressions on `self`" + return "default destroy event arguments must be literals, member access expressions, indexed access expressions on dictionaries, or attachment accesses" } // DefaultDestroyInvalidArgumentError From a986a76bc20f6a48e7d1d9c350e222cd6e27f0a1 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 22 Sep 2023 14:09:17 -0400 Subject: [PATCH 07/15] permit paths --- runtime/sema/check_composite_declaration.go | 14 +++- runtime/sema/errors.go | 2 +- runtime/tests/checker/events_test.go | 77 ++++++++++++++++----- 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index c4f2d7e8a2..af1487d799 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -2032,9 +2032,20 @@ func (checker *Checker) checkDefaultDestroyEvent( *ast.NilExpression, *ast.IntegerExpression, *ast.FixedPointExpression, - *ast.IdentifierExpression, *ast.PathExpression: break + case *ast.IdentifierExpression: + // these are guaranteed to exist at time of destruction, so we allow them + if arg.Identifier.Identifier == SelfIdentifier || arg.Identifier.Identifier == BaseIdentifier { + break + } + // if it's an attachment, then it's also okay + if checker.typeActivations.Find(arg.Identifier.Identifier) != nil { + break + } + checker.report(&DefaultDestroyInvalidArgumentError{ + ast.NewRangeFromPositioned(checker.memoryGauge, arg), + }) case *ast.MemberExpression: checkParamExpressionKind(arg.Expression) case *ast.IndexExpression: @@ -2079,6 +2090,7 @@ func (checker *Checker) checkDefaultDestroyEvent( if !IsSubType(unwrappedParamType, StringType) && !IsSubType(unwrappedParamType, NumberType) && !IsSubType(unwrappedParamType, TheAddressType) && + !IsSubType(unwrappedParamType, PathType) && !IsSubType(unwrappedParamType, BoolType) { checker.report(&DefaultDestroyInvalidParameterError{ ParamType: paramType, diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index 366799ea5b..bc356b6523 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -4672,7 +4672,7 @@ func (*DefaultDestroyInvalidArgumentError) isSemanticError() {} func (*DefaultDestroyInvalidArgumentError) IsUserError() {} func (e *DefaultDestroyInvalidArgumentError) Error() string { - return "default destroy event arguments must be literals, member access expressions, indexed access expressions on dictionaries, or attachment accesses" + return "default destroy event arguments must be literals, member access expressions on `self` or `base`, indexed access expressions on dictionaries, or attachment accesses" } // DefaultDestroyInvalidArgumentError diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index 226501d514..ae6584561f 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -738,6 +738,18 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { require.NoError(t, err) }) + t.Run("path expr", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: PublicPath = /public/foo) + } + `) + require.NoError(t, err) + }) + t.Run("float", func(t *testing.T) { t.Parallel() @@ -812,20 +824,22 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` - let s: S = S() - + let r2 <- create R2() + let ref = &r2 as &R2 + resource R { - event ResourceDestroyed(name: UFix64 = s.field) + event ResourceDestroyed(name: UFix64 = ref.field) } - struct S { + resource R2 { let field : UFix64 init() { self.field = 0.0034 } } `) - require.NoError(t, err) + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) }) t.Run("double nested field", func(t *testing.T) { @@ -905,10 +919,13 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` - var arr : [String] = [] - resource R { - event ResourceDestroyed(name: String? = arr[0]) + var arr : [String] + event ResourceDestroyed(name: String? = self.arr[0]) + + init() { + self.arr = [] + } } `) errs := RequireCheckerErrors(t, err, 1) @@ -921,10 +938,13 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` - var arr : {Int: String} = {} - resource R { - event ResourceDestroyed(name: String? = arr[0]) + let dict : {Int: String} + event ResourceDestroyed(name: String? = self.dict[0]) + + init() { + self.dict = {} + } } `) require.NoError(t, err) @@ -935,12 +955,11 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` - fun get(): {Int: String} { - return {} - } - resource R { - event ResourceDestroyed(name: String? = get()[0]) + event ResourceDestroyed(name: String? = self.get()[0]) + fun get(): {Int: String} { + return {} + } } `) errs := RequireCheckerErrors(t, err, 1) @@ -952,10 +971,32 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` - var dict : {Int: String} = {} + resource R { + let dict : {Int: String} + event ResourceDestroyed(name: String? = self.dict[0+1]) + init() { + self.dict = {} + } + } + `) + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + + t.Run("external var expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + var index: Int = 3 resource R { - event ResourceDestroyed(name: String? = dict[0+1]) + let dict : {Int: String} + event ResourceDestroyed(name: String? = self.dict[index]) + + init() { + self.dict = {} + } } `) errs := RequireCheckerErrors(t, err, 1) From 8d2169f23887d3ef67340491bb91d46e64633538 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Tue, 26 Sep 2023 16:56:06 -0400 Subject: [PATCH 08/15] fix lint --- runtime/sema/check_composite_declaration.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index a3346d45e4..b37fbb659f 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -2053,7 +2053,7 @@ func (checker *Checker) checkDefaultDestroyEvent( break } checker.report(&DefaultDestroyInvalidArgumentError{ - ast.NewRangeFromPositioned(checker.memoryGauge, arg), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), }) case *ast.MemberExpression: checkParamExpressionKind(arg.Expression) @@ -2067,13 +2067,13 @@ func (checker *Checker) checkDefaultDestroyEvent( switch targetExprType.(type) { case *VariableSizedType, *ConstantSizedType: checker.report(&DefaultDestroyInvalidArgumentError{ - ast.NewRangeFromPositioned(checker.memoryGauge, arg), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), }) } default: checker.report(&DefaultDestroyInvalidArgumentError{ - ast.NewRangeFromPositioned(checker.memoryGauge, arg), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), }) } } From 936f2ba8910b4a3221c67f88cac5fdd88cb4206c Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 28 Sep 2023 11:07:02 -0400 Subject: [PATCH 09/15] correct scoping --- runtime/sema/check_composite_declaration.go | 5 ++++- runtime/tests/checker/events_test.go | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index b37fbb659f..b3b0216395 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -2065,7 +2065,7 @@ func (checker *Checker) checkDefaultDestroyEvent( // indexing expressions on dicts, or composites (for attachments) will return `nil` and thus never fail targetExprType := checker.Elaboration.IndexExpressionTypes(arg).IndexedType switch targetExprType.(type) { - case *VariableSizedType, *ConstantSizedType: + case ArrayType: checker.report(&DefaultDestroyInvalidArgumentError{ Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), }) @@ -2078,6 +2078,9 @@ func (checker *Checker) checkDefaultDestroyEvent( } } + checker.enterValueScope() + defer checker.leaveValueScope(eventDeclaration.EndPosition, true) + for index, param := range eventType.ConstructorParameters { paramType := param.TypeAnnotation.Type paramExpr := constructorFunctionParameters[index] diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index ae6584561f..4ec165504e 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -1041,5 +1041,4 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { `) require.NoError(t, err) }) - } From 8fb0ad2a4785560b4013e1f2f6fafa90772912f5 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Thu, 26 Oct 2023 11:31:42 -0400 Subject: [PATCH 10/15] respond to review --- runtime/tests/checker/events_test.go | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index 4ec165504e..fba6aaecba 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -782,6 +782,23 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) }) + t.Run("self", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: @R = self) + } + `) + errs := RequireCheckerErrors(t, err, 4) + + assert.IsType(t, &sema.DefaultDestroyInvalidParameterError{}, errs[0]) + assert.IsType(t, &sema.ResourceLossError{}, errs[1]) + assert.IsType(t, &sema.InvalidEventParameterTypeError{}, errs[2]) + assert.IsType(t, &sema.InvalidResourceFieldError{}, errs[3]) + }) + t.Run("array field", func(t *testing.T) { t.Parallel() @@ -1041,4 +1058,30 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { `) require.NoError(t, err) }) + + t.Run("field name conflict", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + attachment A for R { + event ResourceDestroyed(name: Int = self.self, x: String = base.base) + let self: Int + let base: String + init() { + self.base = "foo" + self.self = 3 + } + } + + resource R { + let base: String + init() { + self.base = "foo" + } + event ResourceDestroyed(name: String? = self[A]?.base, x: Int? = self[A]?.self) + } + `) + require.NoError(t, err) + }) } From 48fb4b21d031a58b58f5c12b7e812afba1b3b5b0 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 27 Oct 2023 10:24:18 -0400 Subject: [PATCH 11/15] prevent manually emitting default destroy events --- runtime/sema/check_composite_declaration.go | 1 - runtime/sema/check_emit_statement.go | 9 ++++++ runtime/sema/errors.go | 17 ++++++++++ runtime/tests/checker/events_test.go | 35 +++++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 8cff51b91b..bcdbe52b5d 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -849,7 +849,6 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( checker.typeActivations.Find(identifier.Identifier) defaultEventComposite := defaultEventType.Type.(*CompositeType) compositeType.DefaultDestroyEvent = defaultEventComposite - return } declarationMembers.Set( diff --git a/runtime/sema/check_emit_statement.go b/runtime/sema/check_emit_statement.go index e93126b21c..572c0f6df5 100644 --- a/runtime/sema/check_emit_statement.go +++ b/runtime/sema/check_emit_statement.go @@ -45,6 +45,15 @@ func (checker *Checker) VisitEmitStatement(statement *ast.EmitStatement) (_ stru return } + if compositeType.Identifier == ast.ResourceDestructionDefaultEventName { + checker.report( + &EmitDefaultDestroyEventError{ + Range: ast.NewRangeFromPositioned(checker.memoryGauge, statement.InvocationExpression), + }, + ) + return + } + checker.Elaboration.SetEmitStatementEventType(statement, compositeType) // Check that the emitted event is declared in the same location diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index b07bdd86b1..78dff438f1 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -2490,6 +2490,23 @@ func (e *EmitNonEventError) Error() string { ) } +// EmitDefaultDestroyEventError + +type EmitDefaultDestroyEventError struct { + ast.Range +} + +var _ SemanticError = &EmitDefaultDestroyEventError{} +var _ errors.UserError = &EmitDefaultDestroyEventError{} + +func (*EmitDefaultDestroyEventError) isSemanticError() {} + +func (*EmitDefaultDestroyEventError) IsUserError() {} + +func (e *EmitDefaultDestroyEventError) Error() string { + return "default destruction events may not be explicitly emitted" +} + // EmitImportedEventError type EmitImportedEventError struct { diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index fba6aaecba..20b401b0b2 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -636,6 +636,41 @@ func TestCheckDefaultEventDeclaration(t *testing.T) { assert.IsType(t, &sema.RedeclarationError{}, errs[2]) assert.IsType(t, &sema.RedeclarationError{}, errs[3]) }) + + t.Run("explicit emit disallowed", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed() + fun foo() { + emit ResourceDestroyed() + } + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.EmitDefaultDestroyEventError{}, errs[0]) + }) + + t.Run("explicit emit disallowed outside", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed() + } + + fun foo() { + emit R.ResourceDestroyed() + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.EmitDefaultDestroyEventError{}, errs[0]) + }) } func TestCheckDefaultEventParamChecking(t *testing.T) { From 8f044b2fdabc57a0a9142d05d261ec3ac9aab5fd Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 27 Oct 2023 10:40:39 -0400 Subject: [PATCH 12/15] respond to review --- runtime/sema/check_composite_declaration.go | 104 +++++++++++--------- runtime/sema/check_interface_declaration.go | 1 - runtime/tests/checker/events_test.go | 36 +++++++ 3 files changed, 94 insertions(+), 47 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index bcdbe52b5d..77ca0b7bf2 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -1995,6 +1995,63 @@ func (checker *Checker) enumMembersAndOrigins( return } +func (checker *Checker) checkDefaultDestroyParamExpressionKind( + arg ast.Expression, +) { + switch arg := arg.(type) { + case *ast.StringExpression, + *ast.BoolExpression, + *ast.NilExpression, + *ast.IntegerExpression, + *ast.FixedPointExpression, + *ast.PathExpression: + break + case *ast.IdentifierExpression: + // these are guaranteed to exist at time of destruction, so we allow them + if arg.Identifier.Identifier == SelfIdentifier || arg.Identifier.Identifier == BaseIdentifier { + break + } + // if it's an attachment, then it's also okay + if checker.typeActivations.Find(arg.Identifier.Identifier) != nil { + break + } + checker.report(&DefaultDestroyInvalidArgumentError{ + Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), + }) + case *ast.MemberExpression: + checker.checkDefaultDestroyParamExpressionKind(arg.Expression) + case *ast.IndexExpression: + checker.checkDefaultDestroyParamExpressionKind(arg.TargetExpression) + checker.checkDefaultDestroyParamExpressionKind(arg.IndexingExpression) + + // indexing expressions on arrays can fail, and must be disallowed, but + // indexing expressions on dicts, or composites (for attachments) will return `nil` and thus never fail + targetExprType := checker.Elaboration.IndexExpressionTypes(arg).IndexedType + // `nil` indicates that the index is a type-index (i.e. for an attachment access) + if targetExprType == nil { + return + } + + switch targetExprType := targetExprType.(type) { + case *DictionaryType: + return + case *ReferenceType: + if _, isDictionaryType := targetExprType.Type.(*DictionaryType); isDictionaryType { + return + } + } + + checker.report(&DefaultDestroyInvalidArgumentError{ + Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), + }) + + default: + checker.report(&DefaultDestroyInvalidArgumentError{ + Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), + }) + } +} + func (checker *Checker) checkDefaultDestroyEvent( eventType *CompositeType, eventDeclaration ast.CompositeLikeDeclaration, @@ -2007,51 +2064,6 @@ func (checker *Checker) checkDefaultDestroyEvent( functions := members.SpecialFunctions() constructorFunctionParameters := functions[0].FunctionDeclaration.ParameterList.Parameters - var checkParamExpressionKind func(ast.Expression) - checkParamExpressionKind = func(arg ast.Expression) { - switch arg := arg.(type) { - case *ast.StringExpression, - *ast.BoolExpression, - *ast.NilExpression, - *ast.IntegerExpression, - *ast.FixedPointExpression, - *ast.PathExpression: - break - case *ast.IdentifierExpression: - // these are guaranteed to exist at time of destruction, so we allow them - if arg.Identifier.Identifier == SelfIdentifier || arg.Identifier.Identifier == BaseIdentifier { - break - } - // if it's an attachment, then it's also okay - if checker.typeActivations.Find(arg.Identifier.Identifier) != nil { - break - } - checker.report(&DefaultDestroyInvalidArgumentError{ - Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), - }) - case *ast.MemberExpression: - checkParamExpressionKind(arg.Expression) - case *ast.IndexExpression: - checkParamExpressionKind(arg.TargetExpression) - checkParamExpressionKind(arg.IndexingExpression) - - // indexing expressions on arrays can fail, and must be disallowed, but - // indexing expressions on dicts, or composites (for attachments) will return `nil` and thus never fail - targetExprType := checker.Elaboration.IndexExpressionTypes(arg).IndexedType - switch targetExprType.(type) { - case ArrayType: - checker.report(&DefaultDestroyInvalidArgumentError{ - Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), - }) - } - - default: - checker.report(&DefaultDestroyInvalidArgumentError{ - Range: ast.NewRangeFromPositioned(checker.memoryGauge, arg), - }) - } - } - checker.enterValueScope() defer checker.leaveValueScope(eventDeclaration.EndPosition, true) @@ -2084,7 +2096,7 @@ func (checker *Checker) checkDefaultDestroyEvent( }) } - checkParamExpressionKind(paramDefaultArgument) + checker.checkDefaultDestroyParamExpressionKind(paramDefaultArgument) } } diff --git a/runtime/sema/check_interface_declaration.go b/runtime/sema/check_interface_declaration.go index 508c9a1c00..a5e1d6b678 100644 --- a/runtime/sema/check_interface_declaration.go +++ b/runtime/sema/check_interface_declaration.go @@ -448,7 +448,6 @@ func (checker *Checker) declareInterfaceMembersAndValue(declaration *ast.Interfa checker.typeActivations.Find(nestedCompositeDeclaration.Identifier.Identifier) defaultEventComposite := nestedEvent.Type.(*CompositeType) interfaceType.DefaultDestroyEvent = defaultEventComposite - // interfaceType.DefaultDestroyEvent = } else { checker.declareNestedEvent(nestedCompositeDeclaration, eventMembers, interfaceType) } diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index 20b401b0b2..bc5e5d2b60 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -985,6 +985,25 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) }) + t.Run("array reference index expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + var arr : &[String] + event ResourceDestroyed(name: String? = self.arr[0]) + + init() { + self.arr = &[] + } + } + `) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[0]) + }) + t.Run("dict index expression", func(t *testing.T) { t.Parallel() @@ -1002,6 +1021,23 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { require.NoError(t, err) }) + t.Run("dict reference index expression", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + let dict : &{Int: String} + event ResourceDestroyed(name: String? = self.dict[0]) + + init() { + self.dict = &{} + } + } + `) + require.NoError(t, err) + }) + t.Run("function call dict index expression", func(t *testing.T) { t.Parallel() From 3899fdb1b1cf9d724b1b1e8d8a6fbe4fa76151c0 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Fri, 27 Oct 2023 10:44:11 -0400 Subject: [PATCH 13/15] add tests --- runtime/tests/checker/events_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/runtime/tests/checker/events_test.go b/runtime/tests/checker/events_test.go index bc5e5d2b60..4b3cda4c77 100644 --- a/runtime/tests/checker/events_test.go +++ b/runtime/tests/checker/events_test.go @@ -761,6 +761,34 @@ func TestCheckDefaultEventParamChecking(t *testing.T) { require.NoError(t, err) }) + t.Run("type", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: Type = Type<@R>()) + } + `) + errs := RequireCheckerErrors(t, err, 2) + assert.IsType(t, &sema.DefaultDestroyInvalidParameterError{}, errs[0]) + assert.IsType(t, &sema.DefaultDestroyInvalidArgumentError{}, errs[1]) + }) + + t.Run("raw type", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R { + event ResourceDestroyed(name: Type = R) + } + `) + errs := RequireCheckerErrors(t, err, 2) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.DefaultDestroyInvalidParameterError{}, errs[1]) + }) + t.Run("address expr", func(t *testing.T) { t.Parallel() From 18e4c0d5e93d7e494fb2aecd2005c3cfc76af272 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Mon, 30 Oct 2023 12:27:34 -0400 Subject: [PATCH 14/15] respond to review --- runtime/ast/composite.go | 6 +- runtime/parser/declaration.go | 2 +- runtime/sema/check_composite_declaration.go | 77 ++++++++++++--------- runtime/sema/check_emit_statement.go | 2 +- 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/runtime/ast/composite.go b/runtime/ast/composite.go index 05c35dcef4..66681bf42d 100644 --- a/runtime/ast/composite.go +++ b/runtime/ast/composite.go @@ -45,6 +45,10 @@ type CompositeLikeDeclaration interface { const ResourceDestructionDefaultEventName = "ResourceDestroyed" +func IsResourceDestructionDefaultEvent(identifier string) bool { + return identifier == ResourceDestructionDefaultEventName +} + type CompositeDeclaration struct { Members *Members DocString string @@ -284,7 +288,7 @@ func (d *CompositeDeclaration) ConformanceList() []*NominalType { func (d *CompositeDeclaration) IsResourceDestructionDefaultEvent() bool { return d.CompositeKind == common.CompositeKindEvent && - d.Identifier.Identifier == ResourceDestructionDefaultEventName + IsResourceDestructionDefaultEvent(d.Identifier.Identifier) } // FieldDeclarationFlags diff --git a/runtime/parser/declaration.go b/runtime/parser/declaration.go index 9b3b013026..5a3353f397 100644 --- a/runtime/parser/declaration.go +++ b/runtime/parser/declaration.go @@ -876,7 +876,7 @@ func parseEventDeclaration( p.next() // if this is a `ResourceDestroyed` event (i.e., a default event declaration), parse default arguments - parseDefaultArguments := identifier.Identifier == ast.ResourceDestructionDefaultEventName + parseDefaultArguments := ast.IsResourceDestructionDefaultEvent(identifier.Identifier) parameterList, err := parseParameterList(p, parseDefaultArguments) if err != nil { return nil, err diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 77ca0b7bf2..d06cd70059 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -843,7 +843,7 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( nestedCompositeDeclarationVariable := checker.valueActivations.Find(identifier.Identifier) - if identifier.Identifier == ast.ResourceDestructionDefaultEventName { + if ast.IsResourceDestructionDefaultEvent(identifier.Identifier) { // Find the default event's type declaration defaultEventType := checker.typeActivations.Find(identifier.Identifier) @@ -2007,12 +2007,13 @@ func (checker *Checker) checkDefaultDestroyParamExpressionKind( *ast.PathExpression: break case *ast.IdentifierExpression: + identifier := arg.Identifier.Identifier // these are guaranteed to exist at time of destruction, so we allow them - if arg.Identifier.Identifier == SelfIdentifier || arg.Identifier.Identifier == BaseIdentifier { + if identifier == SelfIdentifier || identifier == BaseIdentifier { break } // if it's an attachment, then it's also okay - if checker.typeActivations.Find(arg.Identifier.Identifier) != nil { + if checker.typeActivations.Find(identifier) != nil { break } checker.report(&DefaultDestroyInvalidArgumentError{ @@ -2052,6 +2053,44 @@ func (checker *Checker) checkDefaultDestroyParamExpressionKind( } } +func (checker *Checker) checkDefaultDestroyEventParam( + param Parameter, + index int, + constructorFunctionParameters []*ast.Parameter, + containerType ContainerType, + containerDeclaration ast.Declaration, +) { + paramType := param.TypeAnnotation.Type + paramExpr := constructorFunctionParameters[index] + paramDefaultArgument := paramExpr.DefaultArgument + + // make `self` and `base` available when checking default arguments so the fields of the composite are available + checker.declareSelfValue(containerType, containerDeclaration.DeclarationDocString()) + if compositeContainer, isComposite := containerType.(*CompositeType); isComposite && compositeContainer.Kind == common.CompositeKindAttachment { + checker.declareBaseValue( + compositeContainer.baseType, + compositeContainer, + compositeContainer.baseTypeDocString) + } + param.DefaultArgument = checker.VisitExpression(paramDefaultArgument, paramType) + + unwrappedParamType := UnwrapOptionalType(paramType) + // default events must have default arguments for all their parameters; this is enforced in the parser + // we want to check that these arguments are all either literals or field accesses, and have primitive types + if !IsSubType(unwrappedParamType, StringType) && + !IsSubType(unwrappedParamType, NumberType) && + !IsSubType(unwrappedParamType, TheAddressType) && + !IsSubType(unwrappedParamType, PathType) && + !IsSubType(unwrappedParamType, BoolType) { + checker.report(&DefaultDestroyInvalidParameterError{ + ParamType: paramType, + Range: ast.NewRangeFromPositioned(checker.memoryGauge, paramExpr), + }) + } + + checker.checkDefaultDestroyParamExpressionKind(paramDefaultArgument) +} + func (checker *Checker) checkDefaultDestroyEvent( eventType *CompositeType, eventDeclaration ast.CompositeLikeDeclaration, @@ -2061,42 +2100,14 @@ func (checker *Checker) checkDefaultDestroyEvent( // an event definition always has one "constructor" function in its declaration list members := eventDeclaration.DeclarationMembers() - functions := members.SpecialFunctions() + functions := members.Initializers() constructorFunctionParameters := functions[0].FunctionDeclaration.ParameterList.Parameters checker.enterValueScope() defer checker.leaveValueScope(eventDeclaration.EndPosition, true) for index, param := range eventType.ConstructorParameters { - paramType := param.TypeAnnotation.Type - paramExpr := constructorFunctionParameters[index] - paramDefaultArgument := paramExpr.DefaultArgument - - // make `self` and `base` available when checking default arguments so the fields of the composite are available - checker.declareSelfValue(containerType, containerDeclaration.DeclarationDocString()) - if compositeContainer, isComposite := containerType.(*CompositeType); isComposite && compositeContainer.Kind == common.CompositeKindAttachment { - checker.declareBaseValue( - compositeContainer.baseType, - compositeContainer, - compositeContainer.baseTypeDocString) - } - param.DefaultArgument = checker.VisitExpression(paramDefaultArgument, paramType) - - unwrappedParamType := UnwrapOptionalType(paramType) - // default events must have default arguments for all their parameters; this is enforced in the parser - // we want to check that these arguments are all either literals or field accesses, and have primitive types - if !IsSubType(unwrappedParamType, StringType) && - !IsSubType(unwrappedParamType, NumberType) && - !IsSubType(unwrappedParamType, TheAddressType) && - !IsSubType(unwrappedParamType, PathType) && - !IsSubType(unwrappedParamType, BoolType) { - checker.report(&DefaultDestroyInvalidParameterError{ - ParamType: paramType, - Range: ast.NewRangeFromPositioned(checker.memoryGauge, paramExpr), - }) - } - - checker.checkDefaultDestroyParamExpressionKind(paramDefaultArgument) + checker.checkDefaultDestroyEventParam(param, index, constructorFunctionParameters, containerType, containerDeclaration) } } diff --git a/runtime/sema/check_emit_statement.go b/runtime/sema/check_emit_statement.go index 572c0f6df5..4d15fc8d5a 100644 --- a/runtime/sema/check_emit_statement.go +++ b/runtime/sema/check_emit_statement.go @@ -45,7 +45,7 @@ func (checker *Checker) VisitEmitStatement(statement *ast.EmitStatement) (_ stru return } - if compositeType.Identifier == ast.ResourceDestructionDefaultEventName { + if ast.IsResourceDestructionDefaultEvent(compositeType.Identifier) { checker.report( &EmitDefaultDestroyEventError{ Range: ast.NewRangeFromPositioned(checker.memoryGauge, statement.InvocationExpression), From 7931d902c8cfdad13bca2c85786f6660faf052da Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Mon, 30 Oct 2023 14:07:36 -0400 Subject: [PATCH 15/15] respond to review --- runtime/sema/check_composite_declaration.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index d06cd70059..21650417d2 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -290,7 +290,7 @@ func (checker *Checker) visitCompositeLikeDeclaration(declaration ast.CompositeL } for _, nestedComposite := range members.Composites() { - if compositeType.DefaultDestroyEvent != nil { + if compositeType.DefaultDestroyEvent != nil && nestedComposite.IsResourceDestructionDefaultEvent() { // we enforce elsewhere that each composite can have only one default destroy event checker.checkDefaultDestroyEvent(compositeType.DefaultDestroyEvent, nestedComposite, compositeType, declaration) } @@ -2055,14 +2055,12 @@ func (checker *Checker) checkDefaultDestroyParamExpressionKind( func (checker *Checker) checkDefaultDestroyEventParam( param Parameter, - index int, - constructorFunctionParameters []*ast.Parameter, + astParam *ast.Parameter, containerType ContainerType, containerDeclaration ast.Declaration, ) { paramType := param.TypeAnnotation.Type - paramExpr := constructorFunctionParameters[index] - paramDefaultArgument := paramExpr.DefaultArgument + paramDefaultArgument := astParam.DefaultArgument // make `self` and `base` available when checking default arguments so the fields of the composite are available checker.declareSelfValue(containerType, containerDeclaration.DeclarationDocString()) @@ -2084,7 +2082,7 @@ func (checker *Checker) checkDefaultDestroyEventParam( !IsSubType(unwrappedParamType, BoolType) { checker.report(&DefaultDestroyInvalidParameterError{ ParamType: paramType, - Range: ast.NewRangeFromPositioned(checker.memoryGauge, paramExpr), + Range: ast.NewRangeFromPositioned(checker.memoryGauge, astParam), }) } @@ -2107,7 +2105,7 @@ func (checker *Checker) checkDefaultDestroyEvent( defer checker.leaveValueScope(eventDeclaration.EndPosition, true) for index, param := range eventType.ConstructorParameters { - checker.checkDefaultDestroyEventParam(param, index, constructorFunctionParameters, containerType, containerDeclaration) + checker.checkDefaultDestroyEventParam(param, constructorFunctionParameters[index], containerType, containerDeclaration) } }