New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new prefer_key_path
rule
#5548
base: main
Are you sure you want to change the base?
Conversation
@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of? |
247038c
to
ff7a7e3
Compare
ff7a7e3
to
71a5b62
Compare
Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings. |
So most hits look good, but I think this might be a false alarm (where there is no property named
|
The rule would complain about the closure and suggest |
71a5b62
to
d75d058
Compare
@@ -61,6 +61,11 @@ | |||
[SimplyDanny](https://github.com/SimplyDanny) | |||
[#70](https://github.com/realm/SwiftLint/issues/70) | |||
|
|||
* Add new `prefer_key_path` rule that triggers when a trailing closure on a function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be mentioned that the rule is opt-in or disabled by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that's not necessary as it can be looked up in the Rule Directory.
This new rule triggers on code like
which can be replaced by
as of Swift 5.2.
Automatic fixes fail once the argument label for the closure is named. The fix would need to know its name and insert it in the function call. But that's impossible for a syntax-based rule. We could do auto-fixes for simple cases like
map
andfilter
(which represent most of the findings in OSS projects and SwiftLint itself) at least. I'm not sure if partial fixes are a good thing though.