Tidy up PluginIsolate fields

PluginIsolate.executionPath and .packagesPath were non-nullable strings
with a magic `''` value meaning the plugin isolate could not be started.
Making it nullable makes it more understandable, and I add
documentation. In the "insights" pages, we then don't print weird
blank strings.

Also I missed renaming `_info` to `_isolate`.

Change-Id: Ib8ce52d297cf083239004fffe990b6a9e90c7ddb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447960
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: srawlins

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

@@ -29,12 +29,13 @@ class PluginIsolate {
   /// directory).
   final String _path;
 
-  /// The path to the 'plugin.dart' file that will be executed in an isolate.
-  final String executionPath;
+  /// The path to the 'plugin.dart' file that will be executed in an isolate, or
+  /// `null` if the isolate could not be started.
+  final String? executionPath;
 
   /// The path to the '.packages' file used to control the resolution of
-  /// 'package:' URIs.
-  final String packagesPath;
+  /// 'package:' URIs, or `null` if the isolate could not be started.
+  final String? packagesPath;
 
   /// The object used to manage the receiving and sending of notifications.
   final AbstractNotificationManager _notificationManager;
@@ -79,7 +80,7 @@ class PluginIsolate {
   /// For example, a plugin cannot be started if there was an error with a
   /// previous attempt to start running it or if the plugin is not correctly
   /// configured.
-  bool get _canBeStarted => executionPath.isNotEmpty;
+  bool get _canBeStarted => executionPath != null;
 
   /// Adds the given [contextRoot] to the set of context roots being analyzed by
   /// this plugin.
@@ -181,8 +182,8 @@ class PluginIsolate {
   /// Creates and returns the channel used to communicate with the server.
   ServerCommunicationChannel _createChannel() {
     return ServerIsolateChannel.discovered(
-      Uri.file(executionPath, windows: Platform.isWindows),
-      Uri.file(packagesPath, windows: Platform.isWindows),
+      Uri.file(executionPath!, windows: Platform.isWindows),
+      Uri.file(packagesPath!, windows: Platform.isWindows),
       _instrumentationService,
     );
   }
@@ -217,7 +218,7 @@ class PluginSession {
   static const Duration WAIT_FOR_SHUTDOWN_DURATION = Duration(seconds: 10);
 
   /// The information about the plugin being executed.
-  final PluginIsolate _info;
+  final PluginIsolate _isolate;
 
   /// The completer used to signal when the plugin has stopped.
   Completer<void> pluginStoppedCompleter = Completer<void>();
@@ -248,7 +249,7 @@ class PluginSession {
   /// plugin.
   String? _version;
 
-  PluginSession(this._info);
+  PluginSession(this._isolate);
 
   /// The next request ID, encoded as a string.
   ///
@@ -264,12 +265,12 @@ class PluginSession {
     if (notification.event == PLUGIN_NOTIFICATION_ERROR) {
       var params = PluginErrorParams.fromNotification(notification);
       if (params.isFatal) {
-        _info.stop();
+        _isolate.stop();
         stop();
       }
     }
-    _info._notificationManager.handlePluginNotification(
-      _info.pluginId,
+    _isolate._notificationManager.handlePluginNotification(
+      _isolate.pluginId,
       notification,
     );
   }
@@ -288,7 +289,7 @@ class PluginSession {
     if (error case [String message, String stackTraceString]) {
       var stackTrace = StackTrace.fromString(stackTraceString);
       var exception = PluginException(message);
-      _info.reportException(
+      _isolate.reportException(
         CaughtException.withMessage(message, exception, stackTrace),
       );
     } else {
@@ -307,7 +308,7 @@ class PluginSession {
     if (requestData != null) {
       var responseTime = DateTime.now().millisecondsSinceEpoch;
       var duration = responseTime - requestData.requestTime;
-      PluginManager.recordResponseTime(_info, requestData.method, duration);
+      PluginManager.recordResponseTime(_isolate, requestData.method, duration);
       var completer = requestData.completer;
       completer.complete(response);
     }
@@ -354,15 +355,15 @@ class PluginSession {
         if (error.code == RequestErrorCode.UNKNOWN_REQUEST) {
           // The plugin doesn't support this request. It may just be using an
           // older version of the `analysis_server_plugin` package.
-          _info._instrumentationService.logInfo(
+          _isolate._instrumentationService.logInfo(
             "Plugin cannot handle request '${request.method}' with parameters: "
             '$parameters.',
           );
           return;
         }
         var stackTrace = StackTrace.fromString(error.stackTrace!);
         var exception = PluginException(error.message);
-        _info.reportException(
+        _isolate.reportException(
           CaughtException.withMessage(error.message, exception, stackTrace),
         );
       }
@@ -382,24 +383,24 @@ class PluginSession {
       throw StateError('Missing byte store path');
     }
     if (!isCompatible) {
-      _info.reportException(
+      _isolate.reportException(
         CaughtException(
           PluginException('Plugin is not compatible.'),
           StackTrace.current,
         ),
       );
       return false;
     }
-    if (!_info._canBeStarted) {
-      _info.reportException(
+    if (!_isolate._canBeStarted) {
+      _isolate.reportException(
         CaughtException(
           PluginException('Plugin cannot be started.'),
           StackTrace.current,
         ),
       );
       return false;
     }
-    channel = _info._createChannel();
+    channel = _isolate._createChannel();
     // TODO(brianwilkerson): Determine if await is necessary, if so, change the
     // return type of `channel.listen` to `Future<void>`.
     await (channel!.listen(
@@ -412,7 +413,7 @@ class PluginSession {
     if (channel == null) {
       // If there is an error when starting the isolate, the channel will invoke
       // `handleOnDone`, which will cause `channel` to be set to `null`.
-      _info.reportException(
+      _isolate.reportException(
         CaughtException(
           PluginException('Unrecorded error while starting the plugin.'),
           StackTrace.current,
@@ -430,7 +431,7 @@ class PluginSession {
     _version = result.version;
     if (!isCompatible) {
       unawaited(sendRequest(PluginShutdownParams()));
-      _info.reportException(
+      _isolate.reportException(
         CaughtException(
           PluginException('Plugin is not compatible.'),
           StackTrace.current,

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

@@ -155,8 +155,8 @@ class PluginManager {
       } catch (exception, stackTrace) {
         pluginIsolate = PluginIsolate(
           path,
-          '',
-          '',
+          null,
+          null,
           _notificationManager,
           instrumentationService,
         );

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

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

pkg/analyzer/lib/instrumentation/plugin_data.dart

@@ -7,10 +7,10 @@ class PluginData {
   /// The ID used to uniquely identify the plugin.
   final String pluginId;
 
-  /// The name of the plugin.
+  /// The name of the plugin, or `null` if the plugin is not running.
   final String? name;
 
-  /// The version of the plugin.
+  /// The version of the plugin, or `null` if the plugin is not running.
   final String? version;
 
   PluginData(this.pluginId, this.name, this.version);