languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 16:41:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Share the visibility tracker and suggest local functions

Browse source 
Sharing the visibility tracker this way prevents shadowed elements from
being suggested and improves performance.

Most of these changes can be reverted once the local reference
contributor has been removed.

Change-Id: Ib156ae56455e8d85a31163f9e59ce4dd93f5920b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333124
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Brian Wilkerson 2023-11-01 16:21:19 +00:00 committed by Commit Queue
parent 1afa57fc48
commit 8de066a4b0
8 changed files with 96 additions and 73 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
pkg/analysis_server/lib/src/services/completion/dart/candidate_suggestion.dart
View file

@ -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.

16
pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart
View file

@ -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 = <DartCompletionContributor>[
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;
}
}

30
pkg/analysis_server/lib/src/services/completion/dart/declaration_helper.dart
View file

@ -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<String> 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);
}
}

31
pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart
View file

@ -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<void> {
/// 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<void> {
/// 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<void> {
/// 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<void> {
_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<void> {
@override
void visitCommentReference(CommentReference node) {
declarationHelper.addLexicalDeclarations(node);
declarationHelper().addLexicalDeclarations(node);
}
@override
@ -829,7 +837,7 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
@override
void visitInterpolationExpression(InterpolationExpression node) {
declarationHelper.addLexicalDeclarations(node);
declarationHelper().addLexicalDeclarations(node);
}
@override
@ -1512,7 +1520,7 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
TypedLiteral literal, NodeList<CollectionElement> 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<void> {
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> {
void _forExpression(AstNode? node) {
keywordHelper.addExpressionKeywords(node);
if (node != null) {
declarationHelper.addLexicalDeclarations(node);
declarationHelper().addLexicalDeclarations(node);
}
}
@ -1652,7 +1660,8 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
/// 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<void> {
}
} else if (!node.inKeyword.isSynthetic) {
keywordHelper.addKeyword(Keyword.AWAIT);
declarationHelper.addLexicalDeclarations(node);
declarationHelper().addLexicalDeclarations(node);
}
}

43
pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart
View file

@ -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<String> 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);
}
}

21
pkg/analysis_server/lib/src/services/completion/dart/visibility_tracker.dart Normal file
View file

@ -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<String> 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);
}
}

13
pkg/analysis_server/test/services/completion/dart/completion_test.dart
View file

@ -10049,7 +10049,6 @@ suggestions
''');
}
@failingTest
Future<void> 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<void> test_completion_dartDoc_reference_forFunction_2() async {

10
pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart
View file

@ -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
''');
}
}