Completion. Issue 32677. Suggest overrides when partial '@override' is typed.
Bug: https://github.com/dart-lang/sdk/issues/32677
Change-Id: I44d19f4ba8a4a38b6ec6f770b0c0b45abe5b6e03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356640
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
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 000710a..5edd7da 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
@@ -441,15 +441,21 @@
/// 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
@@ -693,8 +699,12 @@
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;
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 c4ad7b8..0391b27 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
@@ -199,6 +199,25 @@
@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
@@ -489,18 +508,14 @@
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);
@@ -515,8 +530,6 @@
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.
@@ -1535,6 +1548,10 @@
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);
@@ -2274,11 +2291,13 @@
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) {
@@ -2411,7 +2430,9 @@
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
@@ -2637,7 +2658,9 @@
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
@@ -2868,11 +2891,49 @@
}
/// 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,
+ );
+ }
}
}
diff --git a/pkg/analysis_server/lib/src/services/completion/dart/override_helper.dart b/pkg/analysis_server/lib/src/services/completion/dart/override_helper.dart
index 9b292fc..019a1c2 100644
--- a/pkg/analysis_server/lib/src/services/completion/dart/override_helper.dart
+++ b/pkg/analysis_server/lib/src/services/completion/dart/override_helper.dart
@@ -26,8 +26,11 @@
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 =
@@ -39,10 +42,14 @@
// 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,
+ ),
+ );
}
}
}
diff --git a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
index 478ffab..576bb43 100644
--- a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
+++ b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
@@ -18,14 +18,12 @@
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.
@@ -941,16 +939,12 @@
/// 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);
@@ -983,6 +977,9 @@
completion.startsWith(overrideAnnotation)) {
completion = completion.substring(overrideAnnotation.length).trim();
}
+ if (skipAt && completion.startsWith(overrideAnnotation)) {
+ completion = completion.substring('@'.length);
+ }
if (completion.isEmpty) {
return;
}
@@ -1612,9 +1609,10 @@
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}';
}
}
diff --git a/pkg/analysis_server/test/services/completion/dart/location/class_body_test.dart b/pkg/analysis_server/test/services/completion/dart/location/class_body_test.dart
index 877097c..4e41e17 100644
--- a/pkg/analysis_server/test/services/completion/dart/location/class_body_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/class_body_test.dart
@@ -646,8 +646,11 @@
printerConfiguration = printer.Configuration(
filter: (suggestion) {
- if (suggestion.kind == CompletionSuggestionKind.OVERRIDE) {
- return suggestion.completion.contains('foo0');
+ switch (suggestion.kind) {
+ case CompletionSuggestionKind.IDENTIFIER:
+ return suggestion.completion == 'override';
+ case CompletionSuggestionKind.OVERRIDE:
+ return suggestion.completion.contains('foo0');
}
return false;
},
@@ -655,6 +658,244 @@
);
}
+ Future<void> test_class_atOverride_afterField_beforeRightBrace() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ final int bar01 = 0;
+
+ @over^
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void>
+ test_class_atOverride_afterLeftBrace_beforeField_prevLine() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ @over^
+ final int bar01 = 0;
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+''');
+ }
+
+ Future<void>
+ test_class_atOverride_afterLeftBrace_beforeField_skipLine() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ @over^
+
+ final int bar01 = 0;
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void> test_class_atOverride_afterLeftBrace_beforeRightBrace() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ @over^
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void> test_class_atOverride_afterMethod_beforeMethod_prevLine() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ void bar01() {}
+
+ @over^
+ void bar02() {}
+}
+''');
+
+ // Note, no `foo01` override suggestion.
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+''');
+ }
+
+ Future<void> test_class_atOverride_afterMethod_beforeMethod_skipLine() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ void bar01() {}
+
+ @over^
+
+ void bar02() {}
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void> test_class_atOverride_afterMethod_beforeRightBrace() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+class B extends A {
+ void bar() {}
+ @over^
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void> test_class_atOverride_operator() async {
+ await computeSuggestions('''
+class A {
+ @over^
+}
+''');
+
+ printerConfiguration.filter = (suggestion) {
+ switch (suggestion.kind) {
+ case CompletionSuggestionKind.IDENTIFIER:
+ return suggestion.completion == 'override';
+ case CompletionSuggestionKind.OVERRIDE:
+ return suggestion.completion.contains('==');
+ }
+ return false;
+ };
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ bool operator ==(Object other) {
+ // TODO: implement ==
+ return super == other;
+ }
+ kind: override
+ displayText: ==(Object other) { … }
+ selection: 74 22
+''');
+ }
+
Future<void> test_class_inComment() async {
await computeSuggestions('''
class A {
@@ -1139,6 +1380,116 @@
''');
}
+ Future<void> test_mixin_beforeMethod_atOverride_skipLine() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+mixin B on A {
+ @over^
+
+ void bar() {}
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void> test_mixin_beforeRightBrace() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+mixin B on A {
+ ^
+}
+''');
+
+ assertResponse(r'''
+suggestions
+ @override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 60 14
+''');
+ }
+
+ Future<void> test_mixin_beforeRightBrace_atOverride() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+mixin B on A {
+ @over^
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ override
+ kind: topLevelVariable
+ displayText: null
+ override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 59 14
+''');
+ }
+
+ Future<void> test_mixin_beforeRightBrace_partial() async {
+ await computeSuggestions('''
+class A {
+ void foo01() {}
+}
+
+mixin B on A {
+ foo0^
+}
+''');
+
+ assertResponse(r'''
+replacement
+ left: 4
+suggestions
+ @override
+ void foo01() {
+ // TODO: implement foo01
+ super.foo01();
+ }
+ kind: override
+ displayText: foo01() { … }
+ selection: 60 14
+''');
+ }
+
Future<void> test_mixin_method_fromConstraints_alreadyOverridden() async {
await computeSuggestions('''
class A {
