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 an incorrect autocorrect for FactoryBot/ConsistentParenthesesStyle with omit_parentheses option when method name and first argument are not on same line #1443

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

ydah
Copy link
Member

@ydah ydah commented Oct 25, 2022

Fix: #1442

This PR is following cases where the method name and the first argument are different line are not considered offense.

create(
  :user
)
build(
  :user,
  name: 'foo'
)

This is because the absence of parentheses will result in a syntax error.

create
  :user

build
  :user,
  name: 'foo'

In that case, the following modification would be necessary, and we have chosen not to make it an offense here, since we would have to suppress either offense if there is a length limit in Layout/LineLength, etc.

create :user

build :user,
  name: 'foo'

Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Updated documentation.
  • Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

@ydah ydah changed the title Fix an incorrect autocorrect for FactoryBot/ConsistentParenthesesStyle with omit_parentheses option when method and first argument are not on same line Fix an incorrect autocorrect for FactoryBot/ConsistentParenthesesStyle with omit_parentheses option when method name and first argument are not on same line Oct 25, 2022
@ydah ydah force-pushed the fix/1442 branch 2 times, most recently from 45d08e4 to 7c8e11a Compare October 25, 2022 12:41
…le` with `omit_parentheses` option when method name and first argument are not on same line

Fix: rubocop#1442

This PR is following cases where the method name and the first argument are different line are not considered offense.
```ruby
create(
  :user
)
build(
  :user,
  name: 'foo'
)
```

This is because the absence of parentheses will result in a syntax error.
```ruby
create
  :user

build
  :user,
  name: 'foo'
```

In that case, the following modification would be necessary, and we have chosen not to make it an offense here, since we would have to suppress either offense if there is a length limit in Layout/LineLength, etc.

```ruby
create :user

build :user,
  name: 'foo'
```
Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix!

@pirj
Copy link
Member

pirj commented Oct 25, 2022

In general, I agree that this is the way to quick-fix the problem with autocorrection.
However, I'm on the fence if we should still raise an offence, and probably put the first argument on the same line during autocorrection.
Or introduce 'IgnoreMultiline` option.
@bquorning @Darhazer WDYT?

@bquorning
Copy link
Collaborator

bquorning commented Oct 25, 2022

I think this is fine for a bugfix. We can consider the IgnoreMultiline option in a separate PR.

@pirj pirj merged commit 6d3649e into rubocop:master Oct 25, 2022
@pirj
Copy link
Member

pirj commented Oct 25, 2022

@bquorning I take your last comment as an approval.

@ydah Thank you!

bquorning pushed a commit that referenced this pull request Oct 25, 2022
Fix an incorrect autocorrect for `FactoryBot/ConsistentParenthesesStyle` with `omit_parentheses` option when method name and first argument are not on same line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants