Support multiple includes in analysis_options.yaml

[incendial]

According to the docs, we can include only one file into analysis_options.yaml and this might work fine with the current analyzer presets, but if we are talking about custom analyzer plugins that provide additional rules, it requires creating custom extend logic in order to provide presets for the users. It'd be more convenient for the users to include such presets instead.

What do you think about changing the include: entry to be multiline instead, like:

include: |
  package:lints/recommended.yaml
  package:dart_code_metrics/flutter_all.yaml

...

For example, eslint has multiple useful plugins and supports this use-case with the extends keyword.

[bwilkerson]

Sorry for the delay.

The primary reason we don't support more than one included file is because we haven't taken the time to define the semantics of doing so. Your use case is definitely valid, but I don't know whether we'd be able to allocate the time required to work out the semantics.

@jacob314 for visibility

[incendial]

Thank you for the details and yeah, I can totally understand, why the priority for this feature is low comparing to others. I guess, we'll start with the custom extend logic then.

[srawlins]

I'd like to offer a design as I think it's actually not terribly complicated, and there may be some obvious choices.

I think there are only two questions to answer, if we allow multiple include files:

1. What is the order of the traversal? Or... is an order even guaranteed? (Again, I think there are obvious answers)
2. Who wins in merges?

The order of traversal

Let's take an example:

# my_monorepo/packages/package_1/analysis_options.yaml (A)
include: ../../common_analysis_options.yaml (B)
include: package:best_lint_rules/best.yaml (C)
linter:
  # ...

# my_monorepo/common_analysis_options.yaml (B)
include: package:lints/recommended.yaml (D)
analyzer:
  # ...
linter:
  # ...

# package:best_lint_rules/best.yaml (C)
linter:
  # Enabling some great lint rules

We have a few choices: we can treat the set of include directives as unordered, such that there is no guaranteed order. I think this only brings pain. Bad choice 😛. We can traverse the include directives depth-first. And we can traverse the include directives breadth-first.

I think the choice is fairly obvious here, because the alternative choice just doesn't make sense to me: we do depth-first. This is because each analysis options file specifies a set of options which necessarily includes any included options. So when package_1's options (A) include the "common" options (B), it is not just including the narrow set of options directly declared in (B), holding off on any includes found in (D). No. It is including the collective set of the options specified by (B) which include options specified in (D), as (D) is included in (B).

To do a breadth-first traversal would mean the following in this example: To compute the collective options specified in (A), we would first include options directly declared in (B), then the options directly declared in (C), then go back and add the options directly declared in (B)'s included options, namely (D). This seems just non-sensical, as we are mincing apart the included options files into directly declared options and included options, which is not how any given options file is meant to be treated.

Who wins in merges

We already have a merge strategy for includes, since you can have conflicts between two files where one includes the other! If (A) includes (B), then step 1 is to start with a base set of options from (B). Step 2 is to go over the options directly declared in (A), and change any values that came from (B).

(There may be some details in where the merge takes place, which is namely with scalar YAML nodes. For example, if (B) specifies a set of linter options, and (A) specifies a different set of linter options, we do not wholesale replace the linter options from (B) with those from (A). We instead start with the linter options from (B), and then walk the linter options from (A), and replace any scalar values as we find them.)

So the existing algorithm is to walk include directives deep, to start with a base set of options, and then update values with those from the including options files.

Let's keep rolling with the example from above. (A) directly includes (B) and (C), and also (B) includes (D). And it specifies some options of it's own. It seems to only make sense to expand this algorithm by considering sibling include directives in the same order that children include directives are considered. That is, the algorithm first looks at (A)'s includes, starting with (B), then looks at (B)'s includes which is (D), and (D) becomes the base set of options. Backing out, we update any options that were specified in (D) and overridden in (B). Backing out again, we look at the next include in (A), which is (C), and we update any options that were specified in (D+B), which are overridden in (C). Backing out again, we are out of includes in (A), so we update any options specified in (D+B+C), which are overridden in (A), to arrive at the collective set of options specified by (A) and (A)'s includes.

None of this is surprising or innovative. It is reminiscent of OO type hierarchy, Cascading Style Sheets, or Dart's spread operator.

Syntax

YAML does not allow us to write a key in a map multiple times, like:

include: a.yaml
include: b.yaml

So what syntax? I propose we just allow for an incremental bump from a scalar include value to a list:

include:
  - a.yaml
  - b.yaml

This is similar to how we support both lists and maps in the linter section (which has it's downsides, but there it is). It means that only one list of includes is supported, which seems fine. It means that you cannot intersperse directly declared options with includes, which seems fine. Something like:

include: a.yaml
linter:
  prefer_double_quotes: true
include: b.yaml

It's a nice simple design. I don't think I've seen requests for anything more complex.

[srawlins]

To round off suppot once the analyzer recognizes a list of includes, we'll have some polish to add:

• Document that multiple files can be included in analysis options, in analyzer 3.7 maybe site-www#6174

• ensure that autocomplete continues to work, if we have autocomplete for an included path.

  Edit: looks like we don't offer autocomplete 🤷

• Support determining the page_width given a list / lists of multiple included files dart_style#1591

[srawlins]

Core support has landed, but I'm leaving this open to track the above issues.