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>
2024-11-06 19:41:00 +00:00
parent 38541bd095
commit e66a8a4fa3
8 changed files with 171 additions and 50 deletions
@@ -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].
@@ -2,6 +2,8 @@
 // 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
@@ -9,9 +11,9 @@ class PluginPackageGenerator {
  ///
  /// 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();
@@ -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('''
@@ -36,6 +36,7 @@ class PluginServerErrorTest extends PluginServerTestBase {
    newAnalysisOptionsYamlFile(packagePath, '''
 plugins:
  no_bools:
+    path: some/path
    diagnostics:
      - no_bools
 ''');
@@ -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)
@@ -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';
 }
@@ -205,11 +205,14 @@ 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;
      }

@@ -217,18 +220,39 @@ final class AnalysisOptionsBuilder {
        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,
      ));
    });
  }
@@ -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) {