analyzer: Support a list of included analysis options Fixes https://github.com/dart-lang/sdk/issues/47256 See specified behavior at https://github.com/dart-lang/sdk/issues/47256#issuecomment-2374880529 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 <brianwilkerson@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
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 86829728..32e5847 100644 --- a/pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart +++ b/pkg/analyzer/lib/src/analysis_options/analysis_options_provider.dart
@@ -16,15 +16,18 @@ class AnalysisOptionsProvider { /// The source factory used to resolve include declarations /// in analysis options files or `null` if include is not supported. - SourceFactory? sourceFactory; + final SourceFactory? _sourceFactory; - AnalysisOptionsProvider([this.sourceFactory]); + AnalysisOptionsProvider([this._sourceFactory]); - /// Provide the options found in - /// [root]/[file_paths.analysisOptionsYaml]. - /// 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 or cannot be + /// Provides the analysis options that apply to [root]. + /// + /// The analysis options come from either [file_paths.analysisOptionsYaml] + /// found directly in [root] or one of [root]'s ancestor directories. + /// + /// Recursively merges options referenced by any 'include' directives + /// and removes any 'include' directives from the resulting options map. + /// Returns an empty options map if the file does not exist or cannot be /// parsed. YamlMap getOptions(Folder root) { File? optionsFile = getOptionsFile(root); @@ -34,7 +37,7 @@ return getOptionsFromFile(optionsFile); } - /// Return the analysis options file from which options should be read, or + /// Returns the analysis options file from which options should be read, or /// `null` if there is no analysis options file for code in the given [root]. /// /// The given [root] directory will be searched first. If no file is found, @@ -49,33 +52,47 @@ return null; } - /// Provide the options found in [file]. - /// 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 or cannot be + /// Provides the options found in [file]. + /// + /// Recursively merges options referenced by any 'include' directives + /// and removes any 'include' directive from the resulting options map. + /// Returns an empty options map if the file does not exist or cannot be /// parsed. YamlMap getOptionsFromFile(File file) { return getOptionsFromSource(FileSource(file)); } - /// Provide the options found in [source]. + /// Provides 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 or cannot be parsed. + /// Recursively merges options referenced by any `include` directives and + /// removes any `include` directives from the resulting options map. Returns + /// an empty options map if the file does not exist or cannot be parsed. YamlMap getOptionsFromSource(Source source) { try { - YamlMap options = getOptionsFromString(_readAnalysisOptions(source)); - var node = options.valueAt(AnalyzerOptions.include); - var sourceFactory = this.sourceFactory; - if (sourceFactory != null && node is YamlScalar) { - var path = node.value; - if (path is String) { - var parent = sourceFactory.resolveUri(source, path); - if (parent != null) { - options = merge(getOptionsFromSource(parent), options); - } + var options = getOptionsFromString(_readAnalysisOptions(source)); + if (_sourceFactory == null) { + return options; + } + var includeValue = options.valueAt(AnalyzerOptions.include); + if (includeValue case YamlScalar(value: String path)) { + var includeUri = _sourceFactory.resolveUri(source, path); + if (includeUri != null) { + options = merge(getOptionsFromSource(includeUri), options); } + } else if (includeValue is YamlList) { + var includePaths = includeValue.nodes + .whereType<YamlScalar>() + .map((e) => e.value) + .whereType<String>(); + var includeOptions = includePaths.fold(YamlMap(), (options, path) { + var includeUri = _sourceFactory.resolveUri(source, path); + if (includeUri == null) { + // Return the existing options, unchanged. + return options; + } + return merge(options, getOptionsFromSource(includeUri)); + }); + options = merge(includeOptions, options); } return options; } on OptionsFormatException {
diff --git a/pkg/analyzer/lib/src/task/options.dart b/pkg/analyzer/lib/src/task/options.dart index 3053a21..526daef 100644 --- a/pkg/analyzer/lib/src/task/options.dart +++ b/pkg/analyzer/lib/src/task/options.dart
@@ -69,7 +69,7 @@ } } - // Validate the specified options and any included option files. + // Validates the specified options and any included option files. void validate(Source source, YamlMap options, LintRuleProvider? provider) { var sourceIsOptionsForContextRoot = initialIncludeSpan == null; var validationErrors = OptionsFileValidator( @@ -90,85 +90,99 @@ sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot); return; } - var includeSpan = includeNode.span; - initialIncludeSpan ??= includeSpan; - String includeUri = includeSpan.text; - var includedSource = sourceFactory.resolveUri(source, includeUri); - if (includedSource == initialSource) { - errors.add( - AnalysisError.tmp( - source: initialSource, - offset: initialIncludeSpan!.start.offset, - length: initialIncludeSpan!.length, - errorCode: AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, - arguments: [includeUri, source.fullName], - ), - ); - return; - } - if (includedSource == null || !includedSource.exists()) { - errors.add( - AnalysisError.tmp( - source: initialSource, - offset: initialIncludeSpan!.start.offset, - length: initialIncludeSpan!.length, - errorCode: AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND, - arguments: [includeUri, source.fullName, contextRoot], - ), - ); - return; - } - var spanInChain = includeChain[includedSource]; - if (spanInChain != null) { - errors.add( - AnalysisError.tmp( - source: initialSource, - offset: initialIncludeSpan!.start.offset, - length: initialIncludeSpan!.length, - errorCode: AnalysisOptionsWarningCode.INCLUDED_FILE_WARNING, - arguments: [ - includedSource, - spanInChain.start.offset, - spanInChain.length, - 'The file includes itself recursively.', - ], - ), - ); - return; - } - includeChain[includedSource] = includeSpan; - try { - var includedOptions = - optionsProvider.getOptionsFromString(includedSource.contents.data); - validate(includedSource, includedOptions, provider); - 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, - e.span!.start.offset.toString(), - e.span!.end.offset.toString(), - e.message, - ]; - // Report errors for included option files on the `include` directive - // located in the initial options file. - errors.add( - AnalysisError.tmp( - source: initialSource, - offset: initialIncludeSpan!.start.offset, - length: initialIncludeSpan!.length, - errorCode: AnalysisOptionsErrorCode.INCLUDED_FILE_PARSE_ERROR, - arguments: args, - ), - ); + void validateInclude(YamlNode includeNode) { + var includeSpan = includeNode.span; + initialIncludeSpan ??= includeSpan; + var includeUri = includeSpan.text; + + var includedSource = sourceFactory.resolveUri(source, includeUri); + if (includedSource == initialSource) { + errors.add( + AnalysisError.tmp( + source: initialSource, + offset: initialIncludeSpan!.start.offset, + length: initialIncludeSpan!.length, + errorCode: AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, + arguments: [includeUri, source.fullName], + ), + ); + return; + } + if (includedSource == null || !includedSource.exists()) { + errors.add( + AnalysisError.tmp( + source: initialSource, + offset: initialIncludeSpan!.start.offset, + length: initialIncludeSpan!.length, + errorCode: AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND, + arguments: [includeUri, source.fullName, contextRoot], + ), + ); + return; + } + var spanInChain = includeChain[includedSource]; + if (spanInChain != null) { + errors.add( + AnalysisError.tmp( + source: initialSource, + offset: initialIncludeSpan!.start.offset, + length: initialIncludeSpan!.length, + errorCode: AnalysisOptionsWarningCode.INCLUDED_FILE_WARNING, + arguments: [ + includedSource, + spanInChain.start.offset, + spanInChain.length, + 'The file includes itself recursively.', + ], + ), + ); + return; + } + includeChain[includedSource] = includeSpan; + + try { + var includedOptions = + optionsProvider.getOptionsFromString(includedSource.contents.data); + validate(includedSource, includedOptions, provider); + 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, + e.span!.start.offset.toString(), + e.span!.end.offset.toString(), + e.message, + ]; + // Report errors for included option files on the `include` directive + // located in the initial options file. + errors.add( + AnalysisError.tmp( + source: initialSource, + offset: initialIncludeSpan!.start.offset, + length: initialIncludeSpan!.length, + errorCode: AnalysisOptionsErrorCode.INCLUDED_FILE_PARSE_ERROR, + arguments: args, + ), + ); + } + } + + if (includeNode is YamlScalar) { + validateInclude(includeNode); + } else if (includeNode is YamlList) { + for (var includeValue in includeNode.nodes) { + if (includeValue is YamlScalar) { + validateInclude(includeValue); + } + } } }
diff --git a/pkg/analyzer/test/src/diagnostics/analysis_options/recursive_include_file_test.dart b/pkg/analyzer/test/src/diagnostics/analysis_options/recursive_include_file_test.dart index b77b855..da02b10 100644 --- a/pkg/analyzer/test/src/diagnostics/analysis_options/recursive_include_file_test.dart +++ b/pkg/analyzer/test/src/diagnostics/analysis_options/recursive_include_file_test.dart
@@ -29,6 +29,21 @@ ]); } + void test_itself_inList() { + assertErrorsInCode(''' +include: + - analysis_options.yaml +''', [ + error( + AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, + 13, + 21, + text: "The include file 'analysis_options.yaml' " + "in '${convertPath('/analysis_options.yaml')}' includes itself recursively.", + ) + ]); + } + void test_recursive() { newFile('/a.yaml', ''' include: b.yaml @@ -68,6 +83,54 @@ ]); } + void test_recursive_listAtTop() { + newFile('/a.yaml', ''' +include: b.yaml +'''); + newFile('/b.yaml', ''' +include: analysis_options.yaml +'''); + newFile('/empty.yaml', ''' +'''); + assertErrorsInCode(''' +include: + - empty.yaml + - a.yaml +''', [ + error( + AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, + 13, + 10, + text: "The include file 'analysis_options.yaml' " + "in '${convertPath('/b.yaml')}' includes itself recursively.", + ) + ]); + } + + void test_recursive_listIncluded() { + newFile('/a.yaml', ''' +include: + - empty.yaml + - b.yaml +'''); + newFile('/b.yaml', ''' +include: analysis_options.yaml +'''); + newFile('/empty.yaml', ''' +'''); + assertErrorsInCode(''' +include: a.yaml +''', [ + error( + AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, + 9, + 6, + text: "The include file 'analysis_options.yaml' " + "in '${convertPath('/b.yaml')}' includes itself recursively.", + ) + ]); + } + void test_recursive_notInBeginning() { newFile('/a.yaml', ''' include: b.yaml
diff --git a/pkg/analyzer/test/src/options/options_provider_test.dart b/pkg/analyzer/test/src/options/options_provider_test.dart index 10adf1d..fc9de5c 100644 --- a/pkg/analyzer/test/src/options/options_provider_test.dart +++ b/pkg/analyzer/test/src/options/options_provider_test.dart
@@ -110,7 +110,7 @@ ); } - test_include_analyzerErrorSeveritiesAreMerged_multipleIncludes() { + test_include_analyzerErrorSeveritiesAreMerged_chainOfIncludes() { newFile('/first_options.yaml', ''' analyzer: errors: @@ -136,6 +136,34 @@ ); } + test_include_analyzerErrorSeveritiesAreMerged_multipleIncludes() { + newFile('/first_options.yaml', ''' +analyzer: + errors: + error_1: error +'''); + newFile('/second_options.yaml', ''' +analyzer: + errors: + error_2: warning +'''); + newFile(optionsFilePath, r''' +include: + - first_options.yaml + - second_options.yaml +'''); + + var options = _getOptionsObject('/'); + + expect( + options.errorProcessors, + unorderedMatches([ + ErrorProcessorMatcher(ErrorProcessor('error_1', ErrorSeverity.ERROR)), + ErrorProcessorMatcher(ErrorProcessor('error_2', ErrorSeverity.WARNING)), + ]), + ); + } + test_include_analyzerErrorSeveritiesAreMerged_outermostWins() { newFile('/other_options.yaml', ''' analyzer: @@ -163,6 +191,38 @@ ); } + test_include_analyzerErrorSeveritiesAreMerged_subsequentIncludeWins() { + newFile('/first_options.yaml', ''' +analyzer: + errors: + error_1: warning + error_2: warning +'''); + newFile('/second_options.yaml', ''' +analyzer: + errors: + error_1: ignore + error_2: warning +'''); + newFile(optionsFilePath, r''' +include: + - first_options.yaml + - second_options.yaml +'''); + + var options = _getOptionsObject('/'); + + expect( + options.errorProcessors, + unorderedMatches([ + // We want to explicitly state the expected severity. + // ignore: avoid_redundant_argument_values + ErrorProcessorMatcher(ErrorProcessor('error_1', null)), + ErrorProcessorMatcher(ErrorProcessor('error_2', ErrorSeverity.WARNING)), + ]), + ); + } + test_include_analyzerExcludeListsAreMerged() { newFile('/other_options.yaml', ''' analyzer: