[L] Change in dart/sdk[stable]: [stable] [analyzer] Limit the number of plugins-per-context to one

0 views
Skip to first unread message

Samuel Rawlins (Gerrit)

unread,
Feb 7, 2023, 9:55:21 AM2/7/23
to Brian Wilkerson, rev...@dartlang.org

Attention is currently required from: Brian Wilkerson.

Samuel Rawlins would like Brian Wilkerson to review this change.

View Change

[stable] [analyzer] Limit the number of plugins-per-context to one

This accounts for included options files, and the three forms of 'plugins' values that are supported: scalar, list, and map.

This includes an analysis options warning which reports each plugin specified after the first.

Fixes https://github.com/dart-lang/sdk/issues/50981

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/278805
Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart
M pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/lib/src/task/options.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/task/options_test.dart
7 files changed, 498 insertions(+), 59 deletions(-)

diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
index 782801e..620cc37 100644
--- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
+++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -69,6 +69,10 @@
replace the invalid option with the selected replacement.
AnalysisOptionsWarningCode.INVALID_SECTION_FORMAT:
status: needsEvaluation
+AnalysisOptionsWarningCode.MULTIPLE_PLUGINS:
+ status: needsFix
+ notes: |-
+ The fix is to remove the plugin name.
AnalysisOptionsWarningCode.SPEC_MODE_REMOVED:
status: needsFix
notes: |-
diff --git a/pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart b/pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart
index 4269e6b..8975553 100644
--- a/pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart
+++ b/pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart
@@ -56,9 +56,10 @@
}

/// Provide the options found in [source].
- /// Recursively merge options referenced by an include directive
- /// and remove the include directive from the resulting options map.
- /// Return an empty options map if the file does not exist.
+ ///
+ /// Recursively merge options referenced by an `include` directive and remove
+ /// the `include` directive from the resulting options map. Return an empty
+ /// options map if the file does not exist.
YamlMap getOptionsFromSource(Source source) {
YamlMap options = getOptionsFromString(_readAnalysisOptions(source));
var node = options.valueAt(AnalyzerOptions.include);
diff --git a/pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart b/pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart
index 925ddcb..b353be2 100644
--- a/pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart
+++ b/pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart
@@ -156,6 +156,17 @@
"Invalid format for the '{0}' section.",
);

+ /// An error code indicating multiple plugins have been specified as enabled.
+ ///
+ /// Parameters:
+ /// 0: the name of the first plugin
+ static const AnalysisOptionsWarningCode MULTIPLE_PLUGINS =
+ AnalysisOptionsWarningCode(
+ 'MULTIPLE_PLUGINS',
+ "Multiple plugins can't be enabled.",
+ correctionMessage: "Remove all plugins following the first, '{0}'.",
+ );
+
/// An error code indicating that strong-mode: false is has been removed.
static const AnalysisOptionsWarningCode SPEC_MODE_REMOVED =
AnalysisOptionsWarningCode(
diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart
index 924fa8e..e706325 100644
--- a/pkg/analyzer/lib/src/error/error_code_values.g.dart
+++ b/pkg/analyzer/lib/src/error/error_code_values.g.dart
@@ -29,6 +29,7 @@
AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND,
AnalysisOptionsWarningCode.INVALID_OPTION,
AnalysisOptionsWarningCode.INVALID_SECTION_FORMAT,
+ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS,
AnalysisOptionsWarningCode.SPEC_MODE_REMOVED,
AnalysisOptionsWarningCode.UNRECOGNIZED_ERROR_CODE,
AnalysisOptionsWarningCode.UNSUPPORTED_OPTION_WITHOUT_VALUES,
diff --git a/pkg/analyzer/lib/src/task/options.dart b/pkg/analyzer/lib/src/task/options.dart
index b6ef279..05d8a03 100644
--- a/pkg/analyzer/lib/src/task/options.dart
+++ b/pkg/analyzer/lib/src/task/options.dart
@@ -37,12 +37,17 @@
SourceSpan? initialIncludeSpan;
AnalysisOptionsProvider optionsProvider =
AnalysisOptionsProvider(sourceFactory);
+ String? firstPluginName;

- // Validate the specified options and any included option files
- void validate(Source source, YamlMap options) {
- List<AnalysisError> validationErrors =
- OptionsFileValidator(source).validate(options);
- if (initialIncludeSpan != null && validationErrors.isNotEmpty) {
+ // TODO(srawlins): This code is getting quite complex, with multiple local
+ // functions, and should be refactored to a class maintaining state, with less
+ // variable shadowing.
+ void addDirectErrorOrIncludedError(
+ List<AnalysisError> validationErrors, Source source,
+ {required bool sourceIsOptionsForContextRoot}) {
+ if (!sourceIsOptionsForContextRoot) {
+ // [source] is an included file, and we should only report errors in
+ // [initialSource], noting that the included file has warnings.
for (AnalysisError error in validationErrors) {
var args = [
source.fullName,
@@ -58,16 +63,32 @@
args));
}
} else {
+ // [source] is the options file for [contextRoot]. Report all errors
+ // directly.
errors.addAll(validationErrors);
}
+ }

- var node = options.valueAt(AnalyzerOptions.include);
- if (node == null) {
+ // Validate the specified options and any included option files.
+ void validate(Source source, YamlMap options) {
+ var sourceIsOptionsForContextRoot = initialIncludeSpan == null;
+ List<AnalysisError> validationErrors =
+ OptionsFileValidator(source).validate(options);
+ addDirectErrorOrIncludedError(validationErrors, source,
+ sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot);
+
+ var includeNode = options.valueAt(AnalyzerOptions.include);
+ if (includeNode == null) {
+ // Validate the 'plugins' option in [options], understanding that no other
+ // options are included.
+ addDirectErrorOrIncludedError(
+ _validatePluginsOption(source, options: options), source,
+ sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot);
return;
}
- SourceSpan span = node.span;
- initialIncludeSpan ??= span;
- String includeUri = span.text;
+ var includeSpan = includeNode.span;
+ initialIncludeSpan ??= includeSpan;
+ String includeUri = includeSpan.text;
var includedSource = sourceFactory.resolveUri(source, includeUri);
if (includedSource == null || !includedSource.exists()) {
errors.add(AnalysisError(
@@ -79,9 +100,18 @@
return;
}
try {
- YamlMap options =
+ var includedOptions =
optionsProvider.getOptionsFromString(includedSource.contents.data);
- validate(includedSource, options);
+ validate(includedSource, includedOptions);
+ firstPluginName ??= _firstPluginName(includedOptions);
+ // Validate the 'plugins' option in [options], taking into account any
+ // plugins enabled by [includedOptions].
+ addDirectErrorOrIncludedError(
+ _validatePluginsOption(source,
+ options: options, firstEnabledPluginName: firstPluginName),
+ source,
+ sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
+ );
} on OptionsFormatException catch (e) {
var args = [
includedSource.fullName,
@@ -89,8 +119,8 @@
e.span!.end.offset.toString(),
e.message,
];
- // Report errors for included option files
- // on the include directive located in the initial options file.
+ // Report errors for included option files on the `include` directive
+ // located in the initial options file.
errors.add(AnalysisError(
initialSource,
initialIncludeSpan!.start.offset,
@@ -115,6 +145,42 @@
_processor.applyToAnalysisOptions(options, optionMap);
}

+/// Returns the name of the first plugin, if one is specified in [options],
+/// otherwise `null`.
+String? _firstPluginName(YamlMap options) {
+ var analyzerMap = options.valueAt(AnalyzerOptions.analyzer);
+ if (analyzerMap is! YamlMap) {
+ return null;
+ }
+ var plugins = analyzerMap.valueAt(AnalyzerOptions.plugins);
+ if (plugins is YamlScalar) {
+ return plugins.value as String?;
+ } else if (plugins is YamlList) {
+ return plugins.first as String?;
+ } else if (plugins is YamlMap) {
+ return plugins.keys.first as String?;
+ } else {
+ return null;
+ }
+}
+
+/// Validates the 'plugins' options in [options], given
+/// [firstEnabledPluginName].
+List<AnalysisError> _validatePluginsOption(
+ Source source, {
+ required YamlMap options,
+ String? firstEnabledPluginName,
+}) {
+ RecordingErrorListener recorder = RecordingErrorListener();
+ ErrorReporter reporter = ErrorReporter(
+ recorder,
+ source,
+ isNonNullableByDefault: true,
+ );
+ PluginsOptionValidator(firstEnabledPluginName).validate(reporter, options);
+ return recorder.errors;
+}
+
/// `analyzer` analysis options constants.
class AnalyzerOptions {
static const String analyzer = 'analyzer';
@@ -627,6 +693,103 @@
}
}

+/// Validates `analyzer` plugins configuration options.
+class PluginsOptionValidator extends OptionsValidator {
+ /// The name of the first included plugin, if there is one.
+ ///
+ /// If there are no included plugins, this is `null`.
+ final String? _firstIncludedPluginName;
+
+ PluginsOptionValidator(this._firstIncludedPluginName);
+
+ @override
+ void validate(ErrorReporter reporter, YamlMap options) {
+ var analyzer = options.valueAt(AnalyzerOptions.analyzer);
+ if (analyzer is! YamlMap) {
+ return;
+ }
+ var plugins = analyzer.valueAt(AnalyzerOptions.plugins);
+ if (plugins is YamlScalar && plugins.value != null) {
+ if (_firstIncludedPluginName != null &&
+ _firstIncludedPluginName != plugins.value) {
+ reporter.reportErrorForSpan(
+ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS,
+ plugins.span,
+ [_firstIncludedPluginName!],
+ );
+ }
+ } else if (plugins is YamlList) {
+ var pluginValues = plugins.nodes.whereType<YamlNode>().toList();
+ if (_firstIncludedPluginName != null) {
+ // There is already at least one plugin specified in included options.
+ for (var plugin in pluginValues) {
+ if (plugin.value != _firstIncludedPluginName) {
+ reporter.reportErrorForSpan(
+ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS,
+ plugin.span,
+ [_firstIncludedPluginName!],
+ );
+ }
+ }
+ } else if (plugins.length > 1) {
+ String? firstPlugin;
+ for (var plugin in pluginValues) {
+ if (firstPlugin == null) {
+ var pluginValue = plugin.value;
+ if (pluginValue is String) {
+ firstPlugin = pluginValue;
+ continue;
+ } else {
+ // This plugin is bad and should not be marked as the first one.
+ continue;
+ }
+ } else if (plugin.value != firstPlugin) {
+ reporter.reportErrorForSpan(
+ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS,
+ plugin.span,
+ [firstPlugin],
+ );
+ }
+ }
+ }
+ } else if (plugins is YamlMap) {
+ var pluginValues = plugins.nodes.keys.cast<YamlNode?>();
+ if (_firstIncludedPluginName != null) {
+ // There is already at least one plugin specified in included options.
+ for (var plugin in pluginValues) {
+ if (plugin != null && plugin.value != _firstIncludedPluginName) {
+ reporter.reportErrorForSpan(
+ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS,
+ plugin.span,
+ [_firstIncludedPluginName!],
+ );
+ }
+ }
+ } else if (plugins.length > 1) {
+ String? firstPlugin;
+ for (var plugin in pluginValues) {
+ if (firstPlugin == null) {
+ var pluginValue = plugin?.value;
+ if (pluginValue is String) {
+ firstPlugin = pluginValue;
+ continue;
+ } else {
+ // This plugin is bad and should not be marked as the first one.
+ continue;
+ }
+ } else if (plugin != null && plugin.value != firstPlugin) {
+ reporter.reportErrorForSpan(
+ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS,
+ plugin.span,
+ [firstPlugin],
+ );
+ }
+ }
+ }
+ }
+ }
+}
+
/// Validates `analyzer` strong-mode value configuration options.
class StrongModeOptionValueValidator extends OptionsValidator {
final ErrorBuilder _builder = ErrorBuilder(AnalyzerOptions.strongModeOptions);
@@ -775,27 +938,8 @@
_applyUnignorables(options, cannotIgnore);

// Process plugins.
- var names = analyzer.valueAt(AnalyzerOptions.plugins);
- List<String> pluginNames = <String>[];
- var pluginName = _toString(names);
- if (pluginName != null) {
- pluginNames.add(pluginName);
- } else if (names is YamlList) {
- for (var element in names.nodes) {
- var pluginName = _toString(element);
- if (pluginName != null) {
- pluginNames.add(pluginName);
- }
- }
- } else if (names is YamlMap) {
- for (var key in names.nodes.keys) {
- var pluginName = _toString(key);
- if (pluginName != null) {
- pluginNames.add(pluginName);
- }
- }
- }
- options.enabledPluginNames = pluginNames;
+ var plugins = analyzer.valueAt(AnalyzerOptions.plugins);
+ _applyPlugins(options, plugins);
}

// Process the 'code-style' option.
@@ -873,6 +1017,31 @@
}
}

+ void _applyPlugins(AnalysisOptionsImpl options, YamlNode? plugins) {
+ var pluginName = _toString(plugins);
+ if (pluginName != null) {
+ options.enabledPluginNames = [pluginName];
+ } else if (plugins is YamlList) {
+ for (var element in plugins.nodes) {
+ var pluginName = _toString(element);
+ if (pluginName != null) {
+ // Only the first plugin is supported.
+ options.enabledPluginNames = [pluginName];
+ return;
+ }
+ }
+ } else if (plugins is YamlMap) {
+ for (var key in plugins.nodes.keys.cast<YamlNode?>()) {
+ var pluginName = _toString(key);
+ if (pluginName != null) {
+ // Only the first plugin is supported.
+ options.enabledPluginNames = [pluginName];
+ return;
+ }
+ }
+ }
+ }
+
void _applyProcessors(AnalysisOptionsImpl options, YamlNode? codes) {
ErrorConfig config = ErrorConfig(codes);
options.errorProcessors = config.processors;
diff --git a/pkg/analyzer/messages.yaml b/pkg/analyzer/messages.yaml
index b3028c9..0fd311d 100644
--- a/pkg/analyzer/messages.yaml
+++ b/pkg/analyzer/messages.yaml
@@ -106,6 +106,14 @@

Parameters:
0: the section name
+ MULTIPLE_PLUGINS:
+ problemMessage: "Multiple plugins can't be enabled."
+ correctionMessage: "Remove all plugins following the first, '{0}'."
+ comment: |-
+ An error code indicating multiple plugins have been specified as enabled.
+
+ Parameters:
+ 0: the name of the first plugin
SPEC_MODE_REMOVED:
problemMessage: "The option 'strong-mode: false' is no longer supported."
correctionMessage: "It's recommended to remove the 'strong-mode:' setting (and make your code Dart 2 compliant)."
diff --git a/pkg/analyzer/test/src/task/options_test.dart b/pkg/analyzer/test/src/task/options_test.dart
index 2d98e22..341700b 100644
--- a/pkg/analyzer/test/src/task/options_test.dart
+++ b/pkg/analyzer/test/src/task/options_test.dart
@@ -15,12 +15,12 @@
import 'package:analyzer/src/lint/linter.dart';
import 'package:analyzer/src/lint/registry.dart';
import 'package:analyzer/src/task/options.dart';
+import 'package:analyzer/src/test_utilities/resource_provider_mixin.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
import 'package:yaml/yaml.dart';

import '../../generated/test_support.dart';
-import '../../resource_utils.dart';

main() {
defineReflectiveSuite(() {
@@ -155,6 +155,8 @@
}

test_analyzer_plugins_list() {
+ // TODO(srawlins): Test plugins as a list of non-scalar values
+ // (`- angular2: yes`).
configureContext('''
analyzer:
plugins:
@@ -163,10 +165,11 @@
''');

var names = analysisOptions.enabledPluginNames;
- expect(names, ['angular2', 'intl']);
+ expect(names, ['angular2']);
}

test_analyzer_plugins_map() {
+ // TODO(srawlins): Test plugins as a map of scalar values (`angular2: yes`).
configureContext('''
analyzer:
plugins:
@@ -604,25 +607,101 @@
}

@reflectiveTest
-class OptionsProviderTest {
- late final TestPathTranslator pathTranslator;
- late final ResourceProvider resourceProvider;
+class OptionsProviderTest with ResourceProviderMixin {
+ late final SourceFactory sourceFactory;

late final AnalysisOptionsProvider provider;

String get optionsFilePath => '/analysis_options.yaml';

- void setUp() {
- var rawProvider = MemoryResourceProvider();
- resourceProvider = TestResourceProvider(rawProvider);
- pathTranslator = TestPathTranslator(rawProvider);
- provider = AnalysisOptionsProvider(SourceFactory([
- ResourceUriResolver(rawProvider),
- ]));
+ void assertErrorsInList(
+ List<AnalysisError> errors,
+ List<ExpectedError> expectedErrors,
+ ) {
+ GatheringErrorListener errorListener = GatheringErrorListener();
+ errorListener.addAll(errors);
+ errorListener.assertErrors(expectedErrors);
}

- test_perform_include_merge() {
- pathTranslator.newFile('/other_options.yaml', '''
+ void assertErrorsInOptionsFile(
+ String code, List<ExpectedError> expectedErrors) async {
+ newFile(optionsFilePath, code);
+ var errors = analyzeAnalysisOptions(
+ sourceFactory.forUri2(toUri(optionsFilePath))!,
+ code,
+ sourceFactory,
+ '/',
+ );
+
+ assertErrorsInList(errors, expectedErrors);
+ }
+
+ ExpectedError error(
+ ErrorCode code,
+ int offset,
+ int length, {
+ Pattern? correctionContains,
+ String? text,
+ List<Pattern> messageContains = const [],
+ List<ExpectedContextMessage> contextMessages =
+ const <ExpectedContextMessage>[],
+ }) =>
+ ExpectedError(
+ code,
+ offset,
+ length,
+ correctionContains: correctionContains,
+ message: text,
+ messageContains: messageContains,
+ expectedContextMessages: contextMessages,
+ );
+
+ void setUp() {
+ resourceProvider = MemoryResourceProvider();
+ sourceFactory = SourceFactory([ResourceUriResolver(resourceProvider)]);
+ provider = AnalysisOptionsProvider(sourceFactory);
+ }
+
+ test_chooseFirstPlugin() {
+ newFile('/more_options.yaml', '''
+analyzer:
+ plugins:
+ - plugin_ddd
+ - plugin_ggg
+ - plugin_aaa
+''');
+ newFile('/other_options.yaml', '''
+include: more_options.yaml
+analyzer:
+ plugins:
+ - plugin_eee
+ - plugin_hhh
+ - plugin_bbb
+''');
+ String code = r'''
+include: other_options.yaml
+analyzer:
+ plugins:
+ - plugin_fff
+ - plugin_iii
+ - plugin_ccc
+''';
+ newFile(optionsFilePath, code);
+
+ final options = _getOptionsObject('/');
+ expect(options.enabledPluginNames, unorderedEquals(['plugin_ddd']));
+ }
+
+ test_mergeIncludedOptions() {
+ // TODO(https://github.com/dart-lang/sdk/issues/50978): add tests for
+ // INCLUDED_FILE_WARNING.
+ // TODO(https://github.com/dart-lang/sdk/issues/50979): add tests for cyclic
+ // includes.
+ // TODO(srawlins): add tests for multiple includes.
+ // TODO(https://github.com/dart-lang/sdk/issues/50980): add tests with
+ // duplicate plugin names.
+
+ newFile('/other_options.yaml', '''
analyzer:
exclude:
- toplevelexclude.dart
@@ -640,16 +719,13 @@
analyzer:
exclude:
- lowlevelexclude.dart
- plugins:
- lowlevelplugin:
- enabled: true
errors:
lowlevelerror: warning
linter:
rules:
- lowlevellint
''';
- pathTranslator.newFile(optionsFilePath, code);
+ newFile(optionsFilePath, code);

final lowlevellint = TestRule.withName('lowlevellint');
final toplevellint = TestRule.withName('toplevellint');
@@ -658,8 +734,7 @@
final options = _getOptionsObject('/');

expect(options.lintRules, unorderedEquals([toplevellint, lowlevellint]));
- expect(options.enabledPluginNames,
- unorderedEquals(['toplevelplugin', 'lowlevelplugin']));
+ expect(options.enabledPluginNames, unorderedEquals(['toplevelplugin']));
expect(options.excludePatterns,
unorderedEquals(['toplevelexclude.dart', 'lowlevelexclude.dart']));
expect(
@@ -672,8 +747,162 @@
]));
}

+ test_multiplePlugins_firstIsDirectlyIncluded_secondIsDirect_listForm() {
+ newFile(convertPath('/other_options.yaml'), '''
+analyzer:
+ plugins:
+ - plugin_one
+''');
+ assertErrorsInOptionsFile(r'''
+include: other_options.yaml
+analyzer:
+ plugins:
+ - plugin_two
+''', [
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 55, 10),
+ ]);
+ }
+
+ test_multiplePlugins_firstIsDirectlyIncluded_secondIsDirect_mapForm() {
+ newFile('/other_options.yaml', '''
+analyzer:
+ plugins:
+ - plugin_one
+''');
+ assertErrorsInOptionsFile(r'''
+include: other_options.yaml
+analyzer:
+ plugins:
+ plugin_two:
+ foo: bar
+''', [
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 53, 10),
+ ]);
+ }
+
+ test_multiplePlugins_firstIsDirectlyIncluded_secondIsDirect_scalarForm() {
+ newFile('/other_options.yaml', '''
+analyzer:
+ plugins:
+ - plugin_one
+''');
+ assertErrorsInOptionsFile(r'''
+include: other_options.yaml
+analyzer:
+ plugins: plugin_two
+''', [
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 49, 10),
+ ]);
+ }
+
+ test_multiplePlugins_firstIsIndirectlyIncluded_secondIsDirect() {
+ newFile('/more_options.yaml', '''
+analyzer:
+ plugins:
+ - plugin_one
+''');
+ newFile('/other_options.yaml', '''
+include: more_options.yaml
+''');
+ assertErrorsInOptionsFile(r'''
+include: other_options.yaml
+analyzer:
+ plugins:
+ - plugin_two
+''', [
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 55, 10),
+ ]);
+ }
+
+ test_multiplePlugins_firstIsIndirectlyIncluded_secondIsDirectlyIncluded() {
+ newFile('/more_options.yaml', '''
+analyzer:
+ plugins:
+ - plugin_one
+''');
+ newFile('/other_options.yaml', '''
+include: more_options.yaml
+analyzer:
+ plugins:
+ - plugin_two
+''');
+ assertErrorsInOptionsFile(r'''
+include: other_options.yaml
+''', [
+ error(AnalysisOptionsWarningCode.INCLUDED_FILE_WARNING, 9, 18),
+ ]);
+ }
+
+ test_multiplePlugins_multipleDirect_listForm() {
+ assertErrorsInOptionsFile(r'''
+analyzer:
+ plugins:
+ - plugin_one
+ - plugin_two
+ - plugin_three
+''', [
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 44, 10),
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 61, 12),
+ ]);
+ }
+
+ test_multiplePlugins_multipleDirect_listForm_nonString() {
+ assertErrorsInOptionsFile(r'''
+analyzer:
+ plugins:
+ - 7
+ - plugin_one
+''', []);
+ }
+
+ test_multiplePlugins_multipleDirect_listForm_sameName() {
+ assertErrorsInOptionsFile(r'''
+analyzer:
+ plugins:
+ - plugin_one
+ - plugin_one
+''', []);
+ }
+
+ test_multiplePlugins_multipleDirect_mapForm() {
+ assertErrorsInOptionsFile(r'''
+analyzer:
+ plugins:
+ plugin_one: yes
+ plugin_two: sure
+''', [
+ error(AnalysisOptionsWarningCode.MULTIPLE_PLUGINS, 45, 10),
+ ]);
+ }
+
+ test_multiplePlugins_multipleDirect_mapForm_sameName() {
+ assertErrorsInOptionsFile(r'''
+analyzer:
+ plugins:
+ plugin_one: yes
+ plugin_one: sure
+''', [
+ error(AnalysisOptionsErrorCode.PARSE_ERROR, 45, 10),
+ ]);
+ }
+
+ List<AnalysisError> validate(String code, List<ErrorCode> expected) {
+ newFile(optionsFilePath, code);
+ var errors = analyzeAnalysisOptions(
+ sourceFactory.forUri('file://$optionsFilePath')!,
+ code,
+ sourceFactory,
+ '/',
+ );
+ expect(
+ errors.map((AnalysisError e) => e.errorCode),
+ unorderedEquals(expected),
+ );
+ return errors;
+ }
+
YamlMap _getOptions(String posixPath) {
- var resource = pathTranslator.getResource(posixPath) as Folder;
+ var resource = getFolder(posixPath);
return provider.getOptions(resource);
}


To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: sdk
Gerrit-Branch: stable
Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
Gerrit-Change-Number: 281300
Gerrit-PatchSet: 2
Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
Gerrit-MessageType: newchange

Samuel Rawlins (Gerrit)

unread,
Feb 7, 2023, 9:55:21 AM2/7/23
to rev...@dartlang.org, Brian Wilkerson, Commit Queue

Attention is currently required from: Brian Wilkerson.

View Change

    To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: sdk
    Gerrit-Branch: stable
    Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
    Gerrit-Change-Number: 281300
    Gerrit-PatchSet: 2
    Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
    Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Comment-Date: Tue, 07 Feb 2023 14:55:17 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Brian Wilkerson (Gerrit)

    unread,
    Feb 7, 2023, 5:16:26 PM2/7/23
    to Samuel Rawlins, rev...@dartlang.org, Brian Wilkerson, Commit Queue

    Attention is currently required from: Samuel Rawlins.

    Patch set 2:Code-Review +1

    View Change

    1 comment:

    • File pkg/analyzer/lib/src/task/options.dart:

    To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: sdk
    Gerrit-Branch: stable
    Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
    Gerrit-Change-Number: 281300
    Gerrit-PatchSet: 2
    Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
    Gerrit-Attention: Samuel Rawlins <sraw...@google.com>
    Gerrit-Comment-Date: Tue, 07 Feb 2023 22:16:21 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Samuel Rawlins (Gerrit)

    unread,
    Feb 7, 2023, 5:27:15 PM2/7/23
    to rev...@dartlang.org, Brian Wilkerson, Commit Queue

    View Change

    1 comment:

    • File pkg/analyzer/lib/src/task/options.dart:

      • I suppose I did, but I don't think I should change it from what is landed in the main branch, if only to avoid later merge conflicts.

    To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: sdk
    Gerrit-Branch: stable
    Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
    Gerrit-Change-Number: 281300
    Gerrit-PatchSet: 2
    Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
    Gerrit-Comment-Date: Tue, 07 Feb 2023 22:27:11 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Brian Wilkerson <brianwi...@google.com>
    Gerrit-MessageType: comment

    Brian Wilkerson (Gerrit)

    unread,
    Feb 7, 2023, 5:29:25 PM2/7/23
    to Samuel Rawlins, rev...@dartlang.org, Brian Wilkerson, Commit Queue

    Attention is currently required from: Samuel Rawlins.

    View Change

    1 comment:

    • File pkg/analyzer/lib/src/task/options.dart:

      • I suppose I did, but I don't think I should change it from what is landed in the main branch, if onl […]

        I agree.

    To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: sdk
    Gerrit-Branch: stable
    Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
    Gerrit-Change-Number: 281300
    Gerrit-PatchSet: 2
    Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
    Gerrit-Attention: Samuel Rawlins <sraw...@google.com>
    Gerrit-Comment-Date: Tue, 07 Feb 2023 22:29:20 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Samuel Rawlins <sraw...@google.com>

    Jonas Termansen (Gerrit)

    unread,
    Feb 20, 2023, 8:19:12 AM2/20/23
    to Samuel Rawlins, rev...@dartlang.org, Brian Wilkerson, Commit Queue

    Attention is currently required from: Samuel Rawlins.

    View Change

    1 comment:

    • Patchset:

      • Patch Set #2:

        Hi, looks like https://github.com/dart-lang/sdk/issues/51272 has been approved, you can land this change at your leisure when it's finalized and ready. If it's submitted by Tuesday, it can go into a stable patch release on Wednesday, unclear how the US holiday affects the schedule though :)

    To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: sdk
    Gerrit-Branch: stable
    Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
    Gerrit-Change-Number: 281300
    Gerrit-PatchSet: 2
    Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
    Gerrit-CC: Jonas Termansen <sor...@google.com>
    Gerrit-Attention: Samuel Rawlins <sraw...@google.com>
    Gerrit-Comment-Date: Mon, 20 Feb 2023 13:19:07 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Samuel Rawlins (Gerrit)

    unread,
    Feb 21, 2023, 5:08:49 PM2/21/23
    to rev...@dartlang.org, Jonas Termansen, Brian Wilkerson, Commit Queue

    Attention is currently required from: Samuel Rawlins.

    Patch set 2:Commit-Queue +2

    View Change

      To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: sdk
      Gerrit-Branch: stable
      Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
      Gerrit-Change-Number: 281300
      Gerrit-PatchSet: 2
      Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
      Gerrit-CC: Jonas Termansen <sor...@google.com>
      Gerrit-Attention: Samuel Rawlins <sraw...@google.com>
      Gerrit-Comment-Date: Tue, 21 Feb 2023 22:08:47 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      Gerrit-MessageType: comment

      Commit Queue (Gerrit)

      unread,
      Feb 21, 2023, 5:09:04 PM2/21/23
      to Samuel Rawlins, rev...@dartlang.org, Jonas Termansen, Brian Wilkerson

      Commit Queue submitted this change.

      View Change

      Approvals: Brian Wilkerson: Looks good to me, approved Samuel Rawlins: Commit
      [stable] [analyzer] Limit the number of plugins-per-context to one

      This accounts for included options files, and the three forms of 'plugins' values that are supported: scalar, list, and map.

      This includes an analysis options warning which reports each plugin specified after the first.

      Fixes https://github.com/dart-lang/sdk/issues/50981

      Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/278805
      Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
      Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281300
      Reviewed-by: Brian Wilkerson <brianwi...@google.com>
      Commit-Queue: Samuel Rawlins <sraw...@google.com>

      ---
      M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
      M pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart
      M pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart
      M pkg/analyzer/lib/src/error/error_code_values.g.dart
      M pkg/analyzer/lib/src/task/options.dart
      M pkg/analyzer/messages.yaml
      M pkg/analyzer/test/src/task/options_test.dart
      7 files changed, 501 insertions(+), 59 deletions(-)

      To view, visit change 281300. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: sdk
      Gerrit-Branch: stable
      Gerrit-Change-Id: Ib6fe3ddd459854cf02aba3687fdb82b62db18b64
      Gerrit-Change-Number: 281300
      Gerrit-PatchSet: 3
      Gerrit-Owner: Samuel Rawlins <sraw...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Samuel Rawlins <sraw...@google.com>
      Gerrit-CC: Jonas Termansen <sor...@google.com>
      Gerrit-MessageType: merged
      Reply all
      Reply to author
      Forward
      0 new messages