Merge pull request #3684 from onflow/bastian/port-internal-284

[v1.0] Port internal #284

runtime/runtime_test.go

@@ -11548,3 +11548,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)
+}

runtime/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)
@@ -450,19 +437,35 @@ func (checker *Checker) checkTypeIndexingExpression(
 		})
 	}
 
+	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
 	}
 

runtime/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
 			}
 		}

runtime/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(

runtime/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 {
@@ -4826,3 +4816,38 @@ func (*NestedReferenceError) IsUserError() {}
 func (e *NestedReferenceError) Error() string {
 	return fmt.Sprintf("cannot create a nested reference to value of type %s", e.Type.QualifiedString())
 }
+
+// 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)
+}

runtime/tests/checker/account_test.go

@@ -424,15 +424,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])
 			}
 		})
 	}
@@ -552,15 +554,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])
 			}
 		})
 	}
@@ -686,15 +690,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])
 				}
 			})
 
@@ -714,15 +720,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])
 				}
 			})
 		})
@@ -1026,9 +1034,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])
 	})
 }
 

runtime/tests/checker/arrays_dictionaries_test.go

@@ -1280,6 +1280,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.
 		},
 	)
@@ -2659,9 +2660,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) {

runtime/tests/checker/builtinfunctions_test.go

@@ -430,6 +430,7 @@ func TestCheckRevertibleRandom(t *testing.T) {
 		"missing type argument",
 		`let rand = revertibleRandom()`,
 		[]error{
+			&sema.InvocationTypeInferenceError{},
 			&sema.TypeParameterTypeInferenceError{},
 		},
 	)

runtime/tests/checker/capability_test.go

@@ -85,9 +85,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,