Skip to content

Commit

Permalink
Add HintCode for duplicate shown/hidden names
Browse files Browse the repository at this point in the history
Bug: #33182
Change-Id: Ibb82c44357bc59044340c6d8b1904c7815d19215
Reviewed-on: https://dart-review.googlesource.com/57161
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
  • Loading branch information
srawlins authored and commit-bot@chromium.org committed May 30, 2018
1 parent e7bad54 commit cc9c8f9
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pkg/analyzer/lib/error/error.dart
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ const List<ErrorCode> errorCodeValues = const [
HintCode.DEPRECATED_MIXIN_FUNCTION,
HintCode.DIVISION_OPTIMIZATION,
HintCode.DUPLICATE_IMPORT,
HintCode.DUPLICATE_HIDDEN_NAME,
HintCode.DUPLICATE_SHOWN_NAME,
HintCode.FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE,
HintCode.FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE,
HintCode.GENERIC_METHOD_COMMENT,
Expand Down
1 change: 1 addition & 0 deletions pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ class LibraryAnalyzer {
verifier.addImports(unit);
_usedImportedElementsList.forEach(verifier.removeUsedElements);
verifier.generateDuplicateImportHints(errorReporter);
verifier.generateDuplicateShownHiddenNameHints(errorReporter);
verifier.generateUnusedImportHints(errorReporter);
verifier.generateUnusedShownNameHints(errorReporter);
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/analyzer/lib/src/dart/error/hint_codes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,22 @@ class HintCode extends ErrorCode {
'DUPLICATE_IMPORT', "Duplicate import.",
correction: "Try removing all but one import of the library.");

/**
* Duplicate hidden names.
*/
static const HintCode DUPLICATE_HIDDEN_NAME =
const HintCode('DUPLICATE_HIDDEN_NAME', "Duplicate hidden name.",
correction: "Try removing the repeated name from the list of hidden "
"members.");

/**
* Duplicate shown names.
*/
static const HintCode DUPLICATE_SHOWN_NAME =
const HintCode('DUPLICATE_SHOWN_NAME', "Duplicate shown name.",
correction: "Try removing the repeated name from the list of shown "
"members.");

/**
* It is a bad practice for a source file in a package "lib" directory
* hierarchy to traverse outside that directory hierarchy. For example, a
Expand Down
85 changes: 85 additions & 0 deletions pkg/analyzer/lib/src/generated/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4076,6 +4076,20 @@ class ImportsVerifier {
final HashMap<ImportDirective, List<SimpleIdentifier>> _unusedShownNamesMap =
new HashMap<ImportDirective, List<SimpleIdentifier>>();

/**
* A map of names that are hidden more than once.
*/
final HashMap<NamespaceDirective, List<SimpleIdentifier>>
_duplicateHiddenNamesMap =
new HashMap<NamespaceDirective, List<SimpleIdentifier>>();

/**
* A map of names that are shown more than once.
*/
final HashMap<NamespaceDirective, List<SimpleIdentifier>>
_duplicateShownNamesMap =
new HashMap<NamespaceDirective, List<SimpleIdentifier>>();

void addImports(CompilationUnit node) {
for (Directive directive in node.directives) {
if (directive is ImportDirective) {
Expand Down Expand Up @@ -4105,6 +4119,9 @@ class ImportsVerifier {
}
_addShownNames(directive);
}
if (directive is NamespaceDirective) {
_addDuplicateShownHiddenNames(directive);
}
}
if (_unusedImports.length > 1) {
// order the list of unusedImports to find duplicates in faster than
Expand Down Expand Up @@ -4199,6 +4216,37 @@ class ImportsVerifier {
});
}

/**
* Report a [HintCode.DUPLICATE_SHOWN_HIDDEN_NAME] hint for each duplicate
* shown or hidden name.
*
* Only call this method after all of the compilation units have been visited
* by this visitor.
*
* @param errorReporter the error reporter used to report the set of
* [HintCode.UNUSED_SHOWN_NAME] hints
*/
void generateDuplicateShownHiddenNameHints(ErrorReporter reporter) {
_duplicateHiddenNamesMap.forEach(
(NamespaceDirective directive, List<SimpleIdentifier> identifiers) {
int length = identifiers.length;
for (int i = 0; i < length; i++) {
Identifier identifier = identifiers[i];
reporter.reportErrorForNode(
HintCode.DUPLICATE_HIDDEN_NAME, identifier, [identifier.name]);
}
});
_duplicateShownNamesMap.forEach(
(NamespaceDirective directive, List<SimpleIdentifier> identifiers) {
int length = identifiers.length;
for (int i = 0; i < length; i++) {
Identifier identifier = identifiers[i];
reporter.reportErrorForNode(
HintCode.DUPLICATE_SHOWN_NAME, identifier, [identifier.name]);
}
});
}

/**
* Remove elements from [_unusedImports] using the given [usedElements].
*/
Expand Down Expand Up @@ -4262,6 +4310,43 @@ class ImportsVerifier {
}
}

/**
* Add duplicate shown and hidden names from [directive] into
* [_duplicateHiddenNamesMap] and [_duplicateShownNamesMap].
*/
void _addDuplicateShownHiddenNames(NamespaceDirective directive) {
if (directive.combinators == null) {
return;
}
for (Combinator combinator in directive.combinators) {
// Use a Set to find duplicates in faster than O(n^2) time.
Set<Element> identifiers = new Set<Element>();
if (combinator is HideCombinator) {
for (SimpleIdentifier name in combinator.hiddenNames) {
if (name.staticElement != null) {
if (!identifiers.add(name.staticElement)) {
// [name] is a duplicate.
List<SimpleIdentifier> duplicateNames = _duplicateHiddenNamesMap
.putIfAbsent(directive, () => new List<SimpleIdentifier>());
duplicateNames.add(name);
}
}
}
} else if (combinator is ShowCombinator) {
for (SimpleIdentifier name in combinator.shownNames) {
if (name.staticElement != null) {
if (!identifiers.add(name.staticElement)) {
// [name] is a duplicate.
List<SimpleIdentifier> duplicateNames = _duplicateShownNamesMap
.putIfAbsent(directive, () => new List<SimpleIdentifier>());
duplicateNames.add(name);
}
}
}
}
}
}

/**
* Lookup and return the [Namespace] from the [_namespaceMap].
*
Expand Down
1 change: 1 addition & 0 deletions pkg/analyzer/lib/src/task/dart.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2836,6 +2836,7 @@ class GenerateHintsTask extends SourceBasedAnalysisTask {
verifier.addImports(unit);
usedImportedElementsList.forEach(verifier.removeUsedElements);
verifier.generateDuplicateImportHints(errorReporter);
verifier.generateDuplicateShownHiddenNameHints(errorReporter);
verifier.generateUnusedImportHints(errorReporter);
verifier.generateUnusedShownNameHints(errorReporter);
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/analyzer/test/generated/hint_code_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,32 @@ class B {}''');
verify([source]);
}

test_duplicateShownHiddenName_hidden() async {
Source source = addSource(r'''
library L;
export 'lib1.dart' hide A, B, A;''');
addNamedSource("/lib1.dart", r'''
library lib1;
class A {}
class B {}''');
await computeAnalysisResult(source);
assertErrors(source, [HintCode.DUPLICATE_HIDDEN_NAME]);
verify([source]);
}

test_duplicateShownHiddenName_shown() async {
Source source = addSource(r'''
library L;
export 'lib1.dart' show A, B, A;''');
addNamedSource("/lib1.dart", r'''
library lib1;
class A {}
class B {}''');
await computeAnalysisResult(source);
assertErrors(source, [HintCode.DUPLICATE_SHOWN_NAME]);
verify([source]);
}

test_factory__expr_return_null_OK() async {
Source source = addSource(r'''
import 'package:meta/meta.dart';
Expand Down

0 comments on commit cc9c8f9

Please sign in to comment.