Migrate VerticalWhitespaceClosingBracesRule to SwiftSyntax #6118
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-verticalwhitespaceclosingbracesrule-to-swiftsyntax
branch
from
3821d9b to
a45aa1f
Compare
Generated by 🚫 Danger |
jpsim
force-pushed
the
migrate-verticalwhitespaceclosingbracesrule-to-swiftsyntax
branch
from
a45aa1f to
5353d40
Compare
## Summary Convert VerticalWhitespaceClosingBracesRule to use SwiftSyntax instead of SourceKit for improved performance and better detection of empty lines before closing braces. ## Key Technical Improvements - **Enhanced trivia analysis** for accurate empty line detection before closing braces - **Proper position calculation** using positionAfterSkippingLeadingTrivia for line numbers - **Improved trivial line detection** supporting any combination of closing braces (`}`, `]`, `)`) - **SwiftSyntax visitor pattern** replacing regex-based detection for better accuracy - **Unified correction logic** handling all newline types (LF, CR, CRLF) consistently ## Migration Details - Replaced `CorrectableRule, OptInRule` with `@SwiftSyntaxRule(correctable: true, optIn: true)` - Implemented `ViolationsSyntaxVisitor` for detecting violations in token leading trivia - Implemented `ViolationsSyntaxRewriter` for correcting violations by modifying trivia - Added proper handling of `only_enforce_before_trivial_lines` configuration option - Maintained exact position reporting for violation locations - Preserved all existing test cases and rule behavior
SimplyDanny
force-pushed
the
migrate-verticalwhitespaceclosingbracesrule-to-swiftsyntax
branch
from
5353d40 to
7dae4c2
Compare
|
Rebased to get the better summary. |
SimplyDanny
reviewed
Jun 29, 2025
| withTemplate: "$2" | ||
| } | ||
|
|
||
| final class Rewriter: ViolationsSyntaxRewriter<VerticalWhitespaceClosingBracesConfiguration> { |
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.
| } | ||
|
|
||
| private extension VerticalWhitespaceClosingBracesRule { | ||
| final class Visitor: ViolationsSyntaxVisitor<VerticalWhitespaceClosingBracesConfiguration> { |
There was a problem hiding this comment.
Suggested change
| final class Visitor: ViolationsSyntaxVisitor<VerticalWhitespaceClosingBracesConfiguration> { | |
| final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> { |
|
|
||
| let patternRegex: NSRegularExpression = regex(pattern) | ||
| private func isTokenLineTrivialHelper( |
There was a problem hiding this comment.
What does the "helper" suffix stand for?
|
With this rewrite, we see new violations on empty lines preceded by a comment, that is: That's reasonable. So all OSS findings are fine. |
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 VerticalWhitespaceClosingBracesRule to use SwiftSyntax instead
of SourceKit for improved performance and better detection of empty
lines before closing braces.
Key Technical Improvements
before closing braces
positionAfterSkippingLeadingTrivia for line numbers
closing braces (
},],))better accuracy
CRLF) consistently
Migration Details
CorrectableRule, OptInRulewith@SwiftSyntaxRule(correctable: true, optIn: true)ViolationsSyntaxVisitorfor detecting violations intoken leading trivia
ViolationsSyntaxRewriterfor correcting violations bymodifying trivia
only_enforce_before_trivial_linesconfiguration option