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 incorrect shorthand autocorrections in calls inside parentheses #11643

Merged
merged 1 commit into from
Mar 2, 2023

Conversation

gsamokovarov
Copy link
Contributor

@gsamokovarov gsamokovarov commented Mar 1, 2023

If you happen to have a method call without parentheses that is inside outer parentheses like the following example:

(create :integration_shop, account: account)
pass

We would have still thought that we need to add parentheses to the create call during Style/HashSyntax autocorrection like so:

(create(:integration_shop, account:))
pass

While this is not a problem in isolation, it can lead to invalid Ruby autocorrections when Style/HashSyntax is used in combination with Style/RedundantParentheses (enabled by default) and Style/MethodCallWithArgsParentheses (disabled by default) with EnforcedStyle: omit_parentheses:

AllCops:
  TargetRubyVersion: 3.1
Style/HashSyntax:
  EnforcedShorthandSyntax: always
Style/RedundantParentheses:
  Enabled: true
Style/MethodCallWithArgsParentheses:
  Enabled: true
  EnforcedStyle: omit_parentheses

I have two problematic examples:

1.

receipt = create :receipt, account: account, integration_shop: (create :integration_shop, account: account)
pass

Autocorrects to broken Ruby like so:

receipt = create :receipt, account:, integration_shop: create :integration_shop, account:
pass

This happens because Style/HashSyntax puts parentheses in (create(:integration_shop, account:)), which then gets autocorrected by Style/MethodCallWithArgsParentheses to (create :integration_shop, account:) that finally gets processed by Style/RedundantParentheses to create :integration_shop, account:.

2.

bank_transaction_ids = (MatchBankTransactionsToCosts.execute object, bank_account: bank_account).keys
pass

Gets autocorrected to:

bank_transaction_ids = MatchBankTransactionsToCosts.execute object, bank_account:.keys
pass

The reason is the same, the unforunate play between Style/HashSyntax, Style/MethodsAcceptingSymbol with EnforcedStyle: omit_parentheses and Style/RedundantParentheses.

An fix is not to parenthesize the calls with value omission if they are inside parentheses. That's one of the drawbacks of omitting parentheses, sometimes folks decide to put them in funny places like around a method call.

If you happen to have a method call without parentheses that is inside
outer parentheses like the following example:

```ruby
(create :integration_shop, account: account)
```

We would have still thought that we need to parentheses the `create`
call during `Style/HashSyntax` autocorrection like so:

```ruby
(create(:integration_shop, account:))
```

While this is not a problem in isolation, it can lead to problems with
`Style/RedundantParentheses` and `Style/MethodCallWithArgsParentheses`
with the following config:

```yaml
AllCops:
  TargetRubyVersion: 3.1
Style/HashSyntax:
  EnforcedShorthandSyntax: always
Style/RedundantParentheses:
  Enabled: true
Style/MethodCallWithArgsParentheses:
  Enabled: true
  EnforcedStyle: omit_parentheses
```

I have two problematic examples:

**1.**

```ruby
receipt = create :receipt, account: account, integration_shop: (create :integration_shop, account: account)
pass
```

Autocorrects to broken Ruby like so:

```ruby
receipt = create :receipt, account:, integration_shop: create :integration_shop, account:
pass
```

This happens because `Style/HashSyntax` puts parentheses in
`(create(:integration_shop, account:))`, which then gets
autocorrected by `Style/MethodCallWithArgsParentheses` to
`(create :integration_shop, account:)` that finally gets picked up by
`Style/RedundantParentheses` to `create :integration_shop, account:`.

**2.**

```ruby
bank_transaction_ids = (MatchBankTransactionsToCosts.execute object, bank_account: bank_account).keys
pass
```

Gets autocorrected to:

```ruby
bank_transaction_ids = MatchBankTransactionsToCosts.execute object, bank_account:.keys
pass
```

The reason is the same, the unforunate play between `Style/HashSyntax`,
`Style/MethodsAcceptingSymbol` with `EnforcedStyle: omit_parentheses`
and `Style/RedundantParentheses`.

An fix is not to parenthesize the calls with value omission if they are
inside parentheses. That's one of the drawbacks of omitting parentheses,
sometimes folks decide to put them in funny places like around a
method call.
@koic koic merged commit 6a0d195 into rubocop:master Mar 2, 2023
@koic
Copy link
Member

koic commented Mar 2, 2023

Thanks!

@gsamokovarov gsamokovarov deleted the shorthand-syntax-in-kwargs branch March 2, 2023 06:42
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