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]>
dart-lang · Oct 30, 2024 · 52001ff · 52001ff
1 parent 7d00c54
commit 52001ff
Show file tree

Hide file tree

Showing 4 changed files with 260 additions and 106 deletions.
diff --git 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 @@ import 'package:yaml/yaml.dart';
 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 @@ class AnalysisOptionsProvider {
     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 @@ class AnalysisOptionsProvider {
     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
@@ -69,7 +69,7 @@ List<AnalysisError> analyzeAnalysisOptions(
     }
   }
 
-  // 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 @@ List<AnalysisError> analyzeAnalysisOptions(
           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;
+
+    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,
+          ),
+        );
+      }
     }
-    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
@@ -29,6 +29,21 @@ include: analysis_options.yaml
     ]);
   }
 
+  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 @@ include: a.yaml
     ]);
   }
 
+  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