From 25b71e07e7c724933e6bbcec7ca0a5050ed57da3 Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Tue, 1 Jun 2021 23:28:25 +0000 Subject: [PATCH] Add a quick fix to import extensions when members are referenced Change-Id: Ic7f85cecdc06e255264441db7add6bd4a9bda5d5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201800 Reviewed-by: Konstantin Shcheglov Commit-Queue: Brian Wilkerson --- .../lib/plugin/edit/fix/fix_dart.dart | 4 + .../lib/src/analysis_server.dart | 1 + .../lib/src/analysis_server_abstract.dart | 14 ++ .../lib/src/edit/edit_domain.dart | 2 +- .../lsp/handlers/handler_code_actions.dart | 2 +- .../lib/src/lsp/lsp_analysis_server.dart | 2 + .../completion/dart/extension_cache.dart | 105 +++++++++ .../dart/extension_member_contributor.dart | 39 +--- .../correction/dart/abstract_producer.dart | 4 + .../correction/dart/import_library.dart | 220 +++++++++++++++--- .../lib/src/services/correction/fix.dart | 8 +- .../src/services/correction/fix_internal.dart | 9 +- .../lib/src/utilities/extensions/element.dart | 57 +++++ .../fix/create_local_variable_test.dart | 11 + .../correction/fix/fix_processor.dart | 13 ++ .../fix/import_library_project_test.dart | 195 ++++++++++++++++ .../fix/import_library_show_test.dart | 92 ++++++++ 17 files changed, 708 insertions(+), 70 deletions(-) create mode 100644 pkg/analysis_server/lib/src/services/completion/dart/extension_cache.dart diff --git a/pkg/analysis_server/lib/plugin/edit/fix/fix_dart.dart b/pkg/analysis_server/lib/plugin/edit/fix/fix_dart.dart index 4792bedd941..3550b96ef62 100644 --- a/pkg/analysis_server/lib/plugin/edit/fix/fix_dart.dart +++ b/pkg/analysis_server/lib/plugin/edit/fix/fix_dart.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/plugin/edit/fix/fix_core.dart'; +import 'package:analysis_server/src/services/completion/dart/extension_cache.dart'; import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/instrumentation/service.dart'; @@ -12,6 +13,9 @@ import 'package:analyzer_plugin/utilities/change_builder/change_workspace.dart'; /// /// Clients may not extend, implement or mix-in this class. abstract class DartFixContext implements FixContext { + /// Return the extension cache used to find available extensions. + ExtensionCache get extensionCache; + /// Return the instrumentation service used to report errors that prevent a /// fix from being composed. InstrumentationService get instrumentationService; diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart index 344f9974ede..db8335019dd 100644 --- a/pkg/analysis_server/lib/src/analysis_server.dart +++ b/pkg/analysis_server/lib/src/analysis_server.dart @@ -686,6 +686,7 @@ class ServerContextManagerCallbacks extends ContextManagerCallbacks { path, server.doAnalysisError_listFromEngine(result)); } analysisServer.getDocumentationCacheFor(result)?.cacheFromResult(result); + analysisServer.getExtensionCacheFor(result)?.cacheFromResult(result); var unit = result.unit; if (unit != null) { if (analysisServer._hasAnalysisServiceSubscription( diff --git a/pkg/analysis_server/lib/src/analysis_server_abstract.dart b/pkg/analysis_server/lib/src/analysis_server_abstract.dart index b890a418db2..616bf9d3d32 100644 --- a/pkg/analysis_server/lib/src/analysis_server_abstract.dart +++ b/pkg/analysis_server/lib/src/analysis_server_abstract.dart @@ -16,6 +16,7 @@ import 'package:analysis_server/src/plugin/plugin_watcher.dart'; import 'package:analysis_server/src/server/crash_reporting_attachments.dart'; import 'package:analysis_server/src/server/diagnostic_server.dart'; import 'package:analysis_server/src/services/completion/dart/documentation_cache.dart'; +import 'package:analysis_server/src/services/completion/dart/extension_cache.dart'; import 'package:analysis_server/src/services/correction/namespace.dart'; import 'package:analysis_server/src/services/pub/pub_api.dart'; import 'package:analysis_server/src/services/pub/pub_package_service.dart'; @@ -91,6 +92,10 @@ abstract class AbstractAnalysisServer { /// each context. Map documentationForContext = {}; + /// A map from analysis contexts to the extension cache associated with + /// each context. + Map extensionForContext = {}; + /// The DiagnosticServer for this AnalysisServer. If available, it can be used /// to start an http diagnostics server or return the port for an existing /// server. @@ -229,6 +234,7 @@ abstract class AbstractAnalysisServer { void addContextsToDeclarationsTracker() { declarationsTracker?.discardContexts(); documentationForContext.clear(); + extensionForContext.clear(); for (var driver in driverMap.values) { declarationsTracker?.addContext(driver.analysisContext!); driver.resetUriResolution(); @@ -347,6 +353,14 @@ abstract class AbstractAnalysisServer { return element; } + /// Return the object used to cache information about extensions in the + /// context that produced the [result], or `null` if there is no cache for the + /// context. + ExtensionCache? getExtensionCacheFor(ResolvedUnitResult result) { + var context = result.session.analysisContext; + return extensionForContext.putIfAbsent(context, () => ExtensionCache()); + } + /// Return a [Future] that completes with the resolved [AstNode] at the /// given [offset] of the given [file], or with `null` if there is no node as /// the [offset]. diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart index 57d87fe6434..4e08659ec59 100644 --- a/pkg/analysis_server/lib/src/edit/edit_domain.dart +++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart @@ -636,7 +636,7 @@ class EditDomainHandler extends AbstractRequestHandler { result.path!, name, ); - }); + }, extensionCache: server.getExtensionCacheFor(result)); List fixes; try { diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart index 322e9d8d0c3..b4289858609 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart @@ -312,7 +312,7 @@ class CodeActionHandler extends MessageHandler processedUnits = {}; + + /// A map from the name of a non-static public extension member to the set of + /// paths to libraries defining an extension member with that name. + final Map> membersByName = {}; + + /// Initialize a newly created cache. + ExtensionCache(); + + /// Fill the cache with data from the [result]. + void cacheFromResult(ResolvedUnitResult result) { + var element = result.unit?.declaredElement; + if (element != null) { + _cacheFromElement(element); + for (var library in result.libraryElement.importedLibraries) { + _cacheLibrary(library); + } + } + } + + /// Fill the cache with data from the [compilationUnit]. + void _cacheFromElement(CompilationUnitElement compilationUnit) { + // Record that we've cached data for the compilation unit. + var unitPath = _keyForUnit(compilationUnit); + processedUnits.add(unitPath); + + // Flush any data that was previously cached for the compilation unit. + for (var set in membersByName.values) { + set.removeWhere((element) => element.unitPath == unitPath); + } + + // Cache the data for the compilation unit. + var libraryPath = compilationUnit.librarySource.fullName; + for (var extension in compilationUnit.extensions) { + var extensionName = extension.name; + if (extensionName != null && !Identifier.isPrivateName(extensionName)) { + for (var member in extension.accessors) { + if (!member.isSynthetic) { + _recordMember(unitPath, libraryPath, member.displayName); + } + } + for (var member in extension.fields) { + if (!member.isSynthetic) { + _recordMember(unitPath, libraryPath, member.name); + } + } + for (var member in extension.methods) { + _recordMember(unitPath, libraryPath, member.name); + } + } + } + } + + /// Cache the data for the given [library] and every library exported from it + /// if it hasn't already been cached. + void _cacheLibrary(LibraryElement library) { + if (_hasDataFor(library.definingCompilationUnit)) { + return; + } + for (var unit in library.units) { + _cacheFromElement(unit); + } + for (var exported in library.exportedLibraries) { + _cacheLibrary(exported); + } + } + + /// Return `true` if the cache contains data for the [compilationUnit]. + bool _hasDataFor(CompilationUnitElement compilationUnit) { + return processedUnits.contains(_keyForUnit(compilationUnit)); + } + + /// Return the key used in the [extensionCache] for the [compilationUnit]. + String _keyForUnit(CompilationUnitElement compilationUnit) => + compilationUnit.source.fullName; + + /// Record that an extension member with the given [name] is defined in the + /// compilation unit with the [unitPath] in the library with the + /// [libraryPath]. + void _recordMember(String unitPath, String libraryPath, String name) { + membersByName + .putIfAbsent(name, () => {}) + .add(UnitInLibrary(unitPath, libraryPath)); + } +} + +/// A representation of a compilation unit in a library. +class UnitInLibrary { + final String unitPath; + final String libraryPath; + + UnitInLibrary(this.unitPath, this.libraryPath); +} diff --git a/pkg/analysis_server/lib/src/services/completion/dart/extension_member_contributor.dart b/pkg/analysis_server/lib/src/services/completion/dart/extension_member_contributor.dart index 8f452a110da..036786e7391 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/extension_member_contributor.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/extension_member_contributor.dart @@ -4,13 +4,10 @@ import 'package:analysis_server/src/provisional/completion/dart/completion_dart.dart'; import 'package:analysis_server/src/services/completion/dart/suggestion_builder.dart'; +import 'package:analysis_server/src/utilities/extensions/element.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/src/dart/element/generic_inferrer.dart' - show GenericInferrer; -import 'package:analyzer/src/dart/element/type_algebra.dart'; -import 'package:analyzer/src/dart/element/type_system.dart'; import 'package:analyzer/src/dart/resolver/scope.dart'; /// A contributor that produces suggestions based on the members of an @@ -87,7 +84,7 @@ class ExtensionMemberContributor extends DartCompletionContributor { if (type == null) { // Without a type we can't find the extensions that apply. We shouldn't // get to this point, but there's an NPE if we invoke - // `_resolveExtendedType` when `type` is `null`, so we guard against it + // `resolvedExtendedType` when `type` is `null`, so we guard against it // to ensure that we can return the suggestions from other providers. return; } @@ -101,7 +98,7 @@ class ExtensionMemberContributor extends DartCompletionContributor { var nameScope = containingLibrary.scope; for (var extension in nameScope.extensions) { var extendedType = - _resolveExtendedType(containingLibrary, extension, type); + extension.resolvedExtendedType(containingLibrary, type); if (extendedType != null && typeSystem.isSubtypeOf(type, extendedType)) { var inheritanceDistance = 0.0; if (type is InterfaceType && extendedType is InterfaceType) { @@ -141,34 +138,4 @@ class ExtensionMemberContributor extends DartCompletionContributor { accessor: accessor, inheritanceDistance: inheritanceDistance); } } - - /// Use the [type] of the object being extended in the [library] to compute - /// the actual type extended by the [extension]. Return the computed type, - /// or `null` if the type cannot be computed. - DartType? _resolveExtendedType( - LibraryElement library, - ExtensionElement extension, - DartType type, - ) { - var typeParameters = extension.typeParameters; - var inferrer = - GenericInferrer(library.typeSystem as TypeSystemImpl, typeParameters); - inferrer.constrainArgument( - type, - extension.extendedType, - 'extendedType', - ); - var typeArguments = inferrer.infer(typeParameters, - failAtError: true, genericMetadataIsEnabled: true); - if (typeArguments == null) { - return null; - } - var substitution = Substitution.fromPairs( - typeParameters, - typeArguments, - ); - return substitution.substituteType( - extension.extendedType, - ); - } } diff --git a/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart b/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart index 0c52fb4ead6..73ca312d457 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart @@ -6,6 +6,7 @@ import 'dart:math' as math; import 'package:_fe_analyzer_shared/src/scanner/token.dart'; import 'package:analysis_server/plugin/edit/fix/fix_dart.dart'; +import 'package:analysis_server/src/services/completion/dart/extension_cache.dart'; import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analysis_server/src/services/correction/fix/data_driven/transform_override_set.dart'; import 'package:analysis_server/src/services/correction/util.dart'; @@ -393,6 +394,9 @@ abstract class _AbstractCorrectionProducer { /// Returns the EOL to use for this [CompilationUnit]. String get eol => utils.endOfLine; + /// Return the extension cache used to find available extensions. + ExtensionCache get extensionCache => _context.dartFixContext!.extensionCache; + String get file => _context.file; Flutter get flutter => Flutter.instance; diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart index 6c90568438e..69137bf2e06 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart @@ -9,8 +9,11 @@ import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analysis_server/src/services/correction/namespace.dart'; import 'package:analysis_server/src/services/linter/lint_names.dart'; +import 'package:analysis_server/src/utilities/extensions/element.dart'; +import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_dart.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; @@ -24,26 +27,58 @@ class ImportLibrary extends MultiCorrectionProducer { @override Iterable get producers sync* { + final node = this.node; if (_importKind == _ImportKind.dartAsync) { yield* _importLibrary(DartFixKind.IMPORT_ASYNC, Uri.parse('dart:async')); } else if (_importKind == _ImportKind.forExtension) { if (node is SimpleIdentifier) { - var extensionName = (node as SimpleIdentifier).name; + var extensionName = node.name; yield* _importLibraryForElement( extensionName, const [ElementKind.EXTENSION], const [TopLevelDeclarationKind.extension]); } + } else if (_importKind == _ImportKind.forExtensionMember) { + /// Return producers that will import extensions that apply to the + /// [targetType] and that define a member with the given [memberName]. + Iterable importMatchingExtensions( + String memberName, DartType? targetType) sync* { + if (targetType == null) { + return; + } + var definingLibraries = extensionCache.membersByName[memberName]; + if (definingLibraries != null) { + for (var definingLibrary in definingLibraries) { + var libraryPath = definingLibrary.libraryPath; + var uri = sessionHelper.session.uriConverter.pathToUri(libraryPath); + if (uri != null) { + yield* _importExtensionInLibrary(uri, targetType, memberName); + } + } + } + } + + if (node is SimpleIdentifier) { + var memberName = node.name; + if (memberName.startsWith('_')) { + return; + } + yield* importMatchingExtensions(memberName, _targetType(node)); + } else if (node is BinaryExpression) { + var memberName = node.operator.lexeme; + yield* importMatchingExtensions( + memberName, node.leftOperand.staticType); + } } else if (_importKind == _ImportKind.forFunction) { if (node is SimpleIdentifier) { - if (node.parent is MethodInvocation) { - var invocation = node.parent as MethodInvocation; - if (invocation.realTarget != null || invocation.methodName != node) { + var parent = node.parent; + if (parent is MethodInvocation) { + if (parent.realTarget != null || parent.methodName != node) { return; } } - var name = (node as SimpleIdentifier).name; + var name = node.name; yield* _importLibraryForElement(name, const [ ElementKind.FUNCTION, ElementKind.TOP_LEVEL_VARIABLE @@ -127,6 +162,45 @@ class ImportLibrary extends MultiCorrectionProducer { return null; } + Iterable _importExtensionInLibrary( + Uri uri, DartType targetType, String memberName) sync* { + // Look to see whether the library at the [uri] is already imported. If it + // is, then we can check the extension elements without needing to perform + // additional analysis. + var foundImport = false; + for (var imp in libraryElement.imports) { + // prepare element + var importedLibrary = imp.importedLibrary; + if (importedLibrary == null || importedLibrary.source.uri != uri) { + continue; + } + foundImport = true; + for (var extension in importedLibrary.matchingExtensionsWithMember( + libraryElement, targetType, memberName)) { + // If the import has a combinator that needs to be updated, then offer + // to update it. + var combinators = imp.combinators; + if (combinators.length == 1) { + var combinator = combinators[0]; + if (combinator is HideElementCombinator) { + // TODO(brianwilkerson) Support removing the extension name from a + // hide combinator. + } else if (combinator is ShowElementCombinator) { + yield _ImportLibraryShow( + uri.toString(), combinator, extension.name!); + } + } + } + } + + // If the library at the [uri] is not already imported, we return a + // correction producer that will either add an import or not based on the + // result of analyzing the library. + if (!foundImport) { + yield _ImportLibraryContainingExtension(uri, targetType, memberName); + } + } + /// Returns a list of one or two import corrections. /// /// If [relativeUri] is `null`, only one correction, with an absolute import @@ -187,21 +261,22 @@ class ImportLibrary extends MultiCorrectionProducer { } // may be update "show" directive var combinators = imp.combinators; - if (combinators.length == 1 && combinators[0] is ShowElementCombinator) { - var showCombinator = combinators[0] as ShowElementCombinator; - // prepare new set of names to show - Set showNames = SplayTreeSet(); - showNames.addAll(showCombinator.shownNames); - showNames.add(name); - // prepare library name - unit name or 'dart:name' for SDK library - var libraryName = - libraryElement.definingCompilationUnit.source.uri.toString(); - if (libraryElement.isInSdk) { - libraryName = libraryElement.source.shortName; + if (combinators.length == 1) { + var combinator = combinators[0]; + if (combinator is HideElementCombinator) { + // TODO(brianwilkerson) Support removing the element name from a + // hide combinator. + } else if (combinator is ShowElementCombinator) { + // prepare library name - unit name or 'dart:name' for SDK library + var libraryName = + libraryElement.definingCompilationUnit.source.uri.toString(); + if (libraryElement.isInSdk) { + libraryName = libraryElement.source.shortName; + } + // don't add this library again + alreadyImportedWithPrefix.add(libraryElement.source.fullName); + yield _ImportLibraryShow(libraryName, combinator, name); } - // don't add this library again - alreadyImportedWithPrefix.add(libraryElement.source.fullName); - yield _ImportLibraryShow(libraryName, showCombinator, showNames); } } // Find new top-level declarations. @@ -250,6 +325,47 @@ class ImportLibrary extends MultiCorrectionProducer { return false; } + /// If the [node] might represent an access to a member of a type, return the + /// type of the object being accessed, otherwise return `null`. + DartType? _targetType(SimpleIdentifier node) { + var parent = node.parent; + if (parent is MethodInvocation && parent.methodName == node) { + var target = parent.realTarget; + if (target != null) { + return target.staticType; + } + } else if (parent is PropertyAccess && parent.propertyName == node) { + return parent.realTarget.staticType; + } else if (parent is PrefixedIdentifier && parent.identifier == node) { + return parent.prefix.staticType; + } + // If there is no explicit target, then return the type of an implicit + // `this`. + DartType? enclosingThisType(AstNode node) { + var parent = node.parent; + if (parent is ClassOrMixinDeclaration) { + return parent.declaredElement?.thisType; + } else if (parent is ExtensionDeclaration) { + return parent.extendedType.type; + } + } + + while (parent != null) { + if (parent is MethodDeclaration) { + if (!parent.isStatic) { + return enclosingThisType(parent); + } + return null; + } else if (parent is FieldDeclaration) { + if (!parent.isStatic) { + return enclosingThisType(parent); + } + return null; + } + parent = parent.parent; + } + } + /// Return an instance of this class that will add an import of `dart:async`. /// Used as a tear-off in `FixProcessor`. static ImportLibrary dartAsync() => ImportLibrary(_ImportKind.dartAsync); @@ -259,6 +375,9 @@ class ImportLibrary extends MultiCorrectionProducer { static ImportLibrary forExtension() => ImportLibrary(_ImportKind.forExtension); + static ImportLibrary forExtensionMember() => + ImportLibrary(_ImportKind.forExtensionMember); + /// Return an instance of this class that will add an import for a top-level /// function. Used as a tear-off in `FixProcessor`. static ImportLibrary forFunction() => ImportLibrary(_ImportKind.forFunction); @@ -273,8 +392,7 @@ class ImportLibrary extends MultiCorrectionProducer { static ImportLibrary forType() => ImportLibrary(_ImportKind.forType); } -/// A correction processor that can make one of the possible change computed by -/// the [ImportLibrary] producer. +/// A correction processor that can add an import using an absolute URI. class _ImportAbsoluteLibrary extends CorrectionProducer { final FixKind _fixKind; @@ -301,13 +419,53 @@ class _ImportAbsoluteLibrary extends CorrectionProducer { enum _ImportKind { dartAsync, forExtension, + forExtensionMember, forFunction, forTopLevelVariable, forType } -/// A correction processor that can make one of the possible change computed by -/// the [ImportLibrary] producer. +/// A correction processor that can add an import of a library containing an +/// extension, but which does so only if the extension applies to a given type. +class _ImportLibraryContainingExtension extends CorrectionProducer { + /// The URI of the library defining the extension. + Uri uri; + + /// The type of the target that the extension must apply to. + DartType targetType; + + /// The name of the member that the extension must declare. + String memberName; + + /// The URI that is being proposed for the import directive. + String _uriText = ''; + + _ImportLibraryContainingExtension(this.uri, this.targetType, this.memberName); + + @override + List get fixArguments => [_uriText]; + + @override + FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_PROJECT1; + + @override + Future compute(ChangeBuilder builder) async { + var result = await sessionHelper.session.getLibraryByUri2(uri.toString()); + if (result is LibraryElementResult) { + var library = result.element; + if (library + .matchingExtensionsWithMember(libraryElement, targetType, memberName) + .isNotEmpty) { + await builder.addDartFileEdit(file, (builder) { + _uriText = builder.importLibrary(uri); + }); + } + } + } +} + +/// A correction processor that can add a prefix to an identifier defined in a +/// library that is already imported but that is imported with a prefix. class _ImportLibraryPrefix extends CorrectionProducer { final LibraryElement _importedLibrary; final PrefixElement _importPrefix; @@ -334,16 +492,16 @@ class _ImportLibraryPrefix extends CorrectionProducer { } } -/// A correction processor that can make one of the possible change computed by -/// the [ImportLibrary] producer. +/// A correction processor that can add a name to the show combinator of an +/// existing import. class _ImportLibraryShow extends CorrectionProducer { final String _libraryName; final ShowElementCombinator _showCombinator; - final Set _showNames; + final String _addedName; - _ImportLibraryShow(this._libraryName, this._showCombinator, this._showNames); + _ImportLibraryShow(this._libraryName, this._showCombinator, this._addedName); @override List get fixArguments => [_libraryName]; @@ -353,7 +511,10 @@ class _ImportLibraryShow extends CorrectionProducer { @override Future compute(ChangeBuilder builder) async { - var newShowCode = 'show ${_showNames.join(', ')}'; + Set showNames = SplayTreeSet(); + showNames.addAll(_showCombinator.shownNames); + showNames.add(_addedName); + var newShowCode = 'show ${showNames.join(', ')}'; var offset = _showCombinator.offset; var length = _showCombinator.end - offset; var libraryFile = resolvedResult.libraryElement.source.fullName; @@ -363,8 +524,7 @@ class _ImportLibraryShow extends CorrectionProducer { } } -/// A correction processor that can make one of the possible change computed by -/// the [ImportLibrary] producer. +/// A correction processor that can add an import using a relative URI. class _ImportRelativeLibrary extends CorrectionProducer { final FixKind _fixKind; diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index f76a542bf54..97e39a21643 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/plugin/edit/fix/fix_dart.dart'; +import 'package:analysis_server/src/services/completion/dart/extension_cache.dart'; import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analysis_server/src/services/linter/lint_names.dart'; import 'package:analyzer/dart/analysis/results.dart'; @@ -129,11 +130,16 @@ class DartFixContextImpl implements DartFixContext { @override final AnalysisError error; + @override + final ExtensionCache extensionCache; + final List Function(String name) getTopLevelDeclarationsFunction; DartFixContextImpl(this.instrumentationService, this.workspace, - this.resolveResult, this.error, this.getTopLevelDeclarationsFunction); + this.resolveResult, this.error, this.getTopLevelDeclarationsFunction, + {ExtensionCache? extensionCache}) + : extensionCache = extensionCache ?? ExtensionCache(); @override List getTopLevelDeclarations(String name) { diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 4ebbaad2d5d..0ecd3097cd8 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -234,6 +234,7 @@ class FixInFileProcessor { resolveResult, error, (name) => [], + extensionCache: context.extensionCache, ); fixState = await _fixError(fixContext, fixState, generator(), error); } @@ -1131,6 +1132,7 @@ class FixProcessor extends BaseProcessor { ], CompileTimeErrorCode.UNDEFINED_GETTER: [ DataDriven.newInstance, + ImportLibrary.forExtensionMember, ImportLibrary.forTopLevelVariable, ImportLibrary.forType, ], @@ -1143,6 +1145,7 @@ class FixProcessor extends BaseProcessor { ], CompileTimeErrorCode.UNDEFINED_METHOD: [ DataDriven.newInstance, + ImportLibrary.forExtensionMember, ImportLibrary.forFunction, ImportLibrary.forType, ], @@ -1150,9 +1153,13 @@ class FixProcessor extends BaseProcessor { ChangeArgumentName.newInstance, DataDriven.newInstance, ], + CompileTimeErrorCode.UNDEFINED_OPERATOR: [ + ImportLibrary.forExtensionMember, + ], CompileTimeErrorCode.UNDEFINED_SETTER: [ DataDriven.newInstance, - // TODO(brianwilkerson) Support ImportLibrary + // TODO(brianwilkerson) Support ImportLibrary for non-extension members. + ImportLibrary.forExtensionMember, ], CompileTimeErrorCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS: [ DataDriven.newInstance, diff --git a/pkg/analysis_server/lib/src/utilities/extensions/element.dart b/pkg/analysis_server/lib/src/utilities/extensions/element.dart index 73e8eb8a046..764d8abceb0 100644 --- a/pkg/analysis_server/lib/src/utilities/extensions/element.dart +++ b/pkg/analysis_server/lib/src/utilities/extensions/element.dart @@ -2,7 +2,12 @@ // 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 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/src/dart/element/generic_inferrer.dart'; +import 'package:analyzer/src/dart/element/type_algebra.dart'; +import 'package:analyzer/src/dart/element/type_system.dart'; extension ClassElementExtensions on ClassElement { /// Return `true` if this element represents the class `Iterable` from @@ -55,6 +60,58 @@ extension ElementExtension on Element { } } +extension ExtensionElementExtensions on ExtensionElement { + /// Use the [type] of the object being extended in the [library] to compute + /// the actual type extended by this [extension]. Return the computed type, + /// or `null` if the type can't be computed. + DartType? resolvedExtendedType(LibraryElement library, DartType type) { + final typeParameters = this.typeParameters; + var inferrer = + GenericInferrer(library.typeSystem as TypeSystemImpl, typeParameters); + inferrer.constrainArgument( + type, + extendedType, + 'extendedType', + ); + var typeArguments = inferrer.infer(typeParameters, + failAtError: true, genericMetadataIsEnabled: true); + if (typeArguments == null) { + return null; + } + var substitution = Substitution.fromPairs( + typeParameters, + typeArguments, + ); + return substitution.substituteType( + extendedType, + ); + } +} + +extension LibraryElementExtensions on LibraryElement { + /// Return the extensions in this library that can be applied, within the + /// [containingLibrary], to the [targetType] and that define a member with the + /// given [memberName]. + Iterable matchingExtensionsWithMember( + LibraryElement containingLibrary, + DartType targetType, + String memberName) sync* { + for (var unit in units) { + for (var extension in unit.extensions) { + var extensionName = extension.name; + if (extensionName != null && !Identifier.isPrivateName(extensionName)) { + var extendedType = + extension.resolvedExtendedType(containingLibrary, targetType); + if (extendedType != null && + typeSystem.isSubtypeOf(targetType, extendedType)) { + yield extension; + } + } + } + } + } +} + extension MethodElementExtensions on MethodElement { /// Return `true` if this element represents the method `cast` from either /// `Iterable`, `List`, `Map`, or `Set`. diff --git a/pkg/analysis_server/test/src/services/correction/fix/create_local_variable_test.dart b/pkg/analysis_server/test/src/services/correction/fix/create_local_variable_test.dart index 2c1d827c14c..4e3c01b929b 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/create_local_variable_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/create_local_variable_test.dart @@ -79,6 +79,17 @@ main() { '''); } + @failingTest + Future test_propertyAccess() async { + // We should not offer to define a local variable named 'g'. + await resolveTestCode(''' +void f(String s) { + s.g; +} +'''); + await assertNoFix(); + } + Future test_read_typeAssignment() async { await resolveTestCode(''' main() { diff --git a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart index 7ac6c2b696e..fd320f27cb9 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart @@ -3,10 +3,12 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/plugin/edit/fix/fix_core.dart'; +import 'package:analysis_server/src/services/completion/dart/extension_cache.dart'; import 'package:analysis_server/src/services/correction/change_workspace.dart'; import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analysis_server/src/services/correction/fix_internal.dart'; +import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer/src/dart/analysis/byte_store.dart'; import 'package:analyzer/src/dart/error/lint_codes.dart'; @@ -172,9 +174,18 @@ abstract class FixProcessorLintTest extends FixProcessorTest { /// A base class defining support for writing fix processor tests. abstract class FixProcessorTest extends BaseFixProcessorTest { + /// The extension cache used for test purposes. + ExtensionCache extensionCache = ExtensionCache(); + /// Return the kind of fixes being tested by this test class. FixKind get kind; + Future addUnimportedFile(String filePath, String content) async { + addSource(filePath, content); + var result = await session.getResolvedUnit2(filePath); + extensionCache.cacheFromResult(result as ResolvedUnitResult); + } + Future assertHasFix(String expected, {bool Function(AnalysisError)? errorFilter, int? length, @@ -429,6 +440,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest { var tracker = DeclarationsTracker(MemoryByteStore(), resourceProvider); tracker.addContext(analysisContext); + extensionCache.cacheFromResult(testAnalysisResult); var context = DartFixContextImpl( TestInstrumentationService(), @@ -440,6 +452,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest { provider.doTrackerWork(); return provider.get(analysisContext, testFile, name); }, + extensionCache: extensionCache, ); return await DartFixContributor().computeFixes(context); } diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_library_project_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_library_project_test.dart index 98c939bbd8d..ccd65da6a66 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_library_project_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_library_project_test.dart @@ -39,6 +39,201 @@ main() { await assertNoFix(); } + Future test_extension_notImported_field_onThisType_fromClass() async { + addUnimportedFile('/home/test/lib/lib2.dart', ''' +import 'package:test/lib1.dart'; + +extension E on C { + int m() => 0; +} +'''); + addSource('/home/test/lib/lib1.dart', ''' +class C {} +'''); + await resolveTestCode(''' +import 'package:test/lib1.dart'; + +class D extends C { + int f = m(); +} +'''); + await assertHasFix(''' +import 'package:test/lib1.dart'; +import 'package:test/lib2.dart'; + +class D extends C { + int f = m(); +} +'''); + } + + Future test_extension_notImported_getter() async { + addUnimportedFile('/home/test/lib/lib.dart', ''' +extension E on String { + int get m => 0; +} +'''); + await resolveTestCode(''' +void f(String s) { + s.m; +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart'; + +void f(String s) { + s.m; +} +'''); + } + + Future test_extension_notImported_method() async { + addUnimportedFile('/home/test/lib/lib.dart', ''' +extension E on String { + void m() {} +} +'''); + await resolveTestCode(''' +void f(String s) { + s.m(); +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart'; + +void f(String s) { + s.m(); +} +'''); + } + + Future test_extension_notImported_method_extendsGeneric() async { + addUnimportedFile('/home/test/lib/lib.dart', ''' +import 'package:test/lib1.dart'; + +extension E on List { + void m() {} +} +'''); + await resolveTestCode(''' +void f(List l) { + l.m(); +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart'; + +void f(List l) { + l.m(); +} +'''); + } + + Future test_extension_notImported_method_onThisType_fromClass() async { + addUnimportedFile('/home/test/lib/lib2.dart', ''' +import 'package:test/lib1.dart'; + +extension E on C { + void m() {} +} +'''); + addSource('/home/test/lib/lib1.dart', ''' +class C {} +'''); + await resolveTestCode(''' +import 'package:test/lib1.dart'; + +class D extends C { + void m2() { + m(); + } +} +'''); + await assertHasFix(''' +import 'package:test/lib1.dart'; +import 'package:test/lib2.dart'; + +class D extends C { + void m2() { + m(); + } +} +'''); + } + + Future + test_extension_notImported_method_onThisType_fromExtension() async { + addUnimportedFile('/home/test/lib/lib2.dart', ''' +import 'package:test/lib1.dart'; + +extension E on C { + void m() {} +} +'''); + addSource('/home/test/lib/lib1.dart', ''' +class C {} +'''); + await resolveTestCode(''' +import 'package:test/lib1.dart'; + +extension F on C { + void m2() { + m(); + } +} +'''); + await assertHasFix(''' +import 'package:test/lib1.dart'; +import 'package:test/lib2.dart'; + +extension F on C { + void m2() { + m(); + } +} +'''); + } + + Future test_extension_notImported_operator() async { + addUnimportedFile('/home/test/lib/lib.dart', ''' +extension E on String { + String operator -(String other) => this; +} +'''); + await resolveTestCode(''' +void f(String s) { + s - '2'; +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart'; + +void f(String s) { + s - '2'; +} +'''); + } + + Future test_extension_notImported_setter() async { + addUnimportedFile('/home/test/lib/lib.dart', ''' +extension E on String { + set m(int v) {} +} +'''); + await resolveTestCode(''' +void f(String s) { + s.m = 2; +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart'; + +void f(String s) { + s.m = 2; +} +'''); + } + Future test_invalidUri_interpolation() async { addSource('/home/test/lib/lib.dart', r''' class Test { diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_library_show_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_library_show_test.dart index e54adba2fd5..118b44f1e96 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_library_show_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_library_show_test.dart @@ -19,6 +19,98 @@ class ImportLibraryShowTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.IMPORT_LIBRARY_SHOW; + Future test_extension_notShown_getter() async { + addSource('/home/test/lib/lib.dart', ''' +class C {} +extension E on String { + int get m => 0; +} +'''); + await resolveTestCode(''' +import 'package:test/lib.dart' show C; + +void f(String s, C c) { + s.m; +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart' show C, E; + +void f(String s, C c) { + s.m; +} +'''); + } + + Future test_extension_notShown_method() async { + addSource('/home/test/lib/lib.dart', ''' +class C {} +extension E on String { + void m() {} +} +'''); + await resolveTestCode(''' +import 'package:test/lib.dart' show C; + +void f(String s, C c) { + s.m(); +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart' show C, E; + +void f(String s, C c) { + s.m(); +} +'''); + } + + Future test_extension_notShown_operator() async { + addSource('/home/test/lib/lib.dart', ''' +class C {} +extension E on String { + String operator -(String other) => this; +} +'''); + await resolveTestCode(''' +import 'package:test/lib.dart' show C; + +void f(String s, C c) { + s - '2'; +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart' show C, E; + +void f(String s, C c) { + s - '2'; +} +'''); + } + + Future test_extension_notShown_setter() async { + addSource('/home/test/lib/lib.dart', ''' +class C {} +extension E on String { + set m(int v) {} +} +'''); + await resolveTestCode(''' +import 'package:test/lib.dart' show C; + +void f(String s, C c) { + s.m = 2; +} +'''); + await assertHasFix(''' +import 'package:test/lib.dart' show C, E; + +void f(String s, C c) { + s.m = 2; +} +'''); + } + Future test_override_samePackage() async { addSource('/home/test/lib/lib.dart', ''' class A {}