Abstract resolution calls to .isNullAware.

When migrating, we'll need to change some null-aware property accesses
and method calls to non-null-aware (due to the fact that the receiver
is non-nullable, or due to the new `?.` shortcut rules), so when
migration re-resolves, it will need to be able to override
resolution's determination of whether a given property access or
method call is null-aware.

Change-Id: I411f43fb937431e757d4dd9b968874db84158e47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133240
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

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

@@ -15,6 +15,7 @@ import 'package:analyzer/src/dart/resolver/invocation_inference_helper.dart';
 import 'package:analyzer/src/dart/resolver/scope.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/generated/element_type_provider.dart';
+import 'package:analyzer/src/generated/migratable_ast_info_provider.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/super_context.dart';
 import 'package:analyzer/src/generated/variable_type_provider.dart';
@@ -54,6 +55,8 @@ class MethodInvocationResolver {
   final ElementTypeProvider _elementTypeProvider;
   final InvocationInferenceHelper _inferenceHelper;
 
+  final MigratableAstInfoProvider _migratableAstInfoProvider;
+
   /// The invocation being resolved.
   MethodInvocationImpl _invocation;
 
@@ -62,7 +65,8 @@ class MethodInvocationResolver {
 
   MethodInvocationResolver(
     this._resolver,
-    this._elementTypeProvider, {
+    this._elementTypeProvider,
+    this._migratableAstInfoProvider, {
     @required InvocationInferenceHelper inferenceHelper,
   })  : _typeType = _resolver.typeProvider.typeType,
         _inheritance = _resolver.inheritance,
@@ -142,7 +146,8 @@ class MethodInvocationResolver {
     DartType receiverType = receiver.staticType;
     receiverType = _resolveTypeParameter(receiverType);
 
-    if (node.isNullAware && _typeSystem.isNonNullableByDefault) {
+    if (_migratableAstInfoProvider.isMethodInvocationNullAware(node) &&
+        _typeSystem.isNonNullableByDefault) {
       receiverType = _typeSystem.promoteToNonNull(receiverType);
     }
 
@@ -298,7 +303,8 @@ class MethodInvocationResolver {
     if (!inferred) {
       DartType staticStaticType = _inferenceHelper.computeInvokeReturnType(
         node.staticInvokeType,
-        isNullAware: node.isNullAware,
+        isNullAware:
+            _migratableAstInfoProvider.isMethodInvocationNullAware(node),
       );
       _inferenceHelper.recordStaticType(node, staticStaticType);
     }

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

@@ -13,8 +13,8 @@ import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_provider.dart';
 import 'package:analyzer/src/error/codes.dart';
-import 'package:analyzer/src/generated/collection_element_provider.dart';
 import 'package:analyzer/src/generated/engine.dart';
+import 'package:analyzer/src/generated/migratable_ast_info_provider.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/utilities_dart.dart';
 import 'package:meta/meta.dart';
@@ -25,7 +25,7 @@ class TypedLiteralResolver {
   final TypeSystemImpl _typeSystem;
   final TypeProviderImpl _typeProvider;
   final ErrorReporter _errorReporter;
-  final CollectionElementProvider _collectionElementProvider;
+  final MigratableAstInfoProvider _migratableAstInfoProvider;
 
   final bool _strictInference;
   final bool _uiAsCodeEnabled;
@@ -34,8 +34,8 @@ class TypedLiteralResolver {
 
   factory TypedLiteralResolver(ResolverVisitor resolver, FeatureSet featureSet,
       TypeSystemImpl typeSystem, TypeProviderImpl typeProvider,
-      {CollectionElementProvider collectionElementProvider =
-          const CollectionElementProvider()}) {
+      {MigratableAstInfoProvider migratableAstInfoProvider =
+          const MigratableAstInfoProvider()}) {
     var library = resolver.definingLibrary as LibraryElementImpl;
     var analysisOptions = library.context.analysisOptions;
     var analysisOptionsImpl = analysisOptions as AnalysisOptionsImpl;
@@ -48,7 +48,7 @@ class TypedLiteralResolver {
         featureSet.isEnabled(Feature.control_flow_collections) ||
             featureSet.isEnabled(Feature.spread_collections),
         featureSet.isEnabled(Feature.non_nullable),
-        collectionElementProvider);
+        migratableAstInfoProvider);
   }
 
   TypedLiteralResolver._(
@@ -59,7 +59,7 @@ class TypedLiteralResolver {
       this._strictInference,
       this._uiAsCodeEnabled,
       this._isNonNullableByDefault,
-      this._collectionElementProvider);
+      this._migratableAstInfoProvider);
 
   DynamicTypeImpl get _dynamicType => DynamicTypeImpl.instance;
 
@@ -301,10 +301,10 @@ class TypedLiteralResolver {
   }
 
   List<CollectionElement> _getListElements(ListLiteral node) =>
-      _collectionElementProvider.getListElements(node);
+      _migratableAstInfoProvider.getListElements(node);
 
   List<CollectionElement> _getSetOrMapElements(SetOrMapLiteral node) =>
-      _collectionElementProvider.getSetOrMapElements(node);
+      _migratableAstInfoProvider.getSetOrMapElements(node);
 
   _InferredCollectionElementTypeInformation _inferCollectionElementType(
       CollectionElement element) {

pkg/analyzer/lib/src/generated/collection_element_provider.dart

@@ -1,25 +0,0 @@
-// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-import 'package:analyzer/dart/ast/ast.dart';
-
-/// Abstraction layer allowing the mechanism for looking up the elements of
-/// collections to be customized.
-///
-/// This is needed for the NNBD migration engine, which needs to be able to
-/// re-run resolution on code for which certain collection elements have been
-/// removed or changed due to dead code elimination..
-///
-/// This base class implementation gets elements directly from the collections;
-/// for other behaviors, create a class that extends or implements this class.
-class CollectionElementProvider {
-  const CollectionElementProvider();
-
-  /// Gets the elements contained in a [ListLiteral].
-  List<CollectionElement> getListElements(ListLiteral node) => node.elements;
-
-  /// Gets the elements contained in a [SetOrMapLiteral].
-  List<CollectionElement> getSetOrMapElements(SetOrMapLiteral node) =>
-      node.elements;
-}

pkg/analyzer/lib/src/generated/element_resolver.dart

@@ -22,6 +22,7 @@ import 'package:analyzer/src/dart/resolver/type_property_resolver.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/generated/element_type_provider.dart';
 import 'package:analyzer/src/generated/engine.dart';
+import 'package:analyzer/src/generated/migratable_ast_info_provider.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/super_context.dart';
 import 'package:analyzer/src/task/strong/checker.dart';
@@ -126,7 +127,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
    */
   ElementResolver(this._resolver,
       {this.reportConstEvaluationErrors = true,
-      ElementTypeProvider elementTypeProvider = const ElementTypeProvider()})
+      ElementTypeProvider elementTypeProvider = const ElementTypeProvider(),
+      MigratableAstInfoProvider migratableAstInfoProvider =
+          const MigratableAstInfoProvider()})
       : _definingLibrary = _resolver.definingLibrary,
         _extensionResolver = _resolver.extensionResolver,
         _typePropertyResolver = _resolver.typePropertyResolver,
@@ -136,6 +139,7 @@ class ElementResolver extends SimpleAstVisitor<void> {
     _methodInvocationResolver = MethodInvocationResolver(
       _resolver,
       elementTypeProvider,
+      migratableAstInfoProvider,
       inferenceHelper: _resolver.inferenceHelper,
     );
   }

pkg/analyzer/lib/src/generated/migratable_ast_info_provider.dart

@@ -0,0 +1,35 @@
+// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/dart/ast/ast.dart';
+
+/// Abstraction layer allowing the NNBD migration engine to customize the
+/// mechanism for looking up various pieces of information AST nodes.
+///
+/// The information that is abstracted is precisely the information that the
+/// migration process might change, for example the elements of collections
+/// (which might disappear or change due to dead code elimination) and whether
+/// or not a property access is null aware.
+///
+/// This base class implementation gets elements directly from the AST nodes;
+/// for other behaviors, create a class that extends or implements this class.
+class MigratableAstInfoProvider {
+  const MigratableAstInfoProvider();
+
+  /// Gets the elements contained in a [ListLiteral].
+  List<CollectionElement> getListElements(ListLiteral node) => node.elements;
+
+  /// Gets the elements contained in a [SetOrMapLiteral].
+  List<CollectionElement> getSetOrMapElements(SetOrMapLiteral node) =>
+      node.elements;
+
+  /// Queries whether the given [node] is null-aware.
+  bool isIndexExpressionNullAware(IndexExpression node) => node.isNullAware;
+
+  /// Queries whether the given [node] is null-aware.
+  bool isMethodInvocationNullAware(MethodInvocation node) => node.isNullAware;
+
+  /// Queries whether the given [node] is null-aware.
+  bool isPropertyAccessNullAware(PropertyAccess node) => node.isNullAware;
+}

pkg/analyzer/lib/src/generated/migration.dart

@@ -6,12 +6,12 @@ import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'package:analyzer/src/generated/collection_element_provider.dart';
 import 'package:analyzer/src/generated/element_type_provider.dart';
+import 'package:analyzer/src/generated/migratable_ast_info_provider.dart';
 
 /// Hooks used by resolution to communicate with the migration engine.
 abstract class MigrationResolutionHooks
-    implements ElementTypeProvider, CollectionElementProvider {
+    implements ElementTypeProvider, MigratableAstInfoProvider {
   /// Called when the resolver is visiting an if statement, if element, or
   /// conditional expression, to determine whether the condition is known to
   /// evaluate to `true` or `false`.

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

@@ -37,11 +37,11 @@ import 'package:analyzer/src/dart/resolver/typed_literal_resolver.dart';
 import 'package:analyzer/src/error/bool_expression_verifier.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/error/nullable_dereference_verifier.dart';
-import 'package:analyzer/src/generated/collection_element_provider.dart';
 import 'package:analyzer/src/generated/constant.dart';
 import 'package:analyzer/src/generated/element_resolver.dart';
 import 'package:analyzer/src/generated/element_type_provider.dart';
 import 'package:analyzer/src/generated/engine.dart';
+import 'package:analyzer/src/generated/migratable_ast_info_provider.dart';
 import 'package:analyzer/src/generated/migration.dart';
 import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer/src/generated/static_type_analyzer.dart';
@@ -199,7 +199,7 @@ class ResolverVisitor extends ScopedVisitor {
 
   final ElementTypeProvider _elementTypeProvider;
 
-  final CollectionElementProvider _collectionElementProvider;
+  final MigratableAstInfoProvider _migratableAstInfoProvider;
 
   /// Helper for checking expression that should have the `bool` type.
   BoolExpressionVerifier boolExpressionVerifier;
@@ -315,7 +315,7 @@ class ResolverVisitor extends ScopedVisitor {
             reportConstEvaluationErrors,
             flowAnalysisHelper,
             const ElementTypeProvider(),
-            const CollectionElementProvider());
+            const MigratableAstInfoProvider());
 
   ResolverVisitor._(
       this.inheritance,
@@ -330,7 +330,7 @@ class ResolverVisitor extends ScopedVisitor {
       reportConstEvaluationErrors,
       this._flowAnalysis,
       this._elementTypeProvider,
-      this._collectionElementProvider)
+      this._migratableAstInfoProvider)
       : _featureSet = featureSet,
         super(definingLibrary, source, typeProvider, errorListener,
             nameScope: nameScope) {
@@ -346,7 +346,7 @@ class ResolverVisitor extends ScopedVisitor {
     );
     this._typedLiteralResolver = TypedLiteralResolver(
         this, _featureSet, typeSystem, typeProvider,
-        collectionElementProvider: _collectionElementProvider);
+        migratableAstInfoProvider: _migratableAstInfoProvider);
     this.extensionResolver = ExtensionMemberResolver(this);
     this.typePropertyResolver = TypePropertyResolver(this);
     this.inferenceHelper = InvocationInferenceHelper(
@@ -385,7 +385,8 @@ class ResolverVisitor extends ScopedVisitor {
     );
     this.elementResolver = ElementResolver(this,
         reportConstEvaluationErrors: reportConstEvaluationErrors,
-        elementTypeProvider: _elementTypeProvider);
+        elementTypeProvider: _elementTypeProvider,
+        migratableAstInfoProvider: _migratableAstInfoProvider);
     this.inferenceContext = InferenceContext._(this);
     this.typeAnalyzer = StaticTypeAnalyzer(
       this,
@@ -1418,7 +1419,8 @@ class ResolverVisitor extends ScopedVisitor {
   @override
   void visitIndexExpression(IndexExpression node) {
     node.target?.accept(this);
-    if (node.isNullAware && _isNonNullableByDefault) {
+    if (_migratableAstInfoProvider.isIndexExpressionNullAware(node) &&
+        _isNonNullableByDefault) {
       _flowAnalysis.flow.nullAwareAccess_rightBegin(node.target);
       _unfinishedNullShorts.add(node.nullShortingTermination);
     }
@@ -1586,7 +1588,8 @@ class ResolverVisitor extends ScopedVisitor {
     // to be visited in the context of the property access node.
     //
     node.target?.accept(this);
-    if (node.isNullAware && _isNonNullableByDefault) {
+    if (_migratableAstInfoProvider.isPropertyAccessNullAware(node) &&
+        _isNonNullableByDefault) {
       _flowAnalysis.flow.nullAwareAccess_rightBegin(node.target);
       _unfinishedNullShorts.add(node.nullShortingTermination);
     }

pkg/nnbd_migration/lib/src/fix_builder.dart

@@ -203,6 +203,14 @@ class FixBuilder {
     }
   }
 
+  /// Determines whether the given [node], which is a null-aware method
+  /// invocation, property access, or index expression, should remain null-aware
+  /// after migration.
+  bool _shouldStayNullAware(Expression node) {
+    // TODO(paulberry): Implement
+    return true;
+  }
+
   static TypeSystemImpl _makeNnbdTypeSystem(
       TypeProvider nnbdTypeProvider, Dart2TypeSystem typeSystem) {
     // TODO(paulberry): do we need to test both possible values of
@@ -222,6 +230,8 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
 
   final Expando<List<CollectionElement>> _collectionElements = Expando();
 
+  final Expando<bool> _shouldStayNullAware = Expando();
+
   FlowAnalysis<AstNode, Statement, Expression, PromotableElement, DartType>
       _flowAnalysis;
 
@@ -301,6 +311,24 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
         return _fixBuilder._computeMigratedType(variable);
       });
 
+  @override
+  bool isIndexExpressionNullAware(IndexExpression node) {
+    return node.isNullAware &&
+        (_shouldStayNullAware[node] ??= _fixBuilder._shouldStayNullAware(node));
+  }
+
+  @override
+  bool isMethodInvocationNullAware(MethodInvocation node) {
+    return node.isNullAware &&
+        (_shouldStayNullAware[node] ??= _fixBuilder._shouldStayNullAware(node));
+  }
+
+  @override
+  bool isPropertyAccessNullAware(PropertyAccess node) {
+    return node.isNullAware &&
+        (_shouldStayNullAware[node] ??= _fixBuilder._shouldStayNullAware(node));
+  }
+
   @override
   DartType modifyExpressionType(Expression node, DartType type) =>
       _wrapExceptions(node, () => type, () {

pkg/nnbd_migration/test/api_test_base.dart

@@ -24,6 +24,6 @@ class TestMigrationListener implements NullabilityMigrationListener {
   @override
   void reportException(
       Source source, AstNode node, Object exception, StackTrace stackTrace) {
-    fail('Exception reported: $exception');
+    fail('Exception reported: $exception\n$stackTrace');
   }
 }