Fix opening_brace correction
#5620
Conversation
Generated by 🚫 Danger |
swiftty
force-pushed
the
fix/opening_brace-correction
branch
from
b6737c7 to
700c674
Compare
Source/SwiftLintCore/Protocols/SwiftSyntaxCorrectableRule.swift
Outdated
Show resolved
Hide resolved
| @@ -273,6 +273,7 @@ extension SwiftLintFile { | |||
| return violatingRanges.filter { range in | |||
| let region = fileRegions.first { | |||
| $0.contains(Location(file: self, characterOffset: range.location)) | |||
| || $0.contains(Location(file: self, characterOffset: range.location + range.length - 1)) | |||
There was a problem hiding this comment.
While this seems like a reasonable change, it has the potential of changing other corrections of rules as well (which we don't see in the OSS report). I'd rather like to avoid that. This will anyway become a thing if the past once these rules are converted to SwiftSyntax rules.
Actually, the decision if a rule is enabled or not should be the same for violations and corrections. In your example, a violations is reported, but no fix is applied. That's inconsistent. So we have to make sure that the "Is rule enabled?" check is implemented in the same way or at least can rely on the same positions in the "reporting" and "fix" cases, probably by extending ViolationCorrection with a violationPosition. Later, ReasonedRuleViolation and ViolationCorrection can even be merged.
…location" This reverts commit 15f7d17.
SimplyDanny
force-pushed
the
fix/opening_brace-correction
branch
from
700c674 to
e8b0b61
Compare
|
@swiftty, sorry for being too late with my review here. Parts of your PR had already been fixed by other measures or required more extensive refactoring. Thank you nevertheless for the valuable contribution! Please keep up the good work. 😊 I rebased the branch, so that everything should be good to go in a while. |
|
@SimplyDanny, I appreciate your courteous response in fixing the code and merging the PR. |
opening_bracefix is broken #5598 and disable command ignored on correcting