From af63388c9048ab2c9e9321e576bb987e9952e294 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Fri, 20 Jun 2025 14:23:21 -0400 Subject: [PATCH] Migrate VerticalWhitespaceOpeningBracesRule to SwiftSyntax ## Summary Convert VerticalWhitespaceOpeningBracesRule to use SwiftSyntax instead of SourceKit for improved performance and better detection of empty lines after opening braces. ## Key Technical Improvements - **Enhanced trivia analysis** for accurate empty line detection after opening braces - **Proper handling of closure "in" keywords** with context-aware detection - **Improved correction logic** handling all newline types (LF, CR, CRLF) consistently - **SwiftSyntax visitor pattern** replacing regex-based detection for better accuracy - **Comprehensive token analysis** supporting all opening brace types (`{`, `[`, `(`) - **Accurate position tracking** for violation start and end positions ## Migration Details - Replaced `CorrectableRule, OptInRule` with `@SwiftSyntaxRule(correctable: true, optIn: true)` - Implemented `ViolationsSyntaxVisitor` for detecting violations in token trailing trivia - Implemented `ViolationsSyntaxRewriter` for correcting violations across different syntax contexts - Added proper handling of closures, code blocks, arrays, and tuples - Maintained exact position reporting for violation locations - Preserved all existing test cases and rule behavior --- CHANGELOG.md | 1 + .../VerticalWhitespaceOpeningBracesRule.swift | 440 ++++++++++++++++-- 2 files changed, 400 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 375603d4e8..628d6f9ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ * `line_length` * `trailing_whitespace` * `vertical_whitespace` + * `vertical_whitespace_opening_braces` [JP Simard](https://github.com/jpsim) [Matt Pennig](https://github.com/pennig) diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/VerticalWhitespaceOpeningBracesRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/VerticalWhitespaceOpeningBracesRule.swift index 585a99aa1b..6172c20e6b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/VerticalWhitespaceOpeningBracesRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Style/VerticalWhitespaceOpeningBracesRule.swift @@ -1,12 +1,8 @@ -import Foundation -import SourceKittenFramework - -private extension SwiftLintFile { - func violatingRanges(for pattern: String) -> [NSRange] { - match(pattern: pattern, excludingSyntaxKinds: SyntaxKind.commentAndStringKinds) - } -} +// swiftlint:disable file_length +import SwiftLintCore +import SwiftSyntax +@SwiftSyntaxRule(correctable: true, optIn: true) struct VerticalWhitespaceOpeningBracesRule: Rule { var configuration = SeverityConfiguration(.warning) @@ -23,6 +19,20 @@ struct VerticalWhitespaceOpeningBracesRule: Rule { } */ """), + Example(""" + func foo() { + // This is a comment + + let x = 5 + } + """), + Example(""" + if condition { + // Comment explaining the logic + + performAction() + } + """), ] private static let violatingToValidExamples: [Example: Example] = [ @@ -135,12 +145,32 @@ struct VerticalWhitespaceOpeningBracesRule: Rule { }) } """), + Example(""" + func foo() { + ↓ + // This is a comment + let x = 5 + } + """): Example(""" + func foo() { + // This is a comment + let x = 5 + } + """), + Example(""" + if condition { + ↓ + // Comment explaining the logic + performAction() + } + """): Example(""" + if condition { + // Comment explaining the logic + performAction() + } + """), ] - private let pattern = "([{(\\[][ \\t]*(?:[^\\n{]+ in[ \\t]*$)?)((?:\\n[ \\t]*)+)(\\n)" -} - -extension VerticalWhitespaceOpeningBracesRule: OptInRule { static let description = RuleDescription( identifier: "vertical_whitespace_opening_braces", name: "Vertical Whitespace after Opening Braces", @@ -150,43 +180,371 @@ extension VerticalWhitespaceOpeningBracesRule: OptInRule { triggeringExamples: Array(violatingToValidExamples.keys).sorted(), corrections: violatingToValidExamples.removingViolationMarkers() ) +} - func validate(file: SwiftLintFile) -> [StyleViolation] { - let patternRegex: NSRegularExpression = regex(pattern) +private struct TriviaAnalysis { + var consecutiveNewlines = 0 + var violationStartPosition: AbsolutePosition? + var violationEndPosition: AbsolutePosition? + var hasViolation = false + var commentDirectlyAfterBrace = false - return file.violatingRanges(for: pattern).map { violationRange in - let substring = file.contents.substring(from: violationRange.location, length: violationRange.length) - let substringRange = NSRange(location: 0, length: substring.count) - let matchResult = patternRegex.firstMatch(in: substring, options: [], range: substringRange)! - let violatingSubrange = matchResult.range(at: 2) - let characterOffset = violationRange.location + violatingSubrange.location + 1 + mutating func processTriviaPiece( + _ piece: TriviaPiece, + currentPosition: AbsolutePosition + ) -> Int { + switch piece { + case .newlines(let count), .carriageReturns(let count): + var context = NewlineProcessingContext( + currentPosition: currentPosition, + consecutiveNewlines: consecutiveNewlines, + violationStartPosition: violationStartPosition, + violationEndPosition: violationEndPosition + ) + let processResult = context.processNewlines(count: count, bytesPerNewline: 1) + consecutiveNewlines = processResult.newlines + violationStartPosition = context.violationStartPosition + violationEndPosition = context.violationEndPosition + + // If we have 2+ consecutive newlines and haven't seen a comment directly after the brace, + // mark this as a violation + if consecutiveNewlines >= 2 && !commentDirectlyAfterBrace { + hasViolation = true + } - return StyleViolation( - ruleDescription: Self.description, - severity: configuration.severity, - location: Location(file: file, characterOffset: characterOffset) + return processResult.positionAdvance + case .carriageReturnLineFeeds(let count): + var context = NewlineProcessingContext( + currentPosition: currentPosition, + consecutiveNewlines: consecutiveNewlines, + violationStartPosition: violationStartPosition, + violationEndPosition: violationEndPosition ) + let processResult = context.processNewlines(count: count, bytesPerNewline: 2) + consecutiveNewlines = processResult.newlines + violationStartPosition = context.violationStartPosition + violationEndPosition = context.violationEndPosition + + // If we have 2+ consecutive newlines and haven't seen a comment directly after the brace, + // mark this as a violation + if consecutiveNewlines >= 2 && !commentDirectlyAfterBrace { + hasViolation = true + } + + return processResult.positionAdvance + case .spaces, .tabs: + return piece.sourceLength.utf8Length + case .lineComment, .blockComment, .docLineComment, .docBlockComment: + // If we see a comment after only one newline, mark it + if consecutiveNewlines == 1 { + commentDirectlyAfterBrace = true + } + // Comments reset the consecutive newline count + consecutiveNewlines = 0 + // Don't clear violation positions if we already found a violation + if !hasViolation { + violationStartPosition = nil + violationEndPosition = nil + } + return piece.sourceLength.utf8Length + default: + // Any other trivia breaks the sequence + consecutiveNewlines = 0 + // Don't clear violation positions if we already found a violation + if !hasViolation { + violationStartPosition = nil + violationEndPosition = nil + } + return piece.sourceLength.utf8Length } } + + private func processNewlines( + count: Int, + bytesPerNewline: Int, + context: inout NewlineProcessingContext + ) -> (newlines: Int, positionAdvance: Int) { + var newConsecutiveNewlines = context.consecutiveNewlines + var totalAdvance = 0 + + for _ in 0..= 2 newlines. + if newConsecutiveNewlines >= 2 { + context.violationEndPosition = context.currentPosition.advanced(by: totalAdvance + bytesPerNewline) + } + totalAdvance += bytesPerNewline + } + + return (newConsecutiveNewlines, totalAdvance) + } } -extension VerticalWhitespaceOpeningBracesRule: CorrectableRule { - func correct(file: SwiftLintFile) -> Int { - let violatingRanges = file.ruleEnabled(violatingRanges: file.violatingRanges(for: pattern), for: self) - guard violatingRanges.isNotEmpty else { - return 0 - } - let patternRegex = regex(pattern) - var fileContents = file.contents - for violationRange in violatingRanges.reversed() { - fileContents = patternRegex.stringByReplacingMatches( - in: fileContents, - options: [], - range: violationRange, - withTemplate: "$1$3" - ) +private struct CorrectionState { + var result = [TriviaPiece]() + var consecutiveNewlines = 0 + var correctionCount = 0 + var hasViolation = false +} + +private struct NewlineProcessingContext { + let currentPosition: AbsolutePosition + var consecutiveNewlines: Int + var violationStartPosition: AbsolutePosition? + var violationEndPosition: AbsolutePosition? + + mutating func processNewlines(count: Int, bytesPerNewline: Int) -> (newlines: Int, positionAdvance: Int) { + var totalAdvance = 0 + + for _ in 0..= 2 newlines. + if consecutiveNewlines >= 2 { + violationEndPosition = currentPosition.advanced(by: totalAdvance + bytesPerNewline) + } + totalAdvance += bytesPerNewline + } + + return (consecutiveNewlines, totalAdvance) + } +} + +private extension VerticalWhitespaceOpeningBracesRule { + final class Visitor: ViolationsSyntaxVisitor> { + override func visitPost(_ node: TokenSyntax) { + // Check for violations after opening braces + if node.isOpeningBrace { + checkForViolationAfterToken(node) + } else if node.tokenKind == .keyword(.in) { + // Check for violations after "in" keywords in closures + // Check if this "in" is part of a closure signature + if isClosureSignatureIn(node) { + checkForViolationAfterToken(node) + } + } + } + + private func isClosureSignatureIn(_ inToken: TokenSyntax) -> Bool { + // Check if the "in" token is part of a closure signature by looking at its parent + var currentNode = Syntax(inToken) + while let parent = currentNode.parent { + if parent.is(ClosureSignatureSyntax.self) { + return true + } + // Stop traversing if we hit a different expression or declaration + if parent.is(ExprSyntax.self) || parent.is(DeclSyntax.self) { + break + } + currentNode = parent + } + return false + } + + private func checkForViolationAfterToken(_ token: TokenSyntax) { + // We analyze the trivia of the token immediately following the token + if let nextToken = token.nextToken(viewMode: .sourceAccurate) { + let triviaAnalysis = analyzeTriviaForViolations( + trivia: nextToken.leadingTrivia, + position: token.endPositionBeforeTrailingTrivia + ) + + if let violation = triviaAnalysis { + violations.append( + ReasonedRuleViolation( + position: violation.position, + correction: .init( + start: violation.position, + end: violation.endPosition, + replacement: "" + ) + ) + ) + } + } + } + + private func analyzeTriviaForViolations( + trivia: Trivia, + position: AbsolutePosition + ) -> (position: AbsolutePosition, endPosition: AbsolutePosition)? { + let analysis = analyzeTrivia(trivia: trivia, startPosition: position) + + // Only flag violations if we found an empty line that wasn't allowed + guard let startPos = analysis.violationStartPosition, + let endPos = analysis.violationEndPosition, + analysis.hasViolation else { + return nil + } + + return (position: startPos, endPosition: endPos) + } + + private func analyzeTrivia( + trivia: Trivia, + startPosition: AbsolutePosition + ) -> TriviaAnalysis { + var result = TriviaAnalysis() + var currentPosition = startPosition + + for piece in trivia { + let positionAdvance = result.processTriviaPiece(piece, currentPosition: currentPosition) + currentPosition = currentPosition.advanced(by: positionAdvance) + } + + return result + } + } + + final class Rewriter: ViolationsSyntaxRewriter> { + override func visit(_ node: CodeBlockSyntax) -> CodeBlockSyntax { + // Handle code blocks with opening braces + if let firstStatement = node.statements.first { + let leadingTrivia = firstStatement.leadingTrivia + let correctedTrivia = correctTrivia(trivia: leadingTrivia) + if correctedTrivia.hasCorrections { + numberOfCorrections += correctedTrivia.correctionCount + var newStatements = node.statements + newStatements[newStatements.startIndex] = firstStatement + .with(\.leadingTrivia, correctedTrivia.trivia) + return node.with(\.statements, newStatements) + } + } + return super.visit(node) + } + + override func visit(_ node: ClosureExprSyntax) -> ExprSyntax { + // Handle closures with "in" keyword + if let firstStatement = node.statements.first { + let leadingTrivia = firstStatement.leadingTrivia + let correctedTrivia = correctTrivia(trivia: leadingTrivia) + if correctedTrivia.hasCorrections { + numberOfCorrections += correctedTrivia.correctionCount + var newStatements = node.statements + newStatements[newStatements.startIndex] = firstStatement + .with(\.leadingTrivia, correctedTrivia.trivia) + return ExprSyntax(node.with(\.statements, newStatements)) + } + } + return super.visit(node) + } + + override func visit(_ node: ArrayExprSyntax) -> ExprSyntax { + // Handle array literals + if let firstElement = node.elements.first { + let leadingTrivia = firstElement.leadingTrivia + let correctedTrivia = correctTrivia(trivia: leadingTrivia) + if correctedTrivia.hasCorrections { + numberOfCorrections += correctedTrivia.correctionCount + var newElements = node.elements + newElements[newElements.startIndex] = firstElement.with(\.leadingTrivia, correctedTrivia.trivia) + return ExprSyntax(node.with(\.elements, newElements)) + } + } + return super.visit(node) + } + + override func visit(_ node: TupleExprSyntax) -> ExprSyntax { + // Handle tuples and function arguments + if let firstElement = node.elements.first { + let leadingTrivia = firstElement.leadingTrivia + let correctedTrivia = correctTrivia(trivia: leadingTrivia) + if correctedTrivia.hasCorrections { + numberOfCorrections += correctedTrivia.correctionCount + var newElements = node.elements + newElements[newElements.startIndex] = firstElement.with(\.leadingTrivia, correctedTrivia.trivia) + return ExprSyntax(node.with(\.elements, newElements)) + } + } + return super.visit(node) + } + + private func correctTrivia( + trivia: Trivia + ) -> (trivia: Trivia, hasCorrections: Bool, correctionCount: Int) { + var state = CorrectionState() + + for piece in trivia { + processPieceForCorrection(piece: piece, state: &state) + } + + return (trivia: Trivia(pieces: state.result), + hasCorrections: state.correctionCount > 0, + correctionCount: state.correctionCount) + } + + private func processPieceForCorrection(piece: TriviaPiece, state: inout CorrectionState) { + switch piece { + case .newlines(let count), .carriageReturns(let count): + let newlineCreator = piece.isNewline ? TriviaPiece.newlines : TriviaPiece.carriageReturns + processNewlinesForCorrection( + count: count, + newlineCreator: { newlineCreator($0) }, + state: &state + ) + case .carriageReturnLineFeeds(let count): + processNewlinesForCorrection( + count: count, + newlineCreator: { TriviaPiece.carriageReturnLineFeeds($0) }, + state: &state + ) + case .spaces, .tabs: + state.result.append(piece) + default: + // Other trivia breaks the sequence + state.consecutiveNewlines = 0 + state.hasViolation = false + state.result.append(piece) + } + } + + private func processNewlinesForCorrection( + count: Int, + newlineCreator: (Int) -> TriviaPiece, + state: inout CorrectionState + ) { + for _ in 0..