Implement augmentation for analyzer mixin error-checking.

Added additional tests to ensure functionality.

Change-Id: I904902bb8358840646a601fcf53de00f8353ffe8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/490301
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

Author: fshcheglov

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

@@ -86,6 +86,10 @@ class LibraryAnalyzer {
   final Map<FileState, FileAnalysis> _libraryFiles = {};
   late final LibraryVerificationContext _libraryVerificationContext;
 
+  /// One verifier per library so that state of elements can be shared across
+  /// fragments in all files of this library.
+  late final InheritanceOverrideVerifier _inheritanceOverrideVerifier;
+
   final TestingData? _testingData;
   final TypeSystemOperations _typeSystemOperations;
 
@@ -111,6 +115,10 @@ class LibraryAnalyzer {
         typeSystem: _typeSystem,
       ),
     );
+    _inheritanceOverrideVerifier = InheritanceOverrideVerifier(
+      _typeSystem,
+      _inheritance,
+    );
   }
 
   TypeProviderImpl get _typeProvider => _libraryElement.typeProvider;
@@ -463,11 +471,7 @@ class LibraryAnalyzer {
     _computeConstantErrors(fileAnalysis);
 
     // Compute inheritance and override errors.
-    InheritanceOverrideVerifier(
-      _typeSystem,
-      _inheritance,
-      diagnosticReporter,
-    ).verifyUnit(unit);
+    _inheritanceOverrideVerifier.verifyUnit(unit, diagnosticReporter);
 
     // Use the ErrorVerifier to compute errors.
     ErrorVerifier errorVerifier = ErrorVerifier(

pkg/analyzer/lib/src/error/inheritance_override.dart

@@ -22,6 +22,7 @@ import 'package:analyzer/src/error/correct_override.dart';
 import 'package:analyzer/src/error/getter_setter_types_verifier.dart';
 import 'package:analyzer/src/error/inference_error.dart';
 import 'package:analyzer/src/error/listener.dart';
+import 'package:analyzer/src/summary2/types_builder.dart';
 import 'package:analyzer/src/utilities/extensions/element.dart';
 
 final _missingMustBeOverridden = Expando<List<ExecutableElement>>();
@@ -36,16 +37,20 @@ class InheritanceOverrideVerifier {
   final TypeSystemImpl _typeSystem;
   final TypeProvider _typeProvider;
   final InheritanceManager3 _inheritance;
-  final DiagnosticReporter _reporter;
 
-  InheritanceOverrideVerifier(
-    this._typeSystem,
-    this._inheritance,
-    this._reporter,
-  ) : _typeProvider = _typeSystem.typeProvider;
+  final Map<InterfaceElementImpl, _InterfaceElementState>
+  _interfaceElementStates = {};
 
-  void verifyUnit(CompilationUnitImpl unit) {
+  InheritanceOverrideVerifier(this._typeSystem, this._inheritance)
+    : _typeProvider = _typeSystem.typeProvider;
+
+  void verifyUnit(CompilationUnitImpl unit, DiagnosticReporter reporter) {
     var library = unit.declaredFragment!.element;
+
+    _InterfaceElementState interfaceElementState(InterfaceElementImpl element) {
+      return _interfaceElementStates[element] ??= _InterfaceElementState();
+    }
+
     for (var declaration in unit.declarations) {
       _ClassVerifier verifier;
       if (declaration is ClassDeclarationImpl) {
@@ -54,85 +59,76 @@ class InheritanceOverrideVerifier {
           typeSystem: _typeSystem,
           typeProvider: _typeProvider,
           inheritance: _inheritance,
-          reporter: _reporter,
+          reporter: reporter,
           featureSet: unit.featureSet,
           library: library,
           node: declaration,
           classNameToken: declaration.namePart.typeName,
           classElement: fragment.element,
+          classFragment: fragment,
           diagnosticSource: unit.declaredFragment!.source,
           implementsClause: declaration.implementsClause,
           members: declaration.body.members,
           superclass: declaration.extendsClause?.superclass,
           withClause: declaration.withClause,
+          interfaceElementState: interfaceElementState(fragment.element),
         );
-        if (fragment.isAugmentation) {
-          verifier._checkDirectSuperTypes();
-          continue;
-        }
       } else if (declaration is ClassTypeAliasImpl) {
         var fragment = declaration.declaredFragment!;
         verifier = _ClassVerifier(
           typeSystem: _typeSystem,
           typeProvider: _typeProvider,
           inheritance: _inheritance,
-          reporter: _reporter,
+          reporter: reporter,
           featureSet: unit.featureSet,
           library: library,
           node: declaration,
           classNameToken: declaration.name,
           classElement: fragment.element,
+          classFragment: fragment,
           diagnosticSource: unit.declaredFragment!.source,
           implementsClause: declaration.implementsClause,
           superclass: declaration.superclass,
           withClause: declaration.withClause,
+          interfaceElementState: interfaceElementState(fragment.element),
         );
-        if (fragment.isAugmentation) {
-          verifier._checkDirectSuperTypes();
-          continue;
-        }
       } else if (declaration is EnumDeclarationImpl) {
         var fragment = declaration.declaredFragment!;
         verifier = _ClassVerifier(
           typeSystem: _typeSystem,
           typeProvider: _typeProvider,
           inheritance: _inheritance,
-          reporter: _reporter,
+          reporter: reporter,
           featureSet: unit.featureSet,
           library: library,
           node: declaration,
           classNameToken: declaration.namePart.typeName,
           classElement: fragment.element,
+          classFragment: fragment,
           diagnosticSource: unit.declaredFragment!.source,
           implementsClause: declaration.implementsClause,
           members: declaration.body.members,
           withClause: declaration.withClause,
+          interfaceElementState: interfaceElementState(fragment.element),
         );
-        if (fragment.isAugmentation) {
-          verifier._checkDirectSuperTypes();
-          continue;
-        }
       } else if (declaration is MixinDeclarationImpl) {
         var fragment = declaration.declaredFragment!;
         verifier = _ClassVerifier(
           typeSystem: _typeSystem,
           typeProvider: _typeProvider,
           inheritance: _inheritance,
-          reporter: _reporter,
+          reporter: reporter,
           featureSet: unit.featureSet,
           library: library,
           node: declaration,
           classNameToken: declaration.name,
           classElement: fragment.element,
+          classFragment: fragment,
           diagnosticSource: unit.declaredFragment!.source,
           implementsClause: declaration.implementsClause,
           members: declaration.body.members,
           onClause: declaration.onClause,
         );
-        if (fragment.isAugmentation) {
-          verifier._checkDirectSuperTypes();
-          continue;
-        }
       } else {
         continue;
       }
@@ -170,6 +166,7 @@ class _ClassVerifier {
   final LibraryElementImpl library;
   final Uri libraryUri;
   final InterfaceElementImpl classElement;
+  final InterfaceFragmentImpl classFragment;
 
   final CompilationUnitMember node;
   final Token classNameToken;
@@ -178,6 +175,7 @@ class _ClassVerifier {
   final MixinOnClause? onClause;
   final NamedType? superclass;
   final WithClause? withClause;
+  final _InterfaceElementState? interfaceElementState;
 
   final List<InterfaceType> directSuperInterfaces = [];
 
@@ -198,12 +196,14 @@ class _ClassVerifier {
     required this.node,
     required this.classNameToken,
     required this.classElement,
+    required this.classFragment,
     required this.diagnosticSource,
     this.implementsClause,
     this.members = const [],
     this.onClause,
     this.superclass,
     this.withClause,
+    this.interfaceElementState,
   }) : libraryUri = library.uri;
 
   /// Verify inheritance overrides, and return `true` if an error was
@@ -250,12 +250,17 @@ class _ClassVerifier {
     //   class C extends S&M2 { ...members of C... }
     // So, we need to check members of each mixin against superinterfaces
     // of `S`, and superinterfaces of all previous mixins.
-    var mixinNodes = withClause?.mixinTypes;
-    var mixinTypes = element.mixins;
-    for (var i = 0; i < mixinTypes.length; i++) {
-      var mixinType = mixinTypes[i];
-      _checkDeclaredMembers(mixinNodes![i], mixinType, mixinIndex: i);
-      directSuperInterfaces.add(mixinType);
+    var mixinNodes = withClause?.mixinTypes ?? <NamedType>[];
+    for (var node in mixinNodes) {
+      var mixinType = node.type;
+      // When building the element model, we skip incorrect types.
+      // So, here we skip corresponding nodes to keep the index in sync.
+      if (mixinType is InterfaceTypeImpl &&
+          isInterfaceTypeInterface(mixinType)) {
+        var index = interfaceElementState!.mixinIndex++;
+        _checkDeclaredMembers(node, mixinType, mixinIndex: index);
+        directSuperInterfaces.add(mixinType);
+      }
     }
 
     directSuperInterfaces.addAll(element.interfaces);
@@ -1044,3 +1049,10 @@ class _ClassVerifier {
     reporter.report(locatableDiagnostic.at(classNameToken));
   }
 }
+
+/// Maintains an [InterfaceElementImpl]'s mixin index across multiple fragments.
+class _InterfaceElementState {
+  int mixinIndex = 0;
+
+  _InterfaceElementState();
+}

pkg/analyzer/lib/src/summary2/types_builder.dart

@@ -19,16 +19,8 @@ import 'package:analyzer/src/summary2/type_builder.dart';
 import 'package:analyzer/src/utilities/extensions/collection.dart';
 import 'package:analyzer/src/utilities/extensions/element.dart';
 
-/// Return `true` if [type] can be used as a class.
-bool _isInterfaceTypeClass(InterfaceType type) {
-  if (type.element is! ClassElement) {
-    return false;
-  }
-  return _isInterfaceTypeInterface(type);
-}
-
 /// Return `true` if [type] can be used as an interface or a mixin.
-bool _isInterfaceTypeInterface(InterfaceType type) {
+bool isInterfaceTypeInterface(InterfaceType type) {
   if (type.element is EnumElement) {
     return false;
   }
@@ -44,12 +36,20 @@ bool _isInterfaceTypeInterface(InterfaceType type) {
   return true;
 }
 
+/// Return `true` if [type] can be used as a class.
+bool _isInterfaceTypeClass(InterfaceType type) {
+  if (type.element is! ClassElement) {
+    return false;
+  }
+  return isInterfaceTypeInterface(type);
+}
+
 List<InterfaceTypeImpl> _toInterfaceTypeList(List<NamedType>? nodeList) {
   if (nodeList != null) {
     return nodeList
         .map((e) => e.type)
         .whereType<InterfaceTypeImpl>()
-        .where(_isInterfaceTypeInterface)
+        .where(isInterfaceTypeInterface)
         .toList();
   }
   return const [];
@@ -502,7 +502,7 @@ class _MixinInference {
     var result = <InterfaceTypeImpl>[];
     for (var mixinNode in withClause.mixinTypes) {
       var mixinType = _inferSingle(mixinNode);
-      if (mixinType != null && _isInterfaceTypeInterface(mixinType)) {
+      if (mixinType != null && isInterfaceTypeInterface(mixinType)) {
         result.add(mixinType);
         interfacesMerger.addWithSupertypes(mixinType);
       }
@@ -628,7 +628,7 @@ class _MixinInference {
   }
 
   InterfaceTypeImpl? _interfaceType(DartType type) {
-    if (type is InterfaceTypeImpl && _isInterfaceTypeInterface(type)) {
+    if (type is InterfaceTypeImpl && isInterfaceTypeInterface(type)) {
       return type;
     }
     return null;

pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_test.dart

@@ -15,7 +15,6 @@ main() {
 
 @reflectiveTest
 class InconsistentInheritanceTest extends PubPackageResolutionTest {
-  @SkippedTest() // TODO(scheglov): implement augmentation
   test_class_augmentWithInterface_augmentWithMixin() async {
     var a = newFile('$testPackageLibPath/a.dart', r'''
 part 'b.dart';
@@ -45,11 +44,10 @@ augment abstract class C with A {}
 ''');
 
     await assertErrorsInFile2(a, [error(diag.inconsistentInheritance, 122, 1)]);
-    await assertErrorsInFile2(b, []);
-    await assertErrorsInFile2(c, []);
+    await assertErrorsInFile2(b, [error(diag.inconsistentInheritance, 42, 1)]);
+    await assertErrorsInFile2(c, [error(diag.inconsistentInheritance, 42, 1)]);
   }
 
-  @SkippedTest() // TODO(scheglov): implement augmentation
   test_class_augmentWithMixin_augmentWithInterface() async {
     var a = newFile('$testPackageLibPath/a.dart', r'''
 part 'b.dart';
@@ -79,8 +77,30 @@ augment abstract class C implements B {}
 ''');
 
     await assertErrorsInFile2(a, [error(diag.inconsistentInheritance, 122, 1)]);
-    await assertErrorsInFile2(b, []);
-    await assertErrorsInFile2(c, []);
+    await assertErrorsInFile2(b, [error(diag.inconsistentInheritance, 42, 1)]);
+    await assertErrorsInFile2(c, [error(diag.inconsistentInheritance, 42, 1)]);
+  }
+
+  test_class_augmentWithMixin_sameFile() async {
+    await assertErrorsInCode(
+      r'''
+abstract class I {
+  String foo();
+}
+
+mixin M {
+  int foo() => 0;
+}
+
+class A implements I {}
+
+augment class A with M {}
+''',
+      [
+        error(diag.inconsistentInheritance, 75, 1),
+        error(diag.inconsistentInheritance, 108, 1),
+      ],
+    );
   }
 
   /// https://github.com/dart-lang/sdk/issues/47026
@@ -275,7 +295,6 @@ enum E implements A, B {v}
     );
   }
 
-  @SkippedTest() // TODO(scheglov): implement augmentation
   test_enum_returnType_augmentation() async {
     await assertErrorsInCode(
       r'''
@@ -293,7 +312,10 @@ augment enum E implements B {
   augment v;
 }
 ''',
-      [error(diag.inconsistentInheritance, 78, 1)],
+      [
+        error(diag.inconsistentInheritance, 78, 1),
+        error(diag.inconsistentInheritance, 111, 1),
+      ],
     );
   }