Started to add rationales (#5681)

CHANGELOG.md

@@ -21,6 +21,11 @@
   function parameter can be replaced with an opaque `some` type.  
   [SimplyDanny](https://github.com/SimplyDanny)
 
+* Add a new rationale property to rule descriptions, providing a more expansive
+  description of the motivation behind each rule.  
+  [Martin Redington](https://github.com/mildm8nnered)
+  [#5681](https://github.com/realm/SwiftLint/issues/5681)
+
 ### Bug Fixes
 
 * Fix issue referencing the Tests package from another Bazel workspace.  

Source/SwiftLintBuiltInRules/Rules/Idiomatic/AnonymousArgumentInMultilineClosureRule.swift

@@ -8,6 +8,23 @@ struct AnonymousArgumentInMultilineClosureRule: Rule {
         identifier: "anonymous_argument_in_multiline_closure",
         name: "Anonymous Argument in Multiline Closure",
         description: "Use named arguments in multiline closures",
+        rationale: """
+        In multiline closures, for clarity, prefer using named arguments
+
+        ```
+        closure { arg in
+            print(arg)
+        }
+        ```
+
+        to anonymous arguments
+
+        ```
+        closure {
+            print(↓$0)
+        }
+        ```
+        """,
         kind: .idiomatic,
         nonTriggeringExamples: [
             Example("closure { $0 }"),

Source/SwiftLintBuiltInRules/Rules/Lint/AccessibilityLabelForImageRule.swift

@@ -1,14 +1,5 @@
 import SourceKittenFramework
 
-/// In UIKit, a `UIImageView` was by default not an accessibility element, and would only be visible to VoiceOver
-/// and other assistive technologies if the developer explicitly made them an accessibility element. In SwiftUI,
-/// however, an `Image` is an accessibility element by default. If the developer does not explicitly hide them from
-/// accessibility or give them an accessibility label, they will inherit the name of the image file, which often creates
-/// a poor experience when VoiceOver reads things like "close icon white".
-///
-/// Known false negatives for Images declared as instance variables and containers that provide a label but are
-/// not accessibility elements. Known false positives for Images created in a separate function from where they
-/// have accessibility properties applied.
 struct AccessibilityLabelForImageRule: ASTRule, OptInRule {
     var configuration = SeverityConfiguration<Self>(.warning)
 
@@ -17,6 +8,17 @@ struct AccessibilityLabelForImageRule: ASTRule, OptInRule {
         name: "Accessibility Label for Image",
         description: "Images that provide context should have an accessibility label or should be explicitly hidden " +
                      "from accessibility",
+        rationale: """
+        In UIKit, a `UIImageView` was by default not an accessibility element, and would only be visible to VoiceOver \
+        and other assistive technologies if the developer explicitly made them an accessibility element. In SwiftUI, \
+        however, an `Image` is an accessibility element by default. If the developer does not explicitly hide them \
+        from accessibility or give them an accessibility label, they will inherit the name of the image file, which \
+        often creates a poor experience when VoiceOver reads things like "close icon white".
+
+        Known false negatives for Images declared as instance variables and containers that provide a label but are \
+        not accessibility elements. Known false positives for Images created in a separate function from where they \
+        have accessibility properties applied.
+        """,
         kind: .lint,
         minSwiftVersion: .fiveDotOne,
         nonTriggeringExamples: AccessibilityLabelForImageRuleExamples.nonTriggeringExamples,

Source/SwiftLintBuiltInRules/Rules/Lint/AccessibilityTraitForButtonRule.swift

@@ -1,12 +1,5 @@
 import SourceKittenFramework
 
-/// The accessibility button and link traits are used to tell assistive technologies that an element is tappable. When
-/// an element has one of these traits, VoiceOver will automatically read "button" or "link" after the element's label
-/// to let the user know that they can activate it. When using a UIKit `UIButton` or SwiftUI `Button` or
-/// `Link`, the button trait is added by default, but when you manually add a tap gesture recognizer to an
-/// element, you need to explicitly add the button or link trait. In most cases the button trait should be used, but for
-/// buttons that open a URL in an external browser we use the link trait instead. This rule attempts to catch uses of
-/// the SwiftUI `.onTapGesture` modifier where the `.isButton` or `.isLink` trait is not explicitly applied.
 struct AccessibilityTraitForButtonRule: ASTRule, OptInRule {
     var configuration = SeverityConfiguration<Self>(.warning)
 
@@ -15,6 +8,18 @@ struct AccessibilityTraitForButtonRule: ASTRule, OptInRule {
         name: "Accessibility Trait for Button",
         description: "All views with tap gestures added should include the .isButton or the .isLink accessibility " +
                      "traits",
+        rationale: """
+        The accessibility button and link traits are used to tell assistive technologies that an element is tappable. \
+        When an element has one of these traits, VoiceOver will automatically read "button" or "link" after the \
+        element's label to let the user know that they can activate it.
+
+        When using a UIKit `UIButton` or SwiftUI `Button` or `Link`, the button trait is added by default, but when \
+        you manually add a tap gesture recognizer to an element, you need to explicitly add the button or link trait. \
+
+        In most cases the button trait should be used, but for buttons that open a URL in an external browser we use \
+        the link trait instead. This rule attempts to catch uses of the SwiftUI `.onTapGesture` modifier where the \
+        `.isButton` or `.isLink` trait is not explicitly applied.
+        """,
         kind: .lint,
         minSwiftVersion: .fiveDotOne,
         nonTriggeringExamples: AccessibilityTraitForButtonRuleExamples.nonTriggeringExamples,

Source/SwiftLintBuiltInRules/Rules/Lint/ArrayInitRule.swift

@@ -8,6 +8,37 @@ struct ArrayInitRule: Rule, @unchecked Sendable {
         identifier: "array_init",
         name: "Array Init",
         description: "Prefer using `Array(seq)` over `seq.map { $0 }` to convert a sequence into an Array",
+        rationale: """
+        When converting the elements of a 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.
+        """,
         kind: .lint,
         nonTriggeringExamples: [
             Example("Array(foo)"),

Source/SwiftLintBuiltInRules/Rules/Lint/BalancedXCTestLifecycleRule.swift

@@ -8,6 +8,16 @@ struct BalancedXCTestLifecycleRule: Rule {
         identifier: "balanced_xctest_lifecycle",
         name: "Balanced XCTest Life Cycle",
         description: "Test classes must implement balanced setUp and tearDown methods",
+        rationale: """
+        The `setUp` method of `XCTestCase` can be used to set up variables and resources before \
+        each test is run (or with the `class` variant, before all tests are run).
+
+        This rule verifies that every class with an implementation of a `setUp` method also has \
+        a `tearDown` method (and vice versa).
+
+        The `tearDown` method should be used to cleanup or reset any resources that could \
+        otherwise have any effects on subsequent tests, and to free up any instance variables.
+        """,
         kind: .lint,
         nonTriggeringExamples: [
             Example(#"""

Source/SwiftLintBuiltInRules/Rules/Lint/BlanketDisableCommandRule.swift

@@ -9,6 +9,27 @@ struct BlanketDisableCommandRule: Rule, SourceKitFreeRule {
                      single line, or `swiftlint:enable` to re-enable the rules immediately after the violations \
                      to be ignored, instead of disabling the rule for the rest of the file.
                      """,
+        rationale: """
+        The intent of this rule is to prevent code like
+
+        ```
+        // swiftlint:disable force_unwrapping
+        let foo = bar!
+        ```
+
+        which disables the `force_unwrapping` rule for the remainder of the file, instead of just for the specific \
+        violation.
+
+        `next`, `this`, or `previous` can be used to restrict the disable command's scope to a single line, or it \
+        can be re-enabled after the violations.
+
+        To disable this rule in code you will need to do something like
+
+        ```
+        // swiftlint:disable:next blanket_disable_command
+        // swiftlint:disable force_unwrapping
+        ```
+        """,
         kind: .lint,
         nonTriggeringExamples: [
             Example("""

Source/SwiftLintBuiltInRules/Rules/Lint/ClassDelegateProtocolRule.swift

@@ -8,6 +8,24 @@ struct ClassDelegateProtocolRule: Rule {
         identifier: "class_delegate_protocol",
         name: "Class Delegate Protocol",
         description: "Delegate protocols should be class-only so they can be weakly referenced",
+        rationale: """
+        Delegate protocols are usually `weak` to avoid retain cycles, or bad references to deallocated delegates.
+
+        The `weak` operator is only supported for classes, and so this rule enforces that protocols ending in \
+        "Delegate" are class based.
+
+        For example
+
+        ```
+        protocol FooDelegate: class {}
+        ```
+
+        versus
+
+        ```
+        ↓protocol FooDelegate {}
+        ```
+        """,
         kind: .lint,
         nonTriggeringExamples: [
             Example("protocol FooDelegate: class {}"),

Source/SwiftLintBuiltInRules/Rules/Metrics/ClosureBodyLengthRule.swift

@@ -1,10 +1,16 @@
 struct ClosureBodyLengthRule: OptInRule, SwiftSyntaxRule {
-    var configuration = SeverityLevelsConfiguration<Self>(warning: 30, error: 100)
+    private static let defaultWarningThreshold = 30
+    var configuration = SeverityLevelsConfiguration<Self>(warning: Self.defaultWarningThreshold, error: 100)
 
     static let description = RuleDescription(
         identifier: "closure_body_length",
         name: "Closure Body Length",
         description: "Closure bodies should not span too many lines",
+        rationale: """
+        "Closure bodies should not span too many lines" says it all.
+
+        Possibly you could refactor your closure code and extract some of it into a function.
+        """,
         kind: .metrics,
         nonTriggeringExamples: ClosureBodyLengthRuleExamples.nonTriggeringExamples,
         triggeringExamples: ClosureBodyLengthRuleExamples.triggeringExamples

Source/SwiftLintBuiltInRules/Rules/Style/AttributesRule.swift

@@ -11,6 +11,22 @@ struct AttributesRule: Rule {
             Attributes should be on their own lines in functions and types, but on the same line as variables and \
             imports
             """,
+        rationale: """
+        Erica Sadun says:
+
+        > My take on things after the poll and after talking directly with a number of \
+        developers is this: Placing attributes like `@objc`, `@testable`, `@available`, `@discardableResult` on \
+        their own lines before a member declaration has become a conventional Swift style.
+
+        > This approach limits declaration length. It allows a member to float below its attribute and supports \
+        flush-left access modifiers, so `internal`, `public`, etc appear in the leftmost column. Many developers \
+        mix-and-match styles for short Swift attributes like `@objc`
+
+        See https://ericasadun.com/2016/10/02/quick-style-survey/ for discussion.
+
+        SwiftLint's rule requires attributes to be on their own lines for functions and types, but on the same line \
+        for variables and imports.
+        """,
         kind: .style,
         nonTriggeringExamples: AttributesRuleExamples.nonTriggeringExamples,
         triggeringExamples: AttributesRuleExamples.triggeringExamples

Source/SwiftLintCore/Models/RuleDescription.swift

@@ -11,6 +11,12 @@ public struct RuleDescription: Equatable, Sendable {
     /// explanation of the rule's purpose and rationale.
     public let description: String
 
+    /// A longer explanation of the rule's purpose and rationale. Typically defined as a multiline string, long text 
+    /// lines should be wrapped. Markdown formatting is supported. Multiline code blocks will be formatted as
+    /// `swift` code unless otherwise specified, and will automatically be indented by four spaces when printed
+    /// to the console.
+    public let rationale: String?
+
     /// The `RuleKind` that best categorizes this rule.
     public let kind: RuleKind
 
@@ -54,6 +60,16 @@ public struct RuleDescription: Equatable, Sendable {
     /// The console-printable string for this description.
     public var consoleDescription: String { "\(name) (\(identifier)): \(description)" }
 
+    /// The console-printable rationale for this description.
+    public var consoleRationale: String? {
+        rationale?.consoleRationale
+    }
+
+    /// The rationale for this description, with Markdown formatting.
+    public var formattedRationale: String? {
+        rationale?.formattedRationale
+    }
+
     /// All identifiers that have been used to uniquely identify this rule in past and current SwiftLint versions.
     public var allIdentifiers: [String] {
         Array(deprecatedAliases) + [identifier]
@@ -74,6 +90,7 @@ public struct RuleDescription: Equatable, Sendable {
     public init(identifier: String,
                 name: String,
                 description: String,
+                rationale: String? = nil,
                 kind: RuleKind,
                 minSwiftVersion: SwiftVersion = .five,
                 nonTriggeringExamples: [Example] = [],
@@ -84,6 +101,7 @@ public struct RuleDescription: Equatable, Sendable {
         self.identifier = identifier
         self.name = name
         self.description = description
+        self.rationale = rationale
         self.kind = kind
         self.nonTriggeringExamples = nonTriggeringExamples
         self.triggeringExamples = triggeringExamples
@@ -99,3 +117,30 @@ public struct RuleDescription: Equatable, Sendable {
         lhs.identifier == rhs.identifier
     }
 }
+
+private extension String {
+    var formattedRationale: String {
+        formattedRationale(forConsole: false)
+    }
+
+    var consoleRationale: String {
+        formattedRationale(forConsole: true)
+    }
+
+    private func formattedRationale(forConsole: Bool) -> String {
+        var insideMultilineString = false
+        return components(separatedBy: "\n").compactMap { line -> String? in
+            if line.contains("```") {
+                if insideMultilineString {
+                    insideMultilineString = false
+                    return forConsole ? nil : line
+                }
+                insideMultilineString = true
+                if line.hasSuffix("```") {
+                    return forConsole ? nil : (line + "swift")
+                }
+            }
+            return line.indent(by: (insideMultilineString && forConsole) ? 4 : 0)
+        }.joined(separator: "\n")
+    }
+}

Source/SwiftLintFramework/Documentation/RuleDocumentation.swift

@@ -40,6 +40,10 @@ struct RuleDocumentation {
     var fileContents: String {
         let description = ruleType.description
         var content = [h1(description.name), description.description, detailsSummary(ruleType.init())]
+        if let formattedRationale = description.formattedRationale {
+            content += [h2("Rationale")]
+            content.append(formattedRationale)
+        }
         let nonTriggeringExamples = description.nonTriggeringExamples.filter { !$0.excludeFromDocumentation }
         if nonTriggeringExamples.isNotEmpty {
             content += [h2("Non Triggering Examples")]

Source/swiftlint/Commands/Rules.swift

@@ -58,6 +58,9 @@ extension SwiftLint {
             }
 
             print("\(description.consoleDescription)")
+            if let consoleRationale = description.consoleRationale {
+                print("\nRationale:\n\n\(consoleRationale)")
+            }
             let configDescription = rule.createConfigurationDescription()
             if configDescription.hasContent {
                 print("\nConfiguration (YAML):\n")