Migrate StatementPositionRule from SourceKit to SwiftSyntax #6116
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-statementpositionrule-from-sourcekit-to-swiftsyntax
branch
from
11a125c to
a021d60
Compare
Generated by 🚫 Danger |
jpsim
force-pushed
the
migrate-statementpositionrule-from-sourcekit-to-swiftsyntax
branch
from
50caf3d to
0d58b9f
Compare
SimplyDanny
reviewed
Jun 24, 2025
| uncuddledCorrect(file: file) | ||
| private static func calculateIndentation(_ trivia: Trivia) -> Int { | ||
| var indentation = 0 | ||
| for piece in trivia.reversed() { |
There was a problem hiding this comment.
To calculate the indentation of the last line in the trivia, not the first. I'll add a comment.
There was a problem hiding this comment.
Without the reversed(), all tests still pass. So we should probably have a test case that makes sure reversed order is required.
Source/SwiftLintBuiltInRules/Rules/Style/StatementPositionRule.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/Style/StatementPositionRule.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/Style/StatementPositionRuleExamples.swift
Show resolved
Hide resolved
|
Wondering about the fixed violations in the Swift repository. The code doesn't look like it shouldn't trigger the rule anymore as both |
Not sure if you're saying this is a good thing or a bad thing. |
Convert StatementPositionRule to use SwiftSyntax instead of SourceKit for improved performance and more accurate positioning validation. The SwiftSyntax implementation: - Uses ViolationsSyntaxVisitor and ViolationsSyntaxRewriter patterns - Validates else/catch keyword positioning with proper indentation checks - Supports both default (same line) and uncuddled (next line) modes - Correctly handles trailing/leading trivia for whitespace validation - Implements corrections using explicit rewriter for both statement types - Extracts common validation logic to reduce code duplication - Skip correcting when there are comments
jpsim
force-pushed
the
migrate-statementpositionrule-from-sourcekit-to-swiftsyntax
branch
from
63417be to
8524128
Compare
Oh sorry, I think that these examples should still trigger a violation and not appear as "fixed" with the rewrite. |
Well really it's that these two violations merged and moved to a different location: Before
After
|
But in my understanding, all three lines should still trigger, because the keyword is always after the closing brace. |
Instead of at the preceding closing brace.
This was really a matter of violation location, but I think it makes more sense to report violations on the |
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.
Convert StatementPositionRule to use SwiftSyntax instead of SourceKit for
improved performance and more accurate positioning validation.
The SwiftSyntax implementation: