Flag to add Highest and Lowest Level Config Files (in Merge Tree)

[SDGGiesbrecht]

This takes over from @Aranoledur’s side track in #1687. Since this requires merging of nested configurations, this would be a future, independent enhancement after that is completed.

This is also the resurrection of @jpsim’s suggestion in #1323, now that the groundwork (merging, etc.) that was missing at the time is nearly finished.

---

I propose adding two flags that insert config files into the merge tree at the very top and very bottom.

Tentative flag names could be:

• --base-config: The file provided here would be treated as though it were parent to the .swiftlint.yml file in the project’s root directory.
• --override-config: The file provided here would be treated as though it were child to each of the deepest nested .swiftlint.yml files. (This one needs a better name.)

Edit: Here is a diagram of the merge tree:

--base-config     ↳ /.swiftlint.yml         ↳ --override-config

        ↳ /A/.swiftlint.yml             ↳ --override-config

            ↳ /A/C/.swiftlint.yml                 ↳ --override-config

        ↳ /B/.swiftlint.yml             ↳ --override-config

            ↳ /B/D/.swiftlint.yml                 ↳ --override-config

••••••• Edit •••••••

I will maintain an centralized list here of additional related flags proposed in later comments.

• --no-merged-configurations: Disables all merging (completely ignoring nested configurations).

••••••• End Edit •••••••

(I don’t know the current semantics of --config. If useful, it could stick around to mean “use this configuration file and ignore all .swiftlint.yml files.”)

Example Usage

--base-config

Workspace recognizes when it is dealing with a Swift Package Manger project, and instructs SwiftLint to ignore dependencies out of the user’s control. As this currently trumps the user’s customization, the user must turn this automation off and figure it out themselves (#1637) if they want to customize SwiftLint. With --base-config, Workspace could simply use that and the user can still customize to their heart’s content (and even override individual pieces of Workspace’s default) using their own .swiftlint.yml.

--override-config

Let’s say you have a tree of nested config files, and you have a rule active at the root but disabled in several dozen subfolders. Now you want to see how much work it would be to make the disabled folders comply as well. As it stands, you have to remove the disabled rule from all of the dozen nested configs. With --override-config, you could simply enable it there, run SwiftLint, see the nightmare it would be to comply, and give up. That’s only one action, instead of thirteen (12 × enable + 1 revert).

---

Anything to add, @Aranoledur?

[Aranoledur]

Thats awesome, thanks! I was thinking to open an issue, because as I see, many people need this feature. And your description of this feature is complete and clear. Also it would be great to have two flags for base and override. And we have to decide what--config really do after #1687 is completed.

[marcelofabri]

I haven't given a lot of thought into this yet, but my initial impression is that --base-config should be an option in a configuration (.swiftlint.yml) file and instead of --override-config we should have a --no-merged-configurations option or something like that.

[Aranoledur]

I agree with --no-merged-configurationsflag. But --base-config in configuration won't help resolve issue, because if project doesn't have .swiftlint.yml it won't know about base config. Idea is to make base config always do it's work plus ability to override some rules for specific project.

[Aranoledur]

I was thinking about implementation and discovered that ask Configuration about another configuration in Configuration.configuration(for file:) is confusing. Like you have one object and you ask it to create another object with it's own type. Maybe another abstraction needed. For example ConfigurationsTree that is created on start by searching all configurations in all subfolders, merging them correctly and has ability to give you configuration for file by it's path.

[SDGGiesbrecht]

@marcelofabri,

my initial impression is that --base-config should be an option in a configuration (.swiftlint.yml) file

Do you mean some sort of import statement? (I think there is a separate issue about that but I cannot find it now.)

If so, while I see its usefulness as well, it doesn’t help my usage case. The whole point is for an overarching tool to be able to control SwiftLint’s basic defaults without having to write into the user’s configuration files. Writing to a configuration file in the repository is precisely what I want to be able to avoid.

---

instead of --override-config we should have a --no-merged-configurations option or something like that

I see its usefulness too, so I added it to the list at the top. (But please see my question about it there.)

However, it is no help for the usage example above. By stopping all merging, it could cause a landslide of differences, not just the specific rule one is experimenting with.

[marcelofabri]

Do you mean some sort of import statement?

Yes

If so, while I see its usefulness as well, it doesn’t help my usage case.

I think we could support both, but I think the more important one is in the configuration file. If that is supported, you could always create workarounds (e.g. generating files yourself).

--no-merged-configurations: Disables all merging. (Would this ignore nested configurations, or would it be a compatibility mode, where nested configurations would be used as‐is without merging?)

IMO it would ignore nested configurations

By stopping all merging, it could cause a landslide of differences, not just the specific rule one is experimenting with.

Wouldn't that happen if you use --override-config as well?

[SDGGiesbrecht]

Wouldn't that happen if you use --override-config as well?

No... maybe it needs a better name.

This was the description:

The file provided here would be treated as though it were child to each of the deepest nested .swiftlint.yml files.

---

I think we could support both, but I think the more important one is in the configuration file.

I agree both should be supported. I also agree that import statements would affect more people. But it is a lot more work and thought to implement. --base-config can be implemented with only a line or two of code.

---

P.S. Unless you tell me not to, I am planning on doing the work of implementing these myself and submitting a pull request. There is no point in starting until merging (#1687) is finalized, but I figured I could get the evaluation of names and semantics started in the meantime.

[SDGGiesbrecht]

@marcelofabri, I noted your answer about --no-merged-configurations at the top. I also added a diagram to help explain --override-config. Can you think of a name that is clearer?

[marcelofabri]

@SDGGiesbrecht you're right, I misunderstood --override-config.

P.S. Unless you tell me not to, I am planning on doing the work of implementing these myself and submitting a pull request.

I think it's important for us to take some time and figure out the approach we want to go with to solve this family of problems, because they're all so related. If you want to go ahead and submit a PR, it'd be good to have a more concrete input for us to think, but I can't say for sure whether it'd be accepted or not. With that in mind, I'd ask you to do it only if it doesn't take so much effort.

I also want to hear @jpsim about this and related features/requests.

[SDGGiesbrecht]

@marcelofabri, agreed on all counts.

I thought more about the name --override-config:

I originally proposed the name thinking --(the-)overrid(ing)-config (is) <file>, but I realize the brain tends to parse it as --override-(the-)config(s) (with) <file>, which is not what it is supposed to do.

Is --config-overrides <file> more natural?

--final-child-config is even less ambiguous, but I am not sure it is very intuitive.

Other suggestions anyone?

[marcelofabri]

I think we're overcomplicating this. Rubocop seems to take a much more simpler (and predictable) approach:

• Always use the configuration in the same level as the file being lint'ed.
• If it's not there, check go to the parent directory until it reaches root. If no configuration was found, use a default one.
• You can use --config to explicitly use a configuration.
• Configurations can use a inherit_from key to explicitly inherit from another configuration (e.g. from a parent directory). Then, they're merged. (So no implicit inheritance.)
• It's possible to inherit from remote configurations as well, but I don't think it makes sense for us now.
• It doesn't look like the current directory is used in the process.
• All rules configurations support Include and Exclude keys that can be used to disable the rule in certain files. I think this is one of the most common motives to use nested configurations in SwiftLint today (e.g. "We don't care about using force casts in unit tests").

[SDGGiesbrecht]

Thoughts on Rubocop:

• Always use the configuration in the same level as the file being lint'ed.

SwiftLint does that. ✓

• If it's not there, check go to the parent directory until it reaches [the system] root. If no configuration was found, use a default one. • It doesn’t look like the current directory is used in the process.

Simple and logical to me. On the other hand, several issues have already been registered because SwiftLint was getting configurations from outside the project/working directory. So I am not sure this is what users actually generally want. It also means SwiftLint can spit out different results when a repository is cloned to a different machine or location.

• You can use --config to explicitly use a configuration.

Logical without automatic merging (i.e. for Rubocop). Unclear semantics with automatic merging.

• Configurations can use a[n] inherit_from key to explicitly inherit from another configuration. • It’s possible to inherit from remote configurations as well, but I don’t think it makes sense for us now.

Wonderful and robust—definitely a good idea for the future. Requiring explicitness for the most common usage case (parent directory) is, however, very annoying (and goes against recent merged changes).

---

Much of what is discussed here and provided in #1748 is just for the sake of thorough completeness.

My primary motive is to be able to have a script analyze the project and provide more‐sensitive defaults, without modifying or overriding the project’s own configuration. The inheritance is thus:

SwiftLint defaults ← scripted defaults ← project files.

(I do not even particularly care how the in‐project files interact among themselves.)

As far as I can tell, that is impossible with RuboCop. It is certainly impossible with SwiftLint right now, but config-defaults would enable this elegantly:

SwiftLint defaults ← --config-defaults ← project files.

Reasons why inherit_from would not help me:

1. It involves writing into a project file, probably one that is checked into source control.
2. The referenced path is hard‐coded. macOS and Linux have differing best‐practice temporary directories, so the checked, hard‐coded path has to fight between them.
3. If the referenced path is remote, then the file cannot be dynamically generated by the script.
4. The least‐annoying workaround would be to generate the file at a hard‐coded path inside the repository, but then it involves writing into .gitignore as well as .swiftlint.yml.

[SDGGiesbrecht]

Closing along with #1748 to remove clutter.