[CFE] Specific error message when using unavailable variables in expression evaluation

So instead of either being told that a variable doesn't exist, or
using a field or something instead of a local, you'll now get a message
saying "<variable> is unavailable in this expression evaluation".

Bug: #53996
Bug: #53087
Bug: #45913 (sort of?)
Bug: #53688
Change-Id: I6726209584e414ddc40f3e7ff6bffa9e7283e9d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445701
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>

Author: jensjoha

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -3732,6 +3732,31 @@ const MessageCode codeExportedMain = const MessageCode(
   problemMessage: r"""This is exported 'main' declaration.""",
 );
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<Message Function(String name)>
+codeExpressionEvaluationKnownVariableUnavailable = const Template<
+  Message Function(String name)
+>(
+  "ExpressionEvaluationKnownVariableUnavailable",
+  problemMessageTemplate:
+      r"""The variable '#name' is unavailable in this expression evaluation.""",
+  withArguments: _withArgumentsExpressionEvaluationKnownVariableUnavailable,
+);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsExpressionEvaluationKnownVariableUnavailable(
+  String name,
+) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  return new Message(
+    codeExpressionEvaluationKnownVariableUnavailable,
+    problemMessage:
+        """The variable '${name}' is unavailable in this expression evaluation.""",
+    arguments: {'name': name},
+  );
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const MessageCode codeExpressionNotMetadata = const MessageCode(
   "ExpressionNotMetadata",

pkg/front_end/lib/src/base/incremental_compiler.dart

@@ -9,6 +9,7 @@ import 'dart:typed_data';
 import 'package:_fe_analyzer_shared/src/scanner/abstract_scanner.dart'
     show ScannerConfiguration;
 import 'package:front_end/src/base/name_space.dart';
+import 'package:front_end/src/type_inference/inference_results.dart';
 import 'package:kernel/binary/ast_from_binary.dart'
     show
         BinaryBuilderWithMetadata,
@@ -50,7 +51,9 @@ import 'package:kernel/kernel.dart'
         TypeParameter,
         VariableDeclaration,
         VisitorDefault,
-        VisitorVoidMixin;
+        VisitorVoidMixin,
+        VariableGet,
+        VariableSet;
 import 'package:kernel/kernel.dart' as kernel show Combinator;
 import 'package:kernel/reference_from_index.dart';
 import 'package:kernel/target/changed_structure_notifier.dart'
@@ -87,6 +90,9 @@ import '../source/source_compilation_unit.dart' show SourceCompilationUnitImpl;
 import '../source/source_library_builder.dart'
     show ImplicitLanguageVersion, SourceLibraryBuilder;
 import '../source/source_loader.dart';
+import '../type_inference/inference_helper.dart' show InferenceHelper;
+import '../type_inference/inference_visitor.dart'
+    show ExpressionEvaluationHelper;
 import '../util/error_reporter_file_copier.dart' show saveAsGzip;
 import '../util/experiment_environment_getter.dart'
     show enableIncrementalCompilerBenchmarking, getExperimentEnvironment;
@@ -1693,6 +1699,21 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
                     hasDeclaredInitializer: true,
                     initializer: def.value.initializer)
                   ..fileOffset = def.value.fileOffset);
+              } else if (def.value.isInitializingFormal ||
+                  def.value.isSuperInitializingFormal) {
+                // An (super) initializing formal parameter of a constructor
+                // should not shadow the field it was used to initialize,
+                // so we'll ignore it.
+              } else {
+                // Non-const variable we should know about but wasn't told
+                // about. Maybe the variable was optimized out? Maybe it wasn't
+                // captured? Either way there's something shadowing any fields
+                // etc.
+                extraKnownVariables.add(new VariableDeclarationImpl(
+                  def.key,
+                  type: def.value.type,
+                  isConst: false,
+                )..fileOffset = def.value.fileOffset);
               }
             } else if (existingType is DynamicType ||
                 _ExtensionTypeFinder.isOrContainsExtensionType(
@@ -1932,14 +1953,18 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
           new Name(syntheticProcedureName), ProcedureKind.Method, parameters,
           isStatic: isStatic, fileUri: debugLibrary.fileUri);
 
+      ExpressionEvaluationHelper expressionEvaluationHelper =
+          new ExpressionEvaluationHelperImpl(extraKnownVariables);
+
       Expression compiledExpression = await lastGoodKernelTarget.loader
           .buildExpression(
               debugLibrary,
               className ?? extensionName,
               (className != null && !isStatic) || extensionThis != null,
               procedure,
               extensionThis,
-              extraKnownVariables);
+              extraKnownVariables,
+              expressionEvaluationHelper);
 
       parameters.body = new ReturnStatement(compiledExpression)
         ..parent = parameters;
@@ -2160,6 +2185,55 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   }
 }
 
+// Coverage-ignore(suite): Not run.
+class ExpressionEvaluationHelperImpl implements ExpressionEvaluationHelper {
+  final Set<VariableDeclarationImpl> knownButUnavailable = {};
+
+  ExpressionEvaluationHelperImpl(List<VariableDeclarationImpl> extraKnown) {
+    for (VariableDeclarationImpl variable in extraKnown) {
+      if (variable.isConst) {
+        // We allow const variables - these are inlined (we check
+        // `alwaysInlineConstants` in `compileExpression`).
+        continue;
+      }
+      knownButUnavailable.add(variable);
+    }
+  }
+
+  @override
+  ExpressionInferenceResult? visitVariableGet(
+      VariableGet node, DartType typeContext, InferenceHelper helper) {
+    if (knownButUnavailable.contains(node.variable)) {
+      return _returnKnownVariableUnavailable(node, node.variable, helper);
+    }
+    return null;
+  }
+
+  @override
+  ExpressionInferenceResult? visitVariableSet(
+      VariableSet node, DartType typeContext, InferenceHelper helper) {
+    if (knownButUnavailable.contains(node.variable)) {
+      return _returnKnownVariableUnavailable(node, node.variable, helper);
+    }
+    return null;
+  }
+
+  ExpressionInferenceResult _returnKnownVariableUnavailable(
+      Expression node, VariableDeclaration variable, InferenceHelper helper) {
+    return new ExpressionInferenceResult(
+        variable.type,
+        helper.wrapInProblem(
+          node,
+          codeExpressionEvaluationKnownVariableUnavailable
+              .withArguments(variable.name!),
+          node.fileOffset,
+          variable.name!.length,
+          errorHasBeenReported: false,
+          includeExpression: false,
+        ));
+  }
+}
+
 // Coverage-ignore(suite): Not run.
 class _ExtensionTypeFinder extends VisitorDefault<void> with VisitorVoidMixin {
   static bool isOrContainsExtensionType(DartType type) {

pkg/front_end/lib/src/kernel/body_builder.dart

@@ -112,6 +112,8 @@ import '../source/type_parameter_factory.dart';
 import '../source/value_kinds.dart';
 import '../type_inference/inference_results.dart'
     show InitializerInferenceResult;
+import '../type_inference/inference_visitor.dart'
+    show ExpressionEvaluationHelper;
 import '../type_inference/type_inferrer.dart'
     show TypeInferrer, InferredFunctionBody;
 import '../type_inference/type_schema.dart' show UnknownType;
@@ -1253,7 +1255,8 @@ class BodyBuilder extends StackListenerImpl
           _context.memberNameOffset,
           _context.returnTypeContext,
           asyncModifier,
-          body);
+          body,
+          null);
       body = inferredFunctionBody.body;
       function.emittedValueType = inferredFunctionBody.emittedValueType;
       assert(function.asyncMarker == AsyncMarker.Sync ||
@@ -1590,7 +1593,8 @@ class BodyBuilder extends StackListenerImpl
       Parser parser,
       Token token,
       FunctionNode parameters,
-      List<VariableDeclarationImpl> extraKnownVariables) {
+      List<VariableDeclarationImpl> extraKnownVariables,
+      ExpressionEvaluationHelper expressionEvaluationHelper) {
     int fileOffset = offsetForToken(token);
     List<NominalParameterBuilder>? typeParameterBuilders;
     for (TypeParameter typeParameter in parameters.typeParameters) {
@@ -1678,7 +1682,12 @@ class BodyBuilder extends StackListenerImpl
     }
 
     InferredFunctionBody inferredFunctionBody = typeInferrer.inferFunctionBody(
-        this, fileOffset, const DynamicType(), AsyncMarker.Sync, fakeReturn);
+        this,
+        fileOffset,
+        const DynamicType(),
+        AsyncMarker.Sync,
+        fakeReturn,
+        expressionEvaluationHelper);
     assert(
         fakeReturn == inferredFunctionBody.body,
         "Previously implicit assumption about inferFunctionBody "
@@ -9085,13 +9094,17 @@ class BodyBuilder extends StackListenerImpl
   @override
   Expression wrapInProblem(
       Expression expression, Message message, int fileOffset, int length,
-      {List<LocatedMessage>? context}) {
+      {List<LocatedMessage>? context,
+      bool? errorHasBeenReported,
+      bool includeExpression = true}) {
     CfeSeverity severity = message.code.severity;
     if (severity == CfeSeverity.error) {
       return wrapInLocatedProblem(
           expression, message.withLocation(uri, fileOffset, length),
           context: context,
-          errorHasBeenReported: expression is InvalidExpression);
+          errorHasBeenReported:
+              errorHasBeenReported ?? expression is InvalidExpression,
+          includeExpression: includeExpression);
     } else {
       // Coverage-ignore-block(suite): Not run.
       if (expression is! InvalidExpression) {
@@ -9103,7 +9116,9 @@ class BodyBuilder extends StackListenerImpl
 
   @override
   Expression wrapInLocatedProblem(Expression expression, LocatedMessage message,
-      {List<LocatedMessage>? context, bool errorHasBeenReported = false}) {
+      {List<LocatedMessage>? context,
+      bool errorHasBeenReported = false,
+      bool includeExpression = true}) {
     // TODO(askesc): Produce explicit error expression wrapping the original.
     // See [issue 29717](https://github.com/dart-lang/sdk/issues/29717)
     int offset = expression.fileOffset;
@@ -9113,7 +9128,7 @@ class BodyBuilder extends StackListenerImpl
     return buildProblem(
         message.messageObject, message.charOffset, message.length,
         context: context,
-        expression: expression,
+        expression: includeExpression ? expression : null,
         errorHasBeenReported: errorHasBeenReported);
   }
 

pkg/front_end/lib/src/kernel/constant_evaluator.dart

@@ -407,10 +407,7 @@ class ConstantsTransformer extends RemovingTransformer {
             makeConstantExpression(constant, initializer)..parent = node;
 
         // If this constant is inlined, remove it.
-        if (!keepLocals &&
-            // Coverage-ignore(suite): Not run.
-            shouldInline(initializer)) {
-          // Coverage-ignore-block(suite): Not run.
+        if (!keepLocals && shouldInline(initializer)) {
           if (constant is! UnevaluatedConstant) {
             // If the constant is unevaluated we need to keep the expression,
             // so that, in the case the constant contains error but the local
@@ -593,7 +590,6 @@ class ConstantsTransformer extends RemovingTransformer {
       }
     }
     if (storeIndex < node.statements.length) {
-      // Coverage-ignore-block(suite): Not run.
       node.statements.length = storeIndex;
     }
     return node;

pkg/front_end/lib/src/kernel/expression_generator_helper.dart

@@ -149,7 +149,7 @@ abstract class ExpressionGeneratorHelper implements InferenceHelper {
       Message message, int charOffset, int length);
 
   Expression wrapInLocatedProblem(Expression expression, LocatedMessage message,
-      {List<LocatedMessage>? context});
+      {List<LocatedMessage>? context, bool includeExpression = true});
 
   Expression evaluateArgumentsBefore(
       Arguments arguments, Expression expression);

pkg/front_end/lib/src/source/source_loader.dart

@@ -70,6 +70,8 @@ import '../kernel/kernel_helper.dart'
     show DelayedDefaultValueCloner, TypeDependency;
 import '../kernel/kernel_target.dart' show KernelTarget;
 import '../kernel/type_builder_computer.dart' show TypeBuilderComputer;
+import '../type_inference/inference_visitor.dart'
+    show ExpressionEvaluationHelper;
 import '../type_inference/type_inference_engine.dart';
 import '../type_inference/type_inferrer.dart';
 import '../util/reference_map.dart';
@@ -1249,7 +1251,8 @@ severity: $severity
       bool isClassInstanceMember,
       Procedure procedure,
       VariableDeclaration? extensionThis,
-      List<VariableDeclarationImpl> extraKnownVariables) async {
+      List<VariableDeclarationImpl> extraKnownVariables,
+      ExpressionEvaluationHelper expressionEvaluationHelper) async {
     // TODO(johnniwinther): Support expression compilation in a specific
     //  compilation unit.
     LookupScope memberScope =
@@ -1308,14 +1311,16 @@ severity: $severity
     }
 
     return listener.parseSingleExpression(
-        new Parser(listener,
-            useImplicitCreationExpression: useImplicitCreationExpressionInCfe,
-            allowPatterns: libraryBuilder.libraryFeatures.patterns.isEnabled,
-            enableFeatureEnhancedParts:
-                libraryBuilder.libraryFeatures.enhancedParts.isEnabled),
-        token,
-        procedure.function,
-        extraKnownVariables);
+      new Parser(listener,
+          useImplicitCreationExpression: useImplicitCreationExpressionInCfe,
+          allowPatterns: libraryBuilder.libraryFeatures.patterns.isEnabled,
+          enableFeatureEnhancedParts:
+              libraryBuilder.libraryFeatures.enhancedParts.isEnabled),
+      token,
+      procedure.function,
+      extraKnownVariables,
+      expressionEvaluationHelper,
+    );
   }
 
   DietListener createDietListener(SourceLibraryBuilder library,

pkg/front_end/lib/src/type_inference/inference_helper.dart

@@ -24,7 +24,9 @@ abstract class InferenceHelper {
 
   Expression wrapInProblem(
       Expression expression, Message message, int fileOffset, int length,
-      {List<LocatedMessage>? context});
+      {List<LocatedMessage>? context,
+      bool? errorHasBeenReported,
+      bool includeExpression = true});
 
   String superConstructorNameForDiagnostics(String name);
 

pkg/front_end/lib/src/type_inference/inference_visitor.dart

@@ -174,8 +174,17 @@ class InferenceVisitorImpl extends InferenceVisitorBase
   // variable was declared outside the try statement or local function.
   bool _inTryOrLocalFunction = false;
 
-  InferenceVisitorImpl(TypeInferrerImpl inferrer, InferenceHelper helper,
-      this._constructorBuilder, this.operations, this.typeAnalyzerOptions)
+  /// Helper used to issue correct error messages and avoid access to
+  /// unavailable variables upon expression evaluation.
+  final ExpressionEvaluationHelper? expressionEvaluationHelper;
+
+  InferenceVisitorImpl(
+      TypeInferrerImpl inferrer,
+      InferenceHelper helper,
+      this._constructorBuilder,
+      this.operations,
+      this.typeAnalyzerOptions,
+      this.expressionEvaluationHelper)
       : super(inferrer, helper);
 
   @override
@@ -9252,6 +9261,14 @@ class InferenceVisitorImpl extends InferenceVisitorBase
   @override
   ExpressionInferenceResult visitVariableSet(
       VariableSet node, DartType typeContext) {
+    if (expressionEvaluationHelper != null) {
+      // Coverage-ignore-block(suite): Not run.
+      ExpressionInferenceResult? result = expressionEvaluationHelper
+          ?.visitVariableSet(node, typeContext, helper);
+      if (result != null) {
+        return result;
+      }
+    }
     VariableDeclarationImpl variable = node.variable as VariableDeclarationImpl;
     bool isDefinitelyAssigned = flowAnalysis.isAssigned(variable);
     bool isDefinitelyUnassigned = flowAnalysis.isUnassigned(variable);
@@ -9551,6 +9568,14 @@ class InferenceVisitorImpl extends InferenceVisitorBase
   @override
   ExpressionInferenceResult visitVariableGet(
       VariableGet node, DartType typeContext) {
+    if (expressionEvaluationHelper != null) {
+      // Coverage-ignore-block(suite): Not run.
+      ExpressionInferenceResult? result = expressionEvaluationHelper
+          ?.visitVariableGet(node, typeContext, helper);
+      if (result != null) {
+        return result;
+      }
+    }
     if (node is! VariableGetImpl) {
       // Coverage-ignore-block(suite): Not run.
       // This node is created as part of a lowering and doesn't need inference.
@@ -12375,3 +12400,11 @@ class MapEntryInferenceContext extends CollectionElementInferenceContext {
             inferredSpreadTypes: inferredSpreadTypes,
             inferredConditionTypes: inferredConditionTypes);
 }
+
+abstract class ExpressionEvaluationHelper {
+  ExpressionInferenceResult? visitVariableGet(
+      VariableGet node, DartType typeContext, InferenceHelper helper);
+
+  ExpressionInferenceResult? visitVariableSet(
+      VariableSet node, DartType typeContext, InferenceHelper helper);
+}