diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 7f942cbdd..e29363190 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11734,3 +11734,46 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { var redeclarationError *sema.RedeclarationError require.ErrorAs(t, err, &redeclarationError) } + +func TestRuntimeInvocationReturnTypeInferenceFailure(t *testing.T) { + + t.Parallel() + + address := common.MustBytesToAddress([]byte{0x1}) + + newRuntimeInterface := func() Interface { + + return &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]common.Address, error) { + return []common.Address{address}, nil + }, + } + } + + runtime := NewTestInterpreterRuntime() + + nextTransactionLocation := NewTransactionLocationGenerator() + + tx := []byte(` + transaction{ + prepare(signer: auth(Storage) &Account){ + let functions = [signer.storage.save].reverse() + } + } + `) + + err := runtime.ExecuteTransaction( + Script{ + Source: tx, + }, + Context{ + Interface: newRuntimeInterface(), + Location: nextTransactionLocation(), + }, + ) + RequireError(t, err) + + var typeErr *sema.InvocationTypeInferenceError + require.ErrorAs(t, err, &typeErr) +} diff --git a/sema/account_test.go b/sema/account_test.go index 6084a5042..7c7bc60fd 100644 --- a/sema/account_test.go +++ b/sema/account_test.go @@ -425,15 +425,17 @@ func TestCheckAccountStorageLoad(t *testing.T) { ) if domain == common.PathDomainStorage { - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) } else { - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) require.IsType(t, &sema.TypeMismatchError{}, errs[0]) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[2]) } }) } @@ -553,15 +555,17 @@ func TestCheckAccountStorageCopy(t *testing.T) { ) if domain == common.PathDomainStorage { - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) } else { - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) require.IsType(t, &sema.TypeMismatchError{}, errs[0]) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[2]) } }) } @@ -687,15 +691,17 @@ func TestCheckAccountStorageBorrow(t *testing.T) { ) if domain == common.PathDomainStorage { - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) } else { - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) require.IsType(t, &sema.TypeMismatchError{}, errs[0]) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[2]) } }) @@ -715,15 +721,17 @@ func TestCheckAccountStorageBorrow(t *testing.T) { ) if domain == common.PathDomainStorage { - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) } else { - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) require.IsType(t, &sema.TypeMismatchError{}, errs[0]) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[1]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[2]) } }) }) @@ -1027,9 +1035,10 @@ func TestCheckAccountContractsBorrow(t *testing.T) { } `) - errors := RequireCheckerErrors(t, err, 1) + errors := RequireCheckerErrors(t, err, 2) - assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errors[0]) + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errors[0]) + assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errors[1]) }) } diff --git a/sema/arrays_dictionaries_test.go b/sema/arrays_dictionaries_test.go index 6e5a512d0..8ae10c8af 100644 --- a/sema/arrays_dictionaries_test.go +++ b/sema/arrays_dictionaries_test.go @@ -1281,6 +1281,7 @@ func TestCheckArrayMapInvalidArgs(t *testing.T) { `, []sema.SemanticError{ &sema.TypeMismatchError{}, + &sema.InvocationTypeInferenceError{}, // since we're not passing a function. &sema.TypeParameterTypeInferenceError{}, // since we're not passing a function. }, ) @@ -2660,9 +2661,10 @@ func TestCheckArrayToConstantSizedMissingTypeArgument(t *testing.T) { } `) - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) } func TestCheckArrayReferenceTypeInference(t *testing.T) { diff --git a/sema/builtinfunctions_test.go b/sema/builtinfunctions_test.go index 07ca19059..b82471b30 100644 --- a/sema/builtinfunctions_test.go +++ b/sema/builtinfunctions_test.go @@ -431,6 +431,7 @@ func TestCheckRevertibleRandom(t *testing.T) { "missing type argument", `let rand = revertibleRandom()`, []error{ + &sema.InvocationTypeInferenceError{}, &sema.TypeParameterTypeInferenceError{}, }, ) diff --git a/sema/capability_test.go b/sema/capability_test.go index cb430f7b8..ba1dd9f7c 100644 --- a/sema/capability_test.go +++ b/sema/capability_test.go @@ -86,9 +86,10 @@ func TestCheckCapability_borrow(t *testing.T) { let r = capability.borrow() `) - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + require.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) }) for _, auth := range []sema.Access{sema.UnauthorizedAccess, diff --git a/sema/check_expression.go b/sema/check_expression.go index caff4f0ee..2d05b41a9 100644 --- a/sema/check_expression.go +++ b/sema/check_expression.go @@ -417,20 +417,7 @@ func (checker *Checker) visitIndexExpression( return InvalidType } - elementType := checker.checkTypeIndexingExpression(typeIndexedType, indexExpression) - if elementType == InvalidType { - checker.report( - &InvalidTypeIndexingError{ - BaseType: typeIndexedType, - IndexingExpression: indexExpression.IndexingExpression, - Range: ast.NewRangeFromPositioned( - checker.memoryGauge, - indexExpression.IndexingExpression, - ), - }, - ) - } - return elementType + return checker.checkTypeIndexingExpression(typeIndexedType, indexExpression) } reportNonIndexable(targetType) @@ -444,19 +431,35 @@ func (checker *Checker) checkTypeIndexingExpression( targetExpression := indexExpression.TargetExpression + reportInvalid := func() { + checker.report( + &InvalidTypeIndexingError{ + BaseType: targetType, + IndexingExpression: indexExpression.IndexingExpression, + Range: ast.NewRangeFromPositioned( + checker.memoryGauge, + indexExpression.IndexingExpression, + ), + }, + ) + } + expressionType := ast.ExpressionAsType(indexExpression.IndexingExpression) if expressionType == nil { + reportInvalid() return InvalidType } nominalTypeExpression, isNominalType := expressionType.(*ast.NominalType) if !isNominalType { + reportInvalid() return InvalidType } nominalType := checker.convertNominalType(nominalTypeExpression) if !targetType.IsValidIndexingType(nominalType) { + reportInvalid() return InvalidType } diff --git a/sema/check_invocation_expression.go b/sema/check_invocation_expression.go index 49f714f24..d1d5ea6e9 100644 --- a/sema/check_invocation_expression.go +++ b/sema/check_invocation_expression.go @@ -503,7 +503,13 @@ func (checker *Checker) checkInvocation( returnType = functionType.ReturnTypeAnnotation.Type.Resolve(typeArguments) if returnType == nil { - // TODO: report error? does `checkTypeParameterInference` below already do that? + checker.report(&InvocationTypeInferenceError{ + Range: ast.NewRangeFromPositioned( + checker.memoryGauge, + invocationExpression, + ), + }) + returnType = InvalidType } @@ -599,6 +605,12 @@ func (checker *Checker) checkInvocationRequiredArgument( parameterType = parameterType.Resolve(typeParameters) // If the type parameter could not be resolved, use the invalid type. if parameterType == nil { + checker.report(&InvocationTypeInferenceError{ + Range: ast.NewRangeFromPositioned( + checker.memoryGauge, + argument.Expression, + ), + }) parameterType = InvalidType } } @@ -674,6 +686,12 @@ func (checker *Checker) checkInvocationRequiredArgument( parameterType = parameterType.Resolve(typeParameters) // If the type parameter could not be resolved, use the invalid type. if parameterType == nil { + checker.report(&InvocationTypeInferenceError{ + Range: ast.NewRangeFromPositioned( + checker.memoryGauge, + argument.Expression, + ), + }) parameterType = InvalidType } } diff --git a/sema/checker.go b/sema/checker.go index d94b2b64d..a3c0408b9 100644 --- a/sema/checker.go +++ b/sema/checker.go @@ -886,12 +886,12 @@ func (checker *Checker) ConvertType(t ast.Type) Type { case *ast.InstantiationType: return checker.convertInstantiationType(t) - case nil: - // The AST might contain "holes" if parsing failed + default: + checker.report(&UnconvertableTypeError{ + Range: ast.NewRangeFromPositioned(checker.memoryGauge, t), + }) return InvalidType } - - panic(&astTypeConversionError{invalidASTType: t}) } func CheckIntersectionType( @@ -2611,6 +2611,8 @@ func (checker *Checker) visitExpressionWithForceType( actualType = ast.AcceptExpression[Type](expr, checker) + checker.checkErrorsForInvalidExpressionTypes(actualType, expectedType) + if checker.Config.ExtendedElaborationEnabled { checker.Elaboration.SetExpressionTypes( expr, @@ -2645,6 +2647,20 @@ func (checker *Checker) visitExpressionWithForceType( return actualType, actualType } +func (checker *Checker) checkErrorsForInvalidExpressionTypes(actualType Type, expectedType Type) { + // Defensive check: If an invalid type was produced, + // then also an error should have been reported for the invalid program. + // + // Check for errors first, which is cheap, + // before checking for an invalid type, which is more expensive. + + if len(checker.errors) == 0 && + (actualType.IsInvalidType() || (expectedType != nil && expectedType.IsInvalidType())) { + + panic(errors.NewUnexpectedError("invalid type produced without error")) + } +} + func (checker *Checker) expressionRange(expression ast.Expression) ast.Range { if indexExpr, ok := expression.(*ast.IndexExpression); ok { return ast.NewRange( diff --git a/sema/conditions_test.go b/sema/conditions_test.go index afcc7762f..e3a996046 100644 --- a/sema/conditions_test.go +++ b/sema/conditions_test.go @@ -266,10 +266,11 @@ func TestCheckInvalidFunctionPostConditionWithBeforeAndNoArgument(t *testing.T) } `) - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) assert.IsType(t, &sema.InsufficientArgumentsError{}, errs[0]) - assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errs[1]) + assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[2]) }) t.Run("emit condition", func(t *testing.T) { @@ -285,10 +286,11 @@ func TestCheckInvalidFunctionPostConditionWithBeforeAndNoArgument(t *testing.T) } `) - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) assert.IsType(t, &sema.InsufficientArgumentsError{}, errs[0]) - assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errs[1]) + assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[2]) }) } diff --git a/sema/dictionary_test.go b/sema/dictionary_test.go index edd9dd83c..b159e55c9 100644 --- a/sema/dictionary_test.go +++ b/sema/dictionary_test.go @@ -41,7 +41,9 @@ func TestCheckIncompleteDictionaryType(t *testing.T) { }, ) - require.NoError(t, err) + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, errs[0], &sema.UnconvertableTypeError{}) assert.Equal(t, &sema.DictionaryType{ diff --git a/sema/errors.go b/sema/errors.go index 1760e33eb..80daa240c 100644 --- a/sema/errors.go +++ b/sema/errors.go @@ -51,16 +51,6 @@ func ErrorMessageExpectedActualTypes( return } -// astTypeConversionError - -type astTypeConversionError struct { - invalidASTType ast.Type -} - -func (e *astTypeConversionError) Error() string { - return fmt.Sprintf("cannot convert unsupported AST type: %#+v", e.invalidASTType) -} - // unsupportedOperation type unsupportedOperation struct { @@ -4883,3 +4873,38 @@ var _ errors.ErrorNote = ResultVariablePostConditionsNote{} func (ResultVariablePostConditionsNote) Message() string { return "post-conditions declared here" } + +// InvocationTypeInferenceError + +type InvocationTypeInferenceError struct { + ast.Range +} + +var _ SemanticError = &InvocationTypeInferenceError{} +var _ errors.UserError = &InvocationTypeInferenceError{} + +func (e *InvocationTypeInferenceError) isSemanticError() {} + +func (*InvocationTypeInferenceError) IsUserError() {} + +func (e *InvocationTypeInferenceError) Error() string { + return "cannot infer type of invocation" +} + +// UnconvertableTypeError + +type UnconvertableTypeError struct { + Type ast.Type + ast.Range +} + +var _ SemanticError = &UnconvertableTypeError{} +var _ errors.UserError = &UnconvertableTypeError{} + +func (e *UnconvertableTypeError) isSemanticError() {} + +func (*UnconvertableTypeError) IsUserError() {} + +func (e *UnconvertableTypeError) Error() string { + return fmt.Sprintf("cannot convert type `%s`", e.Type) +} diff --git a/sema/genericfunction_test.go b/sema/genericfunction_test.go index f88e9ac00..8a2dd7265 100644 --- a/sema/genericfunction_test.go +++ b/sema/genericfunction_test.go @@ -453,9 +453,10 @@ func TestCheckGenericFunctionInvocation(t *testing.T) { }, ) - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) - assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[0]) + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) + assert.IsType(t, &sema.TypeParameterTypeInferenceError{}, errs[1]) }) t.Run("valid: one type parameter, one type argument, no parameters, no arguments, return type", func(t *testing.T) { diff --git a/sema/invalid_test.go b/sema/invalid_test.go index 51ea25498..26857454c 100644 --- a/sema/invalid_test.go +++ b/sema/invalid_test.go @@ -22,8 +22,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/onflow/cadence/common" + "github.com/onflow/cadence/errors" "github.com/onflow/cadence/sema" + "github.com/onflow/cadence/stdlib" . "github.com/onflow/cadence/test_utils/sema_utils" ) @@ -202,3 +206,118 @@ func TestCheckSpuriousCastWithInvalidValueTypeMismatch(t *testing.T) { assert.IsType(t, &sema.NotDeclaredError{}, errs[0]) } + +func TestCheckInvalidInvocationFunctionReturnType(t *testing.T) { + + t.Parallel() + + typeParameter := &sema.TypeParameter{ + Name: "T", + } + + fType := &sema.FunctionType{ + TypeParameters: []*sema.TypeParameter{ + typeParameter, + }, + ReturnTypeAnnotation: sema.NewTypeAnnotation( + &sema.GenericType{ + TypeParameter: typeParameter, + }, + ), + } + + baseValueActivation := sema.NewVariableActivation(sema.BaseValueActivation) + baseValueActivation.DeclareValue(stdlib.StandardLibraryValue{ + Type: fType, + Name: "f", + Kind: common.DeclarationKindFunction, + }) + + _, err := ParseAndCheckWithOptions(t, + ` + let res = [f].reverse() + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + BaseValueActivationHandler: func(_ common.Location) *sema.VariableActivation { + return baseValueActivation + }, + }, + }, + ) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) +} + +func TestCheckInvalidTypeDefensiveCheck(t *testing.T) { + + t.Parallel() + + baseValueActivation := sema.NewVariableActivation(sema.BaseValueActivation) + baseValueActivation.DeclareValue(stdlib.StandardLibraryValue{ + Type: sema.InvalidType, + Name: "invalid", + Kind: common.DeclarationKindConstant, + }) + + var r any + func() { + defer func() { + r = recover() + }() + + _, _ = ParseAndCheckWithOptions(t, + ` + let res = invalid + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + BaseValueActivationHandler: func(_ common.Location) *sema.VariableActivation { + return baseValueActivation + }, + }, + }, + ) + }() + + require.IsType(t, errors.UnexpectedError{}, r) + err := r.(errors.UnexpectedError) + require.ErrorContains(t, err, "invalid type produced without error") +} + +func TestCheckInvalidTypeIndexing(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + struct S {} + let s = S() + let res = s[[]] + `) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.InvalidTypeIndexingError{}, errs[0]) +} + +func TestCheckInvalidRemove(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + struct S {} + + attachment A for S {} + + fun test() { + let s = S() + remove B from s + } + `) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.NotDeclaredError{}, errs[0]) +} diff --git a/sema/invocation_test.go b/sema/invocation_test.go index 1d0543b2c..38f4cd488 100644 --- a/sema/invocation_test.go +++ b/sema/invocation_test.go @@ -593,3 +593,62 @@ func TestCheckArgumentLabels(t *testing.T) { }) } + +func TestCheckInvocationWithIncorrectTypeParameter(t *testing.T) { + + t.Parallel() + + // function type has incorrect type-arguments: + // `fun Foo(_ a: R)` + // + funcType := &sema.FunctionType{ + ReturnTypeAnnotation: sema.VoidTypeAnnotation, + TypeParameters: []*sema.TypeParameter{ + { + Name: "T", + TypeBound: sema.AnyStructType, + }, + }, + Parameters: []sema.Parameter{ + { + Label: sema.ArgumentLabelNotRequired, + Identifier: "a", + TypeAnnotation: sema.NewTypeAnnotation( + &sema.GenericType{ + TypeParameter: &sema.TypeParameter{ + Name: "R", // This is an incorrect/undefined type-parameter + TypeBound: sema.AnyStructType, + }, + }, + ), + }, + }, + } + + baseValueActivation := sema.NewVariableActivation(sema.BaseValueActivation) + baseValueActivation.DeclareValue(stdlib.NewStandardLibraryStaticFunction( + "foo", + funcType, + "", + nil, // no need, we only type-check + )) + + _, err := ParseAndCheckWithOptions(t, + ` + access(all) fun test() { + foo("hello") + } + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + BaseValueActivationHandler: func(_ common.Location) *sema.VariableActivation { + return baseValueActivation + }, + }, + }, + ) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.InvocationTypeInferenceError{}, errs[0]) +}