Skip to content

Commit

Permalink
Completion. Issue 32677. Suggest overrides when partial '@OverRide' i…
Browse files Browse the repository at this point in the history 
…s typed.

Bug: #32677
Change-Id: I44d19f4ba8a4a38b6ec6f770b0c0b45abe5b6e03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356640
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
  • Loading branch information
@scheglov
scheglov authored and Commit Queue committed Mar 11, 2024
1 parent 28bae4c commit 6ac3ba6
Show file tree
Hide file tree
Showing 5 changed files with 476 additions and 49 deletions.
24 changes: 17 additions & 7 deletions pkg/analysis_server/lib/src/services/completion/dart/candidate_suggestion.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,21 @@ final class OverrideSuggestion extends CandidateSuggestion {
/// Whether `super` should be invoked in the body of the override.
final bool shouldInvokeSuper;

/// The soruce range that should be replaced by the override.
/// If `true`, `@override` is already present, at least partially.
/// So, `@` is already present, and the override text does not need it.
final bool skipAt;

/// The source range that should be replaced by the override.
final SourceRange replacementRange;

/// Initialize a newly created candidate suggestion to suggest the [element] by
/// inserting the [shouldInvokeSuper].
OverrideSuggestion(
{required this.element,
required this.shouldInvokeSuper,
required this.replacementRange});
OverrideSuggestion({
required this.element,
required this.shouldInvokeSuper,
required this.skipAt,
required this.replacementRange,
});

@override
// TODO(brianwilkerson): This needs to be replaced with code to compute the
Expand Down Expand Up @@ -693,8 +699,12 @@ extension SuggestionBuilderExtension on SuggestionBuilder {
case NameSuggestion():
suggestName(suggestion.name);
case OverrideSuggestion():
suggestOverride2(suggestion.element, suggestion.shouldInvokeSuper,
suggestion.replacementRange);
suggestOverride(
element: suggestion.element,
invokeSuper: suggestion.shouldInvokeSuper,
replacementRange: suggestion.replacementRange,
skipAt: suggestion.skipAt,
);
case PropertyAccessSuggestion():
var inheritanceDistance = 0.0;
var referencingClass = suggestion.referencingClass;
Expand Down
105 changes: 83 additions & 22 deletions pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,25 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
@override
void visitAnnotation(Annotation node) {
_forAnnotation(node);

// Look for `@override` with an empty line before the next token.
// So, it is not an annotation for a method, but override request.
if (node.constructorName == null) {
if (node.name case SimpleIdentifier name) {
var classNode = node.parent.parent;
if (classNode is Declaration) {
var lineInfo = state.request.unit.lineInfo;
var nameLocation = lineInfo.getLocation(name.offset);
var nextToken = name.token.next;
if (nextToken != null) {
var nextLocation = lineInfo.getLocation(nextToken.offset);
if (nextLocation.lineNumber > nameLocation.lineNumber + 1) {
_tryOverrideAnnotation(name.token, classNode);
}
}
}
}
}
}

@override
Expand Down Expand Up @@ -489,18 +508,14 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
keywordHelper.addClassDeclarationKeywords(node);
} else if (offset >= node.leftBracket.end &&
offset <= node.rightBracket.offset) {
var members = node.members;
// TODO(brianwilkerson): Generalize this to check for unattatched
// annotations in other places.
var preceedingMember = members.elementBefore(offset);
var token = preceedingMember?.beginToken ?? node.leftBracket.next!;
if (token.type == TokenType.AT) {
// The user is completing at the beginning of an annotation.
// TODO(brianwilkerson): We need to check the next token to see whether
// part of the annotation is already there.
_forAnnotation(node);
if (_tryAnnotationAtEndOfClassBody(node)) {
return;
} else if (token.keyword == Keyword.FINAL) {
}

var members = node.members;
var precedingMember = members.elementBefore(offset);
var token = precedingMember?.beginToken ?? node.leftBracket.next!;
if (token.keyword == Keyword.FINAL) {
// The user is completing after the keyword `final`, so they're likely
// trying to declare a field.
_forTypeAnnotation(node);
Expand All @@ -515,8 +530,6 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
keywordHelper.addFunctionBodyModifiers(body);
}
}
// TODO(brianwilkerson): Consider enabling the generation of overrides in
// this location.
} else {
// The cursor is immediately to the right of the right bracket, so the
// user is starting a new top-level declaration.
Expand Down Expand Up @@ -1535,6 +1548,10 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
return;
}
if (offset >= node.leftBracket.end && offset <= node.rightBracket.offset) {
if (_tryAnnotationAtEndOfClassBody(node)) {
return;
}

collector.completionLocation = 'MixinDeclaration_member';
_forMixinMember(node);
var element = node.members.elementBefore(offset);
Expand Down Expand Up @@ -2274,11 +2291,13 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
keywordHelper.addKeyword(Keyword.SET);
}
if (grandparent.isSingleIdentifier) {
_suggestOverridesFor(switch (container) {
ClassDeclaration() => container.declaredElement,
MixinDeclaration() => container.declaredElement,
_ => null,
});
_suggestOverridesFor(
element: switch (container) {
ClassDeclaration() => container.declaredElement,
MixinDeclaration() => container.declaredElement,
_ => null,
},
);
}
} else if (grandparent is TopLevelVariableDeclaration) {
if (grandparent.externalKeyword == null) {
Expand Down Expand Up @@ -2411,7 +2430,9 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
void _forClassMember(ClassDeclaration node) {
keywordHelper.addClassMemberKeywords();
declarationHelper(mustBeType: true).addLexicalDeclarations(node);
_suggestOverridesFor(node.declaredElement);
_suggestOverridesFor(
element: node.declaredElement,
);
}

/// Add the suggestions that are appropriate when the selection is at the
Expand Down Expand Up @@ -2637,7 +2658,9 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
void _forMixinMember(MixinDeclaration node) {
keywordHelper.addMixinMemberKeywords();
declarationHelper(mustBeType: true).addLexicalDeclarations(node);
_suggestOverridesFor(node.declaredElement);
_suggestOverridesFor(
element: node.declaredElement,
);
}

/// Adds the suggestions that are appropriate when the selection is in the
Expand Down Expand Up @@ -2868,11 +2891,49 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
}

/// If allowed, suggest overrides in the context of the given [element].
void _suggestOverridesFor(InterfaceElement? element) {
void _suggestOverridesFor({
required InterfaceElement? element,
bool skipAt = false,
}) {
// TODO(brianwilkerson): Check whether there's sufficient remaining time
// before computing suggestions for overrides.
if (suggestOverrides && element != null) {
overrideHelper.computeOverridesFor(element, SourceRange(offset, 0));
overrideHelper.computeOverridesFor(
interfaceElement: element,
replacementRange: SourceRange(offset, 0),
skipAt: skipAt,
);
}
}

/// Check for typing `@override` inside a class body, before `}`.
/// There is no node, recover using tokens.
bool _tryAnnotationAtEndOfClassBody(Declaration node) {
var displaced = state.request.target.entity;
if (displaced is Token && displaced.type == TokenType.CLOSE_CURLY_BRACKET) {
var identifier = displaced.previous;
if (identifier != null && identifier.type == TokenType.IDENTIFIER) {
if (identifier.previous?.type == TokenType.AT) {
_forAnnotation(node);
_tryOverrideAnnotation(identifier, node);
return true;
}
}
}
return false;
}

/// If [identifier] at `@` is a start of `@override`, suggest overrides.
void _tryOverrideAnnotation(Token identifier, Declaration node) {
var lexeme = identifier.lexeme;
if (lexeme.isNotEmpty && 'override'.startsWith(lexeme)) {
var declaredElement = node.declaredElement;
if (declaredElement is InterfaceElement) {
_suggestOverridesFor(
element: declaredElement,
skipAt: true,
);
}
}
}

Expand Down
15 changes: 11 additions & 4 deletions pkg/analysis_server/lib/src/services/completion/dart/override_helper.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ class OverrideHelper {
OverrideHelper({required this.state, required this.collector})
: inheritanceManager = state.request.inheritanceManager;

void computeOverridesFor(
InterfaceElement interfaceElement, SourceRange replacementRange) {
void computeOverridesFor({
required InterfaceElement interfaceElement,
required SourceRange replacementRange,
required bool skipAt,
}) {
var interface = inheritanceManager.getInterface(interfaceElement);
var interfaceMap = interface.map;
var namesToOverride =
Expand All @@ -39,10 +42,14 @@ class OverrideHelper {
// Gracefully degrade if the overridden element has not been resolved.
if (element != null) {
var invokeSuper = interface.isSuperImplemented(name);
collector.addSuggestion(OverrideSuggestion(
collector.addSuggestion(
OverrideSuggestion(
element: element,
shouldInvokeSuper: invokeSuper,
replacementRange: replacementRange));
skipAt: skipAt,
replacementRange: replacementRange,
),
);
}
}
}
Expand Down
26 changes: 12 additions & 14 deletions pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ import 'package:analysis_server/src/utilities/extensions/ast.dart';
import 'package:analysis_server/src/utilities/extensions/element.dart';
import 'package:analysis_server/src/utilities/flutter.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/source/source_range.dart';
import 'package:analyzer/src/dartdoc/dartdoc_directive_info.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';

/// A container with enough information to do filtering, and if necessary
/// build the [CompletionSuggestion] instance.
Expand Down Expand Up @@ -941,16 +939,12 @@ class SuggestionBuilder {
/// Add a suggestion to replace the [targetId] with an override of the given
/// [element]. If [invokeSuper] is `true`, then the override will contain an
/// invocation of an overridden member.
Future<void> suggestOverride(
Token targetId, ExecutableElement element, bool invokeSuper) async {
await suggestOverride2(element, invokeSuper, range.token(targetId));
}

/// Add a suggestion to replace the [targetId] with an override of the given
/// [element]. If [invokeSuper] is `true`, then the override will contain an
/// invocation of an overridden member.
Future<void> suggestOverride2(ExecutableElement element, bool invokeSuper,
SourceRange replacementRange) async {
Future<void> suggestOverride({
required ExecutableElement element,
required bool invokeSuper,
required SourceRange replacementRange,
required bool skipAt,
}) async {
var displayTextBuffer = StringBuffer();
var overrideImports = <Uri>{};
var builder = ChangeBuilder(session: request.analysisSession);
Expand Down Expand Up @@ -983,6 +977,9 @@ class SuggestionBuilder {
completion.startsWith(overrideAnnotation)) {
completion = completion.substring(overrideAnnotation.length).trim();
}
if (skipAt && completion.startsWith(overrideAnnotation)) {
completion = completion.substring('@'.length);
}
if (completion.isEmpty) {
return;
}
Expand Down Expand Up @@ -1612,9 +1609,10 @@ class SuggestionBuilder {

static String _textToMatchOverride(ExecutableElement element) {
if (element.isOperator) {
return 'operator';
return 'override_operator';
}
return element.displayName;
// Add "override" to match filter when `@override`.
return 'override_${element.displayName}';
}
}

Expand Down
Loading

0 comments on commit 6ac3ba6

Please sign in to comment.