From 70aed6a93c8121ca149b3237eb78c7c33db03e43 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Fri, 30 Aug 2024 13:56:36 +0000 Subject: [PATCH] Revert "Parts. Track used imports while resolving library files." This reverts commit 49c6679e4a7392eefee88c55392a0d805afa191a. Reason for revert: breaks google3 Original change's description: > Parts. Track used imports while resolving library files. > > Change-Id: Ic23ec9ca49d93382449bfe2650089eb43099c3e4 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382680 > Reviewed-by: Brian Wilkerson > Commit-Queue: Konstantin Shcheglov Change-Id: Ia954aea30c6ef61197ddaeca53a5179745c30908 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382903 Commit-Queue: Konstantin Shcheglov Bot-Commit: Rubber Stamper Reviewed-by: Phil Quitslund --- .../fix/data_driven/end_to_end_test.dart | 4 +- .../data_driven/flutter_use_case_test.dart | 4 +- .../fix/data_driven/rename_test.dart | 28 +-- .../fix/data_driven/replaced_by_test.dart | 4 +- .../fix/data_driven/sdk_fix_test.dart | 2 +- .../fix/data_driven/test_use_case_test.dart | 2 +- .../lib/src/dart/analysis/file_analysis.dart | 2 - .../src/dart/analysis/library_analyzer.dart | 52 ++--- pkg/analyzer/lib/src/dart/element/scope.dart | 182 +++--------------- .../resolver/comment_reference_resolver.dart | 7 - .../resolver/extension_member_resolver.dart | 12 +- .../resolver/method_invocation_resolver.dart | 10 +- .../dart/resolver/named_type_resolver.dart | 6 - .../resolver/simple_identifier_resolver.dart | 3 +- .../lib/src/error/assignment_verifier.dart | 3 +- .../lib/src/error/imports_verifier.dart | 66 +++---- pkg/analyzer/lib/src/generated/resolver.dart | 101 ++++------ .../lib/src/summary2/ast_resolver.dart | 1 - .../test/src/dart/analysis/driver_test.dart | 1 + .../resolution/method_invocation_test.dart | 1 + ...x_identifier_not_followed_by_dot_test.dart | 1 + .../src/diagnostics/unused_import_test.dart | 34 ---- .../conditional_uri_does_not_exist_test.dart | 3 + .../use_key_in_widget_constructors_test.dart | 14 +- 24 files changed, 140 insertions(+), 403 deletions(-) diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/end_to_end_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/end_to_end_test.dart index 02ecc4e1959..c75f6cdcfb5 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/end_to_end_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/end_to_end_test.dart @@ -161,7 +161,7 @@ void f(Old o) {} import '$importUri'; void f(New o) {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_replacedBy() async { @@ -193,6 +193,6 @@ import 'dart:io'; int f() { return FileMode.read; } -'''); +''', errorFilter: ignoreUnusedImport); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart index 37554830ba8..5b32f16252c 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart @@ -95,7 +95,7 @@ import '$importUri'; void f() { CupertinoAlertDialog(content: 'x'); } -'''); +''', errorFilter: ignoreUnusedImport); } Future @@ -171,7 +171,7 @@ import '$importUri'; void f() { CupertinoPopupSurface(child: 'x'); } -'''); +''', errorFilter: ignoreUnusedImport); } Future diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart index d7cde538e63..0b9e999e16a 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart @@ -125,7 +125,7 @@ import '$importUri'; void f() { New.c(); } -'''); +''', errorFilter: ignoreUnusedImport); } Future test_constructor_unnamed_deprecated() async { @@ -175,7 +175,7 @@ import '$importUri'; void f() { New(); } -'''); +''', errorFilter: ignoreUnusedImport); } Future test_constructor_unnamed_removed_prefixed() async { @@ -234,7 +234,7 @@ class C extends Old {} import '$importUri'; class C extends New {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_inImplements_deprecated() async { @@ -270,7 +270,7 @@ class C implements Old {} import '$importUri'; class C implements New {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_inOn_deprecated() async { @@ -306,7 +306,7 @@ extension E on Old {} import '$importUri'; extension E on New {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_inTypeAnnotation_deprecated() async { @@ -342,7 +342,7 @@ void f(Old o) {} import '$importUri'; void f(New o) {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_inTypeArgument_deprecated() async { @@ -430,7 +430,7 @@ class C with Old {} import '$importUri'; class C with New {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_staticField_deprecated() async { @@ -472,7 +472,7 @@ var s = Old.empty; import '$importUri'; var s = New.empty; -'''); +''', errorFilter: ignoreUnusedImport); } } @@ -693,7 +693,7 @@ var l = Old('a').double; import '$importUri'; var l = New('a').double; -'''); +''', errorFilter: ignoreUnusedImport); } Future test_staticField_deprecated() async { @@ -735,7 +735,7 @@ var s = Old.empty; import '$importUri'; var s = New.empty; -'''); +''', errorFilter: ignoreUnusedImport); } Future test_staticField_removed_prefixed() async { @@ -1176,7 +1176,7 @@ import '$importUri'; void f() { b; } -'''); +''', errorFilter: ignoreUnusedImport); } Future test_topLevel_reference_removed_prefixed() async { @@ -1417,7 +1417,7 @@ class C with Old {} import '$importUri'; class C with New {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_inWith_removed_prefixed() async { @@ -1515,7 +1515,7 @@ import '$importUri'; void f() { b(); } -'''); +''', errorFilter: ignoreUnusedImport); } Future test_removed_prefixed() async { @@ -1661,7 +1661,7 @@ void f(Old o) {} import '$importUri'; void f(New o) {} -'''); +''', errorFilter: ignoreUnusedImport); } Future test_removed_prefixed() async { diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart index 9ff7300fe30..1c3f3be30a0 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart @@ -1068,7 +1068,7 @@ import '$importUri'; f() { expect(true, true); } -'''); +''', errorFilter: ignoreUnusedImport); } Future test_new_element_uris_single() async { @@ -1102,7 +1102,7 @@ import '$importUri'; main() { expect(true, true); } -'''); +''', errorFilter: ignoreUnusedImport); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/sdk_fix_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/sdk_fix_test.dart index 76c6fdb0dab..fb75558ef77 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/sdk_fix_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/sdk_fix_test.dart @@ -49,7 +49,7 @@ void f(Foo o) {} import '$importUri'; void f(Bar o) {} -'''); +''', errorFilter: ignoreUnusedImport); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart index 11e3f257638..506bab397ac 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart @@ -96,6 +96,6 @@ import '$importUri'; main() { expect(true, true); } -'''); +''', errorFilter: ignoreUnusedImport); } } diff --git a/pkg/analyzer/lib/src/dart/analysis/file_analysis.dart b/pkg/analyzer/lib/src/dart/analysis/file_analysis.dart index 3f82f5fe722..ec948266fd0 100644 --- a/pkg/analyzer/lib/src/dart/analysis/file_analysis.dart +++ b/pkg/analyzer/lib/src/dart/analysis/file_analysis.dart @@ -6,7 +6,6 @@ import 'package:analyzer/error/listener.dart'; import 'package:analyzer/src/dart/analysis/file_state.dart'; import 'package:analyzer/src/dart/ast/ast.dart'; import 'package:analyzer/src/dart/element/element.dart'; -import 'package:analyzer/src/dart/element/scope.dart'; import 'package:analyzer/src/ignore_comments/ignore_info.dart'; /// Information about a file being analyzed. @@ -17,7 +16,6 @@ class FileAnalysis { final CompilationUnitImpl unit; final CompilationUnitElementImpl element; final IgnoreInfo ignoreInfo; - late ImportsTracking importsTracking; FileAnalysis({ required this.file, diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart index 22b534817ee..7ffa613673c 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart @@ -182,18 +182,11 @@ class LibraryAnalyzer { _testingData?.recordFlowAnalysisDataForTesting( file.uri, flowAnalysisHelper.dataForTesting!); - var resolverVisitor = ResolverVisitor( - _inheritance, - _libraryElement, - libraryResolutionContext, - file.source, - _typeProvider, - errorListener, - featureSet: _libraryElement.featureSet, - analysisOptions: _library.file.analysisOptions, - flowAnalysisHelper: flowAnalysisHelper, - libraryFragment: unitElement, - ); + var resolverVisitor = ResolverVisitor(_inheritance, _libraryElement, + libraryResolutionContext, file.source, _typeProvider, errorListener, + featureSet: _libraryElement.featureSet, + analysisOptions: _library.file.analysisOptions, + flowAnalysisHelper: flowAnalysisHelper); _testingData?.recordTypeConstraintGenerationDataForTesting( file.uri, resolverVisitor.inferenceHelper.dataForTesting!); @@ -505,9 +498,7 @@ class LibraryAnalyzer { // Verify imports. { - var verifier = ImportsVerifier( - fileAnalysis: fileAnalysis, - ); + ImportsVerifier verifier = ImportsVerifier(); verifier.addImports(unit); usedImportedElements.forEach(verifier.removeUsedElements); verifier.generateDuplicateExportWarnings(errorReporter); @@ -627,24 +618,10 @@ class LibraryAnalyzer { fileElement: _libraryElement.definingCompilationUnit, ); - // Configure scopes for all files to track imports usages. - // Associate tracking objects with file objects. - for (var fileAnalysis in _libraryFiles.values) { - var scope = fileAnalysis.element.scope; - var tracking = scope.importsTrackingInit(); - fileAnalysis.importsTracking = tracking; - } - for (var fileAnalysis in _libraryFiles.values) { _resolveFile(fileAnalysis); } - // Stop tracking usages by scopes. - for (var fileAnalysis in _libraryFiles.values) { - var scope = fileAnalysis.element.scope; - scope.importsTrackingDestroy(); - } - _computeConstants(); } @@ -934,18 +911,11 @@ class LibraryAnalyzer { _testingData?.recordFlowAnalysisDataForTesting( fileAnalysis.file.uri, flowAnalysisHelper.dataForTesting!); - var resolver = ResolverVisitor( - _inheritance, - _libraryElement, - libraryResolutionContext, - source, - _typeProvider, - errorListener, - analysisOptions: _library.file.analysisOptions, - featureSet: unit.featureSet, - flowAnalysisHelper: flowAnalysisHelper, - libraryFragment: unitElement, - ); + var resolver = ResolverVisitor(_inheritance, _libraryElement, + libraryResolutionContext, source, _typeProvider, errorListener, + analysisOptions: _library.file.analysisOptions, + featureSet: unit.featureSet, + flowAnalysisHelper: flowAnalysisHelper); unit.accept(resolver); _testingData?.recordTypeConstraintGenerationDataForTesting( fileAnalysis.file.uri, resolver.inferenceHelper.dataForTesting!); diff --git a/pkg/analyzer/lib/src/dart/element/scope.dart b/pkg/analyzer/lib/src/dart/element/scope.dart index c88ec83e461..c9d4229dfe2 100644 --- a/pkg/analyzer/lib/src/dart/element/scope.dart +++ b/pkg/analyzer/lib/src/dart/element/scope.dart @@ -109,78 +109,6 @@ class FormalParameterScope extends EnclosedScope { } } -/// Tracking information for all import in [CompilationUnitElementImpl]. -class ImportsTracking { - /// Tracking information for each import prefix. - final Map map; - - ImportsTracking({ - required this.map, - }); - - void notifyExtensionUsed(ExtensionElement element) { - for (var tracking in map.values) { - tracking.notifyExtensionUsed(element); - } - } -} - -class ImportsTrackingOfPrefix { - final Map> elementImports; - final Set usedImports = {}; - - /// This flag is set to `true` when there is a compile-time error reported - /// against the group of imports with this import prefix. Specifically, - /// currently there are two errors: - /// - /// 1. Unresolved identifier. - /// 2. Using import prefix itself, without `.identifier` after it. - /// - /// When this happens, we don't want to report unused imports. - bool hasErrorReported = false; - - /// We set it temporarily to `false` while resolving combinators. - bool active = true; - - ImportsTrackingOfPrefix({ - required this.elementImports, - }); - - void lookupResult(Element? element) { - if (!active) { - return; - } - - if (element == null) { - return; - } - - if (element is MultiplyDefinedElement) { - notifyErrorReported(); - return; - } - - var imports = elementImports[element]; - if (imports != null) { - usedImports.addAll(imports); - } - } - - /// We cannot resolve something, mark this group of imports as affected. - void notifyErrorReported() { - if (active) { - hasErrorReported = true; - } - } - - void notifyExtensionUsed(ExtensionElement element) { - var imports = elementImports[element]; - if (imports != null) { - usedImports.addAll(imports); - } - } -} - /// The scope defined by an instance element. class InstanceScope extends EnclosedScope { InstanceScope(super.parent, InstanceElement element) { @@ -249,14 +177,6 @@ class LibraryFragmentScope implements Scope { /// The cached result for [accessibleExtensions]. List? _extensions; - /// This field is set temporarily while resolving all files of a library. - /// So, we can track which elements were actually returned, and which imports - /// in which file (including enclosing files) provided these elements. - /// - /// When we are done, we remove the tracker, so that it does not use memory - /// when we are not resolving files of this library. - ImportsTracking? _importsTracking; - factory LibraryFragmentScope(CompilationUnitElementImpl fragment) { return LibraryFragmentScope._( parent: fragment.enclosingElement3?.scope, @@ -297,34 +217,6 @@ class LibraryFragmentScope implements Scope { }.toFixedList(); } - // TODO(scheglov): this is kludge. - // We should not use the fragment scope for resolving combinators. - // We should use the export scope of the imported library. - void importsTrackingActive(bool value) { - if (_importsTracking case var importsTracking?) { - for (var tracking in importsTracking.map.values) { - tracking.active = value; - } - } - } - - void importsTrackingDestroy() { - noPrefixScope.importsTrackingDestroy(); - for (var prefixElement in _prefixElements.values) { - prefixElement.scope.importsTrackingDestroy(); - } - } - - ImportsTracking importsTrackingInit() { - return _importsTracking = ImportsTracking( - map: { - null: noPrefixScope.importsTrackingInit(), - for (var prefixElement in _prefixElements.values) - prefixElement: prefixElement.scope.importsTrackingInit(), - }, - ); - } - @override ScopeLookupResult lookup(String id) { // Try declarations of the whole library. @@ -338,14 +230,10 @@ class LibraryFragmentScope implements Scope { return importResult; } - // No result. + // No parent, no result. return ScopeLookupResultImpl(null, null); } - void notifyExtensionUsed(ExtensionElement element) { - _importsTracking?.notifyExtensionUsed(element); - } - PrefixScope? _getParentPrefixScope(PrefixElementImpl prefix) { var isDeferred = prefix.imports.any((import) { return import.prefix is DeferredImportElementPrefix; @@ -378,7 +266,12 @@ class LibraryFragmentScope implements Scope { } // Try the parent's combined import scope. - return parent?._lookupCombined(id); + var parentResult = parent?._lookupCombined(id); + if (parentResult != null) { + return parentResult; + } + + return null; } ScopeLookupResult? _lookupLibrary(String id) { @@ -417,9 +310,6 @@ class PrefixScope implements Scope { final LibraryElementImpl libraryElement; final PrefixScope? parent; - final List _importElements = []; - final Map> _elementImports = {}; - final Map _getters = {}; final Map _setters = {}; Set? _settersFromDeprecatedExport; @@ -427,53 +317,40 @@ class PrefixScope implements Scope { final Set _extensions = {}; LibraryElement? _deferredLibrary; - ImportsTrackingOfPrefix? _importsTracking; - PrefixScope({ required this.libraryElement, required this.parent, - required List libraryImports, + required List libraryImports, required PrefixElement? prefix, }) { var elementFactory = libraryElement.session.elementFactory; for (var import in libraryImports) { var importedUri = import.uri; - if (importedUri is DirectiveUriWithLibraryImpl && + if (importedUri is DirectiveUriWithLibrary && import.prefix?.element == prefix) { - _importElements.add(import); var importedLibrary = importedUri.library; - var combinators = import.combinators.build(); - for (var exportedReference in importedLibrary.exportedReferences) { - var reference = exportedReference.reference; - var id = reference.name; - if (combinators.allows(id)) { - var element = elementFactory.elementOfReference(reference)!; - if (_shouldAdd(importedLibrary, element)) { - (_elementImports[element] ??= []).add(import); - _add( - element, - importedLibrary.isFromDeprecatedExport(exportedReference), - ); + if (importedLibrary is LibraryElementImpl) { + var combinators = import.combinators.build(); + for (var exportedReference in importedLibrary.exportedReferences) { + var reference = exportedReference.reference; + if (combinators.allows(reference.name)) { + var element = elementFactory.elementOfReference(reference)!; + if (_shouldAdd(importedLibrary, element)) { + _add( + element, + importedLibrary.isFromDeprecatedExport(exportedReference), + ); + } } } - } - if (import.prefix is DeferredImportElementPrefix) { - _deferredLibrary ??= importedLibrary; + if (import.prefix is DeferredImportElementPrefix) { + _deferredLibrary ??= importedLibrary; + } } } } } - void importsTrackingDestroy() { - _importsTracking = null; - } - - ImportsTrackingOfPrefix importsTrackingInit() { - return _importsTracking = ImportsTrackingOfPrefix( - elementImports: _elementImports, - ); - } - @override ScopeLookupResult lookup(String id) { var deferredLibrary = _deferredLibrary; @@ -484,8 +361,6 @@ class PrefixScope implements Scope { var getter = _getters[id]; var setter = _setters[id]; if (getter != null || setter != null) { - _importsTracking?.lookupResult(getter); - _importsTracking?.lookupResult(setter); return PrefixScopeLookupResult( getter, setter, @@ -498,18 +373,9 @@ class PrefixScope implements Scope { return parent.lookup(id); } - _importsTracking?.notifyErrorReported(); return ScopeLookupResultImpl(null, null); } - /// This method is invoked when the import prefix is used by itself, - /// without the `.identifier` after it. This is a compile-time error that - /// we expect the user to fix first, and we don't want to report unused - /// imports until then. - void notifyErrorReported() { - _importsTracking?.notifyErrorReported(); - } - void _add(Element element, bool isFromDeprecatedExport) { if (element is PropertyAccessorElement && element.isSetter) { _addTo( diff --git a/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart index 4a5372bf367..5092d92a8aa 100644 --- a/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart @@ -5,7 +5,6 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/src/dart/ast/ast.dart'; -import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/dart/element/inheritance_manager3.dart'; import 'package:analyzer/src/dart/element/type_provider.dart'; import 'package:analyzer/src/dart/element/type_system.dart'; @@ -144,12 +143,6 @@ class CommentReferenceResolver { var lookupResult = identifier.scopeLookupResult!; var element = lookupResult.getter ?? lookupResult.setter; - // Usually referencing just an import prefix is an error. - // But we allow this in documentation comments. - if (element is PrefixElementImpl) { - element.scope.notifyErrorReported(); - } - if (element == null) { InterfaceType enclosingType; var enclosingClass = _resolver.enclosingClass; diff --git a/pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart index 124c1bedbff..bcf7695ab08 100644 --- a/pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart @@ -96,20 +96,12 @@ class ExtensionMemberResolver { } if (extensions.length == 1) { - var instantiated = extensions[0]; - _resolver.libraryFragment.scope.notifyExtensionUsed( - instantiated.extension, - ); - return instantiated.asResolutionResult; + return extensions[0].asResolutionResult; } var mostSpecific = _chooseMostSpecific(extensions); if (mostSpecific.length == 1) { - var instantiated = mostSpecific.first; - _resolver.libraryFragment.scope.notifyExtensionUsed( - instantiated.extension, - ); - return instantiated.asResolutionResult; + return mostSpecific.first.asResolutionResult; } // The most specific extension is ambiguous. diff --git a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart index 9a696bc53ff..45af99bebe2 100644 --- a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart @@ -265,11 +265,7 @@ class MethodInvocationResolver with ScopeHelpers { ); } - void _reportPrefixIdentifierNotFollowedByDot( - SimpleIdentifier target, - PrefixElementImpl prefixElement, - ) { - prefixElement.scope.notifyErrorReported(); + void _reportPrefixIdentifierNotFollowedByDot(SimpleIdentifier target) { _resolver.errorReporter.atNode( target, CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, @@ -590,10 +586,10 @@ class MethodInvocationResolver with ScopeHelpers { return _rewriteAsFunctionExpressionInvocation(node, targetType); } // TODO(scheglov): This is a questionable distinction. - if (element is PrefixElementImpl) { + if (element is PrefixElement) { _setInvalidTypeResolution(node, whyNotPromotedList: whyNotPromotedList, contextType: contextType); - _reportPrefixIdentifierNotFollowedByDot(nameNode, element); + _reportPrefixIdentifierNotFollowedByDot(nameNode); return null; } _reportInvocationOfNonFunction(node, whyNotPromotedList, diff --git a/pkg/analyzer/lib/src/dart/resolver/named_type_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/named_type_resolver.dart index d0637525508..fcdfe0c8f6e 100644 --- a/pkg/analyzer/lib/src/dart/resolver/named_type_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/named_type_resolver.dart @@ -264,9 +264,6 @@ class NamedTypeResolver with ScopeHelpers { ); return _verifyTypeAliasForContext(node, element, type); } else if (_isInstanceCreation(node)) { - if (element is PrefixElementImpl) { - element.scope.notifyErrorReported(); - } _ErrorHelper(errorReporter).reportNewWithNonType(node); return InvalidTypeImpl.instance; } else if (element is DynamicElementImpl) { @@ -617,9 +614,6 @@ class _ErrorHelper { } if (element != null) { - if (element is PrefixElementImpl) { - element.scope.notifyErrorReported(); - } var errorRange = _getErrorRange(node); errorReporter.atOffset( offset: errorRange.offset, diff --git a/pkg/analyzer/lib/src/dart/resolver/simple_identifier_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/simple_identifier_resolver.dart index 46bbe5ccfff..5a8a6fdf616 100644 --- a/pkg/analyzer/lib/src/dart/resolver/simple_identifier_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/simple_identifier_resolver.dart @@ -198,8 +198,7 @@ class SimpleIdentifierResolver with ScopeHelpers { !identical(element, enclosingClass)) { // This error is now reported by the parser. element = null; - } else if (element is PrefixElementImpl && !_isValidAsPrefix(node)) { - element.scope.notifyErrorReported(); + } else if (element is PrefixElement && !_isValidAsPrefix(node)) { errorReporter.atNode( node, CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, diff --git a/pkg/analyzer/lib/src/error/assignment_verifier.dart b/pkg/analyzer/lib/src/error/assignment_verifier.dart index 5ce6a1178c8..c77e90989df 100644 --- a/pkg/analyzer/lib/src/error/assignment_verifier.dart +++ b/pkg/analyzer/lib/src/error/assignment_verifier.dart @@ -63,8 +63,7 @@ class AssignmentVerifier { node, CompileTimeErrorCode.ASSIGNMENT_TO_METHOD, ); - } else if (recovery is PrefixElementImpl) { - recovery.scope.notifyErrorReported(); + } else if (recovery is PrefixElement) { _errorReporter.atNode( node, CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, diff --git a/pkg/analyzer/lib/src/error/imports_verifier.dart b/pkg/analyzer/lib/src/error/imports_verifier.dart index 756b1e58e00..8265591920f 100644 --- a/pkg/analyzer/lib/src/error/imports_verifier.dart +++ b/pkg/analyzer/lib/src/error/imports_verifier.dart @@ -5,9 +5,7 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/error/listener.dart'; -import 'package:analyzer/src/dart/analysis/file_analysis.dart'; import 'package:analyzer/src/dart/ast/ast.dart'; -import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/dart/resolver/scope.dart'; import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer/src/summary2/combinator.dart'; @@ -243,8 +241,6 @@ class GatherUsedImportedElementsVisitor extends RecursiveAstVisitor { /// this logic built up in this class could be used for such an action in the /// future. class ImportsVerifier { - final FileAnalysis fileAnalysis; - /// All [ImportDirective]s of the current library. final List _allImports = []; @@ -300,10 +296,6 @@ class ImportsVerifier { final Map> _duplicateShownNamesMap = {}; - ImportsVerifier({ - required this.fileAnalysis, - }); - void addImports(CompilationUnit node) { var importsWithLibraries = <_NamespaceDirective>[]; var exportsWithLibraries = <_NamespaceDirective>[]; @@ -439,41 +431,37 @@ class ImportsVerifier { verifier.reportImports(errorReporter); } - /// Report [WarningCode.UNUSED_IMPORT] for each unused import. + /// Report an [HintCode.UNUSED_IMPORT] hint for each unused import. + /// + /// 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_IMPORT] hints void generateUnusedImportHints(ErrorReporter errorReporter) { - for (var importDirective in fileAnalysis.unit.directives) { - if (importDirective is ImportDirectiveImpl) { - var importElement = importDirective.element!; - var prefixElement = importElement.prefix?.element; - var tracking = fileAnalysis.importsTracking.map[prefixElement]!; - - // If there are errors reported against this group (by import prefix) - // of imports, then it does not matter which imports are used. - if (tracking.hasErrorReported) { + int length = _unusedImports.length; + for (int i = 0; i < length; i++) { + ImportDirective unusedImport = _unusedImports[i]; + // Check that the imported URI exists and isn't dart:core + var importElement = unusedImport.element; + if (importElement != null) { + var libraryElement = importElement.importedLibrary; + if (libraryElement == null || + libraryElement.isDartCore || + libraryElement.isSynthetic) { continue; } - - if (importElement.uri case DirectiveUriWithLibraryImpl uri) { - // Ignore explicit dart:core import. - if (uri.library.isDartCore) { - continue; - } - - // The URI target does not exist, reported this elsewhere. - if (uri.library.isSynthetic) { - continue; - } - - var isUsed = tracking.usedImports.contains(importElement); - if (!isUsed) { - errorReporter.atNode( - importDirective.uri, - WarningCode.UNUSED_IMPORT, - arguments: [uri.relativeUriString], - ); - } - } } + StringLiteral uri = unusedImport.uri; + // We can safely assume that `uri.stringValue` is non-`null`, because the + // only way for it to be `null` is if the import contains a string + // interpolation, in which case the import wouldn't have resolved and + // would not have been included in [_unusedImports]. + errorReporter.atNode( + uri, + WarningCode.UNUSED_IMPORT, + arguments: [uri.stringValue!], + ); } } diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index bce7df1fca4..75248cae1ed 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -86,7 +86,6 @@ import 'package:analyzer/src/generated/utilities_dart.dart'; import 'package:analyzer/src/generated/variable_type_provider.dart'; import 'package:analyzer/src/task/inference_error.dart'; import 'package:analyzer/src/util/ast_data_extractor.dart'; -import 'package:analyzer/src/utilities/extensions/object.dart'; /// Function determining which source files should have inference logging /// enabled. @@ -135,9 +134,6 @@ class ResolverVisitor extends ThrowingAstVisitor /// The element for the library containing the compilation unit being visited. final LibraryElementImpl definingLibrary; - /// The library fragment being visited. - final CompilationUnitElementImpl libraryFragment; - /// The context shared between different units of the same library. final LibraryResolutionContext libraryResolutionContext; @@ -308,43 +304,39 @@ class ResolverVisitor extends ThrowingAstVisitor // TODO(paulberry): make [featureSet] a required parameter (this will be a // breaking change). ResolverVisitor( - InheritanceManager3 inheritanceManager, - LibraryElementImpl definingLibrary, - LibraryResolutionContext libraryResolutionContext, - Source source, - TypeProvider typeProvider, - AnalysisErrorListener errorListener, { - required CompilationUnitElementImpl libraryFragment, - required FeatureSet featureSet, - required AnalysisOptionsImpl analysisOptions, - required FlowAnalysisHelper flowAnalysisHelper, - }) : this._( - inheritanceManager, - definingLibrary, - libraryResolutionContext, - source, - definingLibrary.typeSystem, - typeProvider as TypeProviderImpl, - errorListener, - featureSet, - analysisOptions, - flowAnalysisHelper, - libraryFragment: libraryFragment, - ); + InheritanceManager3 inheritanceManager, + LibraryElementImpl definingLibrary, + LibraryResolutionContext libraryResolutionContext, + Source source, + TypeProvider typeProvider, + AnalysisErrorListener errorListener, + {required FeatureSet featureSet, + required AnalysisOptionsImpl analysisOptions, + required FlowAnalysisHelper flowAnalysisHelper}) + : this._( + inheritanceManager, + definingLibrary, + libraryResolutionContext, + source, + definingLibrary.typeSystem, + typeProvider as TypeProviderImpl, + errorListener, + featureSet, + analysisOptions, + flowAnalysisHelper); ResolverVisitor._( - this.inheritance, - this.definingLibrary, - this.libraryResolutionContext, - this.source, - this.typeSystem, - this.typeProvider, - AnalysisErrorListener errorListener, - FeatureSet featureSet, - this.analysisOptions, - this.flowAnalysis, { - required this.libraryFragment, - }) : errorReporter = ErrorReporter(errorListener, source), + this.inheritance, + this.definingLibrary, + this.libraryResolutionContext, + this.source, + this.typeSystem, + this.typeProvider, + AnalysisErrorListener errorListener, + FeatureSet featureSet, + this.analysisOptions, + this.flowAnalysis) + : errorReporter = ErrorReporter(errorListener, source), _featureSet = featureSet, genericMetadataIsEnabled = definingLibrary.featureSet.isEnabled(Feature.generic_metadata), @@ -4991,17 +4983,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { } } - @override - void visitHideCombinator(HideCombinator node) { - var scope = nameScope.ifTypeOrNull(); - scope?.importsTrackingActive(false); - try { - super.visitHideCombinator(node); - } finally { - scope?.importsTrackingActive(true); - } - } - @override void visitIfElement(covariant IfElementImpl node) { _visitIf(node); @@ -5028,9 +5009,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { _visitDocumentationComment(node.documentationComment); } - @override - void visitLibraryIdentifier(LibraryIdentifier node) {} - @override void visitMethodDeclaration(covariant MethodDeclarationImpl node) { node.body.localVariableInfo = _localVariableInfo; @@ -5123,17 +5101,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { node.target?.accept(this); } - @override - void visitShowCombinator(ShowCombinator node) { - var scope = nameScope.ifTypeOrNull(); - scope?.importsTrackingActive(false); - try { - super.visitShowCombinator(node); - } finally { - scope?.importsTrackingActive(true); - } - } - @override void visitSimpleIdentifier(covariant SimpleIdentifierImpl node) { // Ignore if already resolved - declaration or type. @@ -5142,9 +5109,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { } // Ignore if qualified. var parent = node.parent; - if (parent is ConstructorName && parent.name == node) { - return; - } var scopeLookupResult = nameScope.lookup(node.name); node.scopeLookupResult = scopeLookupResult; // Ignore if it cannot be a reference to a local variable. @@ -5156,6 +5120,9 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { parent.fieldName == node) { return; } + if (parent is ConstructorName) { + return; + } if (parent is Label) { return; } diff --git a/pkg/analyzer/lib/src/summary2/ast_resolver.dart b/pkg/analyzer/lib/src/summary2/ast_resolver.dart index aab408f838e..cc8dbbf0591 100644 --- a/pkg/analyzer/lib/src/summary2/ast_resolver.dart +++ b/pkg/analyzer/lib/src/summary2/ast_resolver.dart @@ -58,7 +58,6 @@ class AstResolver { featureSet: _featureSet, analysisOptions: analysisOptions, flowAnalysisHelper: _flowAnalysis, - libraryFragment: _unitElement, ); AstResolver( diff --git a/pkg/analyzer/test/src/dart/analysis/driver_test.dart b/pkg/analyzer/test/src/dart/analysis/driver_test.dart index 3d9fa1c0dbc..0fa72e8601d 100644 --- a/pkg/analyzer/test/src/dart/analysis/driver_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/driver_test.dart @@ -2237,6 +2237,7 @@ void f(A a) {} flags: exists isLibrary errors 25 +1 UNDEFINED_CLASS + 7 +8 UNUSED_IMPORT [stream] ResolvedUnitResult #1 [status] idle diff --git a/pkg/analyzer/test/src/dart/resolution/method_invocation_test.dart b/pkg/analyzer/test/src/dart/resolution/method_invocation_test.dart index b2cc4277ab6..175ba7bd8aa 100644 --- a/pkg/analyzer/test/src/dart/resolution/method_invocation_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/method_invocation_test.dart @@ -2112,6 +2112,7 @@ main() { math?.loadLibrary(); } ''', [ + error(WarningCode.UNUSED_IMPORT, 7, 11), error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 49, 4), ]); diff --git a/pkg/analyzer/test/src/diagnostics/prefix_identifier_not_followed_by_dot_test.dart b/pkg/analyzer/test/src/diagnostics/prefix_identifier_not_followed_by_dot_test.dart index 68813ddbeb6..40bcb1bf8e4 100644 --- a/pkg/analyzer/test/src/diagnostics/prefix_identifier_not_followed_by_dot_test.dart +++ b/pkg/analyzer/test/src/diagnostics/prefix_identifier_not_followed_by_dot_test.dart @@ -132,6 +132,7 @@ f() { p?.loadLibrary(); } ''', [ + error(WarningCode.UNUSED_IMPORT, 7, 10), error(CompileTimeErrorCode.PREFIX_IDENTIFIER_NOT_FOLLOWED_BY_DOT, 41, 1), ]); } diff --git a/pkg/analyzer/test/src/diagnostics/unused_import_test.dart b/pkg/analyzer/test/src/diagnostics/unused_import_test.dart index e619b3a211a..9e45b311cad 100644 --- a/pkg/analyzer/test/src/diagnostics/unused_import_test.dart +++ b/pkg/analyzer/test/src/diagnostics/unused_import_test.dart @@ -475,40 +475,6 @@ f() { ]); } - test_noPrefix_constructorName_name() async { - await assertErrorsInCode(r''' -import 'dart:async'; - -class A { - A.foo(); -} - -void f() { - A.foo(); -} -''', [ - error(WarningCode.UNUSED_IMPORT, 7, 12), - ]); - } - - test_prefixed_commentReference_prefix() async { - await assertNoErrorsInCode(r''' -import 'dart:math' as math; - -/// [math] -void f() {} -'''); - } - - test_prefixed_commentReference_prefixClass() async { - await assertNoErrorsInCode(r''' -import 'dart:math' as math; - -/// [math.Random] -void f() {} -'''); - } - test_show() async { newFile('$testPackageLibPath/lib1.dart', r''' class A {} diff --git a/pkg/linter/test/rules/conditional_uri_does_not_exist_test.dart b/pkg/linter/test/rules/conditional_uri_does_not_exist_test.dart index f6ba5f835b8..efbda36c717 100644 --- a/pkg/linter/test/rules/conditional_uri_does_not_exist_test.dart +++ b/pkg/linter/test/rules/conditional_uri_does_not_exist_test.dart @@ -29,6 +29,7 @@ import '' if (dart.library.async) 'dart:missing_2'; ''', [ + error(WarningCode.UNUSED_IMPORT, 7, 2), lint(35, 16, messageContains: 'dart:missing_1'), lint(120, 16, messageContains: 'dart:missing_2'), ], @@ -46,6 +47,7 @@ import '' if (dart.library.async) 'missing_2.dart'; ''', [ + error(WarningCode.UNUSED_IMPORT, 7, 2), lint(35, 16, messageContains: 'missing_1.dart'), lint(121, 16, messageContains: 'missing_2.dart'), ], @@ -61,6 +63,7 @@ import '' if (dart.library.io) 'package:foo/missing_2.dart'; ''', [ + error(WarningCode.UNUSED_IMPORT, 7, 2), lint(35, 29, messageContains: 'missing_1.dart'), lint(142, 28, messageContains: 'missing_2.dart'), ], diff --git a/pkg/linter/test/rules/use_key_in_widget_constructors_test.dart b/pkg/linter/test/rules/use_key_in_widget_constructors_test.dart index e6695054e50..831b60813be 100644 --- a/pkg/linter/test/rules/use_key_in_widget_constructors_test.dart +++ b/pkg/linter/test/rules/use_key_in_widget_constructors_test.dart @@ -22,8 +22,8 @@ class UseKeyInWidgetConstructorsTest extends LintRuleTest { test_augmentedConstructor_noKey() async { newFile('$testPackageLibPath/a.dart', r''' +import augment 'test.dart'; import 'package:flutter/widgets.dart'; -part 'test.dart'; class W extends StatelessWidget { W(); @@ -34,7 +34,9 @@ class W extends StatelessWidget { '''); await assertNoDiagnostics(r''' -part of 'a.dart'; +augment library 'a.dart'; + +import 'package:flutter/widgets.dart'; augment class W { augment const W(); @@ -82,8 +84,8 @@ augment class W { test_constructorInAugmentedClass_noKeyParam() async { newFile('$testPackageLibPath/a.dart', r''' +import augment 'test.dart'; import 'package:flutter/widgets.dart'; -part 'test.dart'; class W extends StatelessWidget { @override @@ -92,13 +94,15 @@ class W extends StatelessWidget { '''); await assertDiagnostics(r''' -part of 'a.dart'; +augment library 'a.dart'; + +import 'package:flutter/widgets.dart'; augment class W { const W(); } ''', [ - lint(45, 1), + lint(93, 1), ]); }