From 6d689b24222cd3c1b932a011359d83974ac2ec8c Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Wed, 19 Jul 2023 12:59:58 -0400 Subject: [PATCH 1/2] report error on composite definition, not member --- runtime/error_test.go | 226 ++++++++++++++++++++ runtime/sema/check_composite_declaration.go | 7 + runtime/sema/errors.go | 20 +- 3 files changed, 236 insertions(+), 17 deletions(-) diff --git a/runtime/error_test.go b/runtime/error_test.go index 4b3eacd749..874ec568f0 100644 --- a/runtime/error_test.go +++ b/runtime/error_test.go @@ -25,8 +25,10 @@ import ( "github.com/stretchr/testify/require" + "github.com/onflow/cadence" "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" + "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/sema" ) @@ -493,3 +495,227 @@ func TestRuntimeError(t *testing.T) { }) } + +func TestRuntimeDefaultFunctionConflictPrintingError(t *testing.T) { + t.Parallel() + + runtime := newTestInterpreterRuntime() + + makeDeployTransaction := func(name, code string) []byte { + return []byte(fmt.Sprintf( + ` + transaction { + prepare(signer: AuthAccount) { + let acct = AuthAccount(payer: signer) + acct.contracts.add(name: "%s", code: "%s".decodeHex()) + } + } + `, + name, + hex.EncodeToString([]byte(code)), + )) + } + + contractInterfaceCode := ` + access(all) contract TestInterfaces { + + access(all) resource interface A { + access(all) fun foo() { + let x = 3 + } + } + + access(all) resource interface B { + access(all) fun foo() + } + } + ` + + contractCode := ` + import TestInterfaces from 0x2 + access(all) contract TestContract { + access(all) resource R: TestInterfaces.A, TestInterfaces.B {} + // fill space + // fill space + // fill space + // fill space + // fill space + // fill space + // filling lots of space + // filling lots of space + // filling lots of space + } + ` + + accountCodes := map[Location][]byte{} + var events []cadence.Event + + var nextAccount byte = 0x2 + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + createAccount: func(payer Address) (address Address, err error) { + result := interpreter.NewUnmeteredAddressValueFromBytes([]byte{nextAccount}) + nextAccount++ + return result.ToAddress(), nil + }, + getSigningAccounts: func() ([]Address, error) { + return []Address{{0x1}}, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + deployTransaction := makeDeployTransaction("TestInterfaces", contractInterfaceCode) + err := runtime.ExecuteTransaction( + Script{ + Source: deployTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + deployTransaction = makeDeployTransaction("TestContract", contractCode) + err = runtime.ExecuteTransaction( + Script{ + Source: deployTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.Error(t, err) + require.Contains(t, err.Error(), "access(all) resource R: TestInterfaces.A, TestInterfaces.B {}") +} + +func TestRuntimeMultipleInterfaceDefaultImplementationsError(t *testing.T) { + t.Parallel() + + runtime := newTestInterpreterRuntime() + + makeDeployTransaction := func(name, code string) []byte { + return []byte(fmt.Sprintf( + ` + transaction { + prepare(signer: AuthAccount) { + let acct = AuthAccount(payer: signer) + acct.contracts.add(name: "%s", code: "%s".decodeHex()) + } + } + `, + name, + hex.EncodeToString([]byte(code)), + )) + } + + contractInterfaceCode := ` + access(all) contract TestInterfaces { + + access(all) resource interface A { + access(all) fun foo() { + let x = 3 + } + } + + access(all) resource interface B { + access(all) fun foo() { + let x = 4 + } + } + } + ` + + contractCode := ` + import TestInterfaces from 0x2 + access(all) contract TestContract { + access(all) resource R: TestInterfaces.A, TestInterfaces.B {} + // fill space + // fill space + // fill space + // fill space + // fill space + // fill space + // filling lots of space + // filling lots of space + // filling lots of space + } + ` + + accountCodes := map[Location][]byte{} + var events []cadence.Event + + var nextAccount byte = 0x2 + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + createAccount: func(payer Address) (address Address, err error) { + result := interpreter.NewUnmeteredAddressValueFromBytes([]byte{nextAccount}) + nextAccount++ + return result.ToAddress(), nil + }, + getSigningAccounts: func() ([]Address, error) { + return []Address{{0x1}}, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + deployTransaction := makeDeployTransaction("TestInterfaces", contractInterfaceCode) + err := runtime.ExecuteTransaction( + Script{ + Source: deployTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + deployTransaction = makeDeployTransaction("TestContract", contractCode) + err = runtime.ExecuteTransaction( + Script{ + Source: deployTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.Error(t, err) + require.Contains(t, err.Error(), "access(all) resource R: TestInterfaces.A, TestInterfaces.B {}") +} diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 18eefc0dad..f7c8091ed9 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -738,11 +738,14 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( } if _, ok := inheritedMembers.Get(memberName); ok { + errorRange := ast.NewRangeFromPositioned(checker.memoryGauge, declaration.DeclarationIdentifier()) + if member.HasImplementation { checker.report( &MultipleInterfaceDefaultImplementationsError{ CompositeType: nestedCompositeType, Member: member, + Range: errorRange, }, ) } else { @@ -750,6 +753,7 @@ func (checker *Checker) declareCompositeLikeMembersAndValue( &DefaultFunctionConflictError{ CompositeType: nestedCompositeType, Member: member, + Range: errorRange, }, ) } @@ -1243,11 +1247,13 @@ func (checker *Checker) checkCompositeLikeConformance( if interfaceMember.DeclarationKind == common.DeclarationKindFunction { if _, ok := inheritedMembers[name]; ok { + errorRange := ast.NewRangeFromPositioned(checker.memoryGauge, compositeDeclaration.DeclarationIdentifier()) if interfaceMember.HasImplementation { checker.report( &MultipleInterfaceDefaultImplementationsError{ CompositeType: compositeType, Member: interfaceMember, + Range: errorRange, }, ) } else { @@ -1255,6 +1261,7 @@ func (checker *Checker) checkCompositeLikeConformance( &DefaultFunctionConflictError{ CompositeType: compositeType, Member: interfaceMember, + Range: errorRange, }, ) } diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index 9012a60450..c89d5ce57e 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -1418,6 +1418,7 @@ func (e *DuplicateConformanceError) Error() string { type MultipleInterfaceDefaultImplementationsError struct { CompositeType *CompositeType Member *Member + ast.Range } var _ SemanticError = &MultipleInterfaceDefaultImplementationsError{} @@ -1436,14 +1437,6 @@ func (e *MultipleInterfaceDefaultImplementationsError) Error() string { ) } -func (e *MultipleInterfaceDefaultImplementationsError) StartPosition() ast.Position { - return e.Member.Identifier.StartPosition() -} - -func (e *MultipleInterfaceDefaultImplementationsError) EndPosition(memoryGauge common.MemoryGauge) ast.Position { - return e.Member.Identifier.EndPosition(memoryGauge) -} - // SpecialFunctionDefaultImplementationError type SpecialFunctionDefaultImplementationError struct { Container ast.Declaration @@ -1479,6 +1472,7 @@ func (e *SpecialFunctionDefaultImplementationError) EndPosition(memoryGauge comm type DefaultFunctionConflictError struct { CompositeType *CompositeType Member *Member + ast.Range } var _ SemanticError = &DefaultFunctionConflictError{} @@ -1490,21 +1484,13 @@ func (*DefaultFunctionConflictError) IsUserError() {} func (e *DefaultFunctionConflictError) Error() string { return fmt.Sprintf( - "%s `%s` has conflicting requirements for function `%s`", + "%s `%s` has conflicting requirements for function `%s` ", e.CompositeType.Kind.Name(), e.CompositeType.QualifiedString(), e.Member.Identifier.Identifier, ) } -func (e *DefaultFunctionConflictError) StartPosition() ast.Position { - return e.Member.Identifier.StartPosition() -} - -func (e *DefaultFunctionConflictError) EndPosition(memoryGauge common.MemoryGauge) ast.Position { - return e.Member.Identifier.EndPosition(memoryGauge) -} - // MissingConformanceError type MissingConformanceError struct { CompositeType *CompositeType From 1e282ebcccd5cf23415dc96d5488561424e8be1e Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Wed, 19 Jul 2023 13:57:03 -0400 Subject: [PATCH 2/2] assert location in tests --- runtime/error_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/runtime/error_test.go b/runtime/error_test.go index 874ec568f0..1d02f014c3 100644 --- a/runtime/error_test.go +++ b/runtime/error_test.go @@ -30,6 +30,7 @@ import ( "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/sema" + "github.com/onflow/cadence/runtime/stdlib" ) func TestRuntimeError(t *testing.T) { @@ -605,6 +606,22 @@ func TestRuntimeDefaultFunctionConflictPrintingError(t *testing.T) { ) require.Error(t, err) require.Contains(t, err.Error(), "access(all) resource R: TestInterfaces.A, TestInterfaces.B {}") + + var errType *sema.CheckerError + require.ErrorAs(t, err, &errType) + + checkerErr := err.(Error). + Err.(interpreter.Error). + Err.(*stdlib.InvalidContractDeploymentError). + Err.(*ParsingCheckingError). + Err.(*sema.CheckerError) + + var specificErrType *sema.DefaultFunctionConflictError + require.ErrorAs(t, checkerErr.Errors[0], &specificErrType) + + errorRange := checkerErr.Errors[0].(*sema.DefaultFunctionConflictError).Range + + require.Equal(t, errorRange.StartPos.Line, 4) } func TestRuntimeMultipleInterfaceDefaultImplementationsError(t *testing.T) { @@ -718,4 +735,20 @@ func TestRuntimeMultipleInterfaceDefaultImplementationsError(t *testing.T) { ) require.Error(t, err) require.Contains(t, err.Error(), "access(all) resource R: TestInterfaces.A, TestInterfaces.B {}") + + var errType *sema.CheckerError + require.ErrorAs(t, err, &errType) + + checkerErr := err.(Error). + Err.(interpreter.Error). + Err.(*stdlib.InvalidContractDeploymentError). + Err.(*ParsingCheckingError). + Err.(*sema.CheckerError) + + var specificErrType *sema.MultipleInterfaceDefaultImplementationsError + require.ErrorAs(t, checkerErr.Errors[0], &specificErrType) + + errorRange := checkerErr.Errors[0].(*sema.MultipleInterfaceDefaultImplementationsError).Range + + require.Equal(t, errorRange.StartPos.Line, 4) }