analyzer plugins: parse version and path keys in YAML

This enables plugin sources to be specified in one of three ways: * as a YamlScalar, like `plugin_name: ^1.2.3` * with a 'version' key, like `plugin_name:\n version: ^1.2.3`, which allows diagnostic configurations to be alongside. * with a 'path' key, like `plugin_name:\n path: foo/bar`, which allows diagnostic configurations to be alongside. We introduce a PluginSource with a `toYaml` method to convert the values back into YAML for the generates pubspec. We update PluginPackageGenerator to use PluginSource.toYaml. Change-Id: Ic5f372f0339edd6aa638dd9b568a83f41a023b08 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393900 Commit-Queue: Samuel Rawlins <srawlins@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-lang · Nov 6, 2024 · e66a8a4 · e66a8a4
1 parent 38541bd
commit e66a8a4
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 50 deletions.
diff --git a/pkg/analysis_server/lib/src/plugin/plugin_watcher.dart b/pkg/analysis_server/lib/src/plugin/plugin_watcher.dart
@@ -41,8 +41,7 @@ class PluginWatcher implements DriverWatcher {
     var contextRoot = driver.analysisContext!.contextRoot;
     _driverInfo[driver] = _DriverInfo(
         contextRoot, <String>[contextRoot.root.path, _getSdkPath(driver)]);
-    var enabledPlugins = driver.enabledLegacyPluginNames;
-    for (var hostPackageName in enabledPlugins) {
+    for (var hostPackageName in driver.enabledLegacyPluginNames) {
       //
       // Determine whether the package exists and defines a plugin.
       //
@@ -66,6 +65,8 @@ class PluginWatcher implements DriverWatcher {
       manager.addPluginToContextRoot(
           driver.analysisContext!.contextRoot, pluginPath);
     }
+
+    // TODO(srawlins): Generate package for plugin configurations, with PluginPackageGenerator.
   }
 
   /// The context manager has just removed the given analysis [driver].

diff --git a/pkg/analysis_server/lib/src/plugin2/generator.dart b/pkg/analysis_server/lib/src/plugin2/generator.dart
@@ -2,16 +2,18 @@
 // 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:analyzer/dart/analysis/analysis_options.dart';
+
 /// This class can generate various files to make up the shared plugin package.
 class PluginPackageGenerator {
   /// The plugin configuration, a map of plugin names to each plugin's
   /// configuration.
   ///
   /// This typically stems from plugin configuration in an analysis options
   /// file.
-  final Map<String, Object> _pluginConfiguration;
+  final List<PluginConfiguration> _pluginConfigurations;
 
-  PluginPackageGenerator(this._pluginConfiguration);
+  PluginPackageGenerator(this._pluginConfigurations);
 
   /// Generates the Dart entrpoint file which is to be spawned in a Dart
   /// isolate by the analysis server.
@@ -20,8 +22,8 @@ class PluginPackageGenerator {
       "'package:analysis_server_plugin/src/plugin_server.dart'",
       "'package:analyzer/file_system/physical_file_system.dart'",
       "'package:analyzer_plugin/starter.dart'",
-      for (var name in _pluginConfiguration.keys)
-        "'package:$name/main.dart' as $name",
+      for (var configuration in _pluginConfigurations)
+        "'package:${configuration.name}/main.dart' as ${configuration.name}",
     ];
 
     var buffer = StringBuffer("import 'dart:isolate';");
@@ -36,8 +38,8 @@ Future<void> main(List<String> args, SendPort sendPort) async {
     plugins: [
 ''');
     // TODO(srawlins): Format with the formatter, for readability.
-    for (var name in _pluginConfiguration.keys) {
-      buffer.writeln('      $name.plugin,');
+    for (var configuration in _pluginConfigurations) {
+      buffer.writeln('      ${configuration.name}.plugin,');
     }
     buffer.write('''
     ],
@@ -60,18 +62,8 @@ version: 0.0.1
 dependencies:
 ''');
 
-    for (var MapEntry(key: name, value: configuration)
-        in _pluginConfiguration.entries) {
-      switch (configuration) {
-        case String():
-          buffer.writeln('  $name: $configuration');
-        case Map<String, Object>():
-          if (configuration case {'path': String pathValue}) {
-            buffer.writeln('  $name:\n    path: $pathValue');
-          } else if (configuration case {'git': String gitValue}) {
-            buffer.writeln('  $name:\n    git: $gitValue');
-          }
-      }
+    for (var configuration in _pluginConfigurations) {
+      buffer.write(configuration.sourceYaml());
     }
 
     return buffer.toString();

diff --git a/pkg/analysis_server/test/src/plugin2/generator_test.dart b/pkg/analysis_server/test/src/plugin2/generator_test.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/plugin2/generator.dart';
+import 'package:analyzer/src/generated/engine.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
@@ -15,10 +16,16 @@ void main() {
 @reflectiveTest
 class GeneratorTest {
   void test_entrypointImportsPluginEntrypoints() {
-    var pluginPackageGenerator = PluginPackageGenerator({
-      'no_bools': '^1.0.0',
-      'no_ints': '^1.2.0',
-    });
+    var pluginPackageGenerator = PluginPackageGenerator([
+      PluginConfiguration(
+        name: 'no_bools',
+        source: VersionedPluginSource(constraint: '^1.0.0'),
+      ),
+      PluginConfiguration(
+        name: 'no_ints',
+        source: VersionedPluginSource(constraint: '^1.2.0'),
+      ),
+    ]);
     expect(
       pluginPackageGenerator.generateEntrypoint(),
       contains('''
@@ -29,10 +36,16 @@ import 'package:no_ints/main.dart' as no_ints;
   }
 
   void test_entrypointListsPluginInstances() {
-    var pluginPackageGenerator = PluginPackageGenerator({
-      'no_bools': '^1.0.0',
-      'no_ints': '^1.2.0',
-    });
+    var pluginPackageGenerator = PluginPackageGenerator([
+      PluginConfiguration(
+        name: 'no_bools',
+        source: VersionedPluginSource(constraint: '^1.0.0'),
+      ),
+      PluginConfiguration(
+        name: 'no_ints',
+        source: VersionedPluginSource(constraint: '^1.2.0'),
+      ),
+    ]);
     expect(
       pluginPackageGenerator.generateEntrypoint(),
       contains('''
@@ -45,24 +58,34 @@ import 'package:no_ints/main.dart' as no_ints;
   }
 
   void test_pubspecContainsGitDependencies() {
-    var pluginPackageGenerator = PluginPackageGenerator({
-      'no_bools': {'git': 'https://example.com/example.git'},
-    });
+    var pluginPackageGenerator = PluginPackageGenerator([
+      PluginConfiguration(
+        name: 'no_bools',
+        source: GitPluginSource(url: 'https://example.com/example.git'),
+      ),
+    ]);
     expect(
       pluginPackageGenerator.generatePubspec(),
       contains('''
 dependencies:
   no_bools:
-    git: https://example.com/example.git
+    git:
+      url: https://example.com/example.git
 '''),
     );
   }
 
   void test_pubspecContainsPathDependencies() {
-    var pluginPackageGenerator = PluginPackageGenerator({
-      'no_bools': {'path': '../no_bools_plugin'},
-      'no_ints': {'path': 'tools/no_ints_plugin'},
-    });
+    var pluginPackageGenerator = PluginPackageGenerator([
+      PluginConfiguration(
+        name: 'no_bools',
+        source: PathPluginSource(path: '../no_bools_plugin'),
+      ),
+      PluginConfiguration(
+        name: 'no_ints',
+        source: PathPluginSource(path: 'tools/no_ints_plugin'),
+      ),
+    ]);
     expect(
       pluginPackageGenerator.generatePubspec(),
       contains('''
@@ -76,10 +99,16 @@ dependencies:
   }
 
   void test_pubspecContainsVersionedDependencies() {
-    var pluginPackageGenerator = PluginPackageGenerator({
-      'no_bools': '^1.0.0',
-      'no_ints': '^1.2.0',
-    });
+    var pluginPackageGenerator = PluginPackageGenerator([
+      PluginConfiguration(
+        name: 'no_bools',
+        source: VersionedPluginSource(constraint: '^1.0.0'),
+      ),
+      PluginConfiguration(
+        name: 'no_ints',
+        source: VersionedPluginSource(constraint: '^1.2.0'),
+      ),
+    ]);
     expect(
       pluginPackageGenerator.generatePubspec(),
       contains('''

diff --git a/pkg/analysis_server_plugin/test/src/plugin_server_error_test.dart b/pkg/analysis_server_plugin/test/src/plugin_server_error_test.dart
@@ -36,6 +36,7 @@ class PluginServerErrorTest extends PluginServerTestBase {
     newAnalysisOptionsYamlFile(packagePath, '''
 plugins:
   no_bools:
+    path: some/path
     diagnostics:
       - no_bools
 ''');

diff --git a/pkg/analysis_server_plugin/test/src/plugin_server_test.dart b/pkg/analysis_server_plugin/test/src/plugin_server_test.dart
@@ -199,6 +199,7 @@ class PluginServerTest extends PluginServerTestBase {
     var buffer = StringBuffer('''
 plugins:
   no_literals:
+    path: some/path
     diagnostics:
 ''');
     for (var MapEntry(key: diagnosticName, value: isEnabled)

diff --git a/pkg/analyzer/lib/dart/analysis/analysis_options.dart b/pkg/analyzer/lib/dart/analysis/analysis_options.dart
@@ -73,12 +73,55 @@ abstract class AnalysisOptions {
   bool isLintEnabled(String name);
 }
 
+final class GitPluginSource implements PluginSource {
+  final String _url;
+
+  final String? _path;
+
+  final String? _ref;
+
+  GitPluginSource({required String url, String? path, String? ref})
+      : _url = url,
+        _path = path,
+        _ref = ref;
+
+  @override
+  String toYaml({required String name}) {
+    var buffer = StringBuffer()
+      ..writeln('  $name:')
+      ..writeln('    git:')
+      ..writeln('      url: $_url');
+    if (_ref != null) {
+      buffer.writeln('      ref: $_ref');
+    }
+    if (_path != null) {
+      buffer.writeln('      path: $_path');
+    }
+    return buffer.toString();
+  }
+}
+
+final class PathPluginSource implements PluginSource {
+  final String _path;
+
+  PathPluginSource({required String path}) : _path = path;
+
+  @override
+  String toYaml({required String name}) => '''
+  $name:
+    path: $_path
+''';
+}
+
 /// The configuration of a Dart Analysis Server plugin, as specified by
 /// analysis options.
 final class PluginConfiguration {
   /// The name of the plugin being configured.
   final String name;
 
+  /// The source of the plugin being configured.
+  final PluginSource source;
+
   /// The list of specified [DiagnosticConfig]s.
   final Map<String, DiagnosticConfig> diagnosticConfigs;
 
@@ -87,7 +130,34 @@ final class PluginConfiguration {
 
   PluginConfiguration({
     required this.name,
-    required this.diagnosticConfigs,
-    required this.isEnabled,
+    required this.source,
+    this.diagnosticConfigs = const {},
+    this.isEnabled = true,
   });
+
+  String sourceYaml() => source.toYaml(name: name);
+}
+
+/// A description of the source of a plugin.
+
+/// We support all of the source formats documented at
+/// https://dart.dev/tools/pub/dependencies.
+sealed class PluginSource {
+  /// Returns the YAML-formatted source, using [name] as a key, for writing into
+  /// a pubspec 'dependencies' section.
+  String toYaml({required String name});
+}
+
+/// A plugin source using a version constraint, hosted either at pub.dev or
+/// another host.
+// TODO(srawlins): Support a different 'hosted' URL.
+final class VersionedPluginSource implements PluginSource {
+  /// The specified version constraint.
+  final String _constraint;
+
+  VersionedPluginSource({required String constraint})
+      : _constraint = constraint;
+
+  @override
+  String toYaml({required String name}) => '  $name: $_constraint\n';
 }
diff --git a/pkg/analyzer/lib/src/dart/analysis/analysis_options.dart b/pkg/analyzer/lib/src/dart/analysis/analysis_options.dart
@@ -205,30 +205,54 @@ final class AnalysisOptionsBuilder {
         return;
       }
       var pluginName = nameNode.toString();
-      if (pluginNode is YamlScalar) {
-        // TODO(srawlins): There may be value in storing the version constraint,
-        // `value`.
+
+      // If the plugin name just maps to a String, then that is the version
+      // constraint; use it and move on.
+      if (pluginNode case YamlScalar(:String value)) {
         pluginConfigurations.add(PluginConfiguration(
-            name: pluginName, diagnosticConfigs: const {}, isEnabled: true));
+          name: pluginName,
+          source: VersionedPluginSource(constraint: value),
+        ));
         return;
       }
 
       if (pluginNode is! YamlMap) {
         return;
       }
 
-      var diagnostics = pluginNode.valueAt(AnalyzerOptions.diagnostics);
-      if (diagnostics == null) {
+      // Grab either the source value from 'version', 'git', or 'path'. In the
+      // erroneous case that multiple are specified, just take the first. A
+      // warning should be reported by `OptionsFileValidator`.
+      // TODO(srawlins): In adition to 'version' and 'path', try 'git'.
+
+      PluginSource? source;
+      var versionSource = pluginNode.valueAt(AnalyzerOptions.version);
+      if (versionSource case YamlScalar(:String value)) {
+        // TODO(srawlins): Handle the 'hosted' key.
+        source = VersionedPluginSource(constraint: value);
+      } else {
+        var pathSource = pluginNode.valueAt(AnalyzerOptions.path);
+        if (pathSource case YamlScalar(:String value)) {
+          source = PathPluginSource(path: value);
+        }
+      }
+
+      if (source == null) {
+        // Either the source data is malformed, or neither 'version' nor 'git'
+        // was provided. A warning should be reported by OptionsFileValidator.
         return;
       }
 
-      var diagnosticConfigurations = parseDiagnosticsSection(diagnostics);
+      var diagnostics = pluginNode.valueAt(AnalyzerOptions.diagnostics);
+      var diagnosticConfigurations = diagnostics == null
+          ? const <String, RuleConfig>{}
+          : parseDiagnosticsSection(diagnostics);
 
       pluginConfigurations.add(PluginConfiguration(
         name: pluginName,
+        source: source,
         diagnosticConfigs: diagnosticConfigurations,
         // TODO(srawlins): Implement `enabled: false`.
-        isEnabled: true,
       ));
     });
   }

diff --git a/pkg/analyzer/lib/src/task/options.dart b/pkg/analyzer/lib/src/task/options.dart
@@ -285,6 +285,8 @@ class AnalyzerOptions {
 
   /// Plugin options.
   static const String diagnostics = 'diagnostics';
+  static const String path = 'path';
+  static const String version = 'version';
 
   static const String propagateLinterExceptions = 'propagate-linter-exceptions';
 
@@ -849,6 +851,7 @@ class OptionsFileValidator {
             sdkVersionConstraint: sdkVersionConstraint,
             sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
           ),
+          // TODO(srawlins): validate the top-level 'plugins' section.
         ];
 
   List<AnalysisError> validate(YamlMap options) {