Skip to content

Commit

Permalink
Use the budget to control whether overrides are suggested
Browse files Browse the repository at this point in the history
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 <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
bwilkerson authored and Commit Queue committed Apr 3, 2024
1 parent c6a82eb commit 769cfb3
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClassMember>();
}

/// 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) {
Expand Down Expand Up @@ -70,16 +76,16 @@ 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);
}
}

// 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3468,8 +3468,10 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
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,
Expand Down

0 comments on commit 769cfb3

Please sign in to comment.