Add unused_control_flow_label rule #2545
Conversation
|
Thanks for doing this! I haven't reviewed yet, but here are some edge cases we should make sure to support: if #available(iOS 11, *) {} else {
// Impossible to negate an `#available` condition
}
if case .loading = state {} else {
// Impossible to negate a case binding
} |
|
Oops, disregard my comment, I completely misunderstood what this rule was validating 😅 |
Generated by 🚫 Danger |
There was a problem hiding this comment.
I added a shitty implementation of correction if you're interested in using it as a prototype for adding real correction support. See unused_control_flow_label-shitty-correctable
CHANGELOG.md
Outdated
| @@ -27,6 +27,11 @@ | |||
| `.last(where: { /* ... */ })` is more efficient. | |||
| [Marcelo Fabri](https://github.com/marcelofabri) | |||
|
|
|||
| * Add `unused_control_flow_label` rule to validate that control flow labels are | |||
| being used. | |||
|
I thought about adding autocorrection but given how rare it’s to use control labels, I thought that maybe it would be better to let a human evaluate if the right fix is to remove the label (vs using it) |
|
IMO it’s easier to review corrections & revert if necessary than to go through the warnings and manually fix them. |
|
Fair enough, will do that later 👍 |
|
Pushed 41868b3 with a few changes on your prototype (I wouldn't call it shitty though 😅) |
|
Thanks! Gonna do another quick review pass. |
|
Ran OSSCheck locally since performance numbers from CI were concerning, but got -3% to +3%. |
and inline firstToken(afterByteOffset:)
They're already ordered last to first in the file from the sorting applied in `violationRanges(in file:)`
|
Thanks @jpsim 🎉 |
Fixes #2227
Adding as a default rule because I think there shouldn't be false positives. Let's wait for oss-check.