[analyzer] add HintCode.DUPLICATE_EXPORT

Fixes #49439

Change-Id: I511205c6b0960f6b19a2ac45211bc1348568f52c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/260703
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -1293,6 +1293,10 @@ HintCode.DEPRECATED_NEW_IN_COMMENT_REFERENCE:
   status: hasFix
 HintCode.DIVISION_OPTIMIZATION:
   status: hasFix
+HintCode.DUPLICATE_EXPORT:
+  status: needsFix
+  notes: |-
+    One fix is to remove the duplicated export.
 HintCode.DUPLICATE_HIDDEN_NAME:
   status: hasFix
 HintCode.DUPLICATE_IGNORE:

pkg/analyzer/lib/error/error.dart

@@ -571,6 +571,7 @@ const List<ErrorCode> errorCodeValues = [
   HintCode.DEPRECATED_MIXIN_FUNCTION,
   HintCode.DEPRECATED_NEW_IN_COMMENT_REFERENCE,
   HintCode.DIVISION_OPTIMIZATION,
+  HintCode.DUPLICATE_EXPORT,
   HintCode.DUPLICATE_HIDDEN_NAME,
   HintCode.DUPLICATE_IGNORE,
   HintCode.DUPLICATE_IMPORT,

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

@@ -371,6 +371,7 @@ class LibraryAnalyzer {
       ImportsVerifier verifier = ImportsVerifier();
       verifier.addImports(unit);
       usedImportedElements.forEach(verifier.removeUsedElements);
+      verifier.generateDuplicateExportHints(errorReporter);
       verifier.generateDuplicateImportHints(errorReporter);
       verifier.generateDuplicateShownHiddenNameHints(errorReporter);
       verifier.generateUnusedImportHints(errorReporter);

pkg/analyzer/lib/src/dart/ast/ast.dart

@@ -7018,10 +7018,12 @@ class ImportDirectiveImpl extends NamespaceDirectiveImpl
   /// to the same absolute URI, so to the same library, regardless of the used
   /// syntax (absolute, relative, not normalized).
   static bool areSyntacticallyIdenticalExceptUri(
-    ImportDirective node1,
-    ImportDirective node2,
+    NamespaceDirective node1,
+    NamespaceDirective node2,
   ) {
-    if (node1.prefix?.name != node2.prefix?.name) {
+    if (node1 is ImportDirective &&
+        node2 is ImportDirective &&
+        node1.prefix?.name != node2.prefix?.name) {
       return false;
     }
 

pkg/analyzer/lib/src/dart/error/hint_codes.g.dart

@@ -205,6 +205,15 @@ class HintCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  ///  Duplicate exports.
+  ///
+  ///  No parameters.
+  static const HintCode DUPLICATE_EXPORT = HintCode(
+    'DUPLICATE_EXPORT',
+    "Duplicate export.",
+    correctionMessage: "Try removing all but one export of the library.",
+  );
+
   ///  No parameters.
   static const HintCode DUPLICATE_HIDDEN_NAME = HintCode(
     'DUPLICATE_HIDDEN_NAME',

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

@@ -2,8 +2,6 @@
 // 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 'dart:collection';
-
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
@@ -199,7 +197,7 @@ class GatherUsedImportedElementsVisitor extends RecursiveAstVisitor<void> {
 /// future.
 class ImportsVerifier {
   /// All [ImportDirective]s of the current library.
-  final List<ImportDirective> _allImports = <ImportDirective>[];
+  final List<ImportDirective> _allImports = [];
 
   /// A list of [ImportDirective]s that the current library imports, but does
   /// not use.
@@ -210,23 +208,25 @@ class ImportsVerifier {
   /// this list represents the set of unused imports.
   ///
   /// See [ImportsVerifier.generateUnusedImportErrors].
-  final List<ImportDirective> _unusedImports = <ImportDirective>[];
+  final List<ImportDirective> _unusedImports = [];
 
   /// After the list of [unusedImports] has been computed, this list is a proper
   /// subset of the unused imports that are listed more than once.
-  final List<ImportDirective> _duplicateImports = <ImportDirective>[];
+  final List<ImportDirective> _duplicateImports = [];
+
+  /// This list is a proper subset of the unused exports that are listed more
+  /// than once.
+  final List<ExportDirective> _duplicateExports = [];
 
   /// The cache of [Namespace]s for [ImportDirective]s.
-  final HashMap<ImportDirective, Namespace> _namespaceMap =
-      HashMap<ImportDirective, Namespace>();
+  final Map<ImportDirective, Namespace> _namespaceMap = {};
 
   /// This is a map between prefix elements and the import directives from which
   /// they are derived. In cases where a type is referenced via a prefix
   /// element, the import directive can be marked as used (removed from the
   /// unusedImports) by looking at the resolved `lib` in `lib.X`, instead of
   /// looking at which library the `lib.X` resolves.
-  final HashMap<PrefixElement, List<ImportDirective>> _prefixElementMap =
-      HashMap<PrefixElement, List<ImportDirective>>();
+  final Map<PrefixElement, List<ImportDirective>> _prefixElementMap = {};
 
   /// A map of identifiers that the current library's imports show, but that the
   /// library does not use.
@@ -241,22 +241,20 @@ class ImportsVerifier {
   /// shown elements.
   ///
   /// See [ImportsVerifier.generateUnusedShownNameHints].
-  final HashMap<ImportDirective, List<SimpleIdentifier>> _unusedShownNamesMap =
-      HashMap<ImportDirective, List<SimpleIdentifier>>();
+  final Map<ImportDirective, List<SimpleIdentifier>> _unusedShownNamesMap = {};
 
   /// A map of names that are hidden more than once.
-  final HashMap<NamespaceDirective, List<SimpleIdentifier>>
-      _duplicateHiddenNamesMap =
-      HashMap<NamespaceDirective, List<SimpleIdentifier>>();
+  final Map<NamespaceDirective, List<SimpleIdentifier>>
+      _duplicateHiddenNamesMap = {};
 
   /// A map of names that are shown more than once.
-  final HashMap<NamespaceDirective, List<SimpleIdentifier>>
-      _duplicateShownNamesMap =
-      HashMap<NamespaceDirective, List<SimpleIdentifier>>();
+  final Map<NamespaceDirective, List<SimpleIdentifier>>
+      _duplicateShownNamesMap = {};
 
   void addImports(CompilationUnit node) {
-    final importsWithLibraries = <_ImportDirective>[];
-    for (Directive directive in node.directives) {
+    final importsWithLibraries = <_NamespaceDirective>[];
+    final exportsWithLibraries = <_NamespaceDirective>[];
+    for (var directive in node.directives) {
       if (directive is ImportDirective) {
         var libraryElement = directive.element?.importedLibrary;
         if (libraryElement == null) {
@@ -265,9 +263,9 @@ class ImportsVerifier {
         _allImports.add(directive);
         _unusedImports.add(directive);
         importsWithLibraries.add(
-          _ImportDirective(
+          _NamespaceDirective(
             node: directive,
-            importedLibrary: libraryElement,
+            library: libraryElement,
           ),
         );
         //
@@ -289,56 +287,66 @@ class ImportsVerifier {
           }
         }
         _addShownNames(directive);
+      } else if (directive is ExportDirective) {
+        var libraryElement = directive.element?.exportedLibrary;
+        if (libraryElement == null) {
+          continue;
+        }
+        exportsWithLibraries.add(
+          _NamespaceDirective(
+            node: directive,
+            library: libraryElement,
+          ),
+        );
       }
+
       if (directive is NamespaceDirective) {
         _addDuplicateShownHiddenNames(directive);
       }
     }
-    if (importsWithLibraries.length > 1) {
-      // order the list of unusedImports to find duplicates in faster than
-      // O(n^2) time
-      importsWithLibraries.sort((import1, import2) {
-        return import1.libraryUriStr.compareTo(import2.libraryUriStr);
-      });
-      var currentDirective = importsWithLibraries[0];
-      for (var i = 1; i < importsWithLibraries.length; i++) {
-        final nextDirective = importsWithLibraries[i];
-        if (currentDirective.libraryUriStr == nextDirective.libraryUriStr &&
-            ImportDirectiveImpl.areSyntacticallyIdenticalExceptUri(
-              currentDirective.node,
-              nextDirective.node,
-            )) {
-          // Add either the currentDirective or nextDirective depending on which
-          // comes second, this guarantees that the first of the duplicates
-          // won't be highlighted.
-          if (currentDirective.node.offset < nextDirective.node.offset) {
-            _duplicateImports.add(nextDirective.node);
-          } else {
-            _duplicateImports.add(currentDirective.node);
-          }
-        }
-        currentDirective = nextDirective;
-      }
+    var importDuplicates = _duplicates(importsWithLibraries);
+    for (var duplicate in importDuplicates) {
+      _duplicateImports.add(duplicate as ImportDirective);
+    }
+    var exportDuplicates = _duplicates(exportsWithLibraries);
+    for (var duplicate in exportDuplicates) {
+      _duplicateExports.add(duplicate as ExportDirective);
+    }
+  }
+
+  /// Any time after the defining compilation unit has been visited by this
+  /// visitor, this method can be called to report an
+  /// [HintCode.DUPLICATE_EXPORT] hint for each of the export directives in the
+  /// [_duplicateExports] list.
+  ///
+  /// @param errorReporter the error reporter to report the set of
+  ///        [HintCode.DUPLICATE_EXPORT] hints to
+  void generateDuplicateExportHints(ErrorReporter errorReporter) {
+    var length = _duplicateExports.length;
+    for (var i = 0; i < length; i++) {
+      errorReporter.reportErrorForNode(
+          HintCode.DUPLICATE_EXPORT, _duplicateExports[i].uri);
     }
   }
 
   /// Any time after the defining compilation unit has been visited by this
   /// visitor, this method can be called to report an
   /// [HintCode.DUPLICATE_IMPORT] hint for each of the import directives in the
-  /// [duplicateImports] list.
+  /// [_duplicateImports] list.
   ///
   /// @param errorReporter the error reporter to report the set of
   ///        [HintCode.DUPLICATE_IMPORT] hints to
   void generateDuplicateImportHints(ErrorReporter errorReporter) {
-    int length = _duplicateImports.length;
-    for (int i = 0; i < length; i++) {
+    var length = _duplicateImports.length;
+    for (var i = 0; i < length; i++) {
       errorReporter.reportErrorForNode(
           HintCode.DUPLICATE_IMPORT, _duplicateImports[i].uri);
     }
   }
 
-  /// Report a [HintCode.DUPLICATE_SHOWN_HIDDEN_NAME] hint for each duplicate
-  /// shown or hidden name.
+  /// Report a [HintCode.DUPLICATE_SHOWN_NAME] and
+  /// [HintCode.DUPLICATE_HIDDEN_NAME] hints for each duplicate shown or hidden
+  /// name.
   ///
   /// Only call this method after all of the compilation units have been visited
   /// by this visitor.
@@ -526,29 +534,29 @@ class ImportsVerifier {
   /// Add duplicate shown and hidden names from [directive] into
   /// [_duplicateHiddenNamesMap] and [_duplicateShownNamesMap].
   void _addDuplicateShownHiddenNames(NamespaceDirective directive) {
-    for (Combinator combinator in directive.combinators) {
+    for (var combinator in directive.combinators) {
       // Use a Set to find duplicates in faster than O(n^2) time.
-      Set<Element> identifiers = <Element>{};
+      var identifiers = <Element>{};
       if (combinator is HideCombinator) {
-        for (SimpleIdentifier name in combinator.hiddenNames) {
+        for (var name in combinator.hiddenNames) {
           var element = name.staticElement;
           if (element != null) {
             if (!identifiers.add(element)) {
               // [name] is a duplicate.
-              List<SimpleIdentifier> duplicateNames = _duplicateHiddenNamesMap
-                  .putIfAbsent(directive, () => <SimpleIdentifier>[]);
+              List<SimpleIdentifier> duplicateNames =
+                  _duplicateHiddenNamesMap.putIfAbsent(directive, () => []);
               duplicateNames.add(name);
             }
           }
         }
       } else if (combinator is ShowCombinator) {
-        for (SimpleIdentifier name in combinator.shownNames) {
+        for (var name in combinator.shownNames) {
           var element = name.staticElement;
           if (element != null) {
             if (!identifiers.add(element)) {
               // [name] is a duplicate.
-              List<SimpleIdentifier> duplicateNames = _duplicateShownNamesMap
-                  .putIfAbsent(directive, () => <SimpleIdentifier>[]);
+              List<SimpleIdentifier> duplicateNames =
+                  _duplicateShownNamesMap.putIfAbsent(directive, () => []);
               duplicateNames.add(name);
             }
           }
@@ -572,6 +580,38 @@ class ImportsVerifier {
     }
   }
 
+  /// Return the duplicates in [directives].
+  List<NamespaceDirective> _duplicates(List<_NamespaceDirective> directives) {
+    var duplicates = <NamespaceDirective>[];
+    if (directives.length > 1) {
+      // order the list of directives to find duplicates in faster than
+      // O(n^2) time
+      directives.sort((import1, import2) {
+        return import1.libraryUriStr.compareTo(import2.libraryUriStr);
+      });
+      var currentDirective = directives[0];
+      for (var i = 1; i < directives.length; i++) {
+        final nextDirective = directives[i];
+        if (currentDirective.libraryUriStr == nextDirective.libraryUriStr &&
+            ImportDirectiveImpl.areSyntacticallyIdenticalExceptUri(
+              currentDirective.node,
+              nextDirective.node,
+            )) {
+          // Add either the currentDirective or nextDirective depending on which
+          // comes second, this guarantees that the first of the duplicates
+          // won't be highlighted.
+          if (currentDirective.node.offset < nextDirective.node.offset) {
+            duplicates.add(nextDirective.node);
+          } else {
+            duplicates.add(currentDirective.node);
+          }
+        }
+        currentDirective = nextDirective;
+      }
+    }
+    return duplicates;
+  }
+
   /// Remove [element] from the list of names shown by [importDirective].
   void _removeFromUnusedShownNamesMap(
       Element element, ImportDirective importDirective) {
@@ -623,18 +663,18 @@ class UsedImportedElements {
   final Set<ExtensionElement> usedExtensions = {};
 }
 
-/// [ImportDirective] with non-null imported [LibraryElement].
-class _ImportDirective {
-  final ImportDirective node;
-  final LibraryElement importedLibrary;
+/// [NamespaceDirective] with non-null imported or exported [LibraryElement].
+class _NamespaceDirective {
+  final NamespaceDirective node;
+  final LibraryElement library;
 
-  _ImportDirective({
+  _NamespaceDirective({
     required this.node,
-    required this.importedLibrary,
+    required this.library,
   });
 
-  /// Returns the absolute URI of the imported library.
-  String get libraryUriStr => '${importedLibrary.source.uri}';
+  /// Returns the absolute URI of the library.
+  String get libraryUriStr => '${library.source.uri}';
 }
 
 /// A class which verifies (and reports) whether any import directives are

pkg/analyzer/messages.yaml

@@ -18025,6 +18025,14 @@ HintCode:
 
       var x = pi;
       ```
+  DUPLICATE_EXPORT:
+    problemMessage: Duplicate export.
+    correctionMessage: Try removing all but one export of the library.
+    hasPublishedDocs: false
+    comment: |-
+      Duplicate exports.
+
+      No parameters.
   DUPLICATE_IGNORE:
     problemMessage: "The diagnostic '{0}' doesn't need to be ignored here because it's already being ignored."
     correctionMessage: Try removing the name from the list, or removing the whole comment if this is the only name in the list.

pkg/analyzer/test/generated/non_error_resolver_test.dart

@@ -206,11 +206,13 @@ export 'lib2.dart' show C;
 library lib;
 class N {}
 ''');
-    await assertNoErrorsInCode(r'''
+    await assertErrorsInCode(r'''
 library L;
 export 'lib.dart';
 export 'lib.dart';
-''');
+''', [
+      error(HintCode.DUPLICATE_EXPORT, 37, 10),
+    ]);
   }
 
   test_ambiguousImport_dart_implicitHide() async {

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

@@ -10,10 +10,51 @@ import '../dart/resolution/context_collection_resolution.dart';
 
 main() {
   defineReflectiveSuite(() {
+    defineReflectiveTests(DuplicateExportTest);
     defineReflectiveTests(DuplicateImportTest);
   });
 }
 
+@reflectiveTest
+class DuplicateExportTest extends PubPackageResolutionTest {
+  test_duplicateExport() async {
+    newFile('$testPackageLibPath/lib1.dart', r'''
+class A {}
+class B {}
+''');
+    await assertErrorsInCode('''
+export 'lib1.dart';
+export 'lib1.dart';
+''', [
+      error(HintCode.DUPLICATE_EXPORT, 27, 11),
+    ]);
+  }
+
+  test_duplicateExport_differentShow() async {
+    newFile('$testPackageLibPath/lib1.dart', r'''
+class A {}
+class B {}
+''');
+    await assertNoErrorsInCode('''
+export 'lib1.dart' show A;
+export 'lib1.dart' show B;
+''');
+  }
+
+  test_duplicateExport_sameShow() async {
+    newFile('$testPackageLibPath/lib1.dart', r'''
+class A {}
+class B {}
+''');
+    await assertErrorsInCode('''
+export 'lib1.dart' show A;
+export 'lib1.dart' show A;
+''', [
+      error(HintCode.DUPLICATE_EXPORT, 34, 11),
+    ]);
+  }
+}
+
 @reflectiveTest
 class DuplicateImportTest extends PubPackageResolutionTest {
   test_duplicateImport_absolute_absolute() async {

pkg/analyzer/tool/diagnostics/generate.dart

@@ -12,7 +12,7 @@ import '../messages/error_code_info.dart';
 
 /// Generate the file `diagnostics.md` based on the documentation associated
 /// with the declarations of the error codes.
-void main() async {
+Future<void> main() async {
   var sink = File(computeOutputPath()).openWrite();
   var generator = DocumentationGenerator();
   generator.writeDocumentation(sink);

pkg/analyzer/tool/messages/generate.dart

@@ -24,7 +24,7 @@ import 'package:path/path.dart';
 
 import 'error_code_info.dart';
 
-void main() async {
+Future<void> main() async {
   await GeneratedContent.generateAll(analyzerPkgPath, allTargets);
 
   _SyntacticErrorGenerator()

pkg/analyzer/tool/messages/generate_test.dart

@@ -11,7 +11,7 @@ import 'package:path/path.dart';
 import 'error_code_info.dart';
 import 'generate.dart';
 
-void main() async {
+Future<void> main() async {
   await GeneratedContent.checkAll(analyzerPkgPath,
       join(analyzerPkgPath, 'tool', 'messages', 'generate.dart'), allTargets);
 }