Code cleanup in log player and server driver

Setting the command-line arguments separately from the protocol used to
be necessary, but it no long is, so it's cleaner to just set the command
line arguments when creating the server and moving the handling of the
protocol option into the server. This resulted in a cleaner API and
implementation.

Change-Id: I9456e2a43310bb855b54c010af64ce0328a08ff7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464785
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Keerti Parthasarathy <[email protected]>

Author: bwilkerson

pkg/analysis_server/tool/log_player/log_player.dart

@@ -6,7 +6,6 @@ import 'package:analysis_server/src/server/driver.dart';
 import 'package:analysis_server/src/session_logger/entry_kind.dart';
 import 'package:analysis_server/src/session_logger/log_entry.dart';
 import 'package:analysis_server/src/session_logger/process_id.dart';
-import 'package:collection/collection.dart';
 
 import 'log.dart';
 import 'server_driver.dart';
@@ -49,24 +48,7 @@ class LogPlayer {
               'Analysis server already started, only one instance is allowed.',
             );
           }
-          // TODO(brianwilkerson): Stop parsing out the protocol. Instead,
-          //  consider verifying that a protocol was specified, throwing if not,
-          //  and then just pass all of the arguments to the constructor. This
-          //  would require allow-listing the protocol option.
-          var parsedArgs = driverArgParser.parse(entry.argList);
-          var protocolOption = parsedArgs.option(Driver.serverProtocolOption);
-          var protocol = switch (protocolOption) {
-            Driver.protocolAnalyzer => ServerProtocol.legacy,
-            Driver.protocolLsp => ServerProtocol.lsp,
-            _ => throw StateError('Unrecognized protocol $protocolOption'),
-          };
-          var server = this.server = ServerDriver(protocol: protocol);
-          server.additionalArguments.addAll(
-            entry.argList.whereNot(
-              (element) =>
-                  element.startsWith('--${Driver.serverProtocolOption}'),
-            ),
-          );
+          var server = this.server = ServerDriver(arguments: entry.argList);
           await server.start();
         case EntryKind.message:
           if (entry.receiver == ProcessId.server) {

pkg/analysis_server/tool/log_player/server_driver.dart

@@ -15,9 +15,8 @@ class ServerDriver {
   /// The protocol being used by the server.
   final ServerProtocol _protocol;
 
-  /// A list of additional arguments from the comman-line used to start the
-  /// server.
-  final List<String> additionalArguments = [];
+  /// A list of arguments from the command-line used to start the server.
+  final List<String> arguments;
 
   /// The sink used to send messages from the IDE to the server's stdin, or
   /// `null` if the server has not been started using [start].
@@ -37,24 +36,38 @@ class ServerDriver {
   /// The server is run in a separate process.
   // TODO(brianwilkerson): Add a flag controlling whether the server is in the
   //  same process as the driver or in a separate process.
-  ServerDriver({required ServerProtocol protocol}) : _protocol = protocol;
-
-  /// Returns the list of additional arguments that were passes on the command
-  /// line when the server was started that should be passed to the server
-  /// created by this driver.
-  List<String> get filteredAdditionalArguments {
-    var arguments = <String>[];
-    var nextIndex = 0;
-    while (nextIndex < additionalArguments.length) {
-      var nextArgument = additionalArguments[nextIndex];
-      if (nextArgument == Driver.withFineDependenciesOption) {
-        arguments.add(nextArgument);
-      }
-      nextIndex++;
-    }
-    return arguments;
+  factory ServerDriver({required List<String> arguments}) {
+    var parsedArgs = Driver.createArgParser().parse(arguments);
+
+    var protocolOption = parsedArgs.option(Driver.serverProtocolOption);
+    var protocol = switch (protocolOption) {
+      Driver.protocolAnalyzer => ServerProtocol.legacy,
+      Driver.protocolLsp => ServerProtocol.lsp,
+      null => throw StateError('No protocol specified'),
+      _ => throw StateError('Unrecognized protocol $protocolOption'),
+    };
+
+    var useFineDependencies = parsedArgs.wasParsed(
+      Driver.withFineDependenciesOption,
+    );
+
+    return ServerDriver._(
+      arguments: [
+        '--${Driver.serverProtocolOption}=${protocol.flagValue}',
+        if (useFineDependencies) '--${Driver.withFineDependenciesOption}',
+      ],
+      protocol: protocol,
+    );
   }
 
+  /// Creates a new driver that can be used to communicate with a server.
+  ///
+  /// When the server is [start]ed, it will use the given [protocol].
+  ///
+  /// The server is run in a separate process.
+  ServerDriver._({required this.arguments, required ServerProtocol protocol})
+    : _protocol = protocol;
+
   /// Returns the path to the `dart` executable.
   String get _dartExecutable {
     return Platform.resolvedExecutable;
@@ -173,9 +186,8 @@ class ServerDriver {
     }
     var process = await Process.start(_dartExecutable, [
       'language-server',
-      '--protocol=${_protocol.flagValue}',
       '--suppress-analytics',
-      ...filteredAdditionalArguments,
+      ...arguments,
     ]);
     _stdinSink = process.stdin;
     if (_protocol == ServerProtocol.lsp) {