diff --git a/pkg/analysis_server/lib/src/services/completion/dart/candidate_suggestion.dart b/pkg/analysis_server/lib/src/services/completion/dart/candidate_suggestion.dart index 45bd9cef25b..1b0822553d1 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/candidate_suggestion.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/candidate_suggestion.dart @@ -120,7 +120,7 @@ final class LabelSuggestion extends CandidateSuggestion { /// The information about a candidate suggestion based on a local function. final class LocalFunctionSuggestion extends CandidateSuggestion { /// The element on which the suggestion is based. - final ExecutableElement element; + final FunctionElement element; /// Initialize a newly created candidate suggestion to suggest the [element]. LocalFunctionSuggestion(this.element); @@ -160,8 +160,7 @@ extension SuggestionBuilderExtension on SuggestionBuilder { case LabelSuggestion(): suggestLabel(suggestion.label); case LocalFunctionSuggestion(): - // TODO(brianwilkerson) Add support for suggesting a local function. - break; + suggestTopLevelFunction(suggestion.element); case LocalVariableSuggestion(): // TODO(brianwilkerson) Enhance `suggestLocalVariable` to allow the // distance to be passed in. diff --git a/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart b/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart index 1f0d8509eba..4bd81bce0da 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart @@ -33,6 +33,7 @@ import 'package:analysis_server/src/services/completion/dart/super_formal_contri import 'package:analysis_server/src/services/completion/dart/type_member_contributor.dart'; import 'package:analysis_server/src/services/completion/dart/uri_contributor.dart'; import 'package:analysis_server/src/services/completion/dart/variable_name_contributor.dart'; +import 'package:analysis_server/src/services/completion/dart/visibility_tracker.dart'; import 'package:analysis_server/src/utilities/selection.dart'; import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/analysis/results.dart'; @@ -145,6 +146,7 @@ class DartCompletionManager { // Compute the list of contributors that will be run. var builder = SuggestionBuilder(request, useFilter: useFilter, listener: listener); + var localReferenceContributor = LocalReferenceContributor(request, builder); var contributors = [ ArgListContributor(request, builder), ClosureContributor(request, builder), @@ -155,7 +157,7 @@ class DartCompletionManager { LibraryMemberContributor(request, builder), LibraryPrefixContributor(request, builder), LocalLibraryContributor(request, builder), - LocalReferenceContributor(request, builder), + localReferenceContributor, NamedConstructorContributor(request, builder), if (enableOverrideContributor) OverrideContributor(request, builder), RecordLiteralContributor(request, builder), @@ -185,12 +187,16 @@ class DartCompletionManager { } try { + VisibilityTracker? visibilityTracker; await performance.runAsync( 'InScopeCompletionPass', (performance) async { - _runFirstPass(request, builder); + visibilityTracker = _runFirstPass(request, builder); }, ); + if (visibilityTracker != null) { + localReferenceContributor.visibilityTracker = visibilityTracker!; + } for (var contributor in contributors) { await performance.runAsync( '${contributor.runtimeType}', @@ -298,7 +304,10 @@ class DartCompletionManager { } // Run the first pass of the code completion algorithm. - void _runFirstPass(DartCompletionRequest request, SuggestionBuilder builder) { + VisibilityTracker? _runFirstPass( + DartCompletionRequest request, SuggestionBuilder builder) { + // TODO(brianwilkerson) Stop returning the visibility tracker when the + // `LocalReferenceContributor` has been deleted. var collector = SuggestionCollector(); var selection = request.unit.select(offset: request.offset, length: 0); if (selection == null) { @@ -308,6 +317,7 @@ class DartCompletionManager { var pass = InScopeCompletionPass(state: state, collector: collector); pass.computeSuggestions(); builder.suggestFromCandidates(collector.suggestions); + return pass.visibilityTracker; } } diff --git a/pkg/analysis_server/lib/src/services/completion/dart/declaration_helper.dart b/pkg/analysis_server/lib/src/services/completion/dart/declaration_helper.dart index fce8f2074c5..f7de3f46914 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/declaration_helper.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/declaration_helper.dart @@ -4,6 +4,7 @@ import 'package:analysis_server/src/services/completion/dart/candidate_suggestion.dart'; import 'package:analysis_server/src/services/completion/dart/suggestion_collector.dart'; +import 'package:analysis_server/src/services/completion/dart/visibility_tracker.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; @@ -23,7 +24,7 @@ class DeclarationHelper { /// The visibility tracker used to prevent suggesting elements that have been /// shadowed by local declarations. - final _VisibilityTracker _visibilityTracker = _VisibilityTracker(); + final VisibilityTracker visibilityTracker = VisibilityTracker(); /// A flag indicating whether suggestions should be limited to only include /// valid constants. @@ -73,6 +74,9 @@ class DeclarationHelper { case FieldDeclaration(): return currentNode; case ForElement(forLoopParts: var parts): + if (parts != previousNode) { + _visitForLoopParts(parts); + } case ForStatement(forLoopParts: var parts): if (parts != previousNode) { _visitForLoopParts(parts); @@ -117,7 +121,7 @@ class DeclarationHelper { /// Add a suggestion for the local function represented by the [element]. void _suggestFunction(ExecutableElement element) { - if (_visibilityTracker.isVisible(element)) { + if (element is FunctionElement && visibilityTracker.isVisible(element)) { var suggestion = LocalFunctionSuggestion(element); collector.addSuggestion(suggestion); } @@ -128,7 +132,7 @@ class DeclarationHelper { if (mustBeConstant && !element.isConst) { return; } - if (_visibilityTracker.isVisible(element) && !_isUnused(element.name)) { + if (visibilityTracker.isVisible(element) && !_isUnused(element.name)) { var suggestion = FormalParameterSuggestion(element); collector.addSuggestion(suggestion); } @@ -139,7 +143,7 @@ class DeclarationHelper { if (mustBeConstant && !element.isConst) { return; } - if (_visibilityTracker.isVisible(element)) { + if (visibilityTracker.isVisible(element)) { var suggestion = LocalVariableSuggestion(element, _variableDistance++); collector.addSuggestion(suggestion); } @@ -162,6 +166,8 @@ class DeclarationHelper { switch (member) { case ConstructorDeclaration(): _visitParameterList(member.parameters); + case FunctionDeclaration(): + _visitParameterList(member.functionExpression.parameters); case FunctionExpression(): _visitParameterList(member.parameters); case MethodDeclaration(): @@ -363,19 +369,3 @@ class DeclarationHelper { } } } - -/// This class tracks the set of names already added in the completion list in -/// order to prevent suggesting elements that have been shadowed by local -/// declarations. -class _VisibilityTracker { - /// The set of known previously declared names in this contributor. - final Set declaredNames = {}; - - /// Before completions are added by this contributor, we verify with this - /// method if the element has already been added, this prevents suggesting - /// [Element]s that are shadowed. - bool isVisible(Element? element) { - var name = element?.name; - return name != null && declaredNames.add(name); - } -} diff --git a/pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart b/pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart index 0eb54b017d9..3824c884adb 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart @@ -7,6 +7,7 @@ import 'package:analysis_server/src/services/completion/dart/declaration_helper. import 'package:analysis_server/src/services/completion/dart/keyword_helper.dart'; import 'package:analysis_server/src/services/completion/dart/label_helper.dart'; import 'package:analysis_server/src/services/completion/dart/suggestion_collector.dart'; +import 'package:analysis_server/src/services/completion/dart/visibility_tracker.dart'; import 'package:analysis_server/src/utilities/extensions/ast.dart'; import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/ast/ast.dart'; @@ -32,10 +33,6 @@ class InScopeCompletionPass extends SimpleAstVisitor { /// The suggestion collector to which suggestions will be added. final SuggestionCollector collector; - /// The helper used to suggest declarations that are in scope. - late final DeclarationHelper declarationHelper = - DeclarationHelper(collector: collector, offset: offset); - /// The helper used to suggest keywords. late final KeywordHelper keywordHelper = KeywordHelper( collector: collector, featureSet: featureSet, offset: offset); @@ -43,6 +40,9 @@ class InScopeCompletionPass extends SimpleAstVisitor { /// The helper used to suggest labels. late final LabelHelper labelHelper = LabelHelper(collector: collector); + /// The helper used to suggest declarations that are in scope. + DeclarationHelper? _declarationHelper; + /// Initialize a newly created completion visitor that can use the [state] to /// add candidate suggestions to the [collector]. InScopeCompletionPass({required this.state, required this.collector}); @@ -54,6 +54,10 @@ class InScopeCompletionPass extends SimpleAstVisitor { /// Return the offset at which completion was requested. int get offset => state.selection.offset; + /// Returns the visibility tracker used by this pass. + VisibilityTracker? get visibilityTracker => + _declarationHelper?.visibilityTracker; + /// Return the node that should be used as the context in which completion is /// occurring. /// @@ -95,6 +99,10 @@ class InScopeCompletionPass extends SimpleAstVisitor { _completionNode.accept(this); } + /// Return the helper used to suggest declarations that are in scope. + DeclarationHelper declarationHelper() => _declarationHelper = + DeclarationHelper(collector: collector, offset: offset); + @override void visitAdjacentStrings(AdjacentStrings node) { _visitParentIfAtOrBeforeNode(node); @@ -295,7 +303,7 @@ class InScopeCompletionPass extends SimpleAstVisitor { @override void visitCommentReference(CommentReference node) { - declarationHelper.addLexicalDeclarations(node); + declarationHelper().addLexicalDeclarations(node); } @override @@ -829,7 +837,7 @@ class InScopeCompletionPass extends SimpleAstVisitor { @override void visitInterpolationExpression(InterpolationExpression node) { - declarationHelper.addLexicalDeclarations(node); + declarationHelper().addLexicalDeclarations(node); } @override @@ -1512,7 +1520,7 @@ class InScopeCompletionPass extends SimpleAstVisitor { TypedLiteral literal, NodeList elements) { var preceedingElement = elements.elementBefore(offset); keywordHelper.addCollectionElementKeywords(literal, elements); - declarationHelper.addLexicalDeclarations(preceedingElement ?? literal); + declarationHelper().addLexicalDeclarations(preceedingElement ?? literal); } /// Add the suggestions that are appropriate when the selection is at the @@ -1539,7 +1547,7 @@ class InScopeCompletionPass extends SimpleAstVisitor { var inConstantContext = node is Expression && node.inConstantContext; keywordHelper.addConstantExpressionKeywords( inConstantContext: inConstantContext); - declarationHelper.addLexicalDeclarations(node, mustBeConstant: true); + declarationHelper().addLexicalDeclarations(node, mustBeConstant: true); } /// Add the suggestions that are appropriate when the selection is at the @@ -1561,7 +1569,7 @@ class InScopeCompletionPass extends SimpleAstVisitor { void _forExpression(AstNode? node) { keywordHelper.addExpressionKeywords(node); if (node != null) { - declarationHelper.addLexicalDeclarations(node); + declarationHelper().addLexicalDeclarations(node); } } @@ -1652,7 +1660,8 @@ class InScopeCompletionPass extends SimpleAstVisitor { /// beginning of a pattern. void _forPattern(AstNode node, {bool mustBeConst = true}) { keywordHelper.addPatternKeywords(); - declarationHelper.addLexicalDeclarations(node, mustBeConstant: mustBeConst); + declarationHelper() + .addLexicalDeclarations(node, mustBeConstant: mustBeConst); } /// Add the suggestions that are appropriate when the selection is at the @@ -1804,7 +1813,7 @@ class InScopeCompletionPass extends SimpleAstVisitor { } } else if (!node.inKeyword.isSynthetic) { keywordHelper.addKeyword(Keyword.AWAIT); - declarationHelper.addLexicalDeclarations(node); + declarationHelper().addLexicalDeclarations(node); } } diff --git a/pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart b/pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart index fc708b232a1..537758326a9 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart @@ -7,6 +7,7 @@ import 'package:analysis_server/src/protocol_server.dart' import 'package:analysis_server/src/provisional/completion/dart/completion_dart.dart'; import 'package:analysis_server/src/services/completion/dart/completion_manager.dart'; import 'package:analysis_server/src/services/completion/dart/suggestion_builder.dart'; +import 'package:analysis_server/src/services/completion/dart/visibility_tracker.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; @@ -31,7 +32,7 @@ class LocalReferenceContributor extends DartCompletionContributor { /// The [_VisibilityTracker] tracks the set of elements already added in the /// completion list, this object helps prevents suggesting elements that have /// been shadowed by local declarations. - final _VisibilityTracker _visibilityTracker = _VisibilityTracker(); + VisibilityTracker visibilityTracker = VisibilityTracker(); LocalReferenceContributor(super.request, super.builder); @@ -72,7 +73,7 @@ class LocalReferenceContributor extends DartCompletionContributor { try { builder.laterReplacesEarlier = false; var localVisitor = _LocalVisitor( - request, builder, _visibilityTracker, + request, builder, visibilityTracker, suggestLocalFields: suggestLocalFields); localVisitor.visit(node); } finally { @@ -107,7 +108,7 @@ class LocalReferenceContributor extends DartCompletionContributor { if (!isFunctionalArgument) { for (var accessor in type.accessors) { if (!accessor.isStatic) { - if (_visibilityTracker._isVisible(accessor.declaration)) { + if (visibilityTracker.isVisible(accessor.declaration)) { if (accessor.isGetter) { if (opType.includeReturnValueSuggestions) { memberBuilder.addSuggestionForAccessor( @@ -127,7 +128,7 @@ class LocalReferenceContributor extends DartCompletionContributor { } for (var method in type.methods) { if (!method.isStatic) { - if (_visibilityTracker._isVisible(method.declaration)) { + if (visibilityTracker.isVisible(method.declaration)) { if (method.returnType is! VoidType) { if (opType.includeReturnValueSuggestions) { memberBuilder.addSuggestionForMethod( @@ -206,7 +207,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { /// clause. bool inExtendsClause = false; - _VisibilityTracker visibilityTracker; + VisibilityTracker visibilityTracker; _LocalVisitor(this.request, this.builder, this.visibilityTracker, {required this.suggestLocalFields}) @@ -245,7 +246,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { void declaredExtension(ExtensionDeclaration declaration) { var declaredElement = declaration.declaredElement; if (declaredElement != null && - visibilityTracker._isVisible(declaredElement) && + visibilityTracker.isVisible(declaredElement) && opType.includeReturnValueSuggestions && declaration.name != null) { builder.suggestExtension(declaredElement, kind: _defaultKind); @@ -256,7 +257,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { void declaredField(FieldDeclaration fieldDecl, VariableDeclaration varDecl) { var field = varDecl.declaredElement; if (field is FieldElement && - ((visibilityTracker._isVisible(field) && + ((visibilityTracker.isVisible(field) && opType.includeReturnValueSuggestions && (!opType.inStaticMethodBody || fieldDecl.isStatic)) || suggestLocalFields)) { @@ -277,7 +278,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { @override void declaredFunction(FunctionDeclaration declaration) { - if (visibilityTracker._isVisible(declaration.declaredElement) && + if (visibilityTracker.isVisible(declaration.declaredElement) && (opType.includeReturnValueSuggestions || opType.includeVoidReturnSuggestions)) { if (declaration.isSetter) { @@ -324,7 +325,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { @override void declaredMethod(MethodDeclaration declaration) { var element = declaration.declaredElement; - if (visibilityTracker._isVisible(element) && + if (visibilityTracker.isVisible(element) && (opType.includeReturnValueSuggestions || opType.includeVoidReturnSuggestions) && (!opType.inStaticMethodBody || declaration.isStatic)) { @@ -354,7 +355,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { var declaredElement = declaration.declaredElement; if (!inExtendsClause && declaredElement != null && - visibilityTracker._isVisible(declaredElement) && + visibilityTracker.isVisible(declaredElement) && opType.includeTypeNameSuggestions) { builder.suggestInterface(declaredElement); } @@ -365,7 +366,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { VariableDeclarationList varList, VariableDeclaration varDecl) { var variableElement = varDecl.declaredElement; if (variableElement is TopLevelVariableElement && - visibilityTracker._isVisible(variableElement) && + visibilityTracker.isVisible(variableElement) && (opType.includeReturnValueSuggestions || (opType.includeAnnotationSuggestions && variableElement.isConst))) { var getter = variableElement.getter; @@ -379,7 +380,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { void declaredTypeParameter(TypeParameter node) { var declaredElement = node.declaredElement; if (declaredElement != null && - visibilityTracker._isVisible(declaredElement) && + visibilityTracker.isVisible(declaredElement) && opType.includeTypeNameSuggestions) { builder.suggestTypeParameter(declaredElement); } @@ -392,7 +393,7 @@ class _LocalVisitor extends LocalDeclarationVisitor { } void _declaredInterfaceElement(InterfaceElement? element) { - if (element != null && visibilityTracker._isVisible(element)) { + if (element != null && visibilityTracker.isVisible(element)) { if (opType.includeTypeNameSuggestions) { builder.suggestInterface(element); } @@ -441,19 +442,3 @@ class _LocalVisitor extends LocalDeclarationVisitor { return false; } } - -/// This class tracks the set of elements already added in the completion list, -/// this object helps prevents suggesting elements that have been shadowed by -/// local declarations. -class _VisibilityTracker { - /// The set of known previously declared names in this contributor. - final Set declaredNames = {}; - - /// Before completions are added by this contributor, we verify with this - /// method if the element has already been added, this prevents suggesting - /// [Element]s that are shadowed. - bool _isVisible(Element? element) { - var name = element?.name; - return name != null && declaredNames.add(name); - } -} diff --git a/pkg/analysis_server/lib/src/services/completion/dart/visibility_tracker.dart b/pkg/analysis_server/lib/src/services/completion/dart/visibility_tracker.dart new file mode 100644 index 00000000000..97a168c0a66 --- /dev/null +++ b/pkg/analysis_server/lib/src/services/completion/dart/visibility_tracker.dart @@ -0,0 +1,21 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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/element/element.dart'; + +/// This class tracks the set of names already added in the completion list in +/// order to prevent suggesting elements that have been shadowed by local +/// declarations. +class VisibilityTracker { + /// The set of known previously declared names. + final Set declaredNames = {}; + + /// Before completions are added by the helper, we verify with this method + /// whether the name of the element has already been added, in order to + /// prevent suggesting elements that are shadowed. + bool isVisible(Element? element) { + var name = element?.name; + return name != null && declaredNames.add(name); + } +} diff --git a/pkg/analysis_server/test/services/completion/dart/completion_test.dart b/pkg/analysis_server/test/services/completion/dart/completion_test.dart index edbeff67b09..61146465f4f 100644 --- a/pkg/analysis_server/test/services/completion/dart/completion_test.dart +++ b/pkg/analysis_server/test/services/completion/dart/completion_test.dart @@ -10049,7 +10049,6 @@ suggestions '''); } - @failingTest Future test_completion_dartDoc_reference_forFunction_1() async { allowedIdentifiers = {'aaa', 'bbb'}; await computeSuggestions(''' @@ -10061,7 +10060,16 @@ suggestions functionA(aaa, bbb) {} functionB() {} '''); - assertResponse(r''' + if (isProtocolVersion2) { + assertResponse(r''' +replacement + left: 2 +suggestions + aaa + kind: parameter +'''); + } else { + assertResponse(r''' replacement left: 2 suggestions @@ -10070,6 +10078,7 @@ suggestions bbb kind: parameter '''); + } } Future test_completion_dartDoc_reference_forFunction_2() async { diff --git a/pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart b/pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart index a2d0b9a460f..5cb90eae914 100644 --- a/pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart +++ b/pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart @@ -493,7 +493,7 @@ suggestions b0 kind: function b1 - kind: function + kind: functionInvocation h0 kind: function '''); @@ -511,7 +511,7 @@ suggestions b0 kind: function b1 - kind: function + kind: functionInvocation b2 kind: functionInvocation h0 @@ -553,7 +553,7 @@ suggestions b0 kind: function b1 - kind: function + kind: functionInvocation b2 kind: function h0 @@ -573,7 +573,7 @@ suggestions b0 kind: function b1 - kind: function + kind: functionInvocation b2 kind: functionInvocation h0 @@ -6902,7 +6902,7 @@ replacement left: 4 suggestions f0 - kind: function + kind: functionInvocation '''); } }