Skip to content

Commit 83ecfc3

Browse files
FMorschelCommit Queue
authored andcommitted
[DAS] Adds some warnings for the analysis_options file
Adds `incompatible_lint` for included rules (and some variants of it) and `unsupported_value` error for linter rules values. Original change: https://dart-review.googlesource.com/c/sdk/+/419982 Original change reverted at: https://dart-review.googlesource.com/c/sdk/+/429381 Fixes: #60125 Change-Id: I0bc9a1a3c101c6455b8731cc0a40a39b6dfb5aac Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429240 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Auto-Submit: Felipe Morschel <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent e01d4b3 commit 83ecfc3

File tree

18 files changed

+1121
-147
lines changed

18 files changed

+1121
-147
lines changed

pkg/analysis_server/lib/src/context_manager.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ class ContextManagerImpl implements ContextManager {
398398
driver.sourceFactory,
399399
driver.currentSession.analysisContext.contextRoot.root.path,
400400
sdkVersionConstraint,
401+
resourceProvider,
401402
);
402403
var converter = AnalyzerConverter();
403404
convertedErrors = converter.convertAnalysisErrors(

pkg/analysis_server/lib/src/handler/legacy/edit_get_fixes.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ class EditGetFixesHandler extends LegacyHandler
133133
sourceFactory,
134134
analysisContext.contextRoot.root.path,
135135
sdkVersionConstraint,
136+
resourceProvider,
136137
);
137138
var options = _getOptions(sourceFactory, content);
138139
if (options == null) {

pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
8787
sourceFactory,
8888
contextRoot.root.path,
8989
sdkVersionConstraint,
90+
resourceProvider,
9091
);
9192

9293
var codeActions = <CodeActionWithPriority>[];

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,20 @@ AnalysisOptionsWarningCode.INCLUDED_FILE_WARNING:
8686
notes: |-
8787
The fix needs to be made in the included file.
8888
AnalysisOptionsWarningCode.INCOMPATIBLE_LINT:
89-
status: noFix
89+
status: needsFix
90+
notes: |-
91+
Allowing the user to select which rule they would prefer to use so we can
92+
remove the incompatible ones.
93+
AnalysisOptionsWarningCode.INCOMPATIBLE_LINT_FILES:
94+
status: needsFix
95+
notes: |-
96+
Allowing the user to select which rule they would prefer to use so we can
97+
disable the incompatible ones.
98+
AnalysisOptionsWarningCode.INCOMPATIBLE_LINT_INCLUDED:
99+
status: needsFix
90100
notes: |-
91-
Without knowing which rule the user would prefer to use, we cannot remove
92-
the reported one.
101+
Allowing the user to select which rule they would prefer to use so we can
102+
disable the incompatible ones.
93103
AnalysisOptionsWarningCode.INVALID_OPTION:
94104
status: needsFix
95105
notes: |-

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:analyzer/source/source.dart';
99
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
1010
import 'package:analyzer/src/analysis_options/error/option_codes.dart';
1111
import 'package:analyzer/src/lint/options_rule_validator.dart';
12+
import 'package:analyzer_testing/resource_provider_mixin.dart';
1213
import 'package:linter/src/rules.dart';
1314
import 'package:test/test.dart';
1415
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -20,14 +21,17 @@ void main() {
2021
}
2122

