[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>
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
index b9eb8aa..c662700 100644
--- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
+++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -1293,6 +1293,10 @@
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:
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index 8aab96d..cfaaff0 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -571,6 +571,7 @@
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,
diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
index 6bf363c..8a626d9 100644
--- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
@@ -371,6 +371,7 @@
ImportsVerifier verifier = ImportsVerifier();
verifier.addImports(unit);
usedImportedElements.forEach(verifier.removeUsedElements);
+ verifier.generateDuplicateExportHints(errorReporter);
verifier.generateDuplicateImportHints(errorReporter);
verifier.generateDuplicateShownHiddenNameHints(errorReporter);
verifier.generateUnusedImportHints(errorReporter);
diff --git a/pkg/analyzer/lib/src/dart/ast/ast.dart b/pkg/analyzer/lib/src/dart/ast/ast.dart
index 4968e0a..13e53dd 100644
--- a/pkg/analyzer/lib/src/dart/ast/ast.dart
+++ b/pkg/analyzer/lib/src/dart/ast/ast.dart
@@ -7018,10 +7018,12 @@
/// 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;
}
diff --git a/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart b/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
index e0b35ac..0e03dc8 100644
--- a/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
+++ b/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
@@ -205,6 +205,15 @@
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',
diff --git a/pkg/analyzer/lib/src/error/imports_verifier.dart b/pkg/analyzer/lib/src/error/imports_verifier.dart
index de7ce08..319cda1 100644
--- a/pkg/analyzer/lib/src/error/imports_verifier.dart
+++ b/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 @@
/// 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 @@
/// 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 @@
/// 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 @@
_allImports.add(directive);
_unusedImports.add(directive);
importsWithLibraries.add(
- _ImportDirective(
+ _NamespaceDirective(
node: directive,
- importedLibrary: libraryElement,
+ library: libraryElement,
),
);
//
@@ -289,56 +287,66 @@
}
}
_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 @@
/// 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 @@
}
}
+ /// 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 @@
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
diff --git a/pkg/analyzer/messages.yaml b/pkg/analyzer/messages.yaml
index 6d0d882..4168ce7 100644
--- a/pkg/analyzer/messages.yaml
+++ b/pkg/analyzer/messages.yaml
@@ -18025,6 +18025,14 @@
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.
diff --git a/pkg/analyzer/test/generated/non_error_resolver_test.dart b/pkg/analyzer/test/generated/non_error_resolver_test.dart
index 8503da8..65622a4 100644
--- a/pkg/analyzer/test/generated/non_error_resolver_test.dart
+++ b/pkg/analyzer/test/generated/non_error_resolver_test.dart
@@ -206,11 +206,13 @@
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 {
diff --git a/pkg/analyzer/test/src/diagnostics/duplicate_import_test.dart b/pkg/analyzer/test/src/diagnostics/duplicate_import_test.dart
index 494f159..baa86ad 100644
--- a/pkg/analyzer/test/src/diagnostics/duplicate_import_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/duplicate_import_test.dart
@@ -10,11 +10,52 @@
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 {
newFile('$testPackageLibPath/a.dart', r'''
diff --git a/pkg/analyzer/tool/diagnostics/generate.dart b/pkg/analyzer/tool/diagnostics/generate.dart
index 7299840..7c29e74 100644
--- a/pkg/analyzer/tool/diagnostics/generate.dart
+++ b/pkg/analyzer/tool/diagnostics/generate.dart
@@ -12,7 +12,7 @@
/// 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);
diff --git a/pkg/analyzer/tool/messages/generate.dart b/pkg/analyzer/tool/messages/generate.dart
index 77931f3..8d07ca3 100644
--- a/pkg/analyzer/tool/messages/generate.dart
+++ b/pkg/analyzer/tool/messages/generate.dart
@@ -24,7 +24,7 @@
import 'error_code_info.dart';
-void main() async {
+Future<void> main() async {
await GeneratedContent.generateAll(analyzerPkgPath, allTargets);
_SyntacticErrorGenerator()
diff --git a/pkg/analyzer/tool/messages/generate_test.dart b/pkg/analyzer/tool/messages/generate_test.dart
index ffba861..2b37eec 100644
--- a/pkg/analyzer/tool/messages/generate_test.dart
+++ b/pkg/analyzer/tool/messages/generate_test.dart
@@ -11,7 +11,7 @@
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);
}
