Skip to content

Commit 8183de1

Browse files
srawlinsCommit Queue
authored andcommitted
analyzer: Properly validate basics of new plugins in analysis options
* validate that the value of any top-level `plugins` key is a YamlMap. * validate that each value of each key in the `plugins` YamlMap is: * a scalar, or * a map, and that map only contains keys, `version`, `path`, or `git`. Change-Id: Ibec13304d96dd2bc9e776a4f599405c83a548896 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396720 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Phil Quitslund <[email protected]>
1 parent 004e115 commit 8183de1

File tree

2 files changed

+96
-7
lines changed

2 files changed

+96
-7
lines changed

pkg/analyzer/lib/src/task/options.dart

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,7 @@ class OptionsFileValidator {
384384
sdkVersionConstraint: sdkVersionConstraint,
385385
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
386386
),
387-
_PluginsTopLevelOptionsValidator(),
388-
// TODO(srawlins): validate everything inside the top-level 'plugins'
389-
// section.
387+
_PluginsOptionsValidator(),
390388
];
391389

392390
List<AnalysisError> validate(YamlMap options) {
@@ -989,10 +987,66 @@ class _OptionalChecksValueValidator extends OptionsValidator {
989987
}
990988
}
991989

992-
/// Validates `plugins` top-level options.
993-
class _PluginsTopLevelOptionsValidator extends _TopLevelOptionValidator {
994-
_PluginsTopLevelOptionsValidator()
995-
: super(AnalysisOptionsFile.plugins, AnalysisOptionsFile._pluginsOptions);
990+
/// Validates options for each `plugins` map value.
991+
class _PluginsOptionsValidator extends OptionsValidator {
992+
final _ErrorBuilder _builder =
993+
_ErrorBuilder(AnalysisOptionsFile._pluginsOptions);
994+
995+
@override
996+
void validate(ErrorReporter reporter, YamlMap options) {
997+
var plugins = options.valueAt(AnalysisOptionsFile.plugins);
998+
switch (plugins) {
999+
case YamlMap():
1000+
plugins.nodes.forEach((pluginName, pluginValue) {
1001+
if (pluginName is! String) {
1002+
return;
1003+
}
1004+
switch (pluginValue) {
1005+
case YamlScalar(value: String()):
1006+
// Valid enough. We could validate that it is a legal VersionConstraint
1007+
// from the pub_semver package.
1008+
break;
1009+
case YamlMap():
1010+
_validatePluginMap(reporter, pluginName, pluginValue);
1011+
default:
1012+
reporter.atSourceSpan(
1013+
plugins.span,
1014+
AnalysisOptionsWarningCode.INVALID_SECTION_FORMAT,
1015+
arguments: ['${AnalysisOptionsFile.plugins}/$pluginName'],
1016+
);
1017+
}
1018+
});
1019+
case YamlList():
1020+
reporter.atSourceSpan(
1021+
plugins.span,
1022+
AnalysisOptionsWarningCode.INVALID_SECTION_FORMAT,
1023+
arguments: [AnalysisOptionsFile.plugins],
1024+
);
1025+
case YamlScalar(:var value):
1026+
if (value != null) {
1027+
reporter.atSourceSpan(
1028+
plugins.span,
1029+
AnalysisOptionsWarningCode.INVALID_SECTION_FORMAT,
1030+
arguments: [AnalysisOptionsFile.plugins],
1031+
);
1032+
}
1033+
}
1034+
}
1035+
1036+
void _validatePluginMap(
1037+
ErrorReporter reporter, String pluginName, YamlMap pluginValue) {
1038+
pluginValue.nodes.forEach((pluginMapKeyNode, pluginMapValueNode) {
1039+
if (pluginMapKeyNode case YamlScalar(value: String pluginMapKey)) {
1040+
if (!AnalysisOptionsFile._pluginsOptions.contains(pluginMapKey)) {
1041+
_builder.reportError(reporter,
1042+
'${AnalysisOptionsFile.plugins}/$pluginName', pluginMapKeyNode);
1043+
}
1044+
}
1045+
// TODO(srawlins): Validate 'path' is a YamlScalar.
1046+
// TODO(srawlins): Validate 'git' value is a YamlScalar. Change when
1047+
// supporting refs.
1048+
});
1049+
}
9961050
}
9971051

9981052
/// Validates `analyzer` strong-mode value configuration options.

pkg/analyzer/test/src/task/options_test.dart

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,41 @@ linter:
414414
''', [AnalysisOptionsWarningCode.UNSUPPORTED_OPTION_WITH_LEGAL_VALUE]);
415415
}
416416

417+
test_plugins_each_invalid_mapKey() {
418+
validate('''
419+
plugins:
420+
one:
421+
ppath: foo/bar
422+
''', []);
423+
}
424+
425+
test_plugins_each_valid_mapKey() {
426+
validate('''
427+
plugins:
428+
one:
429+
path: foo/bar
430+
''', []);
431+
}
432+
433+
test_plugins_each_valid_scalar() {
434+
validate('''
435+
plugins:
436+
one: ^1.2.3
437+
''', []);
438+
}
439+
440+
test_plugins_invalid_scalar() {
441+
validate('''
442+
plugins: 7
443+
''', [AnalysisOptionsWarningCode.INVALID_SECTION_FORMAT]);
444+
}
445+
446+
test_plugins_valid_empty() {
447+
validate('''
448+
plugins:
449+
''', []);
450+
}
451+
417452
List<AnalysisError> validate(String source, List<ErrorCode> expected) {
418453
var options = optionsProvider.getOptionsFromString(source);
419454
var errors = validator.validate(options);

0 commit comments

Comments
 (0)