Use OpenUriNotificationSender to replace separate flag and sending method.

If the sender is not `null`, then we can send.
No need for a flag, and assert() in the sending method.
If the sender is `null`, then the sending code will not able to invoke it.
Types FTW :-)

Change-Id: I7d153ba014c578a73dce2eab28aceedbb8814711
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298440
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Konstantin Shcheglov 2023-04-26 18:17:37 +00:00 committed by Commit Queue
parent 4bbdc97d1c
commit 8d2bafa624
7 changed files with 64 additions and 55 deletions

View file

@ -67,6 +67,9 @@ import 'package:collection/collection.dart';
import 'package:http/http.dart' as http;
import 'package:meta/meta.dart';
/// The function for sending `openUri` request to the client.
typedef OpenUriNotificationSender = FutureOr<void> Function(Uri uri);
/// Implementations of [AnalysisServer] implement a server that listens
/// on a [CommunicationChannel] for analysis messages and process them.
abstract class AnalysisServer {
@ -289,8 +292,9 @@ abstract class AnalysisServer {
Map<Folder, analysis.AnalysisDriver> get driverMap =>
contextManager.driverMap;
/// Whether or not the client supports openUri requests/notifications.
bool get supportsOpenUriNotification;
/// Returns the function that can send `openUri` request to the client.
/// Returns `null` is the client does not support it.
OpenUriNotificationSender? get openUriNotificationSender;
/// Whether or not the client supports showMessageRequest to show the user
/// a message and allow them to respond by clicking buttons.
@ -680,9 +684,6 @@ abstract class AnalysisServer {
return null;
}
/// Sends a notification/request to the client asking it to open [uri].
FutureOr<void> sendOpenUriNotification(Uri uri);
/// Sends an error notification to the user.
void sendServerErrorNotification(
String message,

View file

@ -429,6 +429,20 @@ class LegacyAnalysisServer extends AnalysisServer {
return _onAnalysisStartedController.stream;
}
@override
OpenUriNotificationSender? get openUriNotificationSender {
if (!clientCapabilities.requests.contains('openUrlRequest')) {
return null;
}
return (Uri uri) async {
final requestId = '${nextServerRequestId++}';
await sendRequest(
ServerOpenUrlRequestParams('$uri').toRequest(requestId),
);
};
}
RefactoringManager? get refactoringManager {
var refactoringManager = _refactoringManager;
if (refactoringManager == null) {
@ -445,10 +459,6 @@ class LegacyAnalysisServer extends AnalysisServer {
return sdkManager.defaultSdkDirectory;
}
@override
bool get supportsOpenUriNotification =>
clientCapabilities.requests.contains('openUrlRequest');
@override
bool get supportsShowMessageRequest =>
clientCapabilities.requests.contains('showMessageRequest');
@ -592,15 +602,6 @@ class LegacyAnalysisServer extends AnalysisServer {
channel.sendNotification(notification);
}
@override
Future<void> sendOpenUriNotification(Uri uri) {
assert(supportsOpenUriNotification);
var requestId = (nextServerRequestId++).toString();
var request =
ServerOpenUrlRequestParams(uri.toString()).toRequest(requestId);
return sendRequest(request);
}
/// Send the given [request] to the client.
Future<Response> sendRequest(Request request) {
var completer = Completer<Response>();

View file

@ -234,6 +234,23 @@ class LspAnalysisServer extends AnalysisServer {
LspNotificationManager get notificationManager =>
super.notificationManager as LspNotificationManager;
@override
OpenUriNotificationSender? get openUriNotificationSender {
if (!initializationOptions.allowOpenUri) {
return null;
}
return (Uri uri) {
final params = OpenUriParams(uri: uri);
final message = NotificationMessage(
method: CustomMethods.openUri,
params: params,
jsonrpc: jsonRpcVersion,
);
sendNotification(message);
};
}
@override
set pluginManager(PluginManager value) {
if (AnalysisServer.supportsPlugins) {
@ -249,10 +266,6 @@ class LspAnalysisServer extends AnalysisServer {
RefactoringWorkspace get refactoringWorkspace => _refactoringWorkspace ??=
RefactoringWorkspace(driverMap.values, searchEngine);
/// Whether or not the client supports openUri notifications.
@override
bool get supportsOpenUriNotification => initializationOptions.allowOpenUri;
/// Whether or not the client has advertised support for
/// 'window/showMessageRequest'.
@override
@ -707,18 +720,6 @@ class LspAnalysisServer extends AnalysisServer {
channel.sendNotification(notification);
}
@override
void sendOpenUriNotification(Uri uri) {
assert(supportsOpenUriNotification);
final params = OpenUriParams(uri: uri);
final message = NotificationMessage(
method: CustomMethods.openUri,
params: params,
jsonrpc: jsonRpcVersion,
);
sendNotification(message);
}
/// Send the given [request] to the client and wait for a response. Completes
/// with the raw [ResponseMessage] which could be an error response.
Future<ResponseMessage> sendRequest(Method method, Object params) {

View file

@ -5,7 +5,7 @@
import 'dart:async';
import 'package:analysis_server/src/analysis_server.dart'
show AnalysisServer, MessageType;
show AnalysisServer, MessageType, OpenUriNotificationSender;
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
import 'package:analysis_server/src/services/correction/bulk_fix_processor.dart';
import 'package:analysis_server/src/services/correction/change_workspace.dart';
@ -113,7 +113,9 @@ class DartFixPromptManager {
}
@visibleForTesting
Future<void> showPrompt() async {
Future<void> showPrompt({
required OpenUriNotificationSender openUriNotificationSender,
}) async {
_hasPromptedThisSession = true;
// Note: It's possible the user never responds to this until we shut down
@ -129,7 +131,7 @@ class DartFixPromptManager {
switch (response) {
case learnMoreActionText:
_handleLearnMore();
openUriNotificationSender(learnMoreUri);
case doNotShowAgainActionText:
preferences.showDartFixPrompts = false;
default:
@ -153,19 +155,17 @@ class DartFixPromptManager {
);
}
void _handleLearnMore() {
server.sendOpenUriNotification(learnMoreUri);
}
/// Performs a check to see if "dart fix" may be able to fix diagnostics in
/// the project and if so, prompts the user.
///
/// The check/prompt may be skipped if not supported or the check has been run
/// recently. If an existing check is in-progress, it will be aborted.
Future<void> _performCheckAndPrompt() async {
final openUriNotificationSender = server.openUriNotificationSender;
if (_hasPromptedThisSession ||
!server.supportsShowMessageRequest ||
!server.supportsOpenUriNotification ||
openUriNotificationSender == null ||
!preferences.showDartFixPrompts) {
return;
}
@ -186,6 +186,8 @@ class DartFixPromptManager {
return;
}
await showPrompt();
await showPrompt(
openUriNotificationSender: openUriNotificationSender,
);
}
}

View file

@ -81,8 +81,8 @@ class ServerDomainTest extends PubPackageAnalysisServerTest {
server.clientCapabilities.requests = ['openUrlRequest'];
// Send the request.
var responseFuture =
server.sendOpenUriNotification(toUri('https://dart.dev'));
var uri = toUri('https://dart.dev');
var responseFuture = server.openUriNotificationSender!.call(uri);
expect(serverChannel.serverRequestsSent, hasLength(1));
// Simulate the response.
@ -112,7 +112,8 @@ class ServerDomainTest extends PubPackageAnalysisServerTest {
requestId++;
expect(server.clientCapabilities.requests, requests);
expect(server.supportsOpenUriNotification, openUrlRequest);
expect(server.openUriNotificationSender,
openUrlRequest ? isNotNull : isNull);
expect(server.supportsShowMessageRequest, showMessageRequest);
}

View file

@ -24,18 +24,16 @@ class OpenUriTest extends AbstractLspAnalysisServerTest {
}
Future<void> test_assertsSupported() async {
final testUri = Uri.parse("https://example.org");
await initialize(); // no support
expect(() => server.sendOpenUriNotification(testUri),
throwsA(TypeMatcher<AssertionError>()));
expect(server.openUriNotificationSender, isNull);
}
Future<void> test_openUri() async {
await initializeWithUriSupport();
final notificationFuture = openUriNotifications.first;
server.sendOpenUriNotification(exampleUri);
server.openUriNotificationSender!.call(exampleUri);
final notification = await notificationFuture;
expect(notification.uri, exampleUri);

View file

@ -2,6 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:analysis_server/src/analysis_server.dart';
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
import 'package:analysis_server/src/services/user_prompts/dart_fix_prompt_manager.dart';
@ -110,7 +111,7 @@ class DartFixPromptTest with ResourceProviderMixin {
}
Future<void> test_check_notIfNoOpenUriSupport() async {
server.supportsOpenUriNotification = false;
server.openUriNotificationSender = null;
promptManager.triggerCheck();
await pumpEventQueue(times: 5000);
expect(promptManager.checksPerformed, 0);
@ -169,7 +170,7 @@ class DartFixPromptTest with ResourceProviderMixin {
}
Future<void> test_prompt_notIfNoOpenUriSupport() async {
server.supportsOpenUriNotification = false;
server.openUriNotificationSender = null;
promptManager.triggerCheck();
await pumpEventQueue(times: 5000);
expect(promptManager.promptsShown, 0);
@ -201,9 +202,13 @@ class TestDartFixPromptManager extends DartFixPromptManager {
}
@override
Future<void> showPrompt() {
Future<void> showPrompt({
required OpenUriNotificationSender openUriNotificationSender,
}) {
promptsShown++;
return super.showPrompt();
return super.showPrompt(
openUriNotificationSender: openUriNotificationSender,
);
}
}
@ -215,7 +220,7 @@ class TestServer implements LspAnalysisServer {
bool supportsShowMessageRequest = true;
@override
bool supportsOpenUriNotification = true;
OpenUriNotificationSender? openUriNotificationSender = (_) {};
TestServer(this.instrumentationService);