From 769cfb32baf779bba7e7b67b50f330e46c0640ea Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Wed, 3 Apr 2024 00:30:00 +0000 Subject: [PATCH] Use the budget to control whether overrides are suggested I believe that this is the right thing to do from a UX perspective, but this change could potentially cause flakiness in tests. There's support in the LSP path for setting a bigger budget, but not for legacy based tests (at least not yet). Change-Id: I76353fd3e4cb4cf8fb7a55c27a3353415143dc06 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360740 Reviewed-by: Keerti Parthasarathy Commit-Queue: Brian Wilkerson --- .../completion/dart/completion_manager.dart | 2 +- .../completion/dart/completion_state.dart | 34 +++++++++++-------- .../dart/in_scope_completion_pass.dart | 6 ++-- 3 files changed, 25 insertions(+), 17 deletions(-) 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 126018ceed88..76d6223c827d 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 @@ -159,7 +159,7 @@ class DartCompletionManager { if (selection == null) { throw AbortCompletion(); } - var state = CompletionState(request, selection); + var state = CompletionState(request, selection, budget); var pass = InScopeCompletionPass( state: state, collector: collector, diff --git a/pkg/analysis_server/lib/src/services/completion/dart/completion_state.dart b/pkg/analysis_server/lib/src/services/completion/dart/completion_state.dart index ea4b2f81bd28..5e961810dbba 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/completion_state.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/completion_state.dart @@ -14,35 +14,41 @@ class CompletionState { /// The completion request being processed. final DartCompletionRequest request; - /// The selection at the time completion was requested. The selection is - /// required to have a length of zero. + /// The selection at the time completion was requested. + /// + /// The selection is required to have a length of zero. final Selection selection; + /// The budget controlling how much time can be spent computing completion + /// suggestions. + final CompletionBudget budget; + /// Initialize a newly created completion state. - CompletionState(this.request, this.selection) : assert(selection.length == 0); + CompletionState(this.request, this.selection, this.budget) + : assert(selection.length == 0); - /// Returns the type of value required by the context in which completion was + /// The type of value required by the context in which completion was /// requested. DartType? get contextType => request.contextType; - /// Return the [ClassMember] that encloses the completion location, or `null` - /// if the completion location isn't in a class member. + /// The [ClassMember] that encloses the completion location, or `null` if the + /// completion location isn't in a class member. ClassMember? get enclosingMember { return selection.coveringNode.thisOrAncestorOfType(); } - /// Return `true` if the completion location is inside an instance member, and - /// hence there is a binding for `this`. + /// Whether the completion location is inside an instance member, and hence + /// whether there is a binding for `this`. bool get inInstanceScope { var member = enclosingMember; return member != null && !member.isStatic; } - /// Returns the element of the library containing the completion location. + /// The element of the library containing the completion location. LibraryElement get libraryElement => request.libraryElement; - /// Return the type of `this` at the completion location, or `null` - /// if the completion location doesn't allow `this` to be used. + /// The type of `this` at the completion location, or `null` if the completion + /// location doesn't allow `this` to be used. DartType? get thisType { AstNode? node = selection.coveringNode; while (node != null) { @@ -70,8 +76,8 @@ class CompletionState { return null; } - /// Return `true` if the given `feature` is enabled in the library containing - /// the selection. + /// Whether the given `feature` is enabled in the library containing the + /// selection. bool isFeatureEnabled(Feature feature) { return libraryElement.featureSet.isEnabled(feature); } @@ -79,7 +85,7 @@ class CompletionState { // TODO(brianwilkerson): Move to 'package:analysis_server/src/utilities/extensions/ast.dart' extension on ClassMember { - /// Return `true` if this member is a static member. + /// Whether this member is a static member. bool get isStatic { var self = this; if (self is MethodDeclaration) { 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 fa69193089f1..888bc45916f1 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 @@ -3468,8 +3468,10 @@ class InScopeCompletionPass extends SimpleAstVisitor { required InterfaceElement? element, bool skipAt = false, }) { - // TODO(brianwilkerson): Check whether there's sufficient remaining time - // before computing suggestions for overrides. + if (state.budget.isEmpty) { + // Don't suggest overrides if the time budget has already been spent. + return; + } if (suggestOverrides && element != null) { overrideHelper.computeOverridesFor( interfaceElement: element,