Skip to content

Commit 6ac7afd

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Fix missing selection in override completions
When completing an override, the `throw UnimplementedError()` code should be selected automatically. I think this regressed recently because it broke Dart-Code's integration tests (but apparently we didn't have a test for it here). This fixes the issue and adds a test. Fixes Dart-Code/Dart-Code#5652 Change-Id: I9576f2494249c70968566d13fdf95854f0285944 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448180 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent a9e1a92 commit 6ac7afd

File tree

2 files changed

+56
-66
lines changed

2 files changed

+56
-66
lines changed

pkg/analysis_server/lib/src/lsp/completion_utils.dart

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -324,19 +324,20 @@ lsp.CompletionItem? toLspCompletionItem(
324324
}
325325

326326
var completion = suggestion.completion;
327-
var selectionOffset = (suggestion is KeywordSuggestion)
328-
? suggestion.selectionOffset
329-
: completion.length;
330-
var selectionLength = 0;
331-
332-
if (suggestion is SuggestionData) {
333-
selectionOffset = (suggestion as SuggestionData).selectionOffset;
334-
} else if (suggestion case TypedSuggestion(
335-
data: TypeImportData(:var selectionOffset?, :var selectionLength?),
336-
)) {
337-
selectionOffset = selectionOffset;
338-
selectionLength = selectionLength;
339-
}
327+
328+
var (selectionOffset, selectionLength) = switch (suggestion) {
329+
KeywordSuggestion(:var selectionOffset) => (selectionOffset, 0),
330+
SuggestionData(:var selectionOffset) => (selectionOffset, 0),
331+
TypedSuggestion(
332+
data: TypeImportData(:var selectionOffset?, :var selectionLength?),
333+
) =>
334+
(selectionOffset, selectionLength),
335+
OverrideSuggestion(
336+
data: TypeImportData(:var selectionOffset?, :var selectionLength?),
337+
) =>
338+
(selectionOffset, selectionLength),
339+
_ => (completion.length, 0),
340+
};
340341

341342
var insertTextInfo = buildInsertText(
342343
supportsSnippets: supportsSnippets,

pkg/analysis_server/test/lsp/completion_dart_test.dart

Lines changed: 42 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2420,12 +2420,7 @@ void main() {
24202420

24212421
var completionLabel = 'MyEnum.value1';
24222422

2423-
await _checkCompletionEdits(
2424-
mainFileUri,
2425-
content,
2426-
completionLabel,
2427-
expectedContent,
2428-
);
2423+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
24292424
}
24302425

24312426
Future<void> test_importedSymbol_libraryImported_showingEnum() async {
@@ -2454,12 +2449,7 @@ void main() {
24542449

24552450
var completionLabel = 'MyEnum.value1';
24562451

2457-
await _checkCompletionEdits(
2458-
mainFileUri,
2459-
content,
2460-
completionLabel,
2461-
expectedContent,
2462-
);
2452+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
24632453
}
24642454

24652455
Future<void> test_insertReplaceRanges() async {
@@ -3267,6 +3257,35 @@ class B extends A {
32673257
expect(labels, contains('override unary-() { … }'));
32683258
}
32693259

3260+
Future<void> test_override() async {
3261+
setCompletionItemSnippetSupport();
3262+
content = '''
3263+
abstract class Base {
3264+
String get name;
3265+
}
3266+
3267+
class Derived extends Base {
3268+
^
3269+
}
3270+
''';
3271+
3272+
var expectedContent = r'''
3273+
abstract class Base {
3274+
String get name;
3275+
}
3276+
3277+
class Derived extends Base {
3278+
@override
3279+
// TODO: implement name
3280+
String get name => ${0:throw UnimplementedError()};
3281+
}
3282+
''';
3283+
3284+
var completionLabel = 'override name => …';
3285+
3286+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
3287+
}
3288+
32703289
Future<void> test_plainText() async {
32713290
content = '''
32723291
class MyClass {
@@ -3894,12 +3913,7 @@ void f() {
38943913

38953914
var completionLabel = 'MyClass';
38963915

3897-
await _checkCompletionEdits(
3898-
mainFileUri,
3899-
content,
3900-
completionLabel,
3901-
expectedContent,
3902-
);
3916+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
39033917
}
39043918

39053919
Future<void>
@@ -3925,12 +3939,7 @@ void f() {
39253939
''';
39263940

39273941
var completionLabel = 'myExtensionMethod()';
3928-
await _checkCompletionEdits(
3929-
mainFileUri,
3930-
content,
3931-
completionLabel,
3932-
expectedContent,
3933-
);
3942+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
39343943
}
39353944

39363945
Future<void>
@@ -4199,12 +4208,7 @@ void f() {
41994208

42004209
var completionLabel = 'MyClass1';
42014210

4202-
await _checkCompletionEdits(
4203-
mainFileUri,
4204-
content,
4205-
completionLabel,
4206-
expectedContent,
4207-
);
4211+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
42084212
}
42094213

42104214
Future<void> test_unimportedSymbols_libraryImported_hidingOne() async {
@@ -4229,12 +4233,7 @@ void f() {
42294233

42304234
var completionLabel = 'MyClass1';
42314235

4232-
await _checkCompletionEdits(
4233-
mainFileUri,
4234-
content,
4235-
completionLabel,
4236-
expectedContent,
4237-
);
4236+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
42384237
}
42394238

42404239
Future<void> test_unimportedSymbols_libraryImported_showingOther() async {
@@ -4259,12 +4258,7 @@ void f() {
42594258

42604259
var completionLabel = 'MyClass1';
42614260

4262-
await _checkCompletionEdits(
4263-
mainFileUri,
4264-
content,
4265-
completionLabel,
4266-
expectedContent,
4267-
);
4261+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
42684262
}
42694263

42704264
// Code completion doesn't include prefixes for auto-imports so when an
@@ -4293,12 +4287,7 @@ void f() {
42934287

42944288
var completionLabel = 'MyClass1';
42954289

4296-
await _checkCompletionEdits(
4297-
mainFileUri,
4298-
content,
4299-
completionLabel,
4300-
expectedContent,
4301-
);
4290+
await _checkCompletionEdits(mainFileUri, completionLabel, expectedContent);
43024291
}
43034292

43044293
/// This test reproduces a bug where the pathKey hash used in
@@ -4495,7 +4484,6 @@ void f() {
44954484

44964485
await _checkCompletionEdits(
44974486
importingFileUri,
4498-
content,
44994487
completionLabel,
45004488
expectedContent,
45014489
);
@@ -4541,7 +4529,6 @@ void f() {
45414529

45424530
await _checkCompletionEdits(
45434531
importingFileUri,
4544-
content,
45454532
completionLabel,
45464533
expectedContent,
45474534
);
@@ -4630,9 +4617,11 @@ void f() {
46304617
///
46314618
/// [content] should contain a `^` at the location where completion should be
46324619
/// invoked/accepted.
4620+
///
4621+
/// [expectedContent] can contain LSP snippet syntax if supported and expected
4622+
/// in the result.
46334623
Future<void> _checkCompletionEdits(
46344624
Uri fileUri,
4635-
String content,
46364625
String completionLabel,
46374626
String expectedContent,
46384627
) async {
@@ -4641,15 +4630,15 @@ void f() {
46414630
await initialAnalysis;
46424631
var res = await getCompletion(fileUri, code.position.position);
46434632

4644-
var completion = res.where((c) => c.label == completionLabel).single;
4633+
var completion = res.singleWhere((c) => c.label == completionLabel);
46454634
var resolvedCompletion = await resolveCompletion(completion);
46464635

46474636
// Apply both the main completion edit and the additionalTextEdits atomically.
46484637
var newContent = applyTextEdits(
46494638
code.code,
46504639
[
46514640
toTextEdit(resolvedCompletion.textEdit!),
4652-
].followedBy(resolvedCompletion.additionalTextEdits!).toList(),
4641+
].followedBy(resolvedCompletion.additionalTextEdits ?? []).toList(),
46534642
);
46544643

46554644
expect(newContent, equalsNormalized(expectedContent));

0 commit comments

Comments
 (0)