Migration: fix class hierarchy handling in FixBuilder.

When running the FixBuilder we need to override the behavior of
ClassElement.interfaces so that it returns the post-migration
interface types.  We also need to override the behavior of
LibraryElement.isNonNullableByDefault so that the library being
migrated is considered opted in during FixBuilder analysis.  This
required adding hooks to the analyzer to allow those overrides.

Additionally, since the analyzer caches the InheritanceManager, we
need to clear that cache out just prior to running the FixBuilder, so
that no pre-migration interface types leak into the FixBuilder.

Fixes #40475.
Fixes #42139.

Change-Id: I45e2c30ebe8c12e4a599e2458d77a3731bad0f98
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150260
Reviewed-by: Janice Collins <jcollins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

pkg/analyzer/lib/src/dart/analysis/driver.dart

@@ -442,7 +442,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
         _unitElementRequestedParts.isNotEmpty) {
       return AnalysisDriverPriority.general;
     }
-    _clearLibraryContext();
+    clearLibraryContext();
     return AnalysisDriverPriority.nothing;
   }
 
@@ -486,6 +486,17 @@ class AnalysisDriver implements AnalysisDriverGeneric {
     _changeFile(path);
   }
 
+  /// Clear the library context and any related data structures. Mostly we do
+  /// this to reduce memory consumption. The library context holds to every
+  /// library that was resynthesized, but after some initial analysis we might
+  /// not get again to many of these libraries. So, we should clear the context
+  /// periodically.
+  @visibleForTesting
+  void clearLibraryContext() {
+    _libraryContext = null;
+    _currentSession.clearHierarchies();
+  }
+
   /// Some state on which analysis depends has changed, so the driver needs to be
   /// re-configured with the new state.
   ///
@@ -1170,7 +1181,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
     _throwIfNotAbsolutePath(path);
     _throwIfChangesAreNotAllowed();
     _fileTracker.removeFile(path);
-    _clearLibraryContext();
+    clearLibraryContext();
     _priorityResults.clear();
   }
 
@@ -1185,7 +1196,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   /// Implementation for [changeFile].
   void _changeFile(String path) {
     _fileTracker.changeFile(path);
-    _clearLibraryContext();
+    clearLibraryContext();
     _priorityResults.clear();
   }
 
@@ -1193,27 +1204,17 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   /// of state.
   void _changeHook(String path) {
     _createNewSession(path);
-    _clearLibraryContext();
+    clearLibraryContext();
     _priorityResults.clear();
     _scheduler.notify(this);
   }
 
-  /// Clear the library context and any related data structures. Mostly we do
-  /// this to reduce memory consumption. The library context holds to every
-  /// library that was resynthesized, but after some initial analysis we might
-  /// not get again to many of these libraries. So, we should clear the context
-  /// periodically.
-  void _clearLibraryContext() {
-    _libraryContext = null;
-    _currentSession.clearHierarchies();
-  }
-
   /// There was an exception during a file analysis, we don't know why.
   /// But it might have been caused by an inconsistency of files state, and
   /// the library context state. Reset the library context, and hope that
   /// we will solve the inconsistency while loading / building summaries.
   void _clearLibraryContextAfterException() {
-    _clearLibraryContext();
+    clearLibraryContext();
   }
 
   /// Return the cached or newly computed analysis result of the file with the
@@ -1504,7 +1505,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   LibraryContext _createLibraryContext(FileState library) {
     if (_libraryContext != null) {
       if (_libraryContext.pack()) {
-        _clearLibraryContext();
+        clearLibraryContext();
       }
     }
 

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

@@ -709,7 +709,14 @@ class ClassElementImpl extends AbstractClassElementImpl
   }
 
   @override
-  List<InterfaceType> get interfaces {
+  List<InterfaceType> get interfaces =>
+      ElementTypeProvider.current.getClassInterfaces(this);
+
+  set interfaces(List<InterfaceType> interfaces) {
+    _interfaces = interfaces;
+  }
+
+  List<InterfaceType> get interfacesInternal {
     if (_interfaces != null) {
       return _interfaces;
     }
@@ -730,10 +737,6 @@ class ClassElementImpl extends AbstractClassElementImpl
     return _interfaces = const <InterfaceType>[];
   }
 
-  set interfaces(List<InterfaceType> interfaces) {
-    _interfaces = interfaces;
-  }
-
   @override
   bool get isAbstract {
     if (linkedNode != null) {
@@ -5155,8 +5158,7 @@ class LibraryElementImpl extends ElementImpl implements LibraryElement {
   @override
   final LinkedUnitContext linkedContext;
 
-  @override
-  final bool isNonNullableByDefault;
+  final bool isNonNullableByDefaultInternal;
 
   /// The compilation unit that defines this library.
   CompilationUnitElement _definingCompilationUnit;
@@ -5202,7 +5204,7 @@ class LibraryElementImpl extends ElementImpl implements LibraryElement {
   /// Initialize a newly created library element in the given [context] to have
   /// the given [name] and [offset].
   LibraryElementImpl(this.context, this.session, String name, int offset,
-      this.nameLength, this.isNonNullableByDefault)
+      this.nameLength, this.isNonNullableByDefaultInternal)
       : linkedContext = null,
         super(name, offset);
 
@@ -5215,7 +5217,7 @@ class LibraryElementImpl extends ElementImpl implements LibraryElement {
       this.linkedContext,
       Reference reference,
       CompilationUnit linkedNode)
-      : isNonNullableByDefault = linkedContext.isNNBD,
+      : isNonNullableByDefaultInternal = linkedContext.isNNBD,
         super.forLinkedNode(null, reference, linkedNode) {
     _name = name;
     _nameOffset = offset;
@@ -5445,6 +5447,10 @@ class LibraryElementImpl extends ElementImpl implements LibraryElement {
     return false;
   }
 
+  @override
+  bool get isNonNullableByDefault =>
+      ElementTypeProvider.current.isLibraryNonNullableByDefault(this);
+
   /// Return `true` if the receiver directly or indirectly imports the
   /// 'dart:html' libraries.
   bool get isOrImportsBrowserLibrary {

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

@@ -32,6 +32,9 @@ class ElementTypeProvider {
   void freshTypeParameterCreated(TypeParameterElement newTypeParameter,
       TypeParameterElement oldTypeParameter) {}
 
+  List<InterfaceType> getClassInterfaces(ClassElementImpl element) =>
+      element.interfacesInternal;
+
   /// Queries the parameters of an executable element's signature.
   ///
   /// Equivalent to `getExecutableType(...).parameters`.
@@ -62,4 +65,8 @@ class ElementTypeProvider {
   /// Queries the type of a variable element.
   DartType getVariableType(VariableElementImpl variable) =>
       variable.typeInternal;
+
+  /// Queries whether NNBD is enabled for a library.
+  bool isLibraryNonNullableByDefault(LibraryElementImpl element) =>
+      element.isNonNullableByDefaultInternal;
 }

pkg/analyzer/pubspec.yaml

@@ -1,5 +1,5 @@
 name: analyzer
-version: 0.39.10
+version: 0.39.11-dev
 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/already_migrated_code_decorator.dart

@@ -8,6 +8,7 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/dart/element/type_provider.dart';
 import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/src/dart/element/type.dart';
+import 'package:analyzer/src/generated/element_type_provider.dart';
 import 'package:nnbd_migration/src/decorated_type.dart';
 import 'package:nnbd_migration/src/edge_origin.dart';
 import 'package:nnbd_migration/src/nullability_node.dart';
@@ -106,7 +107,7 @@ class AlreadyMigratedCodeDecorator {
       allSupertypes.add(supertype);
     }
     allSupertypes.addAll(class_.superclassConstraints);
-    allSupertypes.addAll(class_.interfaces);
+    allSupertypes.addAll(class_.preMigrationInterfaces);
     allSupertypes.addAll(class_.mixins);
     var type = class_.thisType;
     if (type.isDartAsyncFuture) {
@@ -119,3 +120,15 @@ class AlreadyMigratedCodeDecorator {
     ];
   }
 }
+
+extension on ClassElement {
+  List<InterfaceType> get preMigrationInterfaces {
+    var previousElementTypeProvider = ElementTypeProvider.current;
+    try {
+      ElementTypeProvider.current = const ElementTypeProvider();
+      return interfaces;
+    } finally {
+      ElementTypeProvider.current = previousElementTypeProvider;
+    }
+  }
+}

pkg/nnbd_migration/lib/src/fix_builder.dart

@@ -82,6 +82,8 @@ class CompoundAssignmentReadNullable implements Problem {
 /// actually make the changes; it simply reports what changes are necessary
 /// through abstract methods.
 class FixBuilder {
+  final DecoratedClassHierarchy _decoratedClassHierarchy;
+
   /// The type provider providing non-nullable types.
   final TypeProvider typeProvider;
 
@@ -145,7 +147,7 @@ class FixBuilder {
   }
 
   FixBuilder._(
-      DecoratedClassHierarchy decoratedClassHierarchy,
+      this._decoratedClassHierarchy,
       this._typeSystem,
       this._variables,
       this.source,
@@ -157,7 +159,6 @@ class FixBuilder {
       this._graph)
       : typeProvider = _typeSystem.typeProvider {
     migrationResolutionHooks._fixBuilder = this;
-    // TODO(paulberry): make use of decoratedClassHierarchy
     assert(_typeSystem.isNonNullableByDefault);
     assert((typeProvider as TypeProviderImpl).isNonNullableByDefault);
     var inheritanceManager = InheritanceManager3();
@@ -312,6 +313,17 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
         DecoratedTypeParameterBounds.current.get(oldTypeParameter));
   }
 
+  @override
+  List<InterfaceType> getClassInterfaces(ClassElementImpl element) {
+    return _wrapExceptions(
+        _fixBuilder.unit,
+        () => element.interfacesInternal,
+        () => [
+              for (var interface in element.interfacesInternal)
+                _getClassInterface(element, interface.element)
+            ]);
+  }
+
   @override
   bool getConditionalKnownValue(AstNode node) =>
       _wrapExceptions(node, () => null, () {
@@ -422,6 +434,12 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
     return false;
   }
 
+  @override
+  bool isLibraryNonNullableByDefault(LibraryElementImpl element) {
+    return _fixBuilder._graph.isBeingMigrated(element.source) ||
+        element.isNonNullableByDefaultInternal;
+  }
+
   @override
   bool isMethodInvocationNullAware(MethodInvocation node) {
     return node.isNullAware &&
@@ -521,6 +539,14 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
     }
   }
 
+  InterfaceType _getClassInterface(
+      ClassElement class_, ClassElement superclass) {
+    var decoratedSupertype = _fixBuilder._decoratedClassHierarchy
+        .getDecoratedSupertype(class_, superclass);
+    var finalType = _fixBuilder._variables.toFinalType(decoratedSupertype);
+    return finalType as InterfaceType;
+  }
+
   DartType _modifyRValueType(Expression node, DartType type,
       {DartType context}) {
     var hint =

pkg/nnbd_migration/lib/src/nullability_migration_impl.dart

@@ -5,6 +5,7 @@
 import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
+import 'package:analyzer/src/dart/analysis/session.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
@@ -112,6 +113,10 @@ class NullabilityMigrationImpl implements NullabilityMigration {
     var compilationUnit = unit.declaredElement;
     var library = compilationUnit.library;
     var source = compilationUnit.source;
+    // Hierarchies were created assuming the libraries being migrated are opted
+    // out, but the FixBuilder will analyze assuming they're opted in.  So we
+    // need to clear the hierarchies before we continue.
+    (result.session as AnalysisSessionImpl).clearHierarchies();
     var fixBuilder = FixBuilder(
         source,
         _decoratedClassHierarchy,

pkg/nnbd_migration/pubspec.yaml

@@ -6,7 +6,7 @@ environment:
   sdk: '>=2.6.0 <3.0.0'
 dependencies:
   _fe_analyzer_shared: ^4.0.0
-  analyzer: ^0.39.9
+  analyzer: ^0.39.11-dev
   analyzer_plugin: ^0.2.4
   args: ^1.4.4
   charcode: ^1.1.2

pkg/nnbd_migration/test/api_test.dart

@@ -380,6 +380,44 @@ abstract class C {
     await _checkSingleFileChanges(content, expected);
   }
 
+  Future<void> test_call_migrated_base_class_method_non_nullable() async {
+    var content = '''
+abstract class M<V> implements Map<String, V> {}
+void f(bool b, M<int> m, int i) {
+  if (b) {
+    m['x'] = i;
+  }
+}
+void g(bool b, M<int> m) {
+  f(b, m, null);
+}
+''';
+    var expected = '''
+abstract class M<V> implements Map<String, V> {}
+void f(bool b, M<int?> m, int? i) {
+  if (b) {
+    m['x'] = i;
+  }
+}
+void g(bool b, M<int?> m) {
+  f(b, m, null);
+}
+''';
+    await _checkSingleFileChanges(content, expected);
+  }
+
+  Future<void> test_call_migrated_base_class_method_nullable() async {
+    var content = '''
+abstract class M<V> implements Map<String, V> {}
+int f(M<int> m) => m['x'];
+''';
+    var expected = '''
+abstract class M<V> implements Map<String, V> {}
+int? f(M<int> m) => m['x'];
+''';
+    await _checkSingleFileChanges(content, expected);
+  }
+
   Future<void> test_catch_simple() async {
     var content = '''
 void f() {
@@ -6391,6 +6429,6 @@ class _ProvisionalApiTestWithReset extends _ProvisionalApiTestBase
 
   @override
   void _betweenStages() {
-    driver.resetUriResolution();
+    driver.clearLibraryContext();
   }
 }

pkg/nnbd_migration/test/fix_builder_test.dart

@@ -2730,7 +2730,6 @@ _f(_C/*?*/ c) => c?.hashCode;
     visitSubexpression(findNode.propertyAccess('c?.hashCode'), 'int?');
   }
 
-  @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/40475')
   Future<void> test_propertyAccess_nullAware_object_tearoff() async {
     await analyze('''
 class _C {}