Skip to content

Started to add rationales #5681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
e26e9bf
Started to add rationales
mildm8nnered Jul 20, 2024
be6d035
Missing separator
mildm8nnered Jul 20, 2024
689548f
Tweaks
mildm8nnered Jul 20, 2024
c73993c
Tweaked line endings
mildm8nnered Jul 21, 2024
fee4130
Wording
mildm8nnered Jul 21, 2024
e407394
Attributes
mildm8nnered Jul 21, 2024
29cb7f7
Added rationale
mildm8nnered Jul 21, 2024
4959660
Added rationale to Blanket Disable Command
mildm8nnered Jul 21, 2024
13a661b
Added more rationale's
mildm8nnered Jul 21, 2024
ef1124e
Fixed typos
mildm8nnered Jul 22, 2024
8af298f
Documentation update
mildm8nnered Jul 26, 2024
e6bbd12
Documentation tweak
mildm8nnered Jul 26, 2024
8aef239
Better formatting
mildm8nnered Jul 27, 2024
8c8b562
Better text
mildm8nnered Jul 29, 2024
2ff1041
Fixed line lengths
mildm8nnered Aug 1, 2024
2f9ad42
Removed indentation
mildm8nnered Aug 1, 2024
74688af
indent markdown multiline code by 4 spaces for the console
mildm8nnered Aug 1, 2024
8c42cb2
Added more detail
mildm8nnered Aug 11, 2024
4810f88
Apply suggestions from code review
mildm8nnered Nov 5, 2024
03a5554
CR comments
mildm8nnered Nov 5, 2024
2adf0d3
line length fix
mildm8nnered Nov 18, 2024
b7f1bb6
Fixed some indentation and formatting
mildm8nnered Nov 30, 2024
b0162b7
Warning fix
mildm8nnered Dec 1, 2024
9be8d1f
Mention indentation in the comment.
mildm8nnered Dec 1, 2024
97324bd
Don't indent code at source
mildm8nnered Dec 1, 2024
0a34628
superfluous else
mildm8nnered Dec 1, 2024
18e4aa1
Improvements
mildm8nnered Dec 1, 2024
2676493
Fixed compilation
mildm8nnered Dec 28, 2024
d894e07
Fixed indentation
mildm8nnered Dec 30, 2024
e422a26
Removed mention of configuration options
mildm8nnered Jan 4, 2025
99cfddf
Added entry
mildm8nnered Jan 4, 2025
fe27c29
CR comments
mildm8nnered Jan 11, 2025
c889638
Fixed markdown
mildm8nnered Feb 2, 2025
a1feebe
Tweaked unbalanced rationale
mildm8nnered Feb 8, 2025
d7444fd
updated
mildm8nnered Feb 23, 2025
2697a23
fixed heading
mildm8nnered Mar 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }"),
Expand Down
Original file line number Diff line number Diff line change
@@ -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)

Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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)

Expand All @@ -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,
Expand Down
31 changes: 31 additions & 0 deletions Source/SwiftLintBuiltInRules/Rules/Lint/ArrayInitRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(#"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}"),
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
16 changes: 16 additions & 0 deletions Source/SwiftLintBuiltInRules/Rules/Style/AttributesRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions Source/SwiftLintCore/Models/RuleDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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]
Expand All @@ -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] = [],
Expand All @@ -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
Expand All @@ -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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
3 changes: 3 additions & 0 deletions Source/swiftlint/Commands/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down