Make an interface type to represent promotable variables.

And updade flow analysis clients to use this interface, so that we
have no risk of accidentally trying to promote things that shouldn't
be promotable.  (Previously we used VariableElement, which would have
allowed fields).

Fixes https://github.com/dart-lang/sdk/issues/38572.

Change-Id: I225d3adabea503ca7eb9042516cf95d5a257fec6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/119162
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

pkg/analyzer/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.38.5-dev
+* Added the interface `PromotableElement`, which representing
+  variables that can be type promoted (local variables and parameters,
+  but not fields).
+
 ## 0.38.4
 * Bug fixes: #33300, #38484, #38505.
 

pkg/analyzer/lib/dart/element/element.dart

@@ -1423,7 +1423,7 @@ abstract class LocalElement implements Element {
 /// A local variable.
 ///
 /// Clients may not extend, implement or mix-in this class.
-abstract class LocalVariableElement implements LocalElement, VariableElement {}
+abstract class LocalVariableElement implements PromotableElement {}
 
 /// An element that represents a method defined within a class.
 ///
@@ -1472,7 +1472,7 @@ abstract class NamespaceCombinator {}
 ///
 /// Clients may not extend, implement or mix-in this class.
 abstract class ParameterElement
-    implements LocalElement, VariableElement, ConstantEvaluationTarget {
+    implements PromotableElement, ConstantEvaluationTarget {
   /// Return the Dart code of the default value, or `null` if no default value.
   String get defaultValueCode;
 
@@ -1570,6 +1570,12 @@ abstract class PrefixElement implements Element {
   List<LibraryElement> get importedLibraries;
 }
 
+/// A variable that might be subject to type promotion.  This might be a local
+/// variable or a parameter.
+///
+/// Clients may not extend, implement or mix-in this class.
+abstract class PromotableElement implements LocalElement, VariableElement {}
+
 /// A getter or a setter. Note that explicitly defined property accessors
 /// implicitly define a synthetic field. Symmetrically, synthetic accessors are
 /// implicitly created for explicitly defined fields. The following rules apply:

pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart

@@ -13,18 +13,18 @@ import 'package:front_end/src/fasta/flow_analysis/flow_analysis.dart';
 import 'package:meta/meta.dart';
 
 class AnalyzerFunctionBodyAccess
-    implements FunctionBodyAccess<VariableElement> {
+    implements FunctionBodyAccess<PromotableElement> {
   final FunctionBody node;
 
   AnalyzerFunctionBodyAccess(this.node);
 
   @override
-  bool isPotentiallyMutatedInClosure(VariableElement variable) {
+  bool isPotentiallyMutatedInClosure(PromotableElement variable) {
     return node.isPotentiallyMutatedInClosure(variable);
   }
 
   @override
-  bool isPotentiallyMutatedInScope(VariableElement variable) {
+  bool isPotentiallyMutatedInScope(PromotableElement variable) {
     return node.isPotentiallyMutatedInScope(variable);
   }
 }
@@ -51,13 +51,13 @@ class FlowAnalysisHelper {
   final TypeSystemTypeOperations _typeOperations;
 
   /// Precomputed sets of potentially assigned variables.
-  final AssignedVariables<AstNode, VariableElement> assignedVariables;
+  final AssignedVariables<AstNode, PromotableElement> assignedVariables;
 
   /// The result for post-resolution stages of analysis.
   final FlowAnalysisResult result;
 
   /// The current flow, when resolving a function body, or `null` otherwise.
-  FlowAnalysis<Statement, Expression, VariableElement, DartType> flow;
+  FlowAnalysis<Statement, Expression, PromotableElement, DartType> flow;
 
   int _blockFunctionBodyLevel = 0;
 
@@ -163,7 +163,7 @@ class FlowAnalysisHelper {
     if (_blockFunctionBodyLevel > 1) {
       assert(flow != null);
     } else {
-      flow = FlowAnalysis<Statement, Expression, VariableElement, DartType>(
+      flow = FlowAnalysis<Statement, Expression, PromotableElement, DartType>(
         _nodeOperations,
         _typeOperations,
         AnalyzerFunctionBodyAccess(node),
@@ -276,9 +276,9 @@ class FlowAnalysisHelper {
   }
 
   /// Computes the [AssignedVariables] map for the given [node].
-  static AssignedVariables<AstNode, VariableElement> computeAssignedVariables(
+  static AssignedVariables<AstNode, PromotableElement> computeAssignedVariables(
       AstNode node) {
-    var assignedVariables = AssignedVariables<AstNode, VariableElement>();
+    var assignedVariables = AssignedVariables<AstNode, PromotableElement>();
     node.accept(_AssignedVariablesVisitor(assignedVariables));
     return assignedVariables;
   }
@@ -334,7 +334,7 @@ class FlowAnalysisResult {
 }
 
 class TypeSystemTypeOperations
-    implements TypeOperations<VariableElement, DartType> {
+    implements TypeOperations<PromotableElement, DartType> {
   final TypeSystem typeSystem;
 
   TypeSystemTypeOperations(this.typeSystem);
@@ -360,7 +360,7 @@ class TypeSystemTypeOperations
   }
 
   @override
-  DartType variableType(VariableElement variable) {
+  DartType variableType(PromotableElement variable) {
     return variable.type;
   }
 }

pkg/analyzer/pubspec.yaml

@@ -1,5 +1,5 @@
 name: analyzer
-version: 0.38.4
+version: 0.38.5-dev
 author: Dart Team <misc@dartlang.org>
 description: This package provides a library that performs static analysis of Dart code.
 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/analyzer

pkg/nnbd_migration/lib/src/decorated_type_operations.dart

@@ -11,7 +11,7 @@ import 'package:nnbd_migration/src/nullability_node.dart';
 
 /// [TypeOperations] that works with [DecoratedType]s.
 class DecoratedTypeOperations
-    implements TypeOperations<VariableElement, DecoratedType> {
+    implements TypeOperations<PromotableElement, DecoratedType> {
   final TypeSystem _typeSystem;
   final VariableRepository _variableRepository;
   final NullabilityGraph _graph;
@@ -20,7 +20,7 @@ class DecoratedTypeOperations
       this._typeSystem, this._variableRepository, this._graph);
 
   @override
-  bool isLocalVariable(VariableElement element) {
+  bool isLocalVariable(PromotableElement element) {
     return element is LocalVariableElement;
   }
 
@@ -40,7 +40,7 @@ class DecoratedTypeOperations
   }
 
   @override
-  DecoratedType variableType(VariableElement variable) {
+  DecoratedType variableType(PromotableElement variable) {
     return _variableRepository.decoratedElementType(variable);
   }
 }

pkg/nnbd_migration/lib/src/edge_builder.dart

@@ -111,12 +111,12 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
 
   /// If we are visiting a function body or initializer, instance of flow
   /// analysis.  Otherwise `null`.
-  FlowAnalysis<Statement, Expression, VariableElement, DecoratedType>
+  FlowAnalysis<Statement, Expression, PromotableElement, DecoratedType>
       _flowAnalysis;
 
   /// If we are visiting a function body or initializer, assigned variable
   /// information  used in flow analysis.  Otherwise `null`.
-  AssignedVariables<AstNode, VariableElement> _assignedVariables;
+  AssignedVariables<AstNode, PromotableElement> _assignedVariables;
 
   /// For convenience, a [DecoratedType] representing non-nullable `Object`.
   final DecoratedType _notNullType;
@@ -261,7 +261,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
   }
 
   @override
-  DecoratedType visitAssertStatement(AssertStatement node) {
+  DecoratedType visitAssertInitializer(AssertInitializer node) {
     _checkExpressionNotNull(node.condition);
     if (identical(_conditionInfo?.condition, node.condition)) {
       var intentNode = _conditionInfo.trueDemonstratesNonNullIntent;
@@ -276,7 +276,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
   }
 
   @override
-  DecoratedType visitAssertInitializer(AssertInitializer node) {
+  DecoratedType visitAssertStatement(AssertStatement node) {
     _checkExpressionNotNull(node.condition);
     if (identical(_conditionInfo?.condition, node.condition)) {
       var intentNode = _conditionInfo.trueDemonstratesNonNullIntent;
@@ -340,7 +340,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
           // TODO(paulberry): figure out what the rules for isPure should be.
           isPure = true;
           var element = leftOperand.staticElement;
-          if (element is VariableElement) {
+          if (element is PromotableElement) {
             _flowAnalysis.conditionEqNull(node, element, notEqual: notEqual);
           }
         }
@@ -443,7 +443,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
       node.stackTraceParameter
     ]) {
       if (identifier != null) {
-        _flowAnalysis.add(identifier.staticElement as VariableElement,
+        _flowAnalysis.add(identifier.staticElement as PromotableElement,
             assigned: true);
       }
     }
@@ -866,7 +866,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
     expression.accept(this);
     if (expression is SimpleIdentifier) {
       var element = expression.staticElement;
-      if (element is VariableElement) {
+      if (element is PromotableElement) {
         _flowAnalysis.isExpression_end(
             node, element, node.notOperator != null, decoratedType);
       }
@@ -1176,7 +1176,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
   @override
   DecoratedType visitSimpleIdentifier(SimpleIdentifier node) {
     var staticElement = node.staticElement;
-    if (staticElement is VariableElement) {
+    if (staticElement is PromotableElement) {
       if (!node.inDeclarationContext()) {
         var promotedType = _flowAnalysis.promotedType(staticElement);
         if (promotedType != null) return promotedType;
@@ -1372,10 +1372,12 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
     for (var variable in node.variables) {
       variable.metadata.accept(this);
       var initializer = variable.initializer;
-      _flowAnalysis.add(variable.declaredElement,
-          assigned: initializer != null);
+      var declaredElement = variable.declaredElement;
+      if (declaredElement is PromotableElement) {
+        _flowAnalysis.add(declaredElement, assigned: initializer != null);
+      }
       if (initializer != null) {
-        var destinationType = getOrComputeElementType(variable.declaredElement);
+        var destinationType = getOrComputeElementType(declaredElement);
         if (typeAnnotation == null) {
           var initializerType = initializer.accept(this);
           if (initializerType == null) {
@@ -1470,7 +1472,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
     assert(_flowAnalysis == null);
     assert(_assignedVariables == null);
     _flowAnalysis =
-        FlowAnalysis<Statement, Expression, VariableElement, DecoratedType>(
+        FlowAnalysis<Statement, Expression, PromotableElement, DecoratedType>(
             const AnalyzerNodeOperations(),
             DecoratedTypeOperations(_typeSystem, _variables, _graph),
             AnalyzerFunctionBodyAccess(node is FunctionBody ? node : null));
@@ -1600,11 +1602,11 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         (destinationExpression == null) != (destinationType == null),
         'Either destinationExpression or destinationType should be supplied, '
         'but not both');
-    VariableElement destinationLocalVariable;
+    PromotableElement destinationLocalVariable;
     if (destinationType == null) {
       if (destinationExpression is SimpleIdentifier) {
         var element = destinationExpression.staticElement;
-        if (element is VariableElement) {
+        if (element is PromotableElement) {
           destinationLocalVariable = element;
         }
       }
@@ -1863,7 +1865,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         }
       }
       _flowAnalysis.forEach_bodyBegin(_assignedVariables.writtenInNode(node),
-          lhsElement is VariableElement ? lhsElement : null);
+          lhsElement is PromotableElement ? lhsElement : null);
     }
 
     // The condition may fail/iterable may be empty, so the body gets a new