diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/ConditionalReturnsOnNewlineRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/ConditionalReturnsOnNewlineRule.swift index 92d28780d23..06d58e6b88b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/ConditionalReturnsOnNewlineRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Style/ConditionalReturnsOnNewlineRule.swift @@ -1,6 +1,7 @@ +import SwiftBasicFormat import SwiftSyntax -@SwiftSyntaxRule(optIn: true) +@SwiftSyntaxRule(explicitRewriter: true, optIn: true) struct ConditionalReturnsOnNewlineRule: Rule { var configuration = ConditionalReturnsOnNewlineConfiguration() @@ -30,6 +31,33 @@ struct ConditionalReturnsOnNewlineRule: Rule { Example(""" ↓guard condition else { XCTFail(); return } """), + ], + corrections: [ + Example("↓if true { return }"): Example("if true {\n return\n}"), + Example(""" + func f() { + ↓if true { return } + } + """): Example(""" + func f() { + if true { + return + } + } + """), + Example("↓if true { break } else { return }"): + Example("if true { break } else {\n return\n}"), + Example("↓if true { return \"YES\" } else { return \"NO\" }"): + Example("if true {\n return \"YES\"\n} else { return \"NO\" }"), + Example("↓guard true else { return }"): + Example("guard true else {\n return\n}"), + Example(""" + ↓guard condition else { XCTFail(); return } + """): Example(""" + guard condition else { XCTFail(); + return + } + """), ] ) } @@ -67,6 +95,95 @@ private extension ConditionalReturnsOnNewlineRule { locationConverter.location(for: token.positionAfterSkippingLeadingTrivia).line } } + + final class Rewriter: ViolationsSyntaxRewriter { + override func visit(_ node: IfExprSyntax) -> ExprSyntax { + // Match the visitor's logic: if body has violation, only fix body (and return early) + // If body is fine, check else body + if isReturn(node.body.statements.lastReturn, onTheSameLineAs: node.ifKeyword) { + numberOfCorrections += 1 + let modifiedNode = node.with(\.body, fixCodeBlock(node.body, baseToken: node.ifKeyword)) + return super.visit(modifiedNode) + } + + // Check if else body's return is on the same line as the else keyword + if let elseBody = node.elseBody?.as(CodeBlockSyntax.self), let elseKeyword = node.elseKeyword, + isReturn(elseBody.statements.lastReturn, onTheSameLineAs: elseKeyword) { + numberOfCorrections += 1 + let fixedElseBody = fixCodeBlock(elseBody, baseToken: node.ifKeyword) + let modifiedNode = node.with(\.elseBody, .codeBlock(fixedElseBody)) + return super.visit(modifiedNode) + } + + return super.visit(node) + } + + override func visit(_ node: GuardStmtSyntax) -> StmtSyntax { + if configuration.ifOnly { + return super.visit(node) + } + + guard isReturn(node.body.statements.lastReturn, onTheSameLineAs: node.guardKeyword) else { + return super.visit(node) + } + + numberOfCorrections += 1 + let fixedNode = node.with(\.body, fixCodeBlock(node.body, baseToken: node.guardKeyword)) + return super.visit(fixedNode) + } + + private func isReturn(_ returnStmt: ReturnStmtSyntax?, onTheSameLineAs token: TokenSyntax) -> Bool { + guard let returnStmt else { + return false + } + + return locationConverter.location(for: returnStmt.returnKeyword.positionAfterSkippingLeadingTrivia).line == + locationConverter.location(for: token.positionAfterSkippingLeadingTrivia).line + } + + private func fixCodeBlock(_ block: CodeBlockSyntax, baseToken: TokenSyntax) -> CodeBlockSyntax { + // Get the indentation of the base token (e.g., `if` or `guard`) + let baseIndentation = baseToken.leadingTrivia.indentation(isOnNewline: true) ?? Trivia() + let innerIndentation = Trivia(pieces: baseIndentation.pieces + [.spaces(4)]) + + // Check if the first statement is the return (i.e., return is the only statement) + let returnIsFirst = block.statements.first?.item.is(ReturnStmtSyntax.self) == true + + // Fix the left brace: only remove trailing whitespace if return is the first statement + let fixedLeftBrace = returnIsFirst + ? block.leftBrace.with(\.trailingTrivia, Trivia()) + : block.leftBrace + + // Fix the statements: add newline + inner indentation before the return, + // and remove trailing whitespace from the return value and the preceding statement + var statements = Array(block.statements) + for index in statements.indices where statements[index].item.is(ReturnStmtSyntax.self) { + // Remove trailing whitespace from the previous statement if there is one + if index > 0 { + statements[index - 1] = statements[index - 1].with(\.trailingTrivia, Trivia()) + } + // Add newline + indentation before return and remove its trailing whitespace + statements[index] = statements[index] + .with( + \.leadingTrivia, + Trivia(pieces: [.newlines(1)] + innerIndentation.pieces) + ) + .with(\.trailingTrivia, Trivia()) + } + let fixedStatements = CodeBlockItemListSyntax(statements) + + // Fix the closing brace: add newline + base indentation before it + let fixedRightBrace = block.rightBrace.with( + \.leadingTrivia, + Trivia(pieces: [.newlines(1)] + baseIndentation.pieces) + ) + + return block + .with(\.leftBrace, fixedLeftBrace) + .with(\.statements, fixedStatements) + .with(\.rightBrace, fixedRightBrace) + } + } } private extension CodeBlockItemListSyntax { diff --git a/Tests/BuiltInRulesTests/ConditionalReturnsOnNewlineRuleTests.swift b/Tests/BuiltInRulesTests/ConditionalReturnsOnNewlineRuleTests.swift index 96a31dbd7ed..e1c18a8a98f 100644 --- a/Tests/BuiltInRulesTests/ConditionalReturnsOnNewlineRuleTests.swift +++ b/Tests/BuiltInRulesTests/ConditionalReturnsOnNewlineRuleTests.swift @@ -12,6 +12,7 @@ final class ConditionalReturnsOnNewlineRuleTests: SwiftLintTestCase { Example("if textField.returnKeyType == .Next {"), Example("if true { // return }"), Example("/*if true { */ return }"), + // guard statements should not trigger or be corrected Example("guard true else { return }"), ] let triggeringExamples = [ @@ -20,10 +21,19 @@ final class ConditionalReturnsOnNewlineRuleTests: SwiftLintTestCase { Example("↓if true { break } else { return }"), Example("↓if true { return \"YES\" } else { return \"NO\" }"), ] + // Only include `if` corrections - guard corrections should not apply with if_only + let expectedCorrections = [ + Example("↓if true { return }"): Example("if true {\n return\n}"), + Example("↓if true { break } else { return }"): + Example("if true { break } else {\n return\n}"), + Example("↓if true { return \"YES\" } else { return \"NO\" }"): + Example("if true {\n return \"YES\"\n} else { return \"NO\" }"), + ] let description = ConditionalReturnsOnNewlineRule.description .with(triggeringExamples: triggeringExamples) .with(nonTriggeringExamples: nonTriggeringExamples) + .with(corrections: expectedCorrections) verifyRule(description, ruleConfiguration: ["if_only": true]) } diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index 474a656955e..43d33ed082a 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -134,7 +134,7 @@ conditional_returns_on_newline: if_only: false meta: opt-in: true - correctable: false + correctable: true contains_over_filter_count: severity: warning meta: