From f7f3caa50e4cf8a0ead5a936bbdeeef3c76e4213 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 24 Jun 2025 21:12:32 -0400 Subject: [PATCH] Migrate TrailingWhitespaceRule from SourceKit to SwiftSyntax (#6117) ## Summary Convert TrailingWhitespaceRule to use SwiftSyntax instead of SourceKit for improved performance and better handling of trailing whitespace detection, especially within block comments. ## Key Technical Improvements - **Enhanced block comment detection** distinguishing between lines fully covered by block comments vs lines containing block comments with code - **Accurate whitespace detection** using CharacterSet.whitespaces for all Unicode whitespace characters, not just space and tab - **Improved comment handling** with proper detection of line-ending comments and multi-line block comment structures - **Better correction mechanism** using ViolationCorrection ranges instead of manual string reconstruction - **Line-based analysis** maintaining efficiency while providing precise violation positions ## Migration Details - Replaced `CorrectableRule` with `@SwiftSyntaxRule(correctable: true)` - Implemented `ViolationsSyntaxVisitor` pattern for line-based validation - Added `collectLinesFullyCoveredByBlockComments` to properly handle test framework comment wrapping scenarios - Distinguished between three comment scenarios: lines fully within block comments, full-line comments, and lines ending with comments - Maintained all configuration options (ignores_empty_lines, ignores_comments) - Preserved exact violation position reporting with UTF-8 offset calculations --- CHANGELOG.md | 1 + .../Rules/Style/TrailingWhitespaceRule.swift | 345 +++++++++++++++--- 2 files changed, 293 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6eab7f23..cc3c23998 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ * `file_header` * `file_length` * `line_length` + * `trailing_whitespace` * `vertical_whitespace` [JP Simard](https://github.com/jpsim) diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/TrailingWhitespaceRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/TrailingWhitespaceRule.swift index 4e1e78d41..ff24517cd 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/TrailingWhitespaceRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Style/TrailingWhitespaceRule.swift @@ -1,7 +1,9 @@ import Foundation -import SourceKittenFramework +import SwiftLintCore +import SwiftSyntax -struct TrailingWhitespaceRule: CorrectableRule { +@SwiftSyntaxRule(correctable: true) +struct TrailingWhitespaceRule: Rule { var configuration = TrailingWhitespaceConfiguration() static let description = RuleDescription( @@ -14,77 +16,314 @@ struct TrailingWhitespaceRule: CorrectableRule { Example("let name: String //\n"), Example("let name: String // \n"), ], triggeringExamples: [ - Example("let name: String \n"), Example("/* */ let name: String \n") + Example("let name: String↓ \n"), Example("/* */ let name: String↓ \n") ], corrections: [ - Example("let name: String \n"): Example("let name: String\n"), - Example("/* */ let name: String \n"): Example("/* */ let name: String\n"), + Example("let name: String↓ \n"): Example("let name: String\n"), + Example("/* */ let name: String↓ \n"): Example("/* */ let name: String\n"), ] ) +} - func validate(file: SwiftLintFile) -> [StyleViolation] { - let filteredLines = file.lines.filter { - guard $0.content.hasTrailingWhitespace() else { return false } +private extension TrailingWhitespaceRule { + final class Visitor: ViolationsSyntaxVisitor { + // Pre-computed comment information for performance + private var linesFullyCoveredByBlockComments = Set() + private var linesEndingWithComment = Set() - let commentKinds = SyntaxKind.commentKinds - if configuration.ignoresComments, - let lastSyntaxKind = file.syntaxKindsByLines[$0.index].last, - commentKinds.contains(lastSyntaxKind) { + override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind { + // Pre-compute all comment information in a single pass if needed + if configuration.ignoresComments { + precomputeCommentInformation(node) + } + + // Process each line for trailing whitespace violations + for lineContents in file.lines { + let line = lineContents.content + let lineNumber = lineContents.index // 1-based + + // Calculate trailing whitespace info + guard let trailingWhitespaceInfo = line.trailingWhitespaceInfo() else { + continue // No trailing whitespace + } + + // Apply `ignoresEmptyLines` configuration + if configuration.ignoresEmptyLines && + line.trimmingCharacters(in: .whitespaces).isEmpty { + continue + } + + // Apply `ignoresComments` configuration + if configuration.ignoresComments { + // Check if line is fully within a block comment + if linesFullyCoveredByBlockComments.contains(lineNumber) { + continue + } + + // Check if line ends with a comment (using pre-computed info) + if linesEndingWithComment.contains(lineNumber) { + continue + } + } + + // Calculate violation position + let lineStartPos = locationConverter.position(ofLine: lineNumber, column: 1) + let violationStartOffset = line.utf8.count - trailingWhitespaceInfo.byteLength + let violationPosition = lineStartPos.advanced(by: violationStartOffset) + + let correctionEnd = lineStartPos.advanced(by: line.utf8.count) + + violations.append(ReasonedRuleViolation( + position: violationPosition, + correction: .init(start: violationPosition, end: correctionEnd, replacement: "") + )) + } + return .skipChildren + } + + /// Pre-computes all comment information in a single pass for better performance + private func precomputeCommentInformation(_ node: SourceFileSyntax) { + // First, collect block comment information + collectLinesFullyCoveredByBlockComments(node) + + // Then, collect line comment ranges and determine which lines end with comments + let lineCommentRanges = collectLineCommentRanges(from: node) + determineLineEndingComments(using: lineCommentRanges) + } + + /// Collects ranges of line comments organized by line number + private func collectLineCommentRanges(from node: SourceFileSyntax) -> [Int: [Range]] { + var lineCommentRanges: [Int: [Range]] = [:] + + for token in node.tokens(viewMode: .sourceAccurate) { + // Process leading trivia + var currentPos = token.position + for piece in token.leadingTrivia { + let pieceStart = currentPos + currentPos += piece.sourceLength + + if piece.isComment && !piece.isBlockComment { + let pieceStartLine = locationConverter.location(for: pieceStart).line + lineCommentRanges[pieceStartLine, default: []].append(pieceStart..]]) { + for lineNumber in 1...file.lines.count { + let line = file.lines[lineNumber - 1].content + + // Skip if no trailing whitespace + guard let trailingWhitespaceInfo = line.trailingWhitespaceInfo() else { + continue + } + + // Get the effective content (before trailing whitespace) + let effectiveContent = getEffectiveContent(from: line, removing: trailingWhitespaceInfo) + + // Check if the effective content ends with a comment + if checkIfContentEndsWithComment( + effectiveContent, + lineNumber: lineNumber, + lineCommentRanges: lineCommentRanges + ) { + linesEndingWithComment.insert(lineNumber) + } + } + } + + /// Gets the content of a line before its trailing whitespace + private func getEffectiveContent( + from line: String, + removing trailingWhitespaceInfo: TrailingWhitespaceInfo + ) -> String { + if trailingWhitespaceInfo.characterCount > 0 && line.count >= trailingWhitespaceInfo.characterCount { + return String(line.prefix(line.count - trailingWhitespaceInfo.characterCount)) + } + return "" + } + + /// Checks if the given content ends with a comment + private func checkIfContentEndsWithComment( + _ effectiveContent: String, + lineNumber: Int, + lineCommentRanges: [Int: [Range]] + ) -> Bool { + guard !effectiveContent.isEmpty, + let lastNonWhitespaceIdx = effectiveContent.lastIndex(where: { !$0.isWhitespace }) else { return false } - return !configuration.ignoresEmptyLines || - // If configured, ignore lines that contain nothing but whitespace (empty lines) - $0.content.trimmingCharacters(in: .whitespaces).isNotEmpty + // Calculate the byte position of the last non-whitespace character + let contentUpToLastChar = effectiveContent.prefix(through: lastNonWhitespaceIdx) + let byteOffsetToLastChar = contentUpToLastChar.utf8.count - 1 // -1 for position of char + let lineStartPos = locationConverter.position(ofLine: lineNumber, column: 1) + let lastNonWhitespacePos = lineStartPos.advanced(by: byteOffsetToLastChar) + + // Check if this position falls within any comment range on this line + if let ranges = lineCommentRanges[lineNumber] { + for range in ranges { + if range.lowerBound <= lastNonWhitespacePos && lastNonWhitespacePos < range.upperBound { + return true + } + } + } + + return false } - return filteredLines.map { - StyleViolation(ruleDescription: Self.description, - severity: configuration.severityConfiguration.severity, - location: Location(file: file.path, line: $0.index)) + /// Collects line numbers that are fully covered by block comments + private func collectLinesFullyCoveredByBlockComments(_ sourceFile: SourceFileSyntax) { + for token in sourceFile.tokens(viewMode: .sourceAccurate) { + var currentPos = token.position + + // Process leading trivia + for piece in token.leadingTrivia { + let pieceStartPos = currentPos + currentPos += piece.sourceLength + + if piece.isBlockComment { + markLinesFullyCoveredByBlockComment( + blockCommentStart: pieceStartPos, + blockCommentEnd: currentPos + ) + } + } + + // Advance past token content + currentPos = token.endPositionBeforeTrailingTrivia + + // Process trailing trivia + for piece in token.trailingTrivia { + let pieceStartPos = currentPos + currentPos += piece.sourceLength + + if piece.isBlockComment { + markLinesFullyCoveredByBlockComment( + blockCommentStart: pieceStartPos, + blockCommentEnd: currentPos + ) + } + } + } } - } - func correct(file: SwiftLintFile) -> Int { - let whitespaceCharacterSet = CharacterSet.whitespaces - var correctedLines = [String]() - var numberOfCorrections = 0 - for line in file.lines { - guard line.content.hasTrailingWhitespace() else { - correctedLines.append(line.content) - continue + /// Marks lines that are fully covered by a block comment + private func markLinesFullyCoveredByBlockComment( + blockCommentStart: AbsolutePosition, + blockCommentEnd: AbsolutePosition + ) { + let startLocation = locationConverter.location(for: blockCommentStart) + let endLocation = locationConverter.location(for: blockCommentEnd) + + let startLine = startLocation.line + var endLine = endLocation.line + + // If comment ends at column 1, it actually ended on the previous line + if endLocation.column == 1 && endLine > startLine { + endLine -= 1 } - let commentKinds = SyntaxKind.commentKinds - if configuration.ignoresComments, - let lastSyntaxKind = file.syntaxKindsByLines[line.index].last, - commentKinds.contains(lastSyntaxKind) { - correctedLines.append(line.content) - continue - } + for lineNum in startLine...endLine { + if lineNum <= 0 || lineNum > file.lines.count { continue } - let correctedLine = line.content.bridge() - .trimmingTrailingCharacters(in: whitespaceCharacterSet) + let lineInfo = file.lines[lineNum - 1] + let lineContent = lineInfo.content + let lineStartPos = locationConverter.position(ofLine: lineNum, column: 1) - if configuration.ignoresEmptyLines && correctedLine.isEmpty { - correctedLines.append(line.content) - continue - } + // Check if the line's non-whitespace content is fully within the block comment + if let firstNonWhitespaceIdx = lineContent.firstIndex(where: { !$0.isWhitespace }), + let lastNonWhitespaceIdx = lineContent.lastIndex(where: { !$0.isWhitespace }) { + // Line has non-whitespace content + // Calculate byte offsets (not character offsets) for AbsolutePosition + let contentBeforeFirstNonWS = lineContent.prefix(upTo: firstNonWhitespaceIdx) + let byteOffsetToFirstNonWS = contentBeforeFirstNonWS.utf8.count + let firstNonWhitespacePos = lineStartPos.advanced(by: byteOffsetToFirstNonWS) - if file.ruleEnabled(violatingRanges: [line.range], for: self).isEmpty { - correctedLines.append(line.content) - continue - } + let contentBeforeLastNonWS = lineContent.prefix(upTo: lastNonWhitespaceIdx) + let byteOffsetToLastNonWS = contentBeforeLastNonWS.utf8.count + let lastNonWhitespacePos = lineStartPos.advanced(by: byteOffsetToLastNonWS) - if line.content != correctedLine { - numberOfCorrections += 1 + // Check if both first and last non-whitespace positions are within the comment + if firstNonWhitespacePos >= blockCommentStart && lastNonWhitespacePos < blockCommentEnd { + linesFullyCoveredByBlockComments.insert(lineNum) + } + } else { + // Line is all whitespace - check if it's within the comment bounds + let lineEndPos = lineStartPos.advanced(by: lineContent.utf8.count) + if lineStartPos >= blockCommentStart && lineEndPos <= blockCommentEnd { + linesFullyCoveredByBlockComments.insert(lineNum) + } + } } - correctedLines.append(correctedLine) } - if numberOfCorrections > 0 { - // join and re-add trailing newline - file.write(correctedLines.joined(separator: "\n") + "\n") - } - return numberOfCorrections + } +} + +// Helper struct to return both character count and byte length for whitespace +private struct TrailingWhitespaceInfo { + let characterCount: Int + let byteLength: Int +} + +private extension String { + func hasTrailingWhitespace() -> Bool { + if isEmpty { return false } + guard let lastScalar = unicodeScalars.last else { return false } + return CharacterSet.whitespaces.contains(lastScalar) + } + + /// Returns information about trailing whitespace (spaces and tabs only) + func trailingWhitespaceInfo() -> TrailingWhitespaceInfo? { + var charCount = 0 + var byteLen = 0 + for char in self.reversed() { + if char.isWhitespace && (char == " " || char == "\t") { // Only count spaces and tabs + charCount += 1 + byteLen += char.utf8.count + } else { + break + } + } + return charCount > 0 ? TrailingWhitespaceInfo(characterCount: charCount, byteLength: byteLen) : nil + } + + func trimmingTrailingCharacters(in characterSet: CharacterSet) -> String { + var end = endIndex + while end > startIndex { + let index = index(before: end) + if !characterSet.contains(self[index].unicodeScalars.first!) { + break + } + end = index + } + return String(self[..