Skip to content
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

A bad manual correction of the Style/SafeNavigation cop isn't raised as now redundant #10926

Closed
mkissarli opened this issue Aug 16, 2022 · 0 comments · Fixed by #10929
Closed

Comments

@mkissarli
Copy link

This request came about due to an incorrectly formatted piece of code which followed the pattern:

a && a.foo

This was correctly flagged up as an offence of Style/SafeNavigation, and so was manually adjusted to:

a && a&.foo

which was not flagged up any further, despite the now redundant nil check.

Describe the solution you'd like

I think in this situation there should be a further offence flagged up for the now redundant nil check. For example:

a && a&.foo

should cause a Style/RedundantNilCheck flag (or something similar), which itself can be fixed with:

a&.foo

Describe alternatives you've considered

Writing a custom cop for this, although I think it would make a good default.

koic added a commit to koic/rubocop that referenced this issue Aug 17, 2022
…nil check

Fixes rubocop#10926.

This PR makes `Style/SafeNavigation` aware of a redundant nil check.

IMHO, the context of this patch is a minimal update.
OTOH, a new cop like `Style/RedundantNilCheck` cop may be implemented
if it is not suitable for this `Style/SafeNavigation` cop.
(But I'm not sure if it's the best.)
bbatsov pushed a commit that referenced this issue Aug 19, 2022
Fixes #10926.

This PR makes `Style/SafeNavigation` aware of a redundant nil check.

IMHO, the context of this patch is a minimal update.
OTOH, a new cop like `Style/RedundantNilCheck` cop may be implemented
if it is not suitable for this `Style/SafeNavigation` cop.
(But I'm not sure if it's the best.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant