Revert "gate new behaviour (errors for all invalid type cases) behind…

… a feature flag" This reverts commit 3e2ecc99b3a819ffb123723f62777a2655bbbae6.
onflow · Nov 21, 2024 · aecd31a · aecd31a
1 parent b4f27e1
commit aecd31a
Show file tree

Hide file tree

Showing 16 changed files with 340 additions and 750 deletions.
diff --git a/runtime/environment.go b/runtime/environment.go
@@ -22,7 +22,6 @@ import (
 	"time"
 
 	"go.opentelemetry.io/otel/attribute"
-	"golang.org/x/mod/semver"
 
 	"github.com/onflow/cadence"
 	"github.com/onflow/cadence/runtime/activations"
@@ -1462,9 +1461,6 @@ func (e *interpreterEnvironment) newValidateAccountCapabilitiesPublishHandler()
 	}
 }
 
-// TODO:
-const MinimumRequiredVersionForInvalidTypeErrorFixes = "v1.0.3"
-
 func (e *interpreterEnvironment) configureVersionedFeatures() {
 	var (
 		minimumRequiredVersion string
@@ -1477,7 +1473,6 @@ func (e *interpreterEnvironment) configureVersionedFeatures() {
 		panic(err)
 	}
 
-	e.CheckerConfig.InvalidTypeErrorFixesEnabled =
-		minimumRequiredVersion != "" &&
-			semver.Compare(MinimumRequiredVersionForInvalidTypeErrorFixes, minimumRequiredVersion) >= 0
+	// No feature flags yet
+	_ = minimumRequiredVersion
 }
diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go
@@ -11553,66 +11553,41 @@ func TestRuntimeInvocationReturnTypeInferenceFailure(t *testing.T) {
 
 	t.Parallel()
 
-	test := func(invalidTypeErrorFixesEnabled bool) {
-
-		name := fmt.Sprintf(
-			"error fixes enabled: %v",
-			invalidTypeErrorFixesEnabled,
-		)
-
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			address := common.MustBytesToAddress([]byte{0x1})
-
-			runtimeInterface := &TestRuntimeInterface{
-				Storage: NewTestLedger(nil, nil),
-				OnGetSigningAccounts: func() ([]common.Address, error) {
-					return []common.Address{address}, nil
-				},
-				OnMinimumRequiredVersion: func() (string, error) {
-					if invalidTypeErrorFixesEnabled {
-						return MinimumRequiredVersionForInvalidTypeErrorFixes, nil
-					}
-					return "", nil
-				},
-			}
+	address := common.MustBytesToAddress([]byte{0x1})
 
-			runtime := NewTestInterpreterRuntime()
+	newRuntimeInterface := func() Interface {
 
-			nextTransactionLocation := NewTransactionLocationGenerator()
+		return &TestRuntimeInterface{
+			Storage: NewTestLedger(nil, nil),
+			OnGetSigningAccounts: func() ([]common.Address, error) {
+				return []common.Address{address}, nil
+			},
+		}
+	}
 
-			tx := []byte(`
-              transaction{
-                  prepare(signer: auth(Storage) &Account){
-                      let functions = [signer.storage.save].reverse()
-                  }
-              }
-            `)
+	runtime := NewTestInterpreterRuntime()
 
-			err := runtime.ExecuteTransaction(
-				Script{
-					Source: tx,
-				},
-				Context{
-					Interface: runtimeInterface,
-					Location:  nextTransactionLocation(),
-				},
-			)
+	nextTransactionLocation := NewTransactionLocationGenerator()
 
-			RequireError(t, err)
+	tx := []byte(`
+      transaction{
+          prepare(signer: auth(Storage) &Account){
+              let functions = [signer.storage.save].reverse()
+          }
+      }
+    `)
 
-			if invalidTypeErrorFixesEnabled {
-				var typeErr *sema.InvocationTypeInferenceError
-				require.ErrorAs(t, err, &typeErr)
-			} else {
-				var transferErr interpreter.ValueTransferTypeError
-				require.ErrorAs(t, err, &transferErr)
-			}
-		})
-	}
+	err := runtime.ExecuteTransaction(
+		Script{
+			Source: tx,
+		},
+		Context{
+			Interface: newRuntimeInterface(),
+			Location:  nextTransactionLocation(),
+		},
+	)
+	RequireError(t, err)
 
-	for _, invalidTypeErrorFixesEnabled := range []bool{true, false} {
-		test(invalidTypeErrorFixesEnabled)
-	}
+	var typeErr *sema.InvocationTypeInferenceError
+	require.ErrorAs(t, err, &typeErr)
 }
diff --git a/runtime/sema/check_invocation_expression.go b/runtime/sema/check_invocation_expression.go
@@ -503,14 +503,12 @@ func (checker *Checker) checkInvocation(
 
 	returnType = functionType.ReturnTypeAnnotation.Type.Resolve(typeArguments)
 	if returnType == nil {
-		if checker.Config.InvalidTypeErrorFixesEnabled {
-			checker.report(&InvocationTypeInferenceError{
-				Range: ast.NewRangeFromPositioned(
-					checker.memoryGauge,
-					invocationExpression,
-				),
-			})
-		}
+		checker.report(&InvocationTypeInferenceError{
+			Range: ast.NewRangeFromPositioned(
+				checker.memoryGauge,
+				invocationExpression,
+			),
+		})
 
 		returnType = InvalidType
 	}
@@ -607,14 +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 {
-				if checker.Config.InvalidTypeErrorFixesEnabled {
-					checker.report(&InvocationTypeInferenceError{
-						Range: ast.NewRangeFromPositioned(
-							checker.memoryGauge,
-							argument.Expression,
-						),
-					})
-				}
+				checker.report(&InvocationTypeInferenceError{
+					Range: ast.NewRangeFromPositioned(
+						checker.memoryGauge,
+						argument.Expression,
+					),
+				})
 				parameterType = InvalidType
 			}
 		}
@@ -690,14 +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 {
-				if checker.Config.InvalidTypeErrorFixesEnabled {
-					checker.report(&InvocationTypeInferenceError{
-						Range: ast.NewRangeFromPositioned(
-							checker.memoryGauge,
-							argument.Expression,
-						),
-					})
-				}
+				checker.report(&InvocationTypeInferenceError{
+					Range: ast.NewRangeFromPositioned(
+						checker.memoryGauge,
+						argument.Expression,
+					),
+				})
 				parameterType = InvalidType
 			}
 		}

diff --git a/runtime/sema/checker.go b/runtime/sema/checker.go
@@ -886,16 +886,12 @@ func (checker *Checker) ConvertType(t ast.Type) Type {
 	case *ast.InstantiationType:
 		return checker.convertInstantiationType(t)
 
-	case nil:
-		if checker.Config.InvalidTypeErrorFixesEnabled {
-			checker.report(&UnconvertableTypeError{
-				Range: ast.NewRangeFromPositioned(checker.memoryGauge, t),
-			})
-		}
+	default:
+		checker.report(&UnconvertableTypeError{
+			Range: ast.NewRangeFromPositioned(checker.memoryGauge, t),
+		})
 		return InvalidType
 	}
-
-	panic(&astTypeConversionError{invalidASTType: t})
 }
 
 func CheckIntersectionType(
@@ -2615,9 +2611,7 @@ func (checker *Checker) visitExpressionWithForceType(
 
 	actualType = ast.AcceptExpression[Type](expr, checker)
 
-	if checker.Config.InvalidTypeErrorFixesEnabled {
-		checker.checkErrorsForInvalidExpressionTypes(actualType, expectedType)
-	}
+	checker.checkErrorsForInvalidExpressionTypes(actualType, expectedType)
 
 	if checker.Config.ExtendedElaborationEnabled {
 		checker.Elaboration.SetExpressionTypes(

diff --git a/runtime/sema/config.go b/runtime/sema/config.go
@@ -55,6 +55,4 @@ type Config struct {
 	AllowStaticDeclarations bool
 	// AttachmentsEnabled determines if attachments are enabled
 	AttachmentsEnabled bool
-	// InvalidTypeErrorFixesEnabled determines if errors for invalid type errors are reported
-	InvalidTypeErrorFixesEnabled bool
 }
diff --git a/runtime/sema/errors.go b/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 {