diff --git a/CHANGELOG.md b/CHANGELOG.md index f80760840b..57e3c08199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,14 @@ ### Experimental -* None. +* Add auto-correction and multi-violation support to the + `multiline_call_arguments` rule. Auto-correction reformats single-line + calls into multi-line form using the global `indentation` setting from + `.swiftlint.yml`. The per-rule `indentation` option has been removed in + favor of the global one. The rule detects all violations in a single + pass (previously required repeated `--fix`); safely handles nested + calls by suppressing overlapping inner corrections. + [GandaLF2006](https://github.com/GandaLF2006) ### Enhancements diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRule.swift index 52020a037b..1a68258c6d 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRule.swift @@ -1,20 +1,20 @@ +import Foundation +import SourceKittenFramework import SwiftLintCore import SwiftSyntax -@SwiftSyntaxRule(optIn: true) +@SwiftSyntaxRule(correctable: true, optIn: true) struct MultilineCallArgumentsRule: Rule { var configuration = MultilineCallArgumentsConfiguration() enum Reason { - static let singleLineMultipleArgumentsNotAllowed = - "Single-line calls with multiple arguments are not allowed" + static let singleLineMultipleArgumentsNotAllowed = "Single-line calls with multiple arguments are not allowed" static func tooManyArgumentsOnSingleLine(max: Int) -> String { "Too many arguments on a single line (max: \(max))" } - static let eachArgumentMustStartOnOwnLine = - "In multi-line calls, each argument must start on its own line" + static let eachArgumentMustStartOnOwnLine = "In multi-line calls, each argument must start on its own line" static let newlineRequiredAfterCommaInMultilineCall = "In multi-line calls, a newline is required after each comma" @@ -24,24 +24,29 @@ struct MultilineCallArgumentsRule: Rule { identifier: "multiline_call_arguments", name: "Multiline Call Arguments", description: """ - Enforces one-argument-per-line for multi-line calls and requires a newline after commas \ - when arguments are split across lines; - optionally limits or forbids multi-argument single-line calls via configuration. + Enforces one-argument-per-line for multi-line calls and requires a newline after commas + when arguments are split across lines; optionally limits or forbids multi-argument + single-line calls via configuration. + """, + rationale: """ + Keeping each argument on its own line in multi-line calls improves readability and + reduces merge conflicts. Requiring a newline after commas makes the call's structure + immediately visible and avoids ambiguous layouts where arguments appear to share a line. """, kind: .style, nonTriggeringExamples: MultilineCallArgumentsRuleExamples.nonTriggeringExamples, - triggeringExamples: MultilineCallArgumentsRuleExamples.triggeringExamples + triggeringExamples: MultilineCallArgumentsRuleExamples.triggeringExamples, + corrections: MultilineCallArgumentsRuleExamples.corrections ) } private extension MultilineCallArgumentsRule { final class Visitor: ViolationsSyntaxVisitor { - /// Cache line lookups by utf8Offset (stable, cheap key) + /// Cache mapping position offsets to line numbers for performance. private var lineCache: [Int: Int] = [:] override init(configuration: ConfigurationType, file: SwiftLintFile) { super.init(configuration: configuration, file: file) - // Most files trigger O(10–100) unique line lookups for this rule. // Reserving a small initial capacity reduces rehashing; it is NOT a hard limit. lineCache.reserveCapacity(64) @@ -51,116 +56,267 @@ private extension MultilineCallArgumentsRule { // Ignore calls that are part of pattern-matching syntax (patterns only, not bodies). guard !node.isInPatternMatchingPatternPosition else { return } - let args = node.arguments - guard args.count > 1 else { return } - - let argumentPositions = args.map(\.positionAfterSkippingLeadingTrivia) - guard let violation = reasonedViolation(argumentPositions: argumentPositions, arguments: args) else { - return - } - violations.append(violation) + let arguments = Array(node.arguments) + guard arguments.count > 1 else { return } + + let argumentPositions = arguments.map(\.positionAfterSkippingLeadingTrivia) + violations.append( + contentsOf: reasonedViolations( + argumentPositions: argumentPositions, + arguments: arguments, + callNode: node + ) + ) } - private func reasonedViolation( + /// Determines violations for a function call: single-line violations (too many + /// arguments, or `allows_single_line: false`) take priority; for multi-line calls, + /// both duplicate-start-line and missing-newline-after-comma violations are collected. + private func reasonedViolations( argumentPositions: [AbsolutePosition], - arguments: LabeledExprListSyntax - ) -> ReasonedRuleViolation? { - guard let firstPos = argumentPositions.first else { return nil } - - let firstLine = line(for: firstPos) - var allOnSameLine = true - for pos in argumentPositions.dropFirst() where line(for: pos) != firstLine { - allOnSameLine = false - break - } + arguments: [LabeledExprSyntax], + callNode: FunctionCallExprSyntax + ) -> [ReasonedRuleViolation] { + guard let firstPosition = argumentPositions.first else { return [] } + + let firstLine = line(for: firstPosition) + let allOnSameLine = argumentPositions.allSatisfy { line(for: $0) == firstLine } if allOnSameLine { if !configuration.allowsSingleLine { - return ReasonedRuleViolation( + let violation = ReasonedRuleViolation( position: argumentPositions[1], - reason: Reason.singleLineMultipleArgumentsNotAllowed + reason: Reason.singleLineMultipleArgumentsNotAllowed, + correction: singleLineCorrection( + arguments: arguments, + callNode: callNode + ) ) + return [violation] } if let max = configuration.maxNumberOfSingleLineParameters, argumentPositions.count > max { - return ReasonedRuleViolation( + let violation = ReasonedRuleViolation( position: argumentPositions[max], - reason: Reason.tooManyArgumentsOnSingleLine(max: max) + reason: Reason.tooManyArgumentsOnSingleLine(max: max), + correction: singleLineCorrection( + arguments: arguments, + callNode: callNode + ) ) + return [violation] } - return nil - } - - if let startLineViolation = duplicateArgumentStartLineViolation(in: arguments) { - return startLineViolation + return [] } - if let commaViolation = newlineAfterCommaViolation(in: arguments) { - return commaViolation - } - - return nil + return duplicateArgumentStartLineViolations(arguments: arguments, callNode: callNode) + + newlineAfterCommaViolations(arguments: arguments, callNode: callNode) } - private func duplicateArgumentStartLineViolation( - in arguments: LabeledExprListSyntax - ) -> ReasonedRuleViolation? { - let args = Array(arguments) - guard args.count > 1 else { return nil } - + private func duplicateArgumentStartLineViolations( + arguments: [LabeledExprSyntax], + callNode: FunctionCallExprSyntax + ) -> [ReasonedRuleViolation] { var seen: Set = [] - for arg in args { - let startPos = startPosition(of: arg) - let line = line(for: startPos) - if !seen.insert(line).inserted { - return ReasonedRuleViolation( - position: startPos, - reason: Reason.eachArgumentMustStartOnOwnLine + var result: [ReasonedRuleViolation] = [] + for (index, argument) in arguments.enumerated() { + let startPosition = startPosition(of: argument) + let argumentLine = line(for: startPosition) + if !seen.insert(argumentLine).inserted { + let prevArgument = arguments[index - 1] + let correctionStart = + if let comma = prevArgument.trailingComma, comma.presence != .missing { + comma.endPositionBeforeTrailingTrivia + } else { + prevArgument.endPositionBeforeTrailingTrivia + } + + let correction: ReasonedRuleViolation.ViolationCorrection? = hasComments(arguments: arguments) + ? nil + : newlineAndIndentCorrection( + start: correctionStart, + end: startPosition, + callNode: callNode + ) + + result.append( + ReasonedRuleViolation( + position: startPosition, + reason: Reason.eachArgumentMustStartOnOwnLine, + correction: correction + ) ) } } - - return nil + return result } - private func newlineAfterCommaViolation(in arguments: LabeledExprListSyntax) -> ReasonedRuleViolation? { - let args = Array(arguments) - guard args.count > 1 else { return nil } - - for index in args.indices.dropLast() { - let current = args[index] - let next = args[index + 1] + private func newlineAfterCommaViolations( + arguments: [LabeledExprSyntax], + callNode: FunctionCallExprSyntax + ) -> [ReasonedRuleViolation] { + var result: [ReasonedRuleViolation] = [] + for index in arguments.indices.dropLast() { + let current = arguments[index] + let next = arguments[index + 1] guard let comma = current.trailingComma, comma.presence != .missing else { continue } - if let lastToken = current.expression.lastToken(viewMode: .sourceAccurate) { - switch lastToken.tokenKind { - case .rightBrace, - .rightSquare: - continue - default: - break - } + // Skip if expression ends with closing brace/bracket (already properly terminated) + if let lastToken = current.expression.lastToken(viewMode: .sourceAccurate), + [.rightBrace, .rightSquare].contains(lastToken.tokenKind) { + continue } let commaLine = line(for: comma.positionAfterSkippingLeadingTrivia) let currentStartLine = line(for: startPosition(of: current)) - let nextStartPos = startPosition(of: next) - let nextStartLine = line(for: nextStartPos) + let nextStartPosition = startPosition(of: next) + let nextStartLine = line(for: nextStartPosition) + // Comma and next arg share a line, but current arg started on a different line + // → the comma-newline split is missing (e.g., `}, b: 3` after a multiline arg) if commaLine == nextStartLine, currentStartLine != nextStartLine { - return ReasonedRuleViolation( - position: nextStartPos, - reason: Reason.newlineRequiredAfterCommaInMultilineCall + let correction: ReasonedRuleViolation.ViolationCorrection? = hasComments(arguments: arguments) + ? nil + : newlineAndIndentCorrection( + start: comma.endPositionBeforeTrailingTrivia, + end: nextStartPosition, + callNode: callNode + ) + + result.append( + ReasonedRuleViolation( + position: nextStartPosition, + reason: Reason.newlineRequiredAfterCommaInMultilineCall, + correction: correction + ) ) } } + return result + } + + /// Returns the indentation string for argument lines: the call's base indent + /// plus one level of the global indentation from `CurrentRule.indentation` + /// (set by `Linter` from the top-level `Configuration.indentation`). + /// Falls back to `IndentationStyle.default` (4 spaces) when not set, + /// e.g. when the rule is invoked outside of a `Linter` context. + private func argumentIndent(for callNode: FunctionCallExprSyntax) -> String { + let callStartLine = line(for: callNode.positionAfterSkippingLeadingTrivia) + let baseIndent = getLineIndent(lineNumber: callStartLine) + return baseIndent + (CurrentRule.indentation ?? .default).indentationString + } + + private func newlineAndIndentCorrection( + start: AbsolutePosition, + end: AbsolutePosition, + callNode: FunctionCallExprSyntax + ) -> ReasonedRuleViolation.ViolationCorrection { + let indent = argumentIndent(for: callNode) + + return ReasonedRuleViolation.ViolationCorrection( + start: start, + end: end, + replacement: "\n" + indent + ) + } + + /// Produces a correction that reformats a single-line call into multi-line form: + /// each argument on its own line with proper indentation, closing `)` on a separate + /// line at the call's base indent. Returns `nil` when comments are present or the + /// call is nested inside another single-line correction (overlap guard). + private func singleLineCorrection( + arguments: [LabeledExprSyntax], + callNode: FunctionCallExprSyntax + ) -> ReasonedRuleViolation.ViolationCorrection? { + guard let firstArgument = arguments.first else { return nil } + + guard !shouldSuppressSingleLineCorrection(for: callNode) else { return nil } + guard !hasComments(arguments: arguments) else { return nil } + guard let rightParen = callNode.rightParen else { return nil } + + let indent = argumentIndent(for: callNode) + let callStartLine = line(for: callNode.positionAfterSkippingLeadingTrivia) + let baseIndent = getLineIndent(lineNumber: callStartLine) + + let argLines = arguments.enumerated().map { index, arg -> String in + let argText = arg.description.trimmingCharacters(in: .whitespacesAndNewlines) + // Only fires during error recovery (e.g., `foo(a: 1 b: 2)`); + // in valid Swift, non-last arguments always have a trailing comma. + let needsComma = index < arguments.count - 1 && arg.trailingComma?.presence == .missing + let suffix = needsComma ? "," : "" + return indent + argText + suffix + } + + let replacement = "\n" + (argLines + [baseIndent + ")"]).joined(separator: "\n") + + return .init( + start: firstArgument.position, + end: rightParen.endPositionBeforeTrailingTrivia, + replacement: replacement + ) + } + + /// Suppresses `singleLineCorrection` for a call nested in the arguments of another + /// single-line call that would also produce a `singleLineCorrection`, because the + /// outer correction encompasses the inner one. Without this guard, overlapping + /// `replaceSubrange` calls in the framework would silently corrupt the output. + /// + /// Walks up the entire parent chain — an intermediate non-violating call (e.g., a + /// 1-arg wrapper) does NOT suppress the inner call, but a higher ancestor that + /// does violate WILL. Returns `true` only when a qualifying outer call is found. + private func shouldSuppressSingleLineCorrection(for callNode: FunctionCallExprSyntax) -> Bool { + var node = Syntax(callNode) + var inArguments = false + while let parent = node.parent { + if parent.is(LabeledExprSyntax.self) || parent.is(LabeledExprListSyntax.self) { + inArguments = true + } else if let outerCall = parent.as(FunctionCallExprSyntax.self), inArguments { + let outerArguments = Array(outerCall.arguments) + if outerArguments.count > 1 { + let outerPositions = outerArguments.map(\.positionAfterSkippingLeadingTrivia) + let firstLine = outerPositions.first.map { line(for: $0) } ?? 0 + if outerPositions.allSatisfy({ line(for: $0) == firstLine }) { + if !configuration.allowsSingleLine, !hasComments(arguments: outerArguments) { + return true + } + if let max = configuration.maxNumberOfSingleLineParameters, + outerPositions.count > max, !hasComments(arguments: outerArguments) { + return true + } + } + } + } else if inArguments { + break + } + node = parent + } + return false + } + + private func hasComments(arguments: [LabeledExprSyntax]) -> Bool { + arguments.contains { argument in + argument.tokens(viewMode: .sourceAccurate).contains { token in + token.leadingTrivia.containsComments || token.trailingTrivia.containsComments + } + } + } - return nil + /// Extracts the leading whitespace (indentation) from a line. + /// Used to preserve existing indentation when adding new lines. + private func getLineIndent(lineNumber: Int) -> String { + guard lineNumber > 0, lineNumber <= file.lines.count else { + return "" + } + let lineContent = file.lines[lineNumber - 1].content + let leadingWhitespace = lineContent.prefix(while: { $0.isWhitespace && $0 != "\r" }) + return String(leadingWhitespace) } + /// Returns the start position of an argument, preferring label over expression. + /// This ensures consistent position calculation for both labeled and unlabeled arguments. private func startPosition(of argument: LabeledExprSyntax) -> AbsolutePosition { if let label = argument.label, label.presence != .missing { return label.positionAfterSkippingLeadingTrivia diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRuleExamples.swift index c3a8e0803c..905745efc8 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRuleExamples.swift @@ -2,6 +2,30 @@ // swiftlint:disable type_body_length struct MultilineCallArgumentsRuleExamples { static let nonTriggeringExamples: [Example] = [ + // MARK: - All configuration options shown + Example(""" + foo( + param1: 1, + param2: false, + param3: [] + ) + """, + configuration: ["allows_single_line": false] + ), + Example(""" + foo(param1: 1, param2: false) + """, + configuration: ["max_number_of_single_line_parameters": 2] + ), + Example(""" + foo( + \tparam1: 1, + \tparam2: false + ) + """, + configuration: ["allows_single_line": false] + ), + // MARK: - Baseline: multi-line OK Example(""" foo(param1: 1, @@ -404,6 +428,14 @@ struct MultilineCallArgumentsRuleExamples { ) """), + // MARK: - Multi-line: multiple duplicate argument start lines + Example(""" + foo( + a: 1, ↓b: 2, + c: 3, ↓d: 4 + ) + """), + // MARK: - Enum-case constructor calls are linted like normal calls Example(""" enum EnumCase { @@ -508,21 +540,296 @@ struct MultilineCallArgumentsRuleExamples { """, configuration: ["max_number_of_single_line_parameters": 2] ), + // MARK: - Tab indentation (uses global indentation setting) + Example( + "foo(param1: 1, ↓param2: false)", + configuration: ["allows_single_line": false] + ), Example(""" - func foo(a: Int, b: Int, c: Int) -> Int { a + b + c } - enum EnumCase: Error { case caseOne(Int, Int, Int, Int) } - - func mayThrow() throws { + class Test { + func method() { + if condition { + foo(param1: 1, ↓param2: false) + } + } } + """, + configuration: ["allows_single_line": false] + ), + // MARK: - Comments between arguments (violation still detected) + Example( + "foo(param1: 1, /* comment */ ↓param2: false)", + configuration: ["allows_single_line": false] + ), + // MARK: - Nested single-line calls (both violate) + Example( + "foo(bar(1, ↓2), ↓baz: 3)", + configuration: ["allows_single_line": false] + ), + ] - do { - try mayThrow() - } catch let EnumCase.caseOne(_, _, _, _) { - _ = foo(a: 1, b: 2, ↓c: 3) + static let corrections: [Example: Example] = [ + // MARK: - Single-line corrections + Example( + "foo(param1: 1, ↓param2: false)", + configuration: ["allows_single_line": false] + ): Example(""" + foo( + param1: 1, + param2: false + ) + """), + Example( + "foo(param1: 1, param2: false, ↓param3: [])", + configuration: ["max_number_of_single_line_parameters": 2] + ): Example(""" + foo( + param1: 1, + param2: false, + param3: [] + ) + """), + Example( + "foo(1, ↓2)", + configuration: ["allows_single_line": false] + ): Example(""" + foo( + 1, + 2 + ) + """), + Example( + "foo(1, b: 2, ↓3)", + configuration: ["max_number_of_single_line_parameters": 2] + ): Example(""" + foo( + 1, + b: 2, + 3 + ) + """), + // MARK: - Multi-line: duplicate argument start line + Example(""" + foo( + a: 1, ↓b: 2, + c: 3 + ) + """): Example(""" + foo( + a: 1, + b: 2, + c: 3 + ) + """), + // MARK: - Multi-line: four args, two on same line + Example(""" + foo( + a: 1, ↓b: 2, + c: 3, + d: 4 + ) + """): Example(""" + foo( + a: 1, + b: 2, + c: 3, + d: 4 + ) + """), + // MARK: - Multi-line: multiple duplicate argument start lines + Example(""" + foo( + a: 1, ↓b: 2, + c: 3, ↓d: 4 + ) + """): Example(""" + foo( + a: 1, + b: 2, + c: 3, + d: 4 + ) + """), + // MARK: - Multi-line: newline after comma + Example(""" + foo( + a: 1, + b: 2, ↓c: 3 + ) + """): Example(""" + foo( + a: 1, + b: 2, + c: 3 + ) + """), + // MARK: - Call with trailing closure + Example( + "foo(a: 1, ↓b: 2) { _ in }", + configuration: ["allows_single_line": false] + ): Example(""" + foo( + a: 1, + b: 2 + ) { _ in } + """), + // MARK: - Closure as argument + Example( + "foo(a: 1, ↓b: { x in x })", + configuration: ["allows_single_line": false] + ): Example(""" + foo( + a: 1, + b: { x in x } + ) + """), + // MARK: - Enum case call + Example( + "Enum.foo(param1: 1, ↓param2: false)", + configuration: ["allows_single_line": false] + ): Example(""" + Enum.foo( + param1: 1, + param2: false + ) + """), + // MARK: - Tuple argument (stays on same line) + Example( + "foo(a: (1, 2), ↓b: 3)", + configuration: ["max_number_of_single_line_parameters": 1] + ): Example(""" + foo( + a: (1, 2), + b: 3 + ) + """), + // MARK: - Enum-case constructor call + Example( + "EnumCase.first(one: 1, ↓two: 2)", + configuration: ["allows_single_line": false] + ): Example(""" + EnumCase.first( + one: 1, + two: 2 + ) + """), + // MARK: - Multi-line with tuple argument + Example(""" + foo( + a: (1, 2), ↓b: 3 + ) + """, configuration: ["max_number_of_single_line_parameters": 1]): Example(""" + foo( + a: (1, 2), + b: 3 + ) + """), + // MARK: - Nested indentation (4 spaces default) + Example(""" + class Test { + func method() { + if condition { + foo(param1: 1, ↓param2: false) + } + } } """, - configuration: ["max_number_of_single_line_parameters": 2] - ), + configuration: ["allows_single_line": false] + ): Example(""" + class Test { + func method() { + if condition { + foo( + param1: 1, + param2: false + ) + } + } + } + """), + // MARK: - Nested calls (inner call already correct, outer has violation) + Example( + "foo(bar(1), ↓baz: 3)", + configuration: ["allows_single_line": false] + ): Example(""" + foo( + bar(1), + baz: 3 + ) + """), + // MARK: - Nested multiline (inner call already multiline, outer duplicate start line) + Example(""" + foo( + bar( + 1, + 2 + ), ↓baz: 3 + ) + """): Example(""" + foo( + bar( + 1, + 2 + ), + baz: 3 + ) + """), + // MARK: - Nested single-line calls (inner correction suppressed) + Example( + "foo(bar(1, ↓2), ↓baz: 3)", + configuration: ["allows_single_line": false], + excludeFromDocumentation: true + ): Example(""" + foo( + bar(1, 2), + baz: 3 + ) + """), + Example( + "grandFoo(singleArgFoo(bar(1, ↓2)), ↓baz: 3)", + configuration: ["allows_single_line": false], + excludeFromDocumentation: true + ): Example(""" + grandFoo( + singleArgFoo(bar(1, 2)), + baz: 3 + ) + """), + Example( + "foo(bar(1, ↓2, ↓3), ↓baz: 4, ↓qux: 5)", + configuration: ["max_number_of_single_line_parameters": 2], + excludeFromDocumentation: true + ): Example(""" + foo( + bar(1, 2, 3), + baz: 4, + qux: 5 + ) + """), + // MARK: - Nested with comments (inner correction allowed) + Example( + "foo(bar(1, ↓2) /* c */, ↓baz: 3)", + configuration: ["allows_single_line": false], + excludeFromDocumentation: true + ): Example(""" + foo(bar( + 1, + 2 + ) /* c */, baz: 3) + """), + // MARK: - Open paren on new line + Example(""" + foo( + a: 1, ↓b: 2) + """, + configuration: ["allows_single_line": false], + excludeFromDocumentation: true + ): Example(""" + foo( + a: 1, + b: 2 + ) + """), ] } // swiftlint:enable type_body_length diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/MultilineCallArgumentsConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/MultilineCallArgumentsConfiguration.swift index 42463ea7f9..23ba41bfa3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/MultilineCallArgumentsConfiguration.swift +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/MultilineCallArgumentsConfiguration.swift @@ -1,5 +1,8 @@ import SwiftLintCore +/// Configuration for the `multiline_call_arguments` rule. +/// +/// This configuration controls how function calls with multiple arguments should be formatted. @AutoConfigParser struct MultilineCallArgumentsConfiguration: SeverityBasedRuleConfiguration { @ConfigurationElement(key: "severity") diff --git a/Source/SwiftLintCore/Models/CurrentRule.swift b/Source/SwiftLintCore/Models/CurrentRule.swift index 6f21a3e565..a183df21ae 100644 --- a/Source/SwiftLintCore/Models/CurrentRule.swift +++ b/Source/SwiftLintCore/Models/CurrentRule.swift @@ -8,4 +8,8 @@ public enum CurrentRule { /// Allows specific SourceKit requests to be made outside of rule execution context. /// This should only be used for essential operations like getting the Swift version. @TaskLocal public static var allowSourceKitRequestWithoutRule = false + + /// The global indentation style from the top-level configuration, made available to rules + /// without modifying function signatures throughout the codebase. + @TaskLocal public static var indentation: IndentationStyle? } diff --git a/Source/SwiftLintCore/Models/IndentationStyle.swift b/Source/SwiftLintCore/Models/IndentationStyle.swift new file mode 100644 index 0000000000..6b8681407f --- /dev/null +++ b/Source/SwiftLintCore/Models/IndentationStyle.swift @@ -0,0 +1,57 @@ +/// The style of indentation used in a Swift project. +public enum IndentationStyle: Hashable, Sendable { + /// Swift source code should be indented using tabs. + case tabs + /// Swift source code should be indented using spaces with the specified count per indentation level. + case spaces(count: Int) + + /// The default indentation style if none is explicitly provided. + package static let `default` = spaces(count: 4) + + /// The string representation of one level of indentation. + public var indentationString: String { + switch self { + case .tabs: return "\t" + case .spaces(let count): return String(repeating: " ", count: count) + } + } + + /// Creates an indentation style based on an untyped configuration value. + public init?(_ object: Any?) { + switch object { + case let value as Int: self = .spaces(count: value) + case let value as String where value == "tabs": self = .tabs + default: return nil + } + } +} + +/// Conformance to ``AcceptableByConfigurationElement`` for use in rule configurations. +extension IndentationStyle: AcceptableByConfigurationElement { + public func asOption() -> OptionType { + switch self { + case .tabs: return .string("tabs") + case .spaces(let count): return .integer(count) + } + } + + public init(fromAny value: Any, context ruleID: String) throws(Issue) { + switch value { + case let intValue as Int: + guard intValue >= 1 else { + throw Issue.invalidConfiguration( + ruleID: ruleID, + message: "Option 'indentation' must be a positive integer or the string \"tabs\"" + ) + } + self = .spaces(count: intValue) + case let stringValue as String where stringValue == "tabs": + self = .tabs + default: + throw Issue.invalidConfiguration( + ruleID: ruleID, + message: "Option 'indentation' must be a positive integer or the string \"tabs\"" + ) + } + } +} diff --git a/Source/SwiftLintFramework/Configuration/Configuration+IndentationStyle.swift b/Source/SwiftLintFramework/Configuration/Configuration+IndentationStyle.swift deleted file mode 100644 index e66fce9aff..0000000000 --- a/Source/SwiftLintFramework/Configuration/Configuration+IndentationStyle.swift +++ /dev/null @@ -1,23 +0,0 @@ -public extension Configuration { - /// The style of indentation used in a Swift project. - enum IndentationStyle: Hashable, Sendable { - /// Swift source code should be indented using tabs. - case tabs - /// Swift source code should be indented using spaces with `count` spaces per indentation level. - case spaces(count: Int) - - /// The default indentation style if none is explicitly provided. - package static let `default` = spaces(count: 4) - - /// Creates an indentation style based on an untyped configuration value. - /// - /// - parameter object: The configuration value. - internal init?(_ object: Any?) { - switch object { - case let value as Int: self = .spaces(count: value) - case let value as String where value == "tabs": self = .tabs - default: return nil - } - } - } -} diff --git a/Source/SwiftLintFramework/Configuration/Configuration+Parsing.swift b/Source/SwiftLintFramework/Configuration/Configuration+Parsing.swift index c7dbdabe57..0f4597e0b5 100644 --- a/Source/SwiftLintFramework/Configuration/Configuration+Parsing.swift +++ b/Source/SwiftLintFramework/Configuration/Configuration+Parsing.swift @@ -125,7 +125,7 @@ extension Configuration { private static func getIndentationLogIfInvalid(from dict: [String: Any]) -> IndentationStyle { if let rawIndentation = dict[Key.indentation.rawValue] { - if let indentationStyle = Self.IndentationStyle(rawIndentation) { + if let indentationStyle = IndentationStyle(rawIndentation) { return indentationStyle } Issue.invalidConfiguration(ruleID: Key.indentation.rawValue).print() diff --git a/Source/SwiftLintFramework/Models/Linter.swift b/Source/SwiftLintFramework/Models/Linter.swift index d1e46b0e4c..42dd537bb1 100644 --- a/Source/SwiftLintFramework/Models/Linter.swift +++ b/Source/SwiftLintFramework/Models/Linter.swift @@ -102,23 +102,26 @@ private extension Rule { benchmark: Bool, storage: RuleStorage, superfluousDisableCommandRule: SuperfluousDisableCommandRule?, - compilerArguments: [String]) -> LintResult { + compilerArguments: [String], + globalIndentation: IndentationStyle?) -> LintResult { let ruleID = Self.identifier // Wrap entire lint process including shouldRun check in rule context return CurrentRule.$identifier.withValue(ruleID) { - guard shouldRun(onFile: file) else { - return LintResult(violations: [], ruleTime: nil, deprecatedToValidIDPairs: []) - } + CurrentRule.$indentation.withValue(globalIndentation) { + guard shouldRun(onFile: file) else { + return LintResult(violations: [], ruleTime: nil, deprecatedToValidIDPairs: []) + } - return performLint( - file: file, - regions: regions, - benchmark: benchmark, - storage: storage, - superfluousDisableCommandRule: superfluousDisableCommandRule, - compilerArguments: compilerArguments - ) + return performLint( + file: file, + regions: regions, + benchmark: benchmark, + storage: storage, + superfluousDisableCommandRule: superfluousDisableCommandRule, + compilerArguments: compilerArguments + ) + } } } @@ -286,7 +289,9 @@ public struct Linter { let rule = rules[idx] let ruleID = type(of: rule).identifier CurrentRule.$identifier.withValue(ruleID) { - rule.collectInfo(for: file, into: storage, compilerArguments: compilerArguments) + CurrentRule.$indentation.withValue(configuration.indentation) { + rule.collectInfo(for: file, into: storage, compilerArguments: compilerArguments) + } } } return CollectedLinter(from: self) @@ -350,7 +355,8 @@ public struct CollectedLinter { $0.lint(file: file, regions: regions, benchmark: benchmark, storage: storage, superfluousDisableCommandRule: superfluousDisableCommandRule, - compilerArguments: compilerArguments) + compilerArguments: compilerArguments, + globalIndentation: configuration.indentation) } let undefinedSuperfluousCommandViolations = undefinedSuperfluousCommandViolations( regions: regions, configuration: configuration, @@ -423,10 +429,12 @@ public struct CollectedLinter { for rule in rules.compactMap({ $0 as? any CorrectableRule }) { // Set rule context before checking shouldRun to allow file property access let ruleCorrections = CurrentRule.$identifier.withValue(type(of: rule).identifier) { () -> Int? in - guard rule.shouldRun(onFile: file) else { - return nil + CurrentRule.$indentation.withValue(configuration.indentation) { + guard rule.shouldRun(onFile: file) else { + return nil + } + return rule.correct(file: file, using: storage, compilerArguments: compilerArguments) } - return rule.correct(file: file, using: storage, compilerArguments: compilerArguments) } if let corrected = ruleCorrections, corrected != 0 { corrections[type(of: rule).description.identifier] = corrected diff --git a/Tests/BuiltInRulesTests/MultilineCallArgumentsRuleTests.swift b/Tests/BuiltInRulesTests/MultilineCallArgumentsRuleTests.swift index 0ba076096c..bea1346890 100644 --- a/Tests/BuiltInRulesTests/MultilineCallArgumentsRuleTests.swift +++ b/Tests/BuiltInRulesTests/MultilineCallArgumentsRuleTests.swift @@ -1,9 +1,15 @@ +// swiftlint:disable file_length +import Foundation @testable import SwiftLintBuiltInRules +import SwiftLintCore +@testable import SwiftLintFramework import TestHelpers import Testing @Suite(.rulesRegistered) struct MultilineCallArgumentsRuleTests { + // MARK: - Reason tests + @Test func reasonSingleLineMultipleArgumentsNotAllowed() throws { let violations = try validate( @@ -63,6 +69,22 @@ struct MultilineCallArgumentsRuleTests { ) } + @Test + func reasonMultilineEachArgumentMustStartOnItsOwnLineDetectsMultipleViolations() throws { + let violations = try validate(""" + foo( + a: 1, b: 2, + c: 3, d: 4 + ) + """ + ) + + #expect(violations.count == 2) + #expect( + violations.allSatisfy { $0.reason == MultilineCallArgumentsRule.Reason.eachArgumentMustStartOnOwnLine } + ) + } + @Test func reasonMultilineRequiresNewlineAfterCommaInSplitLayout() throws { let violations = try validate(""" @@ -114,6 +136,309 @@ struct MultilineCallArgumentsRuleTests { ) } + // MARK: - Auto-correction: no correction when comments present + + @Test + func correctionWithCommentBetweenArgumentsDoesNotAutoCorrect() throws { + let contents = "foo(a: 1, /* comment */ b: 2)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0) + #expect(file.contents == contents) + } + + @Test + func correctionWithLineCommentBetweenArgumentsDoesNotAutoCorrect() throws { + let contents = "foo(a: 1, // comment\n b: 2)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0) + } + + @Test + func correctionWithCommentAfterExpressionBeforeCommaDoesNotAutoCorrect() throws { + let contents = "foo(a: 1 /* comment */, b: 2)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0) + #expect(file.contents == contents) + } + + @Test + func correctionWithCommentBetweenColonAndExpressionDoesNotAutoCorrect() throws { + let contents = "foo(a: /* comment */ 1, b: 2)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0) + #expect(file.contents == contents) + } + + @Test + func correctionWithCommentBetweenLabelAndColonDoesNotAutoCorrect() throws { + let contents = "foo(a /* comment */: 1, b: 2)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0) + #expect(file.contents == contents) + } + + @Test + func correctionWithCommentAfterLastArgExpressionDoesNotAutoCorrect() throws { + let contents = "foo(a: 1, b: 2 /* comment */)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0) + #expect(file.contents == contents) + } + + @Test + func correctionWithCommentInStringLiteralDoesAutoCorrect() throws { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file1 = SwiftLintFile(contents: "foo(a: \"/* not a comment */\", b: 2)") + #expect(rule.correct(file: file1) == 1) + let file2 = SwiftLintFile(contents: "foo(a: \"/* not a comment */\", b: \"// also not\")") + #expect(rule.correct(file: file2) == 1) + } + + @Test + func correctionMultilineWithCommentsDoesNotAutoCorrect() throws { + let rule = try MultilineCallArgumentsRule(configuration: [:]) + let cases = [ + "foo(\n a: 1, /* comment */ b: 2,\n c: 3\n)", + "foo(\n a: 1 /* comment */, b: 2,\n c: 3\n)", + "foo(\n a: (\n 1,\n 2\n ), /* comment */ b: 3\n)", + ] + for contents in cases { + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 0, "Should not correct: \(contents)") + } + } + + // MARK: - Configuration + + @Test + func configurationInvalidValuesThrow() { + #expect(throws: Issue.self) { + _ = try MultilineCallArgumentsRule(configuration: ["max_number_of_single_line_parameters": 0]) + } + #expect(throws: Issue.self) { + _ = try MultilineCallArgumentsRule(configuration: ["max_number_of_single_line_parameters": -1]) + } + #expect(throws: Issue.self) { + _ = try MultilineCallArgumentsRule(configuration: [ + "allows_single_line": false, + "max_number_of_single_line_parameters": 2, + ]) + } + } + + @Test + func configurationAllowsSingleLineFalseWithMaxParametersOneIsValid() { + #expect(throws: Never.self) { + _ = try MultilineCallArgumentsRule(configuration: [ + "allows_single_line": false, + "max_number_of_single_line_parameters": 1, + ]) + } + } + + // MARK: - Edge cases + + @Test + func correctionWithCRLFLineEndingsCorrectsIndentation() throws { + let contents = "foo(\r\n a: 1, b: 2,\r\n c: 3\r\n)" + let rule = try MultilineCallArgumentsRule(configuration: [:]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 1) + #expect(!file.contents.contains("\r ")) + } + + @Test + func reasonNewlineAfterCommaViolationSkippedForArrayLiteralArgument() throws { + let violations = try validate(""" + func foo(one: [Int], animated: Bool) {} + add(one: [ + 1, + 2, + 3 + ], animated: true) + """) + #expect(violations.isEmpty) + } + + @Test + func reasonNewlineAfterCommaViolationSkippedForClosureArgument() throws { + let violations = try validate(""" + foo(with: { + 9_999 + }, and: { + nil + }) + """) + #expect(violations.isEmpty) + } + + @Test + func reasonMixedDuplicateStartLineAndNewlineAfterComma() throws { + let violations = try validate(""" + foo( + a: 1, b: 2, + c: ( + 3, + 4 + ), d: 5 + ) + """) + #expect(violations.count == 2) + #expect(violations[0].reason == MultilineCallArgumentsRule.Reason.eachArgumentMustStartOnOwnLine) + #expect(violations[1].reason == MultilineCallArgumentsRule.Reason.newlineRequiredAfterCommaInMultilineCall) + } + + @Test + func reasonMultipleNewlineAfterCommaViolations() throws { + let violations = try validate(""" + foo( + a: ( + 1, + 2 + ), b: 3, + c: ( + 4, + 5 + ), d: 6 + ) + """) + #expect(violations.count == 2) + #expect(violations.allSatisfy { + $0.reason == MultilineCallArgumentsRule.Reason.newlineRequiredAfterCommaInMultilineCall + }) + } + + @Test + func violationPositionForUnlabeledArgument() throws { + let violations = try validate("foo(1, 2)", config: ["allows_single_line": false]) + #expect(violations.count == 1) + #expect( + violations.first?.reason == MultilineCallArgumentsRule.Reason.singleLineMultipleArgumentsNotAllowed) + } + + @Test + func violationPositionForMixedLabeledAndUnlabeledArguments() throws { + let violations = try validate( + "foo(1, b: 2, 3)", + config: ["max_number_of_single_line_parameters": 1]) + #expect(violations.count == 1) + #expect( + violations.first?.reason == MultilineCallArgumentsRule.Reason.tooManyArgumentsOnSingleLine(max: 1)) + } + + @Test + func patternMatchingPositionDoesNotViolate() throws { + let violations = try validate(""" + enum EnumCase { case caseOne(Int, Int, Int, Int) } + if case .caseOne(1, 2, 3, 4) = EnumCase.caseOne( + 0, + 0, + 0, + 0 + ) {} + """, + config: ["max_number_of_single_line_parameters": 2]) + #expect(violations.isEmpty) + } + + @Test + func lineCacheMultipleCallsOnDifferentLinesViolationsHaveCorrectLineNumbers() throws { + let contents = """ + foo(a: 1, b: 2, c: 3) + bar(x: 1, y: 2, z: 3, w: 4) + baz(1, 2, 3, 4, 5) + """ + let violations = try validate(contents, config: ["max_number_of_single_line_parameters": 2]) + #expect(violations.count == 3) + #expect(violations[0].location.line == 1) + #expect(violations[1].location.line == 2) + #expect(violations[2].location.line == 3) + } + + // MARK: - Global indentation via CurrentRule + + @Test + func correctionUsesGlobalTabIndentation() throws { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: "foo(a: 1, b: 2)") + CurrentRule.$indentation.withValue(.tabs) { + #expect(rule.correct(file: file) == 1) + } + #expect(file.contents == "foo(\n\ta: 1,\n\tb: 2\n)") + } + + @Test + func correctionUsesGlobal2SpaceIndentation() throws { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: "foo(a: 1, b: 2)") + CurrentRule.$indentation.withValue(.spaces(count: 2)) { + #expect(rule.correct(file: file) == 1) + } + #expect(file.contents == "foo(\n a: 1,\n b: 2\n)") + } + + @Test + func correctionFallsBackToDefaultIndentationWhenNotSet() throws { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: "foo(a: 1, b: 2)") + #expect(rule.correct(file: file) == 1) + #expect(file.contents == "foo(\n a: 1,\n b: 2\n)") + } + + @Test + func correctionMultilineUsesGlobalTabIndentation() throws { + let contents = """ + foo( + a: 1, b: 2, + c: 3 + ) + """ + let rule = try MultilineCallArgumentsRule(configuration: [:]) + let file = SwiftLintFile(contents: contents) + CurrentRule.$indentation.withValue(.tabs) { + #expect(rule.correct(file: file) == 1) + } + // Only the violation (b: 2 on same line as a: 1) is corrected; + // existing indentation of a: 1 and c: 3 is preserved. + #expect(file.contents == "foo(\n a: 1,\n\tb: 2,\n c: 3\n)") + } + + // MARK: - getLineIndent edge cases + + @Test + func correctionCallOnFirstLineHasNoBaseIndent() throws { + let contents = "foo(a: 1, b: 2)" + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: contents) + #expect(rule.correct(file: file) == 1) + #expect(file.contents == "foo(\n a: 1,\n b: 2\n)") + #expect(!file.contents.hasPrefix(" ")) + } + + // MARK: - Deeply nested suppression + + @Test + func correctionDeeplyNestedSuppressesInnermostCorrection() throws { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: "outer(middle(inner(1, 2), 3), 4)") + #expect(rule.correct(file: file) == 1) + #expect(file.contents == "outer(\n middle(inner(1, 2), 3),\n 4\n)") + } + + @Test + func correctionDeeplyNestedThroughNonViolatingMiddleSuppressesInner() throws { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let file = SwiftLintFile(contents: "outer(single(middle(1, 2)), 3)") + #expect(rule.correct(file: file) == 1) + #expect(file.contents == "outer(\n single(middle(1, 2)),\n 3\n)") + } + // MARK: - Helper private func validate(_ contents: String, config: [String: Any] = [:]) throws -> [StyleViolation] { @@ -121,3 +446,89 @@ struct MultilineCallArgumentsRuleTests { return rule.validate(file: SwiftLintFile(contents: contents)) } } + +@Suite(.rulesRegistered) +struct MultilineCallArgumentsLinterTests { + // MARK: - End-to-end: global indentation through Linter pipeline + + private func makeLinter(file: SwiftLintFile, indentation: IndentationStyle) throws -> CollectedLinter { + let rule = try MultilineCallArgumentsRule(configuration: ["allows_single_line": false]) + let config = Configuration( + rulesMode: .onlyConfiguration(["multiline_call_arguments"]), + allRulesWrapped: [(rule, false)], + indentation: indentation + ) + let storage = RuleStorage() + return Linter(file: file, configuration: config).collect(into: storage) + } + + @Test + func linterCorrectionUsesGlobalTabIndentation() throws { + let file = SwiftLintFile(contents: "foo(a: 1, b: 2)") + let linter = try makeLinter(file: file, indentation: .tabs) + let storage = RuleStorage() + let corrections = linter.correct(using: storage) + #expect(corrections["multiline_call_arguments"] == 1) + #expect(file.contents == "foo(\n\ta: 1,\n\tb: 2\n)") + } + + @Test + func linterCorrectionUsesGlobal2SpaceIndentation() throws { + let file = SwiftLintFile(contents: "foo(a: 1, b: 2)") + let linter = try makeLinter(file: file, indentation: .spaces(count: 2)) + let storage = RuleStorage() + let corrections = linter.correct(using: storage) + #expect(corrections["multiline_call_arguments"] == 1) + #expect(file.contents == "foo(\n a: 1,\n b: 2\n)") + } + + @Test + func linterCorrectionUsesDefault4SpaceIndentation() throws { + let file = SwiftLintFile(contents: "foo(a: 1, b: 2)") + let linter = try makeLinter(file: file, indentation: .default) + let storage = RuleStorage() + let corrections = linter.correct(using: storage) + #expect(corrections["multiline_call_arguments"] == 1) + #expect(file.contents == "foo(\n a: 1,\n b: 2\n)") + } +} + +@Suite +struct IndentationStyleTests { + @Test + func initializationAndIndentationString() throws { + let style = try IndentationStyle(fromAny: 4, context: "test_rule") + #expect(style.indentationString == " ") + let tabStyle = try IndentationStyle(fromAny: "tabs", context: "test_rule") + #expect(tabStyle.indentationString == "\t") + #expect(IndentationStyle.spaces(count: 1).indentationString == " ") + #expect(IndentationStyle.spaces(count: 2).indentationString == " ") + #expect(IndentationStyle.spaces(count: 8).indentationString == " ") + } + + @Test + func invalidValueThrows() { + #expect(throws: Issue.self) { + _ = try IndentationStyle(fromAny: 0, context: "test_rule") + } + #expect(throws: Issue.self) { + _ = try IndentationStyle(fromAny: -1, context: "test_rule") + } + #expect(throws: Issue.self) { + _ = try IndentationStyle(fromAny: "spaces", context: "test_rule") + } + #expect(throws: Issue.self) { + _ = try IndentationStyle(fromAny: 3.14, context: "test_rule") + } + } + + @Test + func asOptionAndEquality() { + #expect(IndentationStyle.spaces(count: 4).asOption() == .integer(4)) + #expect(IndentationStyle.tabs.asOption() == .string("tabs")) + // swiftlint:disable:next identical_operands + #expect(IndentationStyle.spaces(count: 4) == IndentationStyle.spaces(count: 4)) + #expect(IndentationStyle.spaces(count: 4) != IndentationStyle.spaces(count: 2)) + #expect(IndentationStyle.tabs != IndentationStyle.spaces(count: 4)) + } +} diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index 59ae9bc2a2..0f0f00fc8c 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -704,7 +704,7 @@ multiline_call_arguments: allows_single_line: true meta: opt-in: true - correctable: false + correctable: true multiline_function_chains: severity: warning meta: