DAS: Collapse the ServerIsolateChannel class hierarchy

We stopped referencing BuiltInServerIsolateChannel a while ago, so the
only concrete subclass of ServerIsolateChannel was
DiscoveredServerIsolateChannel. This change removes the former class
and collapses the latter class into the parent class.

We also make `pluginUri` and `sessionLogger` private.

Also, `packagesUri` was referencing the old `.packages` files, which
are replaced with package config files, so we rename that field to
`_packageConfigUri`.

Change-Id: I93e2f4f10e14d832d85ea63382188019716b5de0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465985
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: srawlins

pkg/analysis_server/lib/src/plugin/plugin_isolate.dart

@@ -34,9 +34,9 @@ class PluginIsolate {
   /// `null` if the isolate could not be started.
   final String? executionPath;
 
-  /// The path to the '.packages' file used to control the resolution of
+  /// The path to the package config file used to control the resolution of
   /// 'package:' URIs, or `null` if the isolate could not be started.
-  final String? packagesPath;
+  final String? packageConfigPath;
 
   /// The object used to manage the receiving and sending of notifications.
   final AbstractNotificationManager _notificationManager;
@@ -62,7 +62,7 @@ class PluginIsolate {
   PluginIsolate(
     this._path,
     this.executionPath,
-    this.packagesPath,
+    this.packageConfigPath,
     this._notificationManager,
     this._instrumentationService,
     this.sessionLogger, {
@@ -208,9 +208,9 @@ class PluginIsolate {
 
   /// Creates and returns the channel used to communicate with the server.
   ServerCommunicationChannel _createChannel() {
-    return ServerIsolateChannel.discovered(
+    return ServerIsolateChannel(
       Uri.file(executionPath!, windows: Platform.isWindows),
-      Uri.file(packagesPath!, windows: Platform.isWindows),
+      Uri.file(packageConfigPath!, windows: Platform.isWindows),
       _instrumentationService,
       sessionLogger,
     );

pkg/analysis_server/lib/src/plugin/server_isolate_channel.dart

@@ -16,77 +16,22 @@ import 'package:analyzer_plugin/protocol/protocol_generated.dart';
 /// The type of the function used to run a built-in plugin in an isolate.
 typedef EntryPoint = void Function(SendPort sendPort);
 
-/// A communication channel appropriate for built-in plugins.
-class BuiltInServerIsolateChannel extends ServerIsolateChannel {
-  /// The entry point
-  final EntryPoint entryPoint;
-
-  @override
-  final String pluginId;
-
-  /// Initialize a newly created channel to communicate with an isolate running
-  /// the given [entryPoint].
-  BuiltInServerIsolateChannel(
-    this.entryPoint,
-    this.pluginId,
-    InstrumentationService instrumentationService,
-    SessionLogger sessionLogger,
-  ) : super._(instrumentationService, sessionLogger);
-
-  @override
-  Future<Isolate> _spawnIsolate() {
-    return Isolate.spawn(
-      (message) => entryPoint(message as SendPort),
-      _receivePort?.sendPort,
-      onError: _errorPort?.sendPort,
-      onExit: _exitPort?.sendPort,
-    );
-  }
-}
-
-/// A communication channel appropriate for discovered plugins.
-class DiscoveredServerIsolateChannel extends ServerIsolateChannel {
-  /// The URI for the Dart file that will be run in the isolate that this
-  /// channel communicates with.
-  final Uri pluginUri;
-
-  /// The URI for the '.packages' file that will control how 'package:' URIs are
-  /// resolved.
-  final Uri packagesUri;
-
-  /// Initialize a newly created channel to communicate with an isolate running
-  /// the code at the given [uri].
-  DiscoveredServerIsolateChannel(
-    this.pluginUri,
-    this.packagesUri,
-    InstrumentationService instrumentationService,
-    SessionLogger sessionLogger,
-  ) : super._(instrumentationService, sessionLogger);
-
-  @override
-  String get pluginId => pluginUri.toString();
-
-  @override
-  Future<Isolate> _spawnIsolate() {
-    return Isolate.spawnUri(
-      pluginUri,
-      <String>[],
-      _receivePort?.sendPort,
-      onError: _errorPort?.sendPort,
-      onExit: _exitPort?.sendPort,
-      packageConfig: packagesUri,
-    );
-  }
-}
-
 /// A communication channel that allows an analysis server to send [Request]s
 /// to, and to receive both [Response]s and [Notification]s from, a plugin.
-abstract class ServerIsolateChannel implements ServerCommunicationChannel {
+final class ServerIsolateChannel implements ServerCommunicationChannel {
   /// The instrumentation service that is being used by the analysis server.
   final InstrumentationService instrumentationService;
 
+  /// The URI for the Dart file that will be run in the isolate that this
+  /// channel communicates with.
+  final Uri _pluginUri;
+
+  /// The URI for the package config file that controls how 'package:' URIs
+  /// are resolved.
+  final Uri _packageConfigUri;
+
   /// The session logger that is to be used by this channel.
-  final SessionLogger sessionLogger;
+  final SessionLogger _sessionLogger;
 
   /// The isolate in which the plugin is running, or `null` if the plugin has
   /// not yet been started by invoking [listen].
@@ -105,33 +50,16 @@ abstract class ServerIsolateChannel implements ServerCommunicationChannel {
   /// The port used to receive notification when the plugin isolate has exited.
   ReceivePort? _exitPort;
 
-  /// Return a communication channel appropriate for communicating with a
-  /// built-in plugin.
-  // TODO(brianwilkerson): This isn't currently referenced. We should decide
-  //  whether there's any value to supporting built-in plugins and remove this
-  //  if we decide there isn't.
-  factory ServerIsolateChannel.builtIn(
-    EntryPoint entryPoint,
-    String pluginId,
-    InstrumentationService instrumentationService,
-    SessionLogger sessionLogger,
-  ) = BuiltInServerIsolateChannel;
-
-  /// Return a communication channel appropriate for communicating with a
-  /// discovered plugin.
-  factory ServerIsolateChannel.discovered(
-    Uri pluginUri,
-    Uri packagesUri,
-    InstrumentationService instrumentationService,
-    SessionLogger sessionLogger,
-  ) = DiscoveredServerIsolateChannel;
-
-  /// Initialize a newly created channel.
-  ServerIsolateChannel._(this.instrumentationService, this.sessionLogger);
-
-  /// Return the id of the plugin running in the isolate, used to identify the
-  /// plugin to the instrumentation service.
-  String get pluginId;
+  ServerIsolateChannel(
+    this._pluginUri,
+    this._packageConfigUri,
+    this.instrumentationService,
+    this._sessionLogger,
+  );
+
+  /// The ID of the plugin running in the isolate, used to identify the plugin
+  /// to the instrumentation service.
+  String get _pluginId => _pluginUri.toString();
 
   @override
   void close() {
@@ -180,7 +108,7 @@ abstract class ServerIsolateChannel implements ServerCommunicationChannel {
       _isolate = await _spawnIsolate();
     } catch (exception, stackTrace) {
       instrumentationService.logPluginError(
-        PluginData(pluginId, null, null),
+        PluginData(_pluginId, null, null),
         RequestErrorCode.PLUGIN_ERROR.toString(),
         exception.toString(),
         stackTrace.toString(),
@@ -203,17 +131,17 @@ abstract class ServerIsolateChannel implements ServerCommunicationChannel {
       } else if (input is Map<String, Object?>) {
         if (input.containsKey('id')) {
           var encodedInput = json.encode(input);
-          instrumentationService.logPluginResponse(pluginId, encodedInput);
-          sessionLogger.logMessage(
+          instrumentationService.logPluginResponse(_pluginId, encodedInput);
+          _sessionLogger.logMessage(
             from: ProcessId.plugin,
             to: ProcessId.server,
             message: input,
           );
           onResponse(Response.fromJson(input));
         } else if (input.containsKey('event')) {
           var encodedInput = json.encode(input);
-          instrumentationService.logPluginNotification(pluginId, encodedInput);
-          sessionLogger.logMessage(
+          instrumentationService.logPluginNotification(_pluginId, encodedInput);
+          _sessionLogger.logMessage(
             from: ProcessId.plugin,
             to: ProcessId.server,
             message: input,
@@ -232,8 +160,8 @@ abstract class ServerIsolateChannel implements ServerCommunicationChannel {
     if (sendPort != null) {
       var jsonData = request.toJson();
       var encodedRequest = json.encode(jsonData);
-      instrumentationService.logPluginRequest(pluginId, encodedRequest);
-      sessionLogger.logMessage(
+      instrumentationService.logPluginRequest(_pluginId, encodedRequest);
+      _sessionLogger.logMessage(
         from: ProcessId.server,
         to: ProcessId.plugin,
         message: jsonData,
@@ -242,6 +170,15 @@ abstract class ServerIsolateChannel implements ServerCommunicationChannel {
     }
   }
 
-  /// Spawn the isolate in which the plugin is running.
-  Future<Isolate> _spawnIsolate();
+  /// Spawns the isolate in which the plugin is running.
+  Future<Isolate> _spawnIsolate() {
+    return Isolate.spawnUri(
+      _pluginUri,
+      [],
+      _receivePort?.sendPort,
+      onError: _errorPort?.sendPort,
+      onExit: _exitPort?.sendPort,
+      packageConfig: _packageConfigUri,
+    );
+  }
 }

pkg/analysis_server/lib/src/status/pages/legacy_plugins_page.dart

@@ -51,8 +51,8 @@ class LegacyPluginsPage extends DiagnosticPageWithNav {
         ['Bootstrap package path', id],
         if (plugin.executionPath case var executionPath?)
           ['Execution path:', executionPath.wordBreakOnSlashes],
-        if (plugin.packagesPath case var packagesPath?)
-          ['Packages file path', packagesPath.wordBreakOnSlashes],
+        if (plugin.packageConfigPath case var packageConfigPath?)
+          ['Package config path', packageConfigPath.wordBreakOnSlashes],
       ]);
 
       if (data.name == null) {

pkg/analysis_server/lib/src/status/pages/plugins_page.dart

@@ -43,8 +43,8 @@ class PluginsPage extends DiagnosticPageWithNav {
         ['Bootstrap package path:', id],
         if (isolate.executionPath case var executionPath?)
           ['Execution path:', executionPath.wordBreakOnSlashes],
-        if (isolate.packagesPath case var packagesPath?)
-          ['Packages file path', packagesPath.wordBreakOnSlashes],
+        if (isolate.packageConfigPath case var packageConfigPath?)
+          ['Package config path', packageConfigPath.wordBreakOnSlashes],
       ]);
 
       if (data.name == null) {