analyzer: Improve unused_import analysis for shared prefix names

Fixes https://github.com/dart-lang/sdk/issues/38784

Change-Id: I42b32261e9ed71018967e228d2ec731a4613205d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/147336
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Sam Rawlins
2020-12-03 04:00:37 +00:00
committed by commit-bot@chromium.org
parent cec14bbf71
commit e0ebe30190
20 changed files with 142 additions and 81 deletions
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analysis_server/src/services/correction/fix.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -126,7 +127,9 @@ main() {
c.Future v = null;
print(v);
}
''');
''', errorFilter: (error) {
return error.errorCode == CompileTimeErrorCode.UNDEFINED_CLASS;
});
}
Future<void> test_class_with() async {
@@ -177,7 +180,9 @@ main() {
c.main();
}
''');
await assertNoFix();
await assertNoFix(errorFilter: (error) {
return error.errorCode == CompileTimeErrorCode.UNDEFINED_FUNCTION;
});
}
Future<void> test_function_thisLibrary() async {
@@ -217,11 +217,6 @@ class ImportsVerifier {
/// 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.
///
/// TODO (jwren) Since multiple [ImportDirective]s can share the same
/// [PrefixElement], it is possible to have an unreported unused import in
/// situations where two imports use the same prefix and at least one import
/// directive is used.
final HashMap<PrefixElement, List<ImportDirective>> _prefixElementMap =
HashMap<PrefixElement, List<ImportDirective>>();
@@ -410,22 +405,21 @@ class ImportsVerifier {
/// Remove elements from [_unusedImports] using the given [usedElements].
void removeUsedElements(UsedImportedElements usedElements) {
// Stop if all the imports and shown names are known to be used.
if (_unusedImports.isEmpty && _unusedShownNamesMap.isEmpty) {
return;
}
bool everythingIsKnownToBeUsed() =>
_unusedImports.isEmpty && _unusedShownNamesMap.isEmpty;
// Process import prefixes.
usedElements.prefixMap
.forEach((PrefixElement prefix, List<Element> elements) {
List<ImportDirective> importDirectives = _prefixElementMap[prefix];
if (importDirectives != null) {
int importLength = importDirectives.length;
for (int i = 0; i < importLength; i++) {
ImportDirective importDirective = importDirectives[i];
_unusedImports.remove(importDirective);
int elementLength = elements.length;
for (int j = 0; j < elementLength; j++) {
Element element = elements[j];
if (everythingIsKnownToBeUsed()) {
return;
}
// Find import directives using namespaces.
for (var importDirective in _prefixElementMap[prefix] ?? []) {
Namespace namespace = _computeNamespace(importDirective);
for (var element in elements) {
if (namespace?.getPrefixed(prefix.name, element.name) != null) {
_unusedImports.remove(importDirective);
_removeFromUnusedShownNamesMap(element, importDirective);
}
}
@@ -433,15 +427,13 @@ class ImportsVerifier {
});
// Process top-level elements.
for (Element element in usedElements.elements) {
// Stop if all the imports and shown names are known to be used.
if (_unusedImports.isEmpty && _unusedShownNamesMap.isEmpty) {
if (everythingIsKnownToBeUsed()) {
return;
}
// Find import directives using namespaces.
String name = element.name;
for (ImportDirective importDirective in _allImports) {
Namespace namespace = _computeNamespace(importDirective);
if (namespace?.get(name) != null) {
if (namespace?.get(element.name) != null) {
_unusedImports.remove(importDirective);
_removeFromUnusedShownNamesMap(element, importDirective);
}
@@ -449,15 +441,13 @@ class ImportsVerifier {
}
// Process extension elements.
for (ExtensionElement extensionElement in usedElements.usedExtensions) {
// Stop if all the imports and shown names are known to be used.
if (_unusedImports.isEmpty && _unusedShownNamesMap.isEmpty) {
if (everythingIsKnownToBeUsed()) {
return;
}
// Find import directives using namespaces.
String name = extensionElement.name;
for (ImportDirective importDirective in _allImports) {
Namespace namespace = _computeNamespace(importDirective);
if (namespace?.get(name) == extensionElement) {
if (namespace?.get(extensionElement.name) == extensionElement) {
_unusedImports.remove(importDirective);
_removeFromUnusedShownNamesMap(extensionElement, importDirective);
}
@@ -271,7 +271,9 @@ import 'dart:collection' as c;
// ignore: undefined_prefixed_name
f() => c.g;
''',
[],
[
error(HintCode.UNUSED_IMPORT, 7, 17),
],
);
}
}
@@ -1533,6 +1533,7 @@ main() {
x = 2;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 37, 1),
]);
@@ -24,6 +24,7 @@ main() {
p; // use
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 12),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 38, 1),
]);
@@ -40,6 +41,7 @@ main() {
for (var x in p) {}
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 12),
error(HintCode.UNUSED_LOCAL_VARIABLE, 47, 1),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 52, 1),
]);
@@ -64,6 +66,7 @@ main() {
var x = new C(p);
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 12),
error(HintCode.UNUSED_LOCAL_VARIABLE, 66, 1),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 76, 1),
]);
@@ -755,6 +755,7 @@ main() {
math?.loadLibrary();
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 49, 4),
]);
@@ -778,6 +779,7 @@ main() {
foo();
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 39, 3),
]);
_assertInvalidInvocation(
@@ -806,6 +808,7 @@ main() {
math.foo(0);
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.UNDEFINED_FUNCTION, 45, 3),
]);
_assertUnresolvedMethodInvocation('foo(0);');
@@ -1309,13 +1312,15 @@ main() {
}
test_hasReceiver_deferredImportPrefix_loadLibrary() async {
await assertNoErrorsInCode(r'''
await assertErrorsInCode(r'''
import 'dart:math' deferred as math;
main() {
math.loadLibrary();
}
''');
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
]);
var import = findElement.importFind('dart:math');
@@ -1337,6 +1342,7 @@ main() {
math.loadLibrary(1 + 2);
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.EXTRA_POSITIONAL_ARGUMENTS, 65, 7),
]);
@@ -1769,6 +1775,7 @@ main() {
math();
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 40, 4),
]);
assertElement(findNode.simple('math()'), findElement.prefix('math'));
@@ -2332,14 +2339,16 @@ class MethodInvocationResolutionWithNullSafetyTest
class A {}
''');
await assertNoErrorsInCode(r'''
await assertErrorsInCode(r'''
// @dart = 2.7
import 'a.dart' deferred as a;
main() {
a.loadLibrary();
}
''');
''', [
error(HintCode.UNUSED_IMPORT, 22, 8),
]);
var import = findElement.importFind('package:test/a.dart');
@@ -278,14 +278,16 @@ class PrefixedIdentifierResolutionWithNullSafetyTest
class A {}
''');
await assertNoErrorsInCode(r'''
await assertErrorsInCode(r'''
// @dart = 2.7
import 'a.dart' deferred as a;
main() {
a.loadLibrary;
}
''');
''', [
error(HintCode.UNUSED_IMPORT, 22, 8),
]);
var import = findElement.importFind('package:test/a.dart');
@@ -197,6 +197,7 @@ main() {
new math.A();
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.NEW_WITH_NON_TYPE, 49, 1),
]);
@@ -23,6 +23,7 @@ void f() {
const lib.A();
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.CONST_WITH_NON_TYPE, 50, 1),
]);
}
@@ -118,6 +118,7 @@ import 'dart:math' as p;
class C extends p.A {}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.EXTENDS_NON_CLASS, 42, 3),
]);
}
@@ -260,6 +260,7 @@ f() {
p?.x;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 8),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 31, 1),
]);
}
@@ -471,6 +472,7 @@ f() {
p?.x = 0;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 8),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 31, 1),
]);
}
@@ -158,6 +158,7 @@ import 'dart:math' as p;
class C with p.M {}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.MIXIN_OF_NON_CLASS, 39, 3),
]);
}
@@ -27,6 +27,7 @@ class C {
}
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 46, 1),
]);
}
@@ -41,6 +42,7 @@ f() {
p += 1;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 32, 1),
]);
}
@@ -57,6 +59,7 @@ class C {
}
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 46, 1),
]);
}
@@ -90,6 +93,7 @@ f() {
p = 1;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 32, 1),
]);
}
@@ -104,6 +108,7 @@ f() {
p += 1;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 32, 1),
]);
}
@@ -133,6 +138,7 @@ f() {
p?.loadLibrary();
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 41, 1),
]);
}
@@ -148,6 +154,7 @@ f() {
return p?.x;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 39, 1),
]);
}
@@ -162,6 +169,7 @@ f() {
return p?.loadLibrary;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 48, 1),
]);
}
@@ -177,6 +185,7 @@ f() {
p?.x = null;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 32, 1),
]);
}
@@ -191,6 +200,7 @@ f() {
p?.loadLibrary = null;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 41, 1),
]);
}
@@ -205,6 +215,7 @@ f() {
return p;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 39, 1),
]);
}
@@ -52,6 +52,7 @@ import 'dart:math' as p;
main() {
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.UNDEFINED_ANNOTATION, 25, 13),
]);
}
@@ -140,6 +140,7 @@ import 'dart:math' as p;
p.A a;
''', [
error(HintCode.UNUSED_IMPORT, 7, 11),
error(CompileTimeErrorCode.UNDEFINED_CLASS, 26, 3),
]);
}
@@ -21,6 +21,7 @@ class UndefinedPrefixedNameTest extends PubPackageResolutionTest {
import 'lib.dart' as p;
f() => p.c;
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.UNDEFINED_PREFIXED_NAME, 33, 1),
]);
}
@@ -33,6 +34,7 @@ f() {
p.c = 0;
}
''', [
error(HintCode.UNUSED_IMPORT, 7, 10),
error(CompileTimeErrorCode.UNDEFINED_PREFIXED_NAME, 34, 1),
]);
}
@@ -41,7 +41,6 @@ one.A a;
}
test_as_equalPrefixes_referenced() async {
// 18818
newFile('$testPackageLibPath/lib1.dart', content: r'''
class A {}
''');
@@ -56,9 +55,25 @@ one.B b;
''');
}
@failingTest
test_as_equalPrefixes_referenced_via_export() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class A {}
''');
newFile('$testPackageLibPath/lib2.dart', content: r'''
class B {}
''');
newFile('$testPackageLibPath/lib3.dart', content: r'''
export 'lib2.dart';
''');
await assertNoErrorsInCode(r'''
import 'lib1.dart' as one;
import 'lib3.dart' as one;
one.A a;
one.B b;
''');
}
test_as_equalPrefixes_unreferenced() async {
// See todo at ImportsVerifier.prefixElementMap.
newFile('$testPackageLibPath/lib1.dart', content: r'''
class A {}
''');
@@ -70,7 +85,59 @@ import 'lib1.dart' as one;
import 'lib2.dart' as one;
one.A a;
''', [
error(HintCode.UNUSED_IMPORT, 32, 11),
error(HintCode.UNUSED_IMPORT, 34, 11),
]);
}
test_as_show_multipleElements() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class A {}
class B {}
''');
await assertNoErrorsInCode(r'''
import 'lib1.dart' as one show A, B;
one.A a = one.A();
one.B b = one.B();
''');
}
test_as_showTopLevelFunction() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class One {}
topLevelFunction() {}
''');
await assertErrorsInCode(r'''
import 'lib1.dart' hide topLevelFunction;
import 'lib1.dart' as one show topLevelFunction;
class A {
static void x() {
One o;
one.topLevelFunction();
}
}
''', [
error(HintCode.UNUSED_LOCAL_VARIABLE, 129, 1),
]);
}
test_as_showTopLevelFunction_multipleDirectives() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class One {}
topLevelFunction() {}
''');
await assertErrorsInCode(r'''
import 'lib1.dart' hide topLevelFunction;
import 'lib1.dart' as one show topLevelFunction;
import 'lib1.dart' as two show topLevelFunction;
class A {
static void x() {
One o;
one.topLevelFunction();
two.topLevelFunction();
}
}
''', [
error(HintCode.UNUSED_LOCAL_VARIABLE, 178, 1),
]);
}
@@ -323,46 +390,6 @@ f() {
]);
}
test_prefix_topLevelFunction() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class One {}
topLevelFunction() {}
''');
await assertErrorsInCode(r'''
import 'lib1.dart' hide topLevelFunction;
import 'lib1.dart' as one show topLevelFunction;
class A {
static void x() {
One o;
one.topLevelFunction();
}
}
''', [
error(HintCode.UNUSED_LOCAL_VARIABLE, 129, 1),
]);
}
test_prefix_topLevelFunction2() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class One {}
topLevelFunction() {}
''');
await assertErrorsInCode(r'''
import 'lib1.dart' hide topLevelFunction;
import 'lib1.dart' as one show topLevelFunction;
import 'lib1.dart' as two show topLevelFunction;
class A {
static void x() {
One o;
one.topLevelFunction();
two.topLevelFunction();
}
}
''', [
error(HintCode.UNUSED_LOCAL_VARIABLE, 178, 1),
]);
}
test_show() async {
newFile('$testPackageLibPath/lib1.dart', content: r'''
class A {}
@@ -49,6 +49,9 @@ class DocumentationValidator {
// Need a way to make auxiliary files that (a) are not included in the
// generated docs or (b) can be made persistent for fixes.
'CompileTimeErrorCode.PART_OF_NON_PART',
// Need to avoid reporting an unused import with a prefix, when the prefix
// is only referenced in an invalid way.
'CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT',
// Produces the diagnostic HintCode.UNUSED_LOCAL_VARIABLE when it shouldn't.
'CompileTimeErrorCode.UNDEFINED_IDENTIFIER_AWAIT',
// The code has been replaced but is not yet removed.
-1
View File
@@ -30,7 +30,6 @@ import 'package:analyzer/src/summary/summary_sdk.dart' show SummaryBasedDartSdk;
import 'package:analyzer/src/summary2/bundle_reader.dart';
import 'package:analyzer/src/summary2/link.dart' as summary2;
import 'package:analyzer/src/summary2/linked_element_factory.dart' as summary2;
import 'package:analyzer/src/summary2/linked_library_context.dart' as summary2;
import 'package:analyzer/src/summary2/package_bundle_format.dart';
import 'package:analyzer/src/summary2/reference.dart' as summary2;
import 'package:analyzer_cli/src/context_cache.dart';
@@ -5,7 +5,6 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/generated/engine.dart' as analyzer;
import 'package:analyzer/src/generated/parser.dart' as analyzer;
import 'package:analyzer/src/test_utilities/find_element.dart';
import 'package:analyzer_plugin/src/utilities/completion/completion_target.dart';
import 'package:test/test.dart';