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

Fix false positives for Lint/ParenthesesAsGroupedExpression #8039

Conversation

CamilleDrapier
Copy link

@CamilleDrapier CamilleDrapier commented May 26, 2020

This PR fixes the following false positive for Lint/ParenthesesAsGroupedExpression

  • when an expression is followed by an operator
func (x) || y
  • when an expression is followed by a chain of functions
func (x).func.func.func.func.func

This PR adds some reproduction tests.


I was initially thinking of changing the auto-correct to produce things such as func((x) || y) and func((x).func.func.func.func.func), but since we probably do not want eq (x).to_i to become suddenly eq((x).to_i) (#7909), I went with ignoring these pasterns. This is because the current auto-correct behavior breaks the code for these paterns. Please do tell me if we would prefer going in another direction 🙏

I'm not very comfortable with these language nodes, so please treat this PR as an humble attempt 🙏


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@CamilleDrapier CamilleDrapier force-pushed the fix_auto_correct_parentheses_as_grouped_expression branch from 72093c4 to af7e9e4 Compare May 26, 2020 11:40
@CamilleDrapier CamilleDrapier changed the title [Lint/ParenthesesAsGroupedExpression] Fix false positives for Lint/ParenthesesAsGroupedExpression [Lint/ParenthesesAsGroupedExpression] Fix false positives for expressions followed by an operator or a chain of functions May 26, 2020
@CamilleDrapier CamilleDrapier changed the title [Lint/ParenthesesAsGroupedExpression] Fix false positives for expressions followed by an operator or a chain of functions [Lint/ParenthesesAsGroupedExpression] Fix false positives May 26, 2020
@CamilleDrapier CamilleDrapier changed the title [Lint/ParenthesesAsGroupedExpression] Fix false positives Fix false positives for Lint/ParenthesesAsGroupedExpression May 26, 2020
@CamilleDrapier CamilleDrapier force-pushed the fix_auto_correct_parentheses_as_grouped_expression branch 2 times, most recently from d5f577a to b2bada9 Compare May 26, 2020 12:03
CHANGELOG.md Outdated Show resolved Hide resolved
@CamilleDrapier CamilleDrapier force-pushed the fix_auto_correct_parentheses_as_grouped_expression branch from cae5dd2 to 6696334 Compare July 1, 2020 05:32
This PR fixes the following false positive for `Lint/ParenthesesAsGroupedExpression`

when an expression is followed by an operator

```ruby
func (x) || y
```

when an expression is followed by a chain of functions

```ruby
func (x).func.func.func.func.func
```

This PR adds some reproduction test
@CamilleDrapier CamilleDrapier force-pushed the fix_auto_correct_parentheses_as_grouped_expression branch from 6696334 to 5421042 Compare July 1, 2020 05:46
@CamilleDrapier CamilleDrapier requested a review from koic July 1, 2020 06:00
@koic koic merged commit 0cad1a2 into rubocop:master Jul 1, 2020
@koic
Copy link
Member

koic commented Jul 1, 2020

Thanks!

@CamilleDrapier CamilleDrapier deleted the fix_auto_correct_parentheses_as_grouped_expression branch July 31, 2020 04:22
koic added a commit to koic/rubocop that referenced this pull request Jun 17, 2022
…edExpression`

Fixes rubocop#10719 and follow up rubocop#8039.

This PR fixes a false positive for `Lint/ParenthesesAsGroupedExpression`
when using safe navigation operator.
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 this pull request may close these issues.

None yet

2 participants