mirror of
https://github.com/realm/SwiftLint.git
synced 2026-05-07 20:12:49 +00:00
Fix indentation_width false positives for multi-line conditions (#6505)
Co-authored-by: Danny Mösch <danny.moesch@icloud.com>
This commit is contained in:
@@ -115,6 +115,14 @@
|
||||
[SimplyDanny](https://github.com/SimplyDanny)
|
||||
[#6080](https://github.com/realm/SwiftLint/issues/6080)
|
||||
|
||||
* Fix false positives in `indentation_width` rule for continuation lines
|
||||
of multi-line `guard`/`if`/`while` conditions. A new option
|
||||
`include_multiline_conditions` (default: `false`) skips these lines by
|
||||
default. When enabled, it validates that continuation lines are aligned
|
||||
with the first condition after the keyword.
|
||||
[tanaev](https://github.com/tanaev)
|
||||
[#4961](https://github.com/realm/SwiftLint/issues/4961)
|
||||
|
||||
* `multiline_call_arguments` no longer reports violations for enum-case patterns in
|
||||
pattern matching (e.g. if case, switch case, for case, catch).
|
||||
[GandaLF2006](https://github.com/GandaLF2006)
|
||||
|
||||
+2
@@ -22,4 +22,6 @@ struct IndentationWidthConfiguration: SeverityBasedRuleConfiguration {
|
||||
private(set) var includeCompilerDirectives = true
|
||||
@ConfigurationElement(key: "include_multiline_strings")
|
||||
private(set) var includeMultilineStrings = true
|
||||
@ConfigurationElement(key: "include_multiline_conditions")
|
||||
private(set) var includeMultilineConditions = false
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import Foundation
|
||||
import SourceKittenFramework
|
||||
import SwiftSyntax
|
||||
|
||||
@DisabledWithoutSourceKit
|
||||
struct IndentationWidthRule: OptInRule {
|
||||
@@ -16,6 +17,28 @@ struct IndentationWidthRule: OptInRule {
|
||||
}
|
||||
}
|
||||
|
||||
/// Parsed information about a line's leading whitespace.
|
||||
private struct IndentationPrefix {
|
||||
let tabCount: Int
|
||||
let spaceCount: Int
|
||||
|
||||
var combinedCount: Int { tabCount + spaceCount }
|
||||
|
||||
init(line: Line, length: Int) {
|
||||
var tabs = 0
|
||||
var spaces = 0
|
||||
for char in line.content.prefix(length) {
|
||||
if char == "\t" { tabs += 1 } else if char == " " { spaces += 1 }
|
||||
}
|
||||
self.tabCount = tabs
|
||||
self.spaceCount = spaces
|
||||
}
|
||||
|
||||
func spacesEquivalent(indentationWidth: Int) -> Int {
|
||||
spaceCount + tabCount * indentationWidth
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Properties
|
||||
var configuration = IndentationWidthConfiguration()
|
||||
|
||||
@@ -31,6 +54,31 @@ struct IndentationWidthRule: OptInRule {
|
||||
Example("firstLine\n\tsecondLine\n\t\tthirdLine\n\n\t\tfourthLine"),
|
||||
Example("firstLine\n\tsecondLine\n\t\tthirdLine\n\t//test\n\t\tfourthLine"),
|
||||
Example("firstLine\n secondLine\n thirdLine\nfourthLine"),
|
||||
Example("""
|
||||
guard let x = foo(),
|
||||
let y = bar() else {
|
||||
return
|
||||
}
|
||||
"""),
|
||||
Example("""
|
||||
if let x = foo(),
|
||||
let y = bar() {
|
||||
doSomething()
|
||||
}
|
||||
"""),
|
||||
Example("""
|
||||
while let x = foo(),
|
||||
let y = bar() {
|
||||
doSomething()
|
||||
}
|
||||
"""),
|
||||
Example("""
|
||||
if let x = foo(),
|
||||
let y = bar(),
|
||||
let z = baz() {
|
||||
doSomething()
|
||||
}
|
||||
"""),
|
||||
],
|
||||
triggeringExamples: [
|
||||
Example("↓ firstLine", testMultiByteOffsets: false, testDisableCommand: false),
|
||||
@@ -42,98 +90,52 @@ struct IndentationWidthRule: OptInRule {
|
||||
|
||||
// MARK: - Initializers
|
||||
// MARK: - Methods: Validation
|
||||
func validate(file: SwiftLintFile) -> [StyleViolation] { // swiftlint:disable:this function_body_length
|
||||
func validate(file: SwiftLintFile) -> [StyleViolation] {
|
||||
var violations: [StyleViolation] = []
|
||||
var previousLineIndentations: [Indentation] = []
|
||||
|
||||
let conditionContinuationInfo = multilineConditionInfo(in: file)
|
||||
|
||||
for line in file.lines {
|
||||
if ignoreCompilerDirective(line: line, in: file) { continue }
|
||||
|
||||
// Skip line if it's a whitespace-only line
|
||||
// Skip whitespace-only lines, comments, compiler directives, multiline strings
|
||||
let indentationCharacterCount = line.content.countOfLeadingCharacters(in: CharacterSet(charactersIn: " \t"))
|
||||
if line.content.count == indentationCharacterCount { continue }
|
||||
if shouldSkipLine(line: line, indentationCharacterCount: indentationCharacterCount, in: file) { continue }
|
||||
|
||||
if ignoreComment(line: line, in: file) || ignoreMultilineStrings(line: line, in: file) { continue }
|
||||
let prefix = IndentationPrefix(line: line, length: indentationCharacterCount)
|
||||
|
||||
// Get space and tab count in prefix
|
||||
let prefix = String(line.content.prefix(indentationCharacterCount))
|
||||
let tabCount = prefix.filter { $0 == "\t" }.count
|
||||
let spaceCount = prefix.filter { $0 == " " }.count
|
||||
|
||||
// Determine indentation
|
||||
let indentation: Indentation
|
||||
if tabCount != 0, spaceCount != 0 {
|
||||
// Catch mixed indentation
|
||||
violations.append(
|
||||
StyleViolation(
|
||||
ruleDescription: Self.description,
|
||||
severity: configuration.severityConfiguration.severity,
|
||||
location: Location(file: file, characterOffset: line.range.location),
|
||||
reason: "Code should be indented with tabs or " +
|
||||
"\(configuration.indentationWidth) spaces, but not both in the same line"
|
||||
)
|
||||
)
|
||||
|
||||
// Model this line's indentation using spaces (although it's tabs & spaces) to let parsing continue
|
||||
indentation = .spaces(spaceCount + tabCount * configuration.indentationWidth)
|
||||
} else if tabCount != 0 {
|
||||
indentation = .tabs(tabCount)
|
||||
} else {
|
||||
indentation = .spaces(spaceCount)
|
||||
if let expectedColumn = conditionContinuationInfo[line.index] {
|
||||
if let violation = checkMultilineConditionAlignment(
|
||||
line: line, expectedColumn: expectedColumn, prefix: prefix, file: file
|
||||
) {
|
||||
violations.append(violation)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Determine indentation from prefix
|
||||
let (indentation, mixedViolation) = parseIndentation(line: line, prefix: prefix, file: file)
|
||||
if let mixedViolation { violations.append(mixedViolation) }
|
||||
|
||||
// Catch indented first line
|
||||
guard previousLineIndentations.isNotEmpty else {
|
||||
previousLineIndentations = [indentation]
|
||||
|
||||
if indentation != .spaces(0) {
|
||||
// There's an indentation although this is the first line!
|
||||
violations.append(
|
||||
StyleViolation(
|
||||
ruleDescription: Self.description,
|
||||
severity: configuration.severityConfiguration.severity,
|
||||
location: Location(file: file, characterOffset: line.range.location),
|
||||
reason: "The first line shall not be indented"
|
||||
)
|
||||
makeViolation(file: file, line: line, reason: "The first line shall not be indented")
|
||||
)
|
||||
}
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
let linesValidationResult = previousLineIndentations.map {
|
||||
validate(indentation: indentation, comparingTo: $0)
|
||||
if let violation = checkIndentationChange(
|
||||
indentation: indentation, previousLineIndentations: previousLineIndentations, line: line, file: file
|
||||
) {
|
||||
violations.append(violation)
|
||||
}
|
||||
|
||||
// Catch wrong indentation or wrong unindentation
|
||||
if !linesValidationResult.contains(true) {
|
||||
let isIndentation = previousLineIndentations.last.map {
|
||||
indentation.spacesEquivalent(indentationWidth: configuration.indentationWidth) >=
|
||||
$0.spacesEquivalent(indentationWidth: configuration.indentationWidth)
|
||||
} ?? true
|
||||
|
||||
let indentWidth = configuration.indentationWidth
|
||||
violations.append(
|
||||
StyleViolation(
|
||||
ruleDescription: Self.description,
|
||||
severity: configuration.severityConfiguration.severity,
|
||||
location: Location(file: file, characterOffset: line.range.location),
|
||||
reason: isIndentation ?
|
||||
"Code should be indented using one tab or \(indentWidth) spaces" :
|
||||
"Code should be unindented by multiples of one tab or multiples of \(indentWidth) spaces"
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
if linesValidationResult.first == true {
|
||||
// Reset previousLineIndentations to this line only
|
||||
// if this line's indentation matches the last valid line's indentation (first in the array)
|
||||
if validate(indentation: indentation, comparingTo: previousLineIndentations[0]) {
|
||||
previousLineIndentations = [indentation]
|
||||
} else {
|
||||
// We not only store this line's indentation, but also keep what was stored before.
|
||||
// Therefore, the next line can be indented either according to the last valid line
|
||||
// or any of the succeeding, failing lines.
|
||||
// This mechanism avoids duplicate warnings.
|
||||
previousLineIndentations.append(indentation)
|
||||
}
|
||||
}
|
||||
@@ -141,6 +143,81 @@ struct IndentationWidthRule: OptInRule {
|
||||
return violations
|
||||
}
|
||||
|
||||
private func shouldSkipLine(line: Line, indentationCharacterCount: Int, in file: SwiftLintFile) -> Bool {
|
||||
line.content.count == indentationCharacterCount ||
|
||||
ignoreCompilerDirective(line: line, in: file) ||
|
||||
ignoreComment(line: line, in: file) ||
|
||||
ignoreMultilineStrings(line: line, in: file)
|
||||
}
|
||||
|
||||
private func checkIndentationChange(
|
||||
indentation: Indentation, previousLineIndentations: [Indentation], line: Line, file: SwiftLintFile
|
||||
) -> StyleViolation? {
|
||||
let isValid = previousLineIndentations.contains { validate(indentation: indentation, comparingTo: $0) }
|
||||
guard !isValid else { return nil }
|
||||
let isIndentation = previousLineIndentations.last.map {
|
||||
indentation.spacesEquivalent(indentationWidth: configuration.indentationWidth) >=
|
||||
$0.spacesEquivalent(indentationWidth: configuration.indentationWidth)
|
||||
} ?? true
|
||||
let indentWidth = configuration.indentationWidth
|
||||
return makeViolation(
|
||||
file: file,
|
||||
line: line,
|
||||
reason: isIndentation ?
|
||||
"Code should be indented using one tab or \(indentWidth) spaces" :
|
||||
"Code should be unindented by multiples of one tab or multiples of \(indentWidth) spaces"
|
||||
)
|
||||
}
|
||||
|
||||
private func makeViolation(file: SwiftLintFile, line: Line, reason: String) -> StyleViolation {
|
||||
StyleViolation(
|
||||
ruleDescription: Self.description,
|
||||
severity: configuration.severityConfiguration.severity,
|
||||
location: Location(file: file, characterOffset: line.range.location),
|
||||
reason: reason
|
||||
)
|
||||
}
|
||||
|
||||
private func parseIndentation(
|
||||
line: Line, prefix: IndentationPrefix, file: SwiftLintFile
|
||||
) -> (Indentation, StyleViolation?) {
|
||||
if prefix.tabCount != 0, prefix.spaceCount != 0 {
|
||||
let violation = makeViolation(
|
||||
file: file,
|
||||
line: line,
|
||||
reason: "Code should be indented with tabs or " +
|
||||
"\(configuration.indentationWidth) spaces, but not both in the same line"
|
||||
)
|
||||
return (.spaces(prefix.spacesEquivalent(indentationWidth: configuration.indentationWidth)), violation)
|
||||
}
|
||||
if prefix.tabCount != 0 {
|
||||
return (.tabs(prefix.tabCount), nil)
|
||||
}
|
||||
return (.spaces(prefix.spaceCount), nil)
|
||||
}
|
||||
|
||||
private func checkMultilineConditionAlignment(
|
||||
line: Line, expectedColumn: Int, prefix: IndentationPrefix, file: SwiftLintFile
|
||||
) -> StyleViolation? {
|
||||
if !configuration.includeMultilineConditions { return nil }
|
||||
let actualColumn = prefix.spacesEquivalent(indentationWidth: configuration.indentationWidth)
|
||||
guard actualColumn != expectedColumn else { return nil }
|
||||
return makeViolation(
|
||||
file: file,
|
||||
line: line,
|
||||
reason: "Multi-line condition should be aligned with the first condition " +
|
||||
"(expected \(expectedColumn) spaces, got \(actualColumn))"
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns a mapping from line index to expected indentation column for continuation lines
|
||||
/// of multi-line conditions. When `include_multiline_conditions` is false, these lines are
|
||||
/// skipped entirely (expected column is still stored so the line is recognized as a continuation).
|
||||
private func multilineConditionInfo(in file: SwiftLintFile) -> [Int: Int] {
|
||||
let visitor = MultilineConditionLineVisitor(locationConverter: file.locationConverter)
|
||||
return visitor.walk(tree: file.syntaxTree, handler: \.continuationLines)
|
||||
}
|
||||
|
||||
private func ignoreCompilerDirective(line: Line, in file: SwiftLintFile) -> Bool {
|
||||
if configuration.includeCompilerDirectives {
|
||||
return false
|
||||
@@ -203,3 +280,39 @@ struct IndentationWidthRule: OptInRule {
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private final class MultilineConditionLineVisitor: SyntaxVisitor {
|
||||
private let locationConverter: SourceLocationConverter
|
||||
/// Maps line index → expected indentation column for continuation lines.
|
||||
private(set) var continuationLines = [Int: Int]()
|
||||
|
||||
init(locationConverter: SourceLocationConverter) {
|
||||
self.locationConverter = locationConverter
|
||||
super.init(viewMode: .sourceAccurate)
|
||||
}
|
||||
|
||||
override func visitPost(_ node: GuardStmtSyntax) {
|
||||
collectContinuationLines(keyword: node.guardKeyword, conditions: node.conditions)
|
||||
}
|
||||
|
||||
override func visitPost(_ node: IfExprSyntax) {
|
||||
collectContinuationLines(keyword: node.ifKeyword, conditions: node.conditions)
|
||||
}
|
||||
|
||||
override func visitPost(_ node: WhileStmtSyntax) {
|
||||
collectContinuationLines(keyword: node.whileKeyword, conditions: node.conditions)
|
||||
}
|
||||
|
||||
private func collectContinuationLines(keyword: TokenSyntax, conditions: ConditionElementListSyntax) {
|
||||
guard conditions.count > 1 else { return }
|
||||
let keywordLine = locationConverter.location(for: keyword.positionAfterSkippingLeadingTrivia).line
|
||||
let firstConditionLoc = locationConverter.location(for: conditions.positionAfterSkippingLeadingTrivia)
|
||||
let conditionsEndLine = locationConverter.location(for: conditions.endPositionBeforeTrailingTrivia).line
|
||||
guard keywordLine < conditionsEndLine else { return }
|
||||
// Expected column is where the first condition starts (0-based → subtract 1)
|
||||
let expectedColumn = firstConditionLoc.column - 1
|
||||
for lineIndex in (keywordLine + 1)...conditionsEndLine {
|
||||
continuationLines[lineIndex] = expectedColumn
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -268,6 +268,39 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
assert1Violation(in: example4, includeMultilineStrings: true)
|
||||
}
|
||||
|
||||
func testMultilineConditionsSkippedByDefault() {
|
||||
assertNoViolation(in: "guard let x = foo(),\n let y = bar() else {\n return\n}")
|
||||
assertNoViolation(in: "if let x = foo(),\n let y = bar() {\n doSomething()\n}")
|
||||
assertNoViolation(in: "while let x = foo(),\n let y = bar() {\n doSomething()\n}")
|
||||
assertNoViolation(in: "guard let x = foo() else {\n return\n}")
|
||||
// Misaligned but skipped when include_multiline_conditions: false
|
||||
assertNoViolation(in: "if let x = foo(),\n let y = bar() {\n doSomething()\n}")
|
||||
}
|
||||
|
||||
func testMultilineConditionsAlignmentChecked() {
|
||||
// Properly aligned — no violations
|
||||
let guardAligned = "guard let x = foo(),\n let y = bar() else {\n return\n}"
|
||||
let ifAligned = "if let x = foo(),\n let y = bar() {\n doSomething()\n}"
|
||||
let whileAligned = "while let x = foo(),\n let y = bar() {\n doSomething()\n}"
|
||||
let guardNextLine = "guard\n let x = foo(),\n let y = bar()\nelse {\n return\n}"
|
||||
let ifThreeAligned = "if let a = foo(),\n let b = bar(),\n let c = baz() {\n doSomething()\n}"
|
||||
assertNoViolation(in: guardAligned, includeMultilineConditions: true)
|
||||
assertNoViolation(in: ifAligned, includeMultilineConditions: true)
|
||||
assertNoViolation(in: whileAligned, includeMultilineConditions: true)
|
||||
assertNoViolation(in: guardNextLine, includeMultilineConditions: true)
|
||||
assertNoViolation(in: ifThreeAligned, includeMultilineConditions: true)
|
||||
}
|
||||
|
||||
func testMultilineConditionsMisaligned() {
|
||||
let ifMisaligned = "if let x = foo(),\n let y = bar() {\n doSomething()\n}"
|
||||
let guardMisaligned = "guard let x = foo(),\n let y = bar() else {\n return\n}"
|
||||
let ifThreeMisaligned =
|
||||
"if let a = foo(),\n let b = bar(),\n let c = baz() {\n doSomething()\n}"
|
||||
assert1Violation(in: ifMisaligned, includeMultilineConditions: true)
|
||||
assert1Violation(in: guardMisaligned, includeMultilineConditions: true)
|
||||
assertViolations(in: ifThreeMisaligned, equals: 2, includeMultilineConditions: true)
|
||||
}
|
||||
|
||||
// MARK: Helpers
|
||||
private func countViolations(
|
||||
in example: Example,
|
||||
@@ -275,6 +308,7 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
includeComments: Bool = true,
|
||||
includeCompilerDirectives: Bool = true,
|
||||
includeMultilineStrings: Bool = true,
|
||||
includeMultilineConditions: Bool = false,
|
||||
file: StaticString = #filePath,
|
||||
line: UInt = #line
|
||||
) -> Int {
|
||||
@@ -285,6 +319,7 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
configDict["include_comments"] = includeComments
|
||||
configDict["include_compiler_directives"] = includeCompilerDirectives
|
||||
configDict["include_multiline_strings"] = includeMultilineStrings
|
||||
configDict["include_multiline_conditions"] = includeMultilineConditions
|
||||
|
||||
guard let config = makeConfig(configDict, IndentationWidthRule.identifier) else {
|
||||
XCTFail("Unable to create rule configuration.", file: (file), line: line)
|
||||
@@ -301,6 +336,7 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
includeComments: Bool = true,
|
||||
includeCompilerDirectives: Bool = true,
|
||||
includeMultilineStrings: Bool = true,
|
||||
includeMultilineConditions: Bool = false,
|
||||
file: StaticString = #filePath,
|
||||
line: UInt = #line
|
||||
) {
|
||||
@@ -311,6 +347,7 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
includeComments: includeComments,
|
||||
includeCompilerDirectives: includeCompilerDirectives,
|
||||
includeMultilineStrings: includeMultilineStrings,
|
||||
includeMultilineConditions: includeMultilineConditions,
|
||||
file: file,
|
||||
line: line
|
||||
),
|
||||
@@ -326,18 +363,15 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
includeComments: Bool = true,
|
||||
includeCompilerDirectives: Bool = true,
|
||||
includeMultilineStrings: Bool = true,
|
||||
includeMultilineConditions: Bool = false,
|
||||
file: StaticString = #filePath,
|
||||
line: UInt = #line
|
||||
) {
|
||||
assertViolations(
|
||||
in: string,
|
||||
equals: 0,
|
||||
indentationWidth: indentationWidth,
|
||||
includeComments: includeComments,
|
||||
includeCompilerDirectives: includeCompilerDirectives,
|
||||
in: string, equals: 0, indentationWidth: indentationWidth,
|
||||
includeComments: includeComments, includeCompilerDirectives: includeCompilerDirectives,
|
||||
includeMultilineStrings: includeMultilineStrings,
|
||||
file: file,
|
||||
line: line
|
||||
includeMultilineConditions: includeMultilineConditions, file: file, line: line
|
||||
)
|
||||
}
|
||||
|
||||
@@ -347,18 +381,15 @@ final class IndentationWidthRuleTests: SwiftLintTestCase {
|
||||
includeComments: Bool = true,
|
||||
includeCompilerDirectives: Bool = true,
|
||||
includeMultilineStrings: Bool = true,
|
||||
includeMultilineConditions: Bool = false,
|
||||
file: StaticString = #filePath,
|
||||
line: UInt = #line
|
||||
) {
|
||||
assertViolations(
|
||||
in: string,
|
||||
equals: 1,
|
||||
indentationWidth: indentationWidth,
|
||||
includeComments: includeComments,
|
||||
includeCompilerDirectives: includeCompilerDirectives,
|
||||
in: string, equals: 1, indentationWidth: indentationWidth,
|
||||
includeComments: includeComments, includeCompilerDirectives: includeCompilerDirectives,
|
||||
includeMultilineStrings: includeMultilineStrings,
|
||||
file: file,
|
||||
line: line
|
||||
includeMultilineConditions: includeMultilineConditions, file: file, line: line
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -546,6 +546,7 @@ indentation_width:
|
||||
include_comments: true
|
||||
include_compiler_directives: true
|
||||
include_multiline_strings: true
|
||||
include_multiline_conditions: false
|
||||
meta:
|
||||
opt-in: true
|
||||
correctable: false
|
||||
|
||||
Reference in New Issue
Block a user