Migrate VerticalWhitespaceOpeningBracesRule to SwiftSyntax #6119
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jpsim
force-pushed
the
migrate-verticalwhitespaceopeningbracesrule-to-swiftsyntax
branch
from
27e3c77 to
b690c0c
Compare
Generated by 🚫 Danger |
jpsim
force-pushed
the
migrate-verticalwhitespaceopeningbracesrule-to-swiftsyntax
branch
from
78d1b0c to
ea4a254
Compare
## 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
SimplyDanny
force-pushed
the
migrate-verticalwhitespaceopeningbracesrule-to-swiftsyntax
branch
from
ea4a254 to
af63388
Compare
|
Rebased to get the better summary. |
|
The difference to the previous implementation are examples like func f() { // comment
}with comments after the opening brace. They trigger now. Another difference is when the f { foo,
bar in
}The previous implementation didn't trigger on the empty line, the new implementation does. I think that both cases are acceptable without introducing an option since there are only ~20 examples of either nature in the scanned repositories. |
SimplyDanny
reviewed
Jun 29, 2025
| } | ||
| } | ||
|
|
||
| private extension TriviaPiece { |
There was a problem hiding this comment.
We should move this to SwiftSyntax+SwiftLint.swift and reused it in other rules.
| } | ||
|
|
||
| private extension VerticalWhitespaceOpeningBracesRule { | ||
| final class Visitor: ViolationsSyntaxVisitor<SeverityConfiguration<VerticalWhitespaceOpeningBracesRule>> { |
There was a problem hiding this comment.
Suggested change
| final class Visitor: ViolationsSyntaxVisitor<SeverityConfiguration<VerticalWhitespaceOpeningBracesRule>> { | |
| final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> { |
| } | ||
| } | ||
|
|
||
| final class Rewriter: ViolationsSyntaxRewriter<SeverityConfiguration<VerticalWhitespaceOpeningBracesRule>> { |
There was a problem hiding this comment.
Suggested change
| final class Rewriter: ViolationsSyntaxRewriter<SeverityConfiguration<VerticalWhitespaceOpeningBracesRule>> { | |
| final class Rewriter: ViolationsSyntaxRewriter<ConfigurationType> { |
Comment on lines
+411
to
+422
| 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) |
There was a problem hiding this comment.
These blocks look all very similar. Common parts could be extracted into a method.
| // Comment explaining the logic | ||
| performAction() | ||
| } | ||
| """), |
There was a problem hiding this comment.
I tried with another example:
if x == 5 { // comment
print("x is 5")
}The violation triggers at the comment and the correction removes on of the leading slashes //.
SimplyDanny
reviewed
Jun 29, 2025
| } | ||
| } | ||
|
|
||
| final class Rewriter: ViolationsSyntaxRewriter<SeverityConfiguration<VerticalWhitespaceOpeningBracesRule>> { |
There was a problem hiding this comment.
The rewriter is currently not used at all since the visitor does the replacement already and the rule doesn't specify explicitRewriter: true.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Convert VerticalWhitespaceOpeningBracesRule to use SwiftSyntax instead
of SourceKit for improved performance and better detection of empty
lines after opening braces.
Key Technical Improvements
opening braces
detection
CRLF) consistently
better accuracy
(
{,[,()Migration Details
CorrectableRule, OptInRulewith@SwiftSyntaxRule(correctable: true, optIn: true)ViolationsSyntaxVisitorfor detecting violations intoken trailing trivia
ViolationsSyntaxRewriterfor correcting violationsacross different syntax contexts