Skip to content

Commit 52001ff

Browse files
srawlinsCommit Queue
srawlins
authored and
Commit Queue
committed
analyzer: Support a list of included analysis options
Fixes #47256 See specified behavior at #47256 (comment) Additionally: * Make AnalysisOptionsProvider.sourceFactory private and final, which allows for promotion, yay! Change-Id: I26e0e73c661d48189078be95ff544fe8dfb7a86d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392100 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 7d00c54 commit 52001ff

File tree

4 files changed

+260
-106
lines changed

4 files changed

+260
-106
lines changed

‎pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart

+43-26
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,18 @@ import 'package:yaml/yaml.dart';
1616
class AnalysisOptionsProvider {
1717
/// The source factory used to resolve include declarations
1818
/// in analysis options files or `null` if include is not supported.
19-
SourceFactory? sourceFactory;
19+
final SourceFactory? _sourceFactory;
2020

21-
AnalysisOptionsProvider([this.sourceFactory]);
21+
AnalysisOptionsProvider([this._sourceFactory]);
2222

23-
/// Provide the options found in
24-
/// [root]/[file_paths.analysisOptionsYaml].
25-
/// Recursively merge options referenced by an include directive
26-
/// and remove the include directive from the resulting options map.
27-
/// Return an empty options map if the file does not exist or cannot be
23+
/// Provides the analysis options that apply to [root].
24+
///
25+
/// The analysis options come from either [file_paths.analysisOptionsYaml]
26+
/// found directly in [root] or one of [root]'s ancestor directories.
27+
///
28+
/// Recursively merges options referenced by any 'include' directives
29+
/// and removes any 'include' directives from the resulting options map.
30+
/// Returns an empty options map if the file does not exist or cannot be
2831
/// parsed.
2932
YamlMap getOptions(Folder root) {
3033
File? optionsFile = getOptionsFile(root);
@@ -34,7 +37,7 @@ class AnalysisOptionsProvider {
3437
return getOptionsFromFile(optionsFile);
3538
}
3639

37-
/// Return the analysis options file from which options should be read, or
40+
/// Returns the analysis options file from which options should be read, or
3841
/// `null` if there is no analysis options file for code in the given [root].
3942
///
4043
/// The given [root] directory will be searched first. If no file is found,
@@ -49,33 +52,47 @@ class AnalysisOptionsProvider {
4952
return null;
5053
}
5154

52-
/// Provide the options found in [file].
53-
/// Recursively merge options referenced by an include directive
54-
/// and remove the include directive from the resulting options map.
55-
/// Return an empty options map if the file does not exist or cannot be
55+
/// Provides the options found in [file].
56+
///
57+
/// Recursively merges options referenced by any 'include' directives
58+
/// and removes any 'include' directive from the resulting options map.
59+
/// Returns an empty options map if the file does not exist or cannot be
5660
/// parsed.
5761
YamlMap getOptionsFromFile(File file) {
5862
return getOptionsFromSource(FileSource(file));
5963
}
6064

61-
/// Provide the options found in [source].
65+
/// Provides the options found in [source].
6266
///
63-
/// Recursively merge options referenced by an `include` directive and remove
64-
/// the `include` directive from the resulting options map. Return an empty
65-
/// options map if the file does not exist or cannot be parsed.
67+
/// Recursively merges options referenced by any `include` directives and
68+
/// removes any `include` directives from the resulting options map. Returns
69+
/// an empty options map if the file does not exist or cannot be parsed.
6670
YamlMap getOptionsFromSource(Source source) {
6771
try {
68-
YamlMap options = getOptionsFromString(_readAnalysisOptions(source));
69-
var node = options.valueAt(AnalyzerOptions.include);
70-
var sourceFactory = this.sourceFactory;
71-
if (sourceFactory != null && node is YamlScalar) {
72-
var path = node.value;
73-
if (path is String) {
74-
var parent = sourceFactory.resolveUri(source, path);
75-
if (parent != null) {
76-
options = merge(getOptionsFromSource(parent), options);
77-
}
72+
var options = getOptionsFromString(_readAnalysisOptions(source));
73+
if (_sourceFactory == null) {
74+
return options;
75+
}
76+
var includeValue = options.valueAt(AnalyzerOptions.include);
77+
if (includeValue case YamlScalar(value: String path)) {
78+
var includeUri = _sourceFactory.resolveUri(source, path);
79+
if (includeUri != null) {
80+
options = merge(getOptionsFromSource(includeUri), options);
7881
}
82+
} else if (includeValue is YamlList) {
83+
var includePaths = includeValue.nodes
84+
.whereType<YamlScalar>()
85+
.map((e) => e.value)
86+
.whereType<String>();
87+
var includeOptions = includePaths.fold(YamlMap(), (options, path) {
88+
var includeUri = _sourceFactory.resolveUri(source, path);
89+
if (includeUri == null) {
90+
// Return the existing options, unchanged.
91+
return options;
92+
}
93+
return merge(options, getOptionsFromSource(includeUri));
94+
});
95+
options = merge(includeOptions, options);
7996
}
8097
return options;
8198
} on OptionsFormatException {

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

+93-79
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ List<AnalysisError> analyzeAnalysisOptions(
6969
}
7070
}
7171

72-
// Validate the specified options and any included option files.
72+
// Validates the specified options and any included option files.
7373
void validate(Source source, YamlMap options, LintRuleProvider? provider) {
7474
var sourceIsOptionsForContextRoot = initialIncludeSpan == null;
7575
var validationErrors = OptionsFileValidator(
@@ -90,85 +90,99 @@ List<AnalysisError> analyzeAnalysisOptions(
9090
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot);
9191
return;
9292
}
93-
var includeSpan = includeNode.span;
94-
initialIncludeSpan ??= includeSpan;
95-
String includeUri = includeSpan.text;
96-
var includedSource = sourceFactory.resolveUri(source, includeUri);
97-
if (includedSource == initialSource) {
98-
errors.add(
99-
AnalysisError.tmp(
100-
source: initialSource,
101-
offset: initialIncludeSpan!.start.offset,
102-
length: initialIncludeSpan!.length,
103-
errorCode: AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE,
104-
arguments: [includeUri, source.fullName],
105-
),
106-
);
107-
return;
108-
}
109-
if (includedSource == null || !includedSource.exists()) {
110-
errors.add(
111-
AnalysisError.tmp(
112-
source: initialSource,
113-
offset: initialIncludeSpan!.start.offset,
114-
length: initialIncludeSpan!.length,
115-
errorCode: AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND,
116-
arguments: [includeUri, source.fullName, contextRoot],
117-
),
118-
);
119-
return;
120-
}
121-
var spanInChain = includeChain[includedSource];
122-
if (spanInChain != null) {
123-
errors.add(
124-
AnalysisError.tmp(
125-
source: initialSource,
126-
offset: initialIncludeSpan!.start.offset,
127-
length: initialIncludeSpan!.length,
128-
errorCode: AnalysisOptionsWarningCode.INCLUDED_FILE_WARNING,
129-
arguments: [
130-
includedSource,
131-
spanInChain.start.offset,
132-
spanInChain.length,
133-
'The file includes itself recursively.',
134-
],
135-
),
136-
);
137-
return;
93+
94+
void validateInclude(YamlNode includeNode) {
95+
var includeSpan = includeNode.span;
96+
initialIncludeSpan ??= includeSpan;
97+
var includeUri = includeSpan.text;
98+
99+
var includedSource = sourceFactory.resolveUri(source, includeUri);
100+
if (includedSource == initialSource) {
101+
errors.add(
102+
AnalysisError.tmp(
103+
source: initialSource,
104+
offset: initialIncludeSpan!.start.offset,
105+
length: initialIncludeSpan!.length,
106+
errorCode: AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE,
107+
arguments: [includeUri, source.fullName],
108+
),
109+
);
110+
return;
111+
}
112+
if (includedSource == null || !includedSource.exists()) {
113+
errors.add(
114+
AnalysisError.tmp(
115+
source: initialSource,
116+
offset: initialIncludeSpan!.start.offset,
117+
length: initialIncludeSpan!.length,
118+
errorCode: AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND,
119+
arguments: [includeUri, source.fullName, contextRoot],
120+
),
121+
);
122+
return;
123+
}
124+
var spanInChain = includeChain[includedSource];
125+
if (spanInChain != null) {
126+
errors.add(
127+
AnalysisError.tmp(
128+
source: initialSource,
129+
offset: initialIncludeSpan!.start.offset,
130+
length: initialIncludeSpan!.length,
131+
errorCode: AnalysisOptionsWarningCode.INCLUDED_FILE_WARNING,
132+
arguments: [
133+
includedSource,
134+
spanInChain.start.offset,
135+
spanInChain.length,
136+
'The file includes itself recursively.',
137+
],
138+
),
139+
);
140+
return;
141+
}
142+
includeChain[includedSource] = includeSpan;
143+
144+
try {
145+
var includedOptions =
146+
optionsProvider.getOptionsFromString(includedSource.contents.data);
147+
validate(includedSource, includedOptions, provider);
148+
firstPluginName ??= _firstPluginName(includedOptions);
149+
// Validate the 'plugins' option in [options], taking into account any
150+
// plugins enabled by [includedOptions].
151+
addDirectErrorOrIncludedError(
152+
_validatePluginsOption(source,
153+
options: options, firstEnabledPluginName: firstPluginName),
154+
source,
155+
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
156+
);
157+
} on OptionsFormatException catch (e) {
158+
var args = [
159+
includedSource.fullName,
160+
e.span!.start.offset.toString(),
161+
e.span!.end.offset.toString(),
162+
e.message,
163+
];
164+
// Report errors for included option files on the `include` directive
165+
// located in the initial options file.
166+
errors.add(
167+
AnalysisError.tmp(
168+
source: initialSource,
169+
offset: initialIncludeSpan!.start.offset,
170+
length: initialIncludeSpan!.length,
171+
errorCode: AnalysisOptionsErrorCode.INCLUDED_FILE_PARSE_ERROR,
172+
arguments: args,
173+
),
174+
);
175+
}
138176
}
139-
includeChain[includedSource] = includeSpan;
140-
141-
try {
142-
var includedOptions =
143-
optionsProvider.getOptionsFromString(includedSource.contents.data);
144-
validate(includedSource, includedOptions, provider);
145-
firstPluginName ??= _firstPluginName(includedOptions);
146-
// Validate the 'plugins' option in [options], taking into account any
147-
// plugins enabled by [includedOptions].
148-
addDirectErrorOrIncludedError(
149-
_validatePluginsOption(source,
150-
options: options, firstEnabledPluginName: firstPluginName),
151-
source,
152-
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
153-
);
154-
} on OptionsFormatException catch (e) {
155-
var args = [
156-
includedSource.fullName,
157-
e.span!.start.offset.toString(),
158-
e.span!.end.offset.toString(),
159-
e.message,
160-
];
161-
// Report errors for included option files on the `include` directive
162-
// located in the initial options file.
163-
errors.add(
164-
AnalysisError.tmp(
165-
source: initialSource,
166-
offset: initialIncludeSpan!.start.offset,
167-
length: initialIncludeSpan!.length,
168-
errorCode: AnalysisOptionsErrorCode.INCLUDED_FILE_PARSE_ERROR,
169-
arguments: args,
170-
),
171-
);
177+
178+
if (includeNode is YamlScalar) {
179+
validateInclude(includeNode);
180+
} else if (includeNode is YamlList) {
181+
for (var includeValue in includeNode.nodes) {
182+
if (includeValue is YamlScalar) {
183+
validateInclude(includeValue);
184+
}
185+
}
172186
}
173187
}
174188

‎pkg/analyzer/test/src/diagnostics/analysis_options/recursive_include_file_test.dart

+63
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ include: analysis_options.yaml
2929
]);
3030
}
3131

32+
void test_itself_inList() {
33+
assertErrorsInCode('''
34+
include:
35+
- analysis_options.yaml
36+
''', [
37+
error(
38+
AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE,
39+
13,
40+
21,
41+
text: "The include file 'analysis_options.yaml' "
42+
"in '${convertPath('/analysis_options.yaml')}' includes itself recursively.",
43+
)
44+
]);
45+
}
46+
3247
void test_recursive() {
3348
newFile('/a.yaml', '''
3449
include: b.yaml
@@ -68,6 +83,54 @@ include: a.yaml
6883
]);
6984
}
7085

86+
void test_recursive_listAtTop() {
87+
newFile('/a.yaml', '''
88+
include: b.yaml
89+
''');
90+
newFile('/b.yaml', '''
91+
include: analysis_options.yaml
92+
''');
93+
newFile('/empty.yaml', '''
94+
''');
95+
assertErrorsInCode('''
96+
include:
97+
- empty.yaml
98+
- a.yaml
99+
''', [
100+
error(
101+
AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE,
102+
13,
103+
10,
104+
text: "The include file 'analysis_options.yaml' "
105+
"in '${convertPath('/b.yaml')}' includes itself recursively.",
106+
)
107+
]);
108+
}
109+
110+
void test_recursive_listIncluded() {
111+
newFile('/a.yaml', '''
112+
include:
113+
- empty.yaml
114+
- b.yaml
115+
''');
116+
newFile('/b.yaml', '''
117+
include: analysis_options.yaml
118+
''');
119+
newFile('/empty.yaml', '''
120+
''');
121+
assertErrorsInCode('''
122+
include: a.yaml
123+
''', [
124+
error(
125+
AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE,
126+
9,
127+
6,
128+
text: "The include file 'analysis_options.yaml' "
129+
"in '${convertPath('/b.yaml')}' includes itself recursively.",
130+
)
131+
]);
132+
}
133+
71134
void test_recursive_notInBeginning() {
72135
newFile('/a.yaml', '''
73136
include: b.yaml

0 commit comments

Comments
 (0)