[analysis_server] Don't fail on invalid/complex setters in LSP code completion Fixes https://github.com/dart-lang/sdk/issues/52050. Change-Id: I6cf5420d5e017b501481cb436a7d1b17f46c0e44 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295542 Commit-Queue: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart index 20b55ad..3184e2a 100644 --- a/pkg/analysis_server/lib/src/lsp/mapping.dart +++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -50,6 +50,10 @@ ], }; +/// A regex to extract the type name from the parameter string of a setter +/// completion item. +final _completionSetterTypePattern = RegExp(r'^\((\S+)\s+\S+\)$'); + /// Pattern for docComplete text on completion items that can be upgraded to /// the "detail" field so that it can be shown more prominently by clients. /// @@ -396,15 +400,17 @@ // appear in the completion list, so displaying them as setters is misleading. // To avoid this, always show only the return type, whether it's a getter // or a setter. - return prefix + - (element?.kind == server.ElementKind.GETTER - ? (returnType ?? '') - // Don't assume setters always have parameters - // See https://github.com/dart-lang/sdk/issues/27747 - : parameters != null && parameters.isNotEmpty - // Extract the type part from '(MyType value)` - ? parameters.substring(1, parameters.lastIndexOf(' ')) - : ''); + if (element?.kind == server.ElementKind.GETTER) { + return prefix + (returnType ?? ''); + } else if (parameters == null) { + return prefix; + } else { + // TODO(dantup): Consider having the type of a setter available on + // CompletionSuggestion (or changing it to a protocol-agnostic class that + // includes it). + return prefix + + (_completionSetterTypePattern.firstMatch(parameters)?.group(1) ?? ''); + } } else if (hasParameters && hasReturnType) { return '$prefix$parameters → $returnType'; } else if (hasReturnType) {
diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart index 4eabfb1..7d776db 100644 --- a/pkg/analysis_server/test/lsp/completion_dart_test.dart +++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart
@@ -1345,9 +1345,9 @@ final content = ''' class MyClass { String get justGetter => ''; - String set justSetter(String value) {} + set justSetter(String value) {} String get getterAndSetter => ''; - String set getterAndSetter(String value) {} + set getterAndSetter(String value) {} } void f() { @@ -2210,6 +2210,41 @@ expect(res.any((c) => c.label == 'UniqueNamedClassForLspThree'), isTrue); } + Future<void> test_setters() async { + final content = ''' + class MyClass { + set stringSetter(String a) {} + set noArgSetter() {} + set multiArgSetter(a, b) {} + set functionSetter(String Function(int a, int b) foo) {} + } + + void f() { + MyClass a; + a.^ + } + '''; + + await initialize(); + await openFile(mainFileUri, withoutMarkers(content)); + final res = await getCompletion(mainFileUri, positionFromMarker(content)); + final setters = res + .where((c) => c.label.endsWith('Setter')) + .map((c) => '${c.label} (${c.detail})') + .toList(); + expect( + setters, + [ + 'stringSetter (String)', + 'noArgSetter ()', + 'multiArgSetter ()', + // Because of how we extract the type name, we don't currently support + // this. + 'functionSetter ()', + ], + ); + } + Future<void> test_unimportedSymbols() async { newFile( join(projectFolderPath, 'other_file.dart'),