From 9bc16b539b4c95d465cb8e3c180894524ad69702 Mon Sep 17 00:00:00 2001 From: Pavlo Tanaiev Date: Sun, 19 Apr 2026 16:08:24 +0300 Subject: [PATCH] Fix `indentation_width` false positives for multi-line conditions (#6505) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- CHANGELOG.md | 8 + .../IndentationWidthConfiguration.swift | 2 + .../Rules/Style/IndentationWidthRule.swift | 251 +++++++++++++----- .../IndentationWidthRuleTests.swift | 59 +++- .../Resources/default_rule_configurations.yml | 1 + 5 files changed, 238 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66f8bc7a1..0fd087386 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/IndentationWidthConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/IndentationWidthConfiguration.swift index 4133e2fc8..7e1b56d5f 100644 --- a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/IndentationWidthConfiguration.swift +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/IndentationWidthConfiguration.swift @@ -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 } diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/IndentationWidthRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/IndentationWidthRule.swift index 2c8f304de..bc8b11ace 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/IndentationWidthRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Style/IndentationWidthRule.swift @@ -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 + } + } +} diff --git a/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift b/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift index 936dd77bf..3e21dad80 100644 --- a/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift +++ b/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift @@ -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 ) } } diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index ba4c5af4b..a9722b64f 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -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