Skip to content

Commit d3ecc0f

Browse files
srawlinsCommit Queue
authored andcommitted
anlyzer: Simplify AnalysisOptionsProvider exception-logic, non-nullable _sourceFactory
Change-Id: I8eb2484340a939107abfad88bbf7a919c1461a75 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475785 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
1 parent 4b6664f commit d3ecc0f

7 files changed

Lines changed: 69 additions & 70 deletions

File tree

pkg/analysis_server/test/services/linter/linter_test.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:analyzer/error/error.dart';
77
import 'package:analyzer/error/listener.dart';
88
import 'package:analyzer/source/source.dart';
99
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
10+
import 'package:analyzer/src/context/source.dart';
1011
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
1112
import 'package:analyzer/src/file_system/file_system.dart';
1213
import 'package:analyzer/src/generated/source.dart';
@@ -31,7 +32,7 @@ class LinterRuleOptionsValidatorTest with ResourceProviderMixin {
3132
List<Diagnostic> get diagnostics => recorder.diagnostics;
3233

3334
LinterRuleOptionsValidator get validator => LinterRuleOptionsValidator(
34-
optionsProvider: AnalysisOptionsProvider(),
35+
optionsProvider: AnalysisOptionsProvider(SourceFactoryImpl([])),
3536
resourceProvider: resourceProvider,
3637
sourceFactory: sourceFactory,
3738
);
@@ -79,7 +80,9 @@ linter:
7980
}
8081

8182
void validate(String source, List<DiagnosticCode> expected) {
82-
var options = AnalysisOptionsProvider().getOptionsFromString(source);
83+
var options = AnalysisOptionsProvider(
84+
SourceFactoryImpl([]),
85+
).getOptionsFromString(source);
8386
validator.validate(reporter, options);
8487
expect(diagnostics.map((e) => e.diagnosticCode), unorderedEquals(expected));
8588
}

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

Lines changed: 41 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ import 'package:yaml/yaml.dart';
1515

1616
/// Provide the options found in the analysis options file.
1717
class AnalysisOptionsProvider {
18-
/// The source factory used to resolve include declarations
19-
/// in analysis options files or `null` if include is not supported.
20-
final SourceFactory? _sourceFactory;
18+
/// The source factory used to resolve include declarations in analysis
19+
/// options files.
20+
final SourceFactory _sourceFactory;
2121

22-
AnalysisOptionsProvider([this._sourceFactory]);
22+
AnalysisOptionsProvider(this._sourceFactory);
2323

2424
/// Provides the analysis options that apply to [root].
2525
///
@@ -74,63 +74,58 @@ class AnalysisOptionsProvider {
7474
Set<Source>? handled,
7575
}) {
7676
handled ??= {};
77+
78+
YamlMap options;
7779
try {
78-
var options = getOptionsFromString(_readAnalysisOptions(source));
79-
if (_sourceFactory == null) {
80-
return options;
81-
}
82-
var includeValue = options.valueAt(AnalysisOptionsFile.include);
83-
var includes = switch (includeValue) {
84-
YamlScalar(:String value) => [value],
85-
YamlList() =>
86-
includeValue.nodes
87-
.whereType<YamlScalar>()
88-
.map((e) => e.value)
89-
.whereType<String>()
90-
.toList(),
91-
_ => <String>[],
92-
};
93-
var includeOptions = includes.fold(YamlMap(), (currentOptions, path) {
94-
var includeSource = _sourceFactory.resolveUri(source, path);
95-
if (includeSource == null || !handled!.add(includeSource)) {
96-
// Return the existing options, unchanged.
97-
return currentOptions;
98-
}
99-
var includedOptions = getOptionsFromSource(
100-
includeSource,
101-
pathContext,
102-
handled: handled,
103-
);
104-
includedOptions = _rewriteRelativePaths(
105-
includedOptions,
106-
pathContext.dirname(includeSource.fullName),
107-
pathContext,
108-
);
109-
return merge(currentOptions, includedOptions);
110-
});
111-
options = merge(includeOptions, options);
112-
return options;
113-
} on OptionsFormatException {
80+
options = getOptionsFromString(source.contents.data);
81+
} on Exception {
11482
return YamlMap();
11583
}
84+
85+
var includeValue = options.valueAt(AnalysisOptionsFile.include);
86+
var includes = switch (includeValue) {
87+
YamlScalar(:String value) => [value],
88+
YamlList() =>
89+
includeValue.nodes
90+
.whereType<YamlScalar>()
91+
.map((e) => e.value)
92+
.whereType<String>()
93+
.toList(),
94+
_ => <String>[],
95+
};
96+
var includeOptions = includes.fold(YamlMap(), (currentOptions, path) {
97+
var includeSource = _sourceFactory.resolveUri(source, path);
98+
if (includeSource == null || !handled!.add(includeSource)) {
99+
// Return the existing options, unchanged.
100+
return currentOptions;
101+
}
102+
var includedOptions = getOptionsFromSource(
103+
includeSource,
104+
pathContext,
105+
handled: handled,
106+
);
107+
includedOptions = _rewriteRelativePaths(
108+
includedOptions,
109+
pathContext.dirname(includeSource.fullName),
110+
pathContext,
111+
);
112+
return merge(currentOptions, includedOptions);
113+
});
114+
options = merge(includeOptions, options);
115+
return options;
116116
}
117117

118118
/// Provide the options found in [content].
119119
///
120120
/// An 'include' directive, if present, will be left as-is, and the referenced
121121
/// options will NOT be merged into the result. Returns an empty options map
122122
/// if the content is null, or not a YAML map.
123-
YamlMap getOptionsFromString(String? content, {Uri? sourceUrl}) {
124-
if (content == null) {
125-
return YamlMap();
126-
}
123+
YamlMap getOptionsFromString(String content, {Uri? sourceUrl}) {
127124
try {
128125
var doc = loadYamlNode(content, sourceUrl: sourceUrl);
129126
return doc is YamlMap ? doc : YamlMap();
130127
} on YamlException catch (e) {
131128
throw OptionsFormatException(e.message, e.span);
132-
} catch (e) {
133-
throw OptionsFormatException('Unable to parse YAML document.');
134129
}
135130
}
136131

@@ -149,17 +144,6 @@ class AnalysisOptionsProvider {
149144
YamlMap merge(YamlMap defaults, YamlMap overrides) =>
150145
Merger().mergeMap(defaults, overrides);
151146

152-
/// Read the contents of [source] as a string.
153-
/// Returns null if source is null or does not exist.
154-
String? _readAnalysisOptions(Source source) {
155-
try {
156-
return source.contents.data;
157-
} catch (e) {
158-
// Source can't be read.
159-
return null;
160-
}
161-
}
162-
163147
/// Walks [options] with semantic knowledge about where paths may appear in an
164148
/// analysis options file, rewriting relative paths (relative to [directory])
165149
/// as absolute paths.

pkg/analyzer/test/source/analysis_options_provider_test.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
6+
import 'package:analyzer/src/context/source.dart';
67
import 'package:analyzer/src/file_system/file_system.dart';
78
import 'package:analyzer/src/generated/source.dart';
89
import 'package:analyzer/src/util/file_paths.dart' as file_paths;
@@ -19,7 +20,7 @@ main() {
1920

2021
group('AnalysisOptionsProvider', () {
2122
void expectMergesTo(String defaults, String overrides, String expected) {
22-
var optionsProvider = AnalysisOptionsProvider();
23+
var optionsProvider = AnalysisOptionsProvider(SourceFactoryImpl([]));
2324
var defaultOptions = optionsProvider.getOptionsFromString(defaults);
2425
var overrideOptions = optionsProvider.getOptionsFromString(overrides);
2526
var merged = optionsProvider.merge(defaultOptions, overrideOptions);
@@ -78,7 +79,7 @@ linter:
7879
strong-mode: true
7980
''';
8081

81-
var optionsProvider = AnalysisOptionsProvider();
82+
var optionsProvider = AnalysisOptionsProvider(SourceFactoryImpl([]));
8283
expect(
8384
() => optionsProvider.getOptionsFromString(src),
8485
throwsA(TypeMatcher<OptionsFormatException>()),
@@ -91,7 +92,7 @@ analyzer:
9192
strong-mode:true # missing space (sdk/issues/24885)
9293
''';
9394

94-
var optionsProvider = AnalysisOptionsProvider();
95+
var optionsProvider = AnalysisOptionsProvider(SourceFactoryImpl([]));
9596
// Should not throw an exception.
9697
var options = optionsProvider.getOptionsFromString(src);
9798
// Should return a non-null options list.

pkg/analyzer/test/source/error_processor_test.dart

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:analyzer/diagnostic/diagnostic.dart';
77
import 'package:analyzer/error/error.dart';
88
import 'package:analyzer/source/error_processor.dart';
99
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
10+
import 'package:analyzer/src/context/source.dart';
1011
import 'package:analyzer/src/dart/analysis/analysis_options.dart';
1112
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
1213
import 'package:collection/collection.dart';
@@ -131,7 +132,9 @@ analyzer:
131132

132133
group('processing', () {
133134
test('yaml map', () {
134-
var options = AnalysisOptionsProvider().getOptionsFromString(config);
135+
var options = AnalysisOptionsProvider(
136+
SourceFactoryImpl([]),
137+
).getOptionsFromString(config);
135138
var errorConfig = ErrorConfig(
136139
(options['analyzer'] as YamlMap)['errors'] as YamlNode?,
137140
);
@@ -184,9 +187,10 @@ analyzer:
184187
});
185188

186189
test('configure lints', () {
187-
var options = AnalysisOptionsProvider().getOptionsFromString(
188-
'analyzer:\n errors:\n annotate_overrides: warning\n',
189-
);
190+
var options = AnalysisOptionsProvider(SourceFactoryImpl([]))
191+
.getOptionsFromString(
192+
'analyzer:\n errors:\n annotate_overrides: warning\n',
193+
);
190194
var errorConfig = ErrorConfig(
191195
(options['analyzer'] as YamlMap)['errors'] as YamlNode?,
192196
);
@@ -204,7 +208,9 @@ class _TestContext {
204208

205209
void configureOptions(String options) {
206210
analysisOptions = AnalysisOptionsImpl.fromYaml(
207-
optionsMap: AnalysisOptionsProvider().getOptionsFromString(options),
211+
optionsMap: AnalysisOptionsProvider(
212+
SourceFactoryImpl([]),
213+
).getOptionsFromString(options),
208214
);
209215
}
210216

pkg/analyzer/test/src/options/analysis_options_test.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:analyzer/diagnostic/diagnostic.dart';
66
import 'package:analyzer/error/error.dart';
77
import 'package:analyzer/file_system/memory_file_system.dart';
88
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
9+
import 'package:analyzer/src/context/source.dart';
910
import 'package:analyzer/src/dart/analysis/analysis_options.dart';
1011
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
1112
import 'package:analyzer/src/file_system/file_system.dart';
@@ -27,7 +28,9 @@ main() {
2728

2829
@reflectiveTest
2930
class AnalysisOptionsTest {
30-
final AnalysisOptionsProvider optionsProvider = AnalysisOptionsProvider();
31+
final AnalysisOptionsProvider optionsProvider = AnalysisOptionsProvider(
32+
SourceFactoryImpl([]),
33+
);
3134

3235
final resourceProvider = MemoryResourceProvider();
3336

pkg/analyzer/test/src/options/options_file_validator_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:analyzer/diagnostic/diagnostic.dart';
99
import 'package:analyzer/error/error.dart';
1010
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
1111
import 'package:analyzer/src/analysis_options/options_file_validator.dart';
12+
import 'package:analyzer/src/context/source.dart';
1213
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
1314
import 'package:analyzer/src/file_system/file_system.dart';
1415
import 'package:analyzer/src/generated/source.dart';
@@ -84,7 +85,7 @@ class OptionsFileValidatorTest
8485
resourceProvider: resourceProvider,
8586
sourceFactory: SourceFactory([ResourceUriResolver(resourceProvider)]),
8687
);
87-
final AnalysisOptionsProvider optionsProvider = AnalysisOptionsProvider();
88+
final optionsProvider = AnalysisOptionsProvider(SourceFactoryImpl([]));
8889

8990
void tearDown() {
9091
unregisterLintRules();

pkg/analyzer_cli/test/driver_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:analyzer/error/error.dart';
99
import 'package:analyzer/source/error_processor.dart';
1010
import 'package:analyzer/source/source.dart';
1111
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
12+
import 'package:analyzer/src/context/source.dart';
1213
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
1314
import 'package:analyzer/src/generated/engine.dart';
1415
import 'package:analyzer/src/util/file_paths.dart' as file_paths;
@@ -274,7 +275,7 @@ linter:
274275
}
275276

276277
YamlMap _parseOptions(String src) =>
277-
AnalysisOptionsProvider().getOptionsFromString(src);
278+
AnalysisOptionsProvider(SourceFactoryImpl([])).getOptionsFromString(src);
278279

279280
Future<void> _runLinter_noLintsFlag() async {
280281
await drive(

0 commit comments

Comments
 (0)