Started to add rationales

[mildm8nnered]

Addresses #4811

The idea is to add a longer explanation of the purpose or motivation behind each rule.

You can now see the output via swiftlint rules rule_name

For example

% swiftlint.debug rules array_init
Array Init (array_init): Prefer using `Array(seq)` over `seq.map { $0 }` to convert a sequence into an Array

Rationale:

When converting the elements of sequence directly into an `Array`, for clarity, prefer using the `Array` constructor over calling `map`. For example

    Array(foo)

rather than

    foo.↓map({ $0 })

If some processing of the elements is required, then using `map` is fine. For example

    foo.map { !$0 }

Constructs like

    enum MyError: Error {}
    let myResult: Result<String, MyError> = .success("")
    let result: Result<Any, MyError> = myResult.map { $0 }

may be picked up as false positives by the `array_init` rule. If your codebase contains constructs like this, consider using the `typesafe_array_init` analyzer rule instead.

And when rendered via Jazzy:

[SwiftLintBot]

	2 Warnings
⚠️	If this is a user-facing change, please include a CHANGELOG entry to credit yourself! You can find it at CHANGELOG.md.
⚠️	This PR may need tests.

	17 Messages
📖	Linting Aerial with this PR took 0.93s vs 0.92s on main (1% slower)
📖	Linting Alamofire with this PR took 1.26s vs 1.27s on main (0% faster)
📖	Linting Brave with this PR took 7.21s vs 7.22s on main (0% faster)
📖	Linting DuckDuckGo with this PR took 5.11s vs 5.09s on main (0% slower)
📖	Linting Firefox with this PR took 10.58s vs 10.6s on main (0% faster)
📖	Linting Kickstarter with this PR took 9.9s vs 9.88s on main (0% slower)
📖	Linting Moya with this PR took 0.53s vs 0.53s on main (0% slower)
📖	Linting NetNewsWire with this PR took 2.71s vs 2.69s on main (0% slower)
📖	Linting Nimble with this PR took 0.78s vs 0.78s on main (0% slower)
📖	Linting PocketCasts with this PR took 8.66s vs 8.63s on main (0% slower)
📖	Linting Quick with this PR took 0.44s vs 0.45s on main (2% faster)
📖	Linting Realm with this PR took 4.51s vs 4.51s on main (0% slower)
📖	Linting Sourcery with this PR took 2.32s vs 2.29s on main (1% slower)
📖	Linting Swift with this PR took 4.5s vs 4.47s on main (0% slower)
📖	Linting VLC with this PR took 1.25s vs 1.26s on main (0% faster)
📖	Linting Wire with this PR took 17.72s vs 17.44s on main (1% slower)
📖	Linting WordPress with this PR took 11.4s vs 11.33s on main (0% slower)

Here's an example of your CHANGELOG entry:

* Started to add rationales.  
  [mildm8nnered](https://github.com/mildm8nnered)
  [#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)

note: There are two invisible spaces after the entry's text.

Generated by 🚫 Danger

[SimplyDanny]

That's an ambitious plan with 238 existing rules. 😅 But I very much appreciate the effort. This definitely something helpful and can be extended over time with the infrastructure as a starting point.

A few thoughts from my side:

• There are currently one description and sometimes additional more specific reasons per rule. Now, we would add another form of documentation with the rational. I wonder if we should rather extend the description and introduce something like a "default reason" or "violation reason".
• Something similar has been on my ToDo list for rule configuration options for quite a while.
• I think that the text should be entirely Markdown.

[mildm8nnered]

That's an ambitious plan with 238 existing rules. 😅 But I very much appreciate the effort. This definitely something helpful and can be extended over time with the infrastructure as a starting point.

238 is a lot yes, but I think definitely achievable. I've had a lot of fun writing the ones so far, and it's the kind of thing that you can knock some off on a rainy afternoon. Keep going, and before you know it you're done.

I think one think to talk about is "voice" - we could go for something very dry and technical, or somewhat more lighthearted, or even something a bit zany.

I think my take is that some kind of middle ground is good, but it would be nice to have some "character" to the rationale's, and to let them be a little bit opionated in places ("almost no-one enables this rule").

A few thoughts from my side:

• There are currently one description and sometimes additional more specific reasons per rule. Now, we would add another form of documentation with the rational. I wonder if we should rather extend the description and introduce something like a "default reason" or "violation reason".

So I think description has a role, and it's short and widely used.

I kind of see rationale as more of a freeform piece of text, that might mention some original source of why X is bad/good, and/or also maybe give some tips about using the rule, or about configuration options that you might want to use, or that people commonly set.

"Why should I care about this rule/how should I get the most out of it", together with a bit more of a narrative explanation of what it actually is - sometimes the examples can be a little obtuse.

• Something similar has been on my ToDo list for rule configuration options for quite a while.

Yep - I think it would be great to have something more formal for config options.

It's kind of a pain, because we probably have a lot, and someone has to write the text (see rationale's), but we could definitely do a better job of explaining what individual options do.

I think mentions of config options in rationale's are of the kind - "this option is one that you might want to use" rather than documenting the config options per se.

• I think that the text should be entirely Markdown.

My kind of tacky solution was

1. needs to look good in swiftlint rules rule_name output (would be so nice if that picked up the system pager, if set)
2. needs to look good in Jazzy output

So strip "```" for the Terminal and add swift for Jazzy, and make sure you get your indentation right for the Terminal yourself.

I think the Terminal case is harder.

Adding swift to "```" I kind of saw a service to rationale authors - it's probably swift code, and I know we'll forget to add those manually.

[SimplyDanny]

That's an ambitious plan with 238 existing rules. 😅 But I very much appreciate the effort. This definitely something helpful and can be extended over time with the infrastructure as a starting point.

238 is a lot yes, but I think definitely achievable. I've had a lot of fun writing the ones so far, and it's the kind of thing that you can knock some off on a rainy afternoon. Keep going, and before you know it you're done.

Oh, perfect. Happy to learn you like writing documentation. 😉

I think one think to talk about is "voice" - we could go for something very dry and technical, or somewhat more lighthearted, or even something a bit zany.

I think my take is that some kind of middle ground is good, but it would be nice to have some "character" to the rationale's, and to let them be a little bit opionated in places ("almost no-one enables this rule").

I think all these styles are fine. And maybe even a mixture makes them interesting for readers. One consistent style throughout the whole set of rules is anyway almost impossible.

• I think that the text should be entirely Markdown.

My kind of tacky solution was

1. needs to look good in swiftlint rules rule_name output (would be so nice if that picked up the system pager, if set)
2. needs to look good in Jazzy output

So strip "```" for the Terminal and add swift for Jazzy, and make sure you get your indentation right for the Terminal yourself.

I think the Terminal case is harder.

The thing is that without using Markdown, there is no good way to use different text styles and add links to other rules or further references. For the terminal output, all the styling modifiers could be removed afterwards.

Adding swift to "```" I kind of saw a service to rationale authors - it's probably swift code, and I know we'll forget to add those manually.

Important is that it allows to choose other languages as well.

[mildm8nnered]

That's an ambitious plan with 238 existing rules. 😅 But I very much appreciate the effort. This definitely something helpful and can be extended over time with the infrastructure as a starting point.

238 is a lot yes, but I think definitely achievable. I've had a lot of fun writing the ones so far, and it's the kind of thing that you can knock some off on a rainy afternoon. Keep going, and before you know it you're done.

Oh, perfect. Happy to learn you like writing documentation. 😉

:-)

I think one think to talk about is "voice" - we could go for something very dry and technical, or somewhat more lighthearted, or even something a bit zany.
I think my take is that some kind of middle ground is good, but it would be nice to have some "character" to the rationale's, and to let them be a little bit opionated in places ("almost no-one enables this rule").

I think all these styles are fine. And maybe even a mixture makes them interesting for readers. One consistent style throughout the whole set of rules is anyway almost impossible.

I hadn't thought of it like that, but actually I think that is absolutely the best approach. Nothing too prescriptive, and then we can catch anything that goes too far off-piste, or lacks clarity in individual CR.

I think comprehensibility to English as a second language speakers is important, so probably we wouldn't want rationale's in Yorkshire Dialect or Runyonese, sadly.

• I think that the text should be entirely Markdown.

My kind of tacky solution was

1. needs to look good in swiftlint rules rule_name output (would be so nice if that picked up the system pager, if set)
2. needs to look good in Jazzy output

So strip "```" for the Terminal and add swift for Jazzy, and make sure you get your indentation right for the Terminal yourself.
I think the Terminal case is harder.

The thing is that without using Markdown, there is no good way to use different text styles and add links to other rules or further references. For the terminal output, all the styling modifiers could be removed afterwards.

Any tips for how you think we should go about the styling modifiers stripping? Not entirely keen on having a markdown parser just so that I can de-markdown it :-)

Adding swift to "```" I kind of saw a service to rationale authors - it's probably swift code, and I know we'll forget to add those manually.

Important is that it allows to choose other languages as well.

So the intention is to ignore any language code that's present, as an override.

[SimplyDanny]

I hadn't thought of it like that, but actually I think that is absolutely the best approach. Nothing too prescriptive, and then we can catch anything that goes too far off-piste, or lacks clarity in individual CR.

I think comprehensibility to English as a second language speakers is important, so probably we wouldn't want rationale's in Yorkshire Dialect or Runyonese, sadly.

Fully support that, especially the last part! 🫣

The thing is that without using Markdown, there is no good way to use different text styles and add links to other rules or further references. For the terminal output, all the styling modifiers could be removed afterwards.

Any tips for how you think we should go about the styling modifiers stripping? Not entirely keen on having a markdown parser just so that I can de-markdown it :-)

Well, there is swiftlang/swift-markdown from Apple which I had been playing around with a little at that time. A new dependency should be well thought of. As these rationales are going to provide such a huge benefit, that would be justified though I guess. Apart from removing any Markdown styling, it would also allow SwiftLint to check that the whole text is actually valid Markdown before rendering the docs in the build.

On the other hand, even unrendered Markdown is so well readable, that I wouldn't complain if we just print the text as is into the console (keeping also URLs). There are also console rendering tools available that could be used to prettify the output. That, however, oughtn't be SwiftLint's job then.

Adding swift to "```" I kind of saw a service to rationale authors - it's probably swift code, and I know we'll forget to add those manually.

Important is that it allows to choose other languages as well.

So the intention is to ignore any language code that's present, as an override.

👍

[mildm8nnered]

Are these CI failures something that I'm doing @SimplyDanny ? - https://buildkite.com/swiftlint/swiftlint/builds/8753 - they all seem to be concurrency related, but it feels like they come and go at random

[SimplyDanny]

Are these CI failures something that I'm doing @SimplyDanny ? - https://buildkite.com/swiftlint/swiftlint/builds/8753 - they all seem to be concurrency related, but it feels like they come and go at random

They fail randomly. It's annoying. A retrigger normally helps. I hope this gets fixed by having strict concurrency checking always on by default. But until then there's still a long way to go.

Optionally, we can disable warnings reported as errors in Bazel builds for the time being.

[mildm8nnered]

Are these CI failures something that I'm doing @SimplyDanny ? - https://buildkite.com/swiftlint/swiftlint/builds/8753 - they all seem to be concurrency related, but it feels like they come and go at random

They fail randomly. It's annoying. A retrigger normally helps. I hope this gets fixed by having strict concurrency checking always on by default. But until then there's still a long way to go.

Optionally, we can disable warnings reported as errors in Bazel builds for the time being.

It feel like mine always fail :-) Apart from pushing an empty commit, is there a better way to retrigger them?

[SimplyDanny]

Are these CI failures something that I'm doing @SimplyDanny ? - https://buildkite.com/swiftlint/swiftlint/builds/8753 - they all seem to be concurrency related, but it feels like they come and go at random

They fail randomly. It's annoying. A retrigger normally helps. I hope this gets fixed by having strict concurrency checking always on by default. But until then there's still a long way to go.
Optionally, we can disable warnings reported as errors in Bazel builds for the time being.

It feel like mine always fail :-) Apart from pushing an empty commit, is there a better way to retrigger them?

This should be much more stable now. I haven't seen the random failures since the update to Swift 6 and a few code adaptions.

[SimplyDanny]

We can also merge this piece by piece. Every new rational improves the rule. No need to wait for all rules being updated.

[SimplyDanny]

I recommend not mentioning options in the rationale and rather plan to add dedicated rationals to the configuration elements.

[SimplyDanny]

You could make use of quotes here like so:

Erica Sadun says:

> My name is Erica.

[SimplyDanny]

Can we make this duplicated piece reusable somehow?

[mildm8nnered]

I'm going to pick this up again soon.

I just tried Copilot for the first time in a while ... quite interesting

[mildm8nnered]

This one took a bit more prompting - initially it just talked in generic terms about modern hash methods.