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>
diff --git a/pkg/analysis_server/lib/src/plugin/plugin_watcher.dart b/pkg/analysis_server/lib/src/plugin/plugin_watcher.dart
index 34ec5b2..236332c 100644
--- a/pkg/analysis_server/lib/src/plugin/plugin_watcher.dart
+++ b/pkg/analysis_server/lib/src/plugin/plugin_watcher.dart
@@ -41,8 +41,7 @@
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 @@
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
index cab3548..725da06 100644
--- a/pkg/analysis_server/lib/src/plugin2/generator.dart
+++ b/pkg/analysis_server/lib/src/plugin2/generator.dart
@@ -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 @@
///
/// 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 @@
"'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 @@
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 @@
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
index 7dfd3d7..b3b5785 100644
--- 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 @@
@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 @@
}
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 @@
}
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 @@
}
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
index e2340a9..39c8fee 100644
--- 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 @@
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
index 8bc4dab..9904dbb 100644
--- 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 @@
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
index 4f2901e..3003146 100644
--- a/pkg/analyzer/lib/dart/analysis/analysis_options.dart
+++ b/pkg/analyzer/lib/dart/analysis/analysis_options.dart
@@ -73,12 +73,55 @@
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 @@
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
index 262ff03..f81be6d 100644
--- a/pkg/analyzer/lib/src/dart/analysis/analysis_options.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/analysis_options.dart
@@ -205,11 +205,14 @@
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 @@
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
index ea16d6e..cbbea02 100644
--- a/pkg/analyzer/lib/src/task/options.dart
+++ b/pkg/analyzer/lib/src/task/options.dart
@@ -285,6 +285,8 @@
/// 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 @@
sdkVersionConstraint: sdkVersionConstraint,
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
),
+ // TODO(srawlins): validate the top-level 'plugins' section.
];
List<AnalysisError> validate(YamlMap options) {