2223
@reflectiveTest
23-
class LinterRuleOptionsValidatorTest {
24+
class LinterRuleOptionsValidatorTest with ResourceProviderMixin {
2425
late RecordingDiagnosticListener recorder;
2526

2627
late DiagnosticReporter reporter;
2728

2829
List<Diagnostic> get diagnostics => recorder.diagnostics;
2930

30-
LinterRuleOptionsValidator get validator => LinterRuleOptionsValidator();
31+
LinterRuleOptionsValidator get validator => LinterRuleOptionsValidator(
32+
optionsProvider: AnalysisOptionsProvider(),
33+
resourceProvider: resourceProvider,
34+
);
3135

3236
void setUp() {
3337
registerLintRules();

pkg/analysis_server/test/src/services/correction/fix/analysis_options/test_support.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class AnalysisOptionsFixTest with ResourceProviderMixin {
5252
sourceFactory,
5353
'/',
5454
dart2_12,
55+
resourceProvider,
5556
);
5657
if (diagnosticFilter != null) {
5758
if (errors.length == 1) {

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

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,34 +67,36 @@ class AnalysisOptionsProvider {
6767
/// Recursively merges options referenced by any `include` directives and
6868
/// removes any `include` directives from the resulting options map. Returns
6969
/// an empty options map if the file does not exist or cannot be parsed.
70-
YamlMap getOptionsFromSource(Source source) {
70+
YamlMap getOptionsFromSource(Source source, {Set<Source>? handled}) {
71+
handled ??= {};
7172
try {
7273
var options = getOptionsFromString(_readAnalysisOptions(source));
7374
if (_sourceFactory == null) {
7475
return options;
7576
}
7677
var includeValue = options.valueAt(AnalysisOptionsFile.include);
77-
if (includeValue case YamlScalar(value: String path)) {
78+
var includes = switch (includeValue) {
79+
YamlScalar(:String value) => [value],
80+
YamlList() =>
81+
includeValue.nodes
82+
.whereType<YamlScalar>()
83+
.map((e) => e.value)
84+
.whereType<String>()
85+
.toList(),
86+
_ => <String>[],
87+
};
88+
var includeOptions = includes.fold(YamlMap(), (options, path) {
7889
var includeUri = _sourceFactory.resolveUri(source, path);
79-
if (includeUri != null) {
80-
options = merge(getOptionsFromSource(includeUri), options);
90+
if (includeUri == null || !handled!.add(includeUri)) {
91+
// Return the existing options, unchanged.
92+
return options;
8193
}
82-
} else if (includeValue is YamlList) {
83-
var includePaths =
84-
includeValue.nodes
85-
.whereType<YamlScalar>()
86-
.map((e) => e.value)
87-
.whereType<String>();
88-
var includeOptions = includePaths.fold(YamlMap(), (options, path) {
89-
var includeUri = _sourceFactory.resolveUri(source, path);
90-
if (includeUri == null) {
91-
// Return the existing options, unchanged.
92-
return options;
93-
}
94-
return merge(options, getOptionsFromSource(includeUri));
95-
});
96-
options = merge(includeOptions, options);
97-
}
94+
return merge(
95+
options,
96+
getOptionsFromSource(includeUri, handled: handled),
97+
);
98+
});
99+
options = merge(includeOptions, options);
98100
return options;
99101
} on OptionsFormatException {
100102
return YamlMap();
@@ -106,12 +108,12 @@ class AnalysisOptionsProvider {
106108
/// An 'include' directive, if present, will be left as-is, and the referenced
107109
/// options will NOT be merged into the result. Returns an empty options map
108110
/// if the content is null, or not a YAML map.
109-
YamlMap getOptionsFromString(String? content) {
111+
YamlMap getOptionsFromString(String? content, {Uri? sourceUrl}) {
110112
if (content == null) {
111113
return YamlMap();
112114
}
113115
try {
114-
var doc = loadYamlNode(content);
116+
var doc = loadYamlNode(content, sourceUrl: sourceUrl);
115117
return doc is YamlMap ? doc : YamlMap();
116118
} on YamlException catch (e) {
117119
throw OptionsFormatException(e.message, e.span);

pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -261,20 +261,75 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
261261

262262
/// An error code indicating an incompatible rule.
263263
///
264+
/// The incompatible rules must be included by context messages.
265+
///
264266
/// Parameters:
265267
/// String p0: the rule name
266-
/// String p1: the incompatible rule
268+
/// String p1: the incompatible rules
267269
static const AnalysisOptionsWarningTemplate<
268270
LocatableDiagnostic Function({required String p0, required String p1})
269271
>
270272
incompatibleLint = AnalysisOptionsWarningTemplate(
271273
'INCOMPATIBLE_LINT',
272-
"The rule '{0}' is incompatible with the rule '{1}'.",
273-
correctionMessage: "Try removing one of the incompatible rules.",
274+
"The rule '{0}' is incompatible with {1}.",
275+
correctionMessage: "Try removing all but one of the incompatible rules.",
274276
withArguments: _withArgumentsIncompatibleLint,
275277
expectedTypes: [ExpectedType.string, ExpectedType.string],
276278
);
277279

280+
/// An error code indicating an incompatible rule.
281+
///
282+
/// The files that enable the referenced rules must be included by context messages.
283+
///
284+
/// Parameters:
285+
/// String p0: the rule name
286+
/// String p1: the incompatible rules
287+
static const AnalysisOptionsWarningTemplate<
288+
LocatableDiagnostic Function({required String p0, required String p1})
289+
>
290+
incompatibleLintFiles = AnalysisOptionsWarningTemplate(
291+
'INCOMPATIBLE_LINT',
292+
"The rule '{0}' is incompatible with {1}.",
293+
correctionMessage:
294+
"Try locally disabling all but one of the conflicting rules or "
295+
"removing one of the incompatible files.",
296+
uniqueName: 'INCOMPATIBLE_LINT_FILES',
297+
withArguments: _withArgumentsIncompatibleLintFiles,
298+
expectedTypes: [ExpectedType.string, ExpectedType.string],
299+
);
300+
301+
/// An error code indicating an incompatible rule.
302+
///
303+
/// Parameters:
304+
/// String p0: the rule name
305+
/// String p1: the incompatible rules
306+
/// int p2: the number of files that include the incompatible rule
307+
/// String p3: plural suffix for the word "file"
308+
static const AnalysisOptionsWarningTemplate<
309+
LocatableDiagnostic Function({
310+
required String p0,
311+
required String p1,
312+
required int p2,
313+
required String p3,
314+
})
315+
>
316+
incompatibleLintIncluded = AnalysisOptionsWarningTemplate(
317+
'INCOMPATIBLE_LINT',
318+
"The rule '{0}' is incompatible with '{1}', which is included from {2} "
319+
"file{3}.",
320+
correctionMessage:
321+
"Try locally disabling all but one of the conflicting rules or "
322+
"removing one of the incompatible files.",
323+
uniqueName: 'INCOMPATIBLE_LINT_INCLUDED',
324+
withArguments: _withArgumentsIncompatibleLintIncluded,
325+
expectedTypes: [
326+
ExpectedType.string,
327+
ExpectedType.string,
328+
ExpectedType.int,
329+
ExpectedType.string,
330+
],
331+
);
332+
278333
/// An error code indicating that a plugin is being configured with an invalid
279334
/// value for an option and a detail message is provided.
280335
///
@@ -481,12 +536,12 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
481536
///
482537
/// Parameters:
483538
/// String p0: the option name
484-
/// int p1: the unsupported value
539+
/// Object p1: the unsupported value
485540
/// String p2: legal values
486541
static const AnalysisOptionsWarningTemplate<
487542
LocatableDiagnostic Function({
488543
required String p0,
489-
required int p1,
544+
required Object p1,
490545
required String p2,
491546
})
492547
>
@@ -495,7 +550,11 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
495550
"The value '{1}' isn't supported by '{0}'.",
496551
correctionMessage: "Try using one of the supported options: {2}.",
497552
withArguments: _withArgumentsUnsupportedValue,
498-
expectedTypes: [ExpectedType.string, ExpectedType.int, ExpectedType.string],
553+
expectedTypes: [
554+
ExpectedType.string,
555+
ExpectedType.object,
556+
ExpectedType.string,
557+
],
499558
);
500559

501560
/// Initialize a newly created error code to have the given [name].
@@ -577,6 +636,22 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
577636
return LocatableDiagnosticImpl(incompatibleLint, [p0, p1]);
578637
}
579638

639+
static LocatableDiagnostic _withArgumentsIncompatibleLintFiles({
640+
required String p0,
641+
required String p1,
642+
}) {
643+
return LocatableDiagnosticImpl(incompatibleLintFiles, [p0, p1]);
644+
}
645+
646+
static LocatableDiagnostic _withArgumentsIncompatibleLintIncluded({
647+
required String p0,
648+
required String p1,
649+
required int p2,
650+
required String p3,
651+
}) {
652+
return LocatableDiagnosticImpl(incompatibleLintIncluded, [p0, p1, p2, p3]);
653+
}
654+
580655
static LocatableDiagnostic _withArgumentsInvalidOption({
581656
required String p0,
582657
required String p1,
@@ -661,7 +736,7 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
661736

662737
static LocatableDiagnostic _withArgumentsUnsupportedValue({
663738
required String p0,
664-
required int p1,
739+
required Object p1,
665740
required String p2,
666741
}) {
667742
return LocatableDiagnosticImpl(unsupportedValue, [p0, p1, p2]);

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:analyzer/dart/analysis/formatter_options.dart';
66
import 'package:analyzer/diagnostic/diagnostic.dart';
77
import 'package:analyzer/error/error.dart';
88
import 'package:analyzer/error/listener.dart';
9+
import 'package:analyzer/file_system/file_system.dart';
910
import 'package:analyzer/source/source.dart';
1011
import 'package:analyzer/src/analysis_options/analysis_options_file.dart';
1112
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
@@ -29,6 +30,7 @@ List<Diagnostic> analyzeAnalysisOptions(
2930
SourceFactory sourceFactory,
3031
String contextRoot,
3132
VersionConstraint? sdkVersionConstraint,
33+
ResourceProvider resourceProvider,
3234
) {
3335
List<Diagnostic> errors = [];
3436
Source initialSource = source;
@@ -81,6 +83,8 @@ List<Diagnostic> analyzeAnalysisOptions(
8183
source,
8284
sdkVersionConstraint: sdkVersionConstraint,
8385
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
86+
optionsProvider: optionsProvider,
87+
resourceProvider: resourceProvider,
8488
).validate(options);
8589
addDirectErrorOrIncludedError(
8690
validationErrors,
@@ -209,7 +213,10 @@ List<Diagnostic> analyzeAnalysisOptions(
209213
}
210214

211215
try {
212-
YamlMap options = optionsProvider.getOptionsFromString(content);
216+
YamlMap options = optionsProvider.getOptionsFromString(
217+
content,
218+
sourceUrl: source.uri,
219+
);
213220
validate(source, options);
214221
} on OptionsFormatException catch (e) {
215222
SourceSpan span = e.span!;
@@ -286,12 +293,16 @@ class OptionsFileValidator {
286293
this._source, {
287294
VersionConstraint? sdkVersionConstraint,
288295
required bool sourceIsOptionsForContextRoot,
296+
required AnalysisOptionsProvider optionsProvider,
297+
required ResourceProvider resourceProvider,
289298
}) : _validators = [
290299
AnalyzerOptionsValidator(),
291300
_CodeStyleOptionsValidator(),
292301
_FormatterOptionsValidator(),
293302
_LinterTopLevelOptionsValidator(),
294303
LinterRuleOptionsValidator(
304+
resourceProvider: resourceProvider,
305+
optionsProvider: optionsProvider,
295306
sdkVersionConstraint: sdkVersionConstraint,
296307
sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
297308
),

pkg/analyzer/lib/src/diagnostic/diagnostic_code_values.g.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ const List<DiagnosticCode> diagnosticCodeValues = [
3535
AnalysisOptionsWarningCode.includedFileWarning,
3636
AnalysisOptionsWarningCode.includeFileNotFound,
3737
AnalysisOptionsWarningCode.incompatibleLint,
38+
AnalysisOptionsWarningCode.incompatibleLintFiles,
39+
AnalysisOptionsWarningCode.incompatibleLintIncluded,
3840
AnalysisOptionsWarningCode.invalidOption,
3941
AnalysisOptionsWarningCode.invalidSectionFormat,
4042
AnalysisOptionsWarningCode.multiplePlugins,

0 commit comments

Comments
 (0)