Skip to content

Commit 1075e3a

Browse files
committed
Clean up reporting HintCode.UNUSED_SHOWN_NAME when a prefixed top-level function is invoked.
R=brianwilkerson@google.com BUG= Review URL: https://codereview.chromium.org/1882633002 .
1 parent 2e4f8a0 commit 1075e3a

File tree

2 files changed

+103
-67
lines changed

2 files changed

+103
-67
lines changed

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

Lines changed: 79 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -4103,27 +4103,9 @@ class GatherUsedImportedElementsVisitor extends RecursiveAstVisitor {
41034103
_visitDirective(node);
41044104
}
41054105

4106-
@override
4107-
void visitPrefixedIdentifier(PrefixedIdentifier node) {
4108-
// If the prefixed identifier references some A.B, where A is a library
4109-
// prefix, then we can lookup the associated ImportDirective in
4110-
// prefixElementMap and remove it from the unusedImports list.
4111-
SimpleIdentifier prefixIdentifier = node.prefix;
4112-
Element element = prefixIdentifier.staticElement;
4113-
if (element is PrefixElement) {
4114-
List<Element> prefixedElements =
4115-
usedElements.prefixMap.putIfAbsent(element, () => <Element>[]);
4116-
prefixedElements.add(node.identifier.staticElement);
4117-
return;
4118-
}
4119-
// Otherwise, pass the prefixed identifier element and name onto
4120-
// visitIdentifier.
4121-
_visitIdentifier(element, prefixIdentifier.name);
4122-
}
4123-
41244106
@override
41254107
void visitSimpleIdentifier(SimpleIdentifier node) {
4126-
_visitIdentifier(node.staticElement, node.name);
4108+
_visitIdentifier(node, node.staticElement);
41274109
}
41284110

41294111
/**
@@ -4134,7 +4116,7 @@ class GatherUsedImportedElementsVisitor extends RecursiveAstVisitor {
41344116
directive.metadata.accept(this);
41354117
}
41364118

4137-
void _visitIdentifier(Element element, String name) {
4119+
void _visitIdentifier(SimpleIdentifier identifier, Element element) {
41384120
if (element == null) {
41394121
return;
41404122
}
@@ -4143,10 +4125,17 @@ class GatherUsedImportedElementsVisitor extends RecursiveAstVisitor {
41434125
if (element is MultiplyDefinedElement) {
41444126
MultiplyDefinedElement multiplyDefinedElement = element;
41454127
for (Element elt in multiplyDefinedElement.conflictingElements) {
4146-
_visitIdentifier(elt, name);
4128+
_visitIdentifier(identifier, elt);
41474129
}
41484130
return;
4149-
} else if (element is PrefixElement) {
4131+
}
4132+
4133+
// Record `importPrefix.identifier` into 'prefixMap'.
4134+
if (_recordPrefixMap(identifier, element)) {
4135+
return;
4136+
}
4137+
4138+
if (element is PrefixElement) {
41504139
usedElements.prefixMap.putIfAbsent(element, () => <Element>[]);
41514140
return;
41524141
} else if (element.enclosingElement is! CompilationUnitElement) {
@@ -4168,6 +4157,30 @@ class GatherUsedImportedElementsVisitor extends RecursiveAstVisitor {
41684157
// Remember the element.
41694158
usedElements.elements.add(element);
41704159
}
4160+
4161+
/**
4162+
* If the given [identifier] is prefixed with a [PrefixElement], fill the
4163+
* corresponding `UsedImportedElements.prefixMap` entry and return `true`.
4164+
*/
4165+
bool _recordPrefixMap(SimpleIdentifier identifier, Element element) {
4166+
bool recordIfTargetIsPrefixElement(Expression target) {
4167+
if (target is SimpleIdentifier && target.staticElement is PrefixElement) {
4168+
List<Element> prefixedElements = usedElements.prefixMap
4169+
.putIfAbsent(target.staticElement, () => <Element>[]);
4170+
prefixedElements.add(element);
4171+
return true;
4172+
}
4173+
return false;
4174+
}
4175+
AstNode parent = identifier.parent;
4176+
if (parent is MethodInvocation && parent.methodName == identifier) {
4177+
return recordIfTargetIsPrefixElement(parent.target);
4178+
}
4179+
if (parent is PrefixedIdentifier && parent.identifier == identifier) {
4180+
return recordIfTargetIsPrefixElement(parent.prefix);
4181+
}
4182+
return false;
4183+
}
41714184
}
41724185

41734186
/**
@@ -4652,15 +4665,16 @@ class ImportsVerifier {
46524665
* hints
46534666
*/
46544667
void generateUnusedShownNameHints(ErrorReporter reporter) {
4655-
_unusedShownNamesMap.forEach((ImportDirective importDirective,
4656-
List<SimpleIdentifier> identifiers) {
4668+
_unusedShownNamesMap.forEach(
4669+
(ImportDirective importDirective, List<SimpleIdentifier> identifiers) {
46574670
if (_unusedImports.contains(importDirective)) {
46584671
// This import is actually wholly unused, not just one or more shown names from it.
46594672
// This is then an "unused import", rather than unused shown names.
46604673
return;
46614674
}
46624675
for (Identifier identifier in identifiers) {
4663-
reporter.reportErrorForNode(HintCode.UNUSED_SHOWN_NAME, identifier, [identifier.name]);
4676+
reporter.reportErrorForNode(
4677+
HintCode.UNUSED_SHOWN_NAME, identifier, [identifier.name]);
46644678
}
46654679
});
46664680
}
@@ -4674,7 +4688,8 @@ class ImportsVerifier {
46744688
return;
46754689
}
46764690
// Process import prefixes.
4677-
usedElements.prefixMap.forEach((PrefixElement prefix, List<Element> elements) {
4691+
usedElements.prefixMap
4692+
.forEach((PrefixElement prefix, List<Element> elements) {
46784693
List<ImportDirective> importDirectives = _prefixElementMap[prefix];
46794694
if (importDirectives != null) {
46804695
for (ImportDirective importDirective in importDirectives) {
@@ -4711,9 +4726,6 @@ class ImportsVerifier {
47114726
String name = element.displayName;
47124727
for (ImportDirective importDirective in importsLibrary) {
47134728
Namespace namespace = _computeNamespace(importDirective);
4714-
if (importDirective.prefix != null) {
4715-
name = "${importDirective.prefix.name}.$name";
4716-
}
47174729
if (namespace != null && namespace.get(name) != null) {
47184730
_unusedImports.remove(importDirective);
47194731
_removeFromUnusedShownNamesMap(element, importDirective);
@@ -4722,35 +4734,6 @@ class ImportsVerifier {
47224734
}
47234735
}
47244736

4725-
/**
4726-
* Remove [element] from the list of names shown by [importDirective].
4727-
*/
4728-
void _removeFromUnusedShownNamesMap(Element element,
4729-
ImportDirective importDirective) {
4730-
List<SimpleIdentifier> identifiers = _unusedShownNamesMap[importDirective];
4731-
if (identifiers == null) {
4732-
return;
4733-
}
4734-
for (Identifier identifier in identifiers) {
4735-
if (element is PropertyAccessorElement) {
4736-
// If the getter or setter of a variable is used, then the variable (the
4737-
// shown name) is used.
4738-
if (identifier.staticElement == element.variable) {
4739-
identifiers.remove(identifier);
4740-
break;
4741-
}
4742-
} else {
4743-
if (identifier.staticElement == element) {
4744-
identifiers.remove(identifier);
4745-
break;
4746-
}
4747-
}
4748-
}
4749-
if (identifiers.isEmpty) {
4750-
_unusedShownNamesMap.remove(importDirective);
4751-
}
4752-
}
4753-
47544737
/**
47554738
* Recursively add any exported library elements into the [libraryMap].
47564739
*/
@@ -4767,6 +4750,24 @@ class ImportsVerifier {
47674750
}
47684751
}
47694752

4753+
/**
4754+
* Add every shown name from [importDirective] into [_unusedShownNamesMap].
4755+
*/
4756+
void _addShownNames(ImportDirective importDirective) {
4757+
if (importDirective.combinators == null) {
4758+
return;
4759+
}
4760+
List<SimpleIdentifier> identifiers = new List<SimpleIdentifier>();
4761+
_unusedShownNamesMap[importDirective] = identifiers;
4762+
for (Combinator combinator in importDirective.combinators) {
4763+
if (combinator is ShowCombinator) {
4764+
for (SimpleIdentifier name in combinator.shownNames) {
4765+
identifiers.add(name);
4766+
}
4767+
}
4768+
}
4769+
}
4770+
47704771
/**
47714772
* Lookup and return the [Namespace] from the [_namespaceMap].
47724773
*
@@ -4808,21 +4809,32 @@ class ImportsVerifier {
48084809
}
48094810

48104811
/**
4811-
* Add every shown name from [importDirective] into [_unusedShownNamesMap].
4812+
* Remove [element] from the list of names shown by [importDirective].
48124813
*/
4813-
void _addShownNames(ImportDirective importDirective) {
4814-
if (importDirective.combinators == null) {
4814+
void _removeFromUnusedShownNamesMap(
4815+
Element element, ImportDirective importDirective) {
4816+
List<SimpleIdentifier> identifiers = _unusedShownNamesMap[importDirective];
4817+
if (identifiers == null) {
48154818
return;
48164819
}
4817-
List<SimpleIdentifier> identifiers = new List<SimpleIdentifier>();
4818-
_unusedShownNamesMap[importDirective] = identifiers;
4819-
for (Combinator combinator in importDirective.combinators) {
4820-
if (combinator is ShowCombinator) {
4821-
for (SimpleIdentifier name in combinator.shownNames) {
4822-
identifiers.add(name);
4820+
for (Identifier identifier in identifiers) {
4821+
if (element is PropertyAccessorElement) {
4822+
// If the getter or setter of a variable is used, then the variable (the
4823+
// shown name) is used.
4824+
if (identifier.staticElement == element.variable) {
4825+
identifiers.remove(identifier);
4826+
break;
4827+
}
4828+
} else {
4829+
if (identifier.staticElement == element) {
4830+
identifiers.remove(identifier);
4831+
break;
48234832
}
48244833
}
48254834
}
4835+
if (identifiers.isEmpty) {
4836+
_unusedShownNamesMap.remove(importDirective);
4837+
}
48264838
}
48274839
}
48284840

pkg/analyzer/test/generated/non_hint_code_test.dart

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,30 @@ topLevelFunction() {}''');
10961096
verify([source]);
10971097
}
10981098

1099+
void test_unusedImport_prefix_topLevelFunction2() {
1100+
Source source = addSource(r'''
1101+
library L;
1102+
import 'lib1.dart' hide topLevelFunction;
1103+
import 'lib1.dart' as one show topLevelFunction;
1104+
import 'lib1.dart' as two show topLevelFunction;
1105+
class A {
1106+
static void x() {
1107+
One o;
1108+
one.topLevelFunction();
1109+
two.topLevelFunction();
1110+
}
1111+
}''');
1112+
addNamedSource(
1113+
"/lib1.dart",
1114+
r'''
1115+
library lib1;
1116+
class One {}
1117+
topLevelFunction() {}''');
1118+
computeLibrarySourceErrors(source);
1119+
assertNoErrors(source);
1120+
verify([source]);
1121+
}
1122+
10991123
void test_useOfVoidResult_implicitReturnValue() {
11001124
Source source = addSource(r'''
11011125
f() {}

0 commit comments

Comments
 (0)