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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow parens for calls in range literals (Style/MethodCallWithArgsParentheses) #6763

Merged

Conversation

gsamokovarov
Copy link
Contributor

With the style of omit_parentheses in the Style/MethodCallWithArgsParentheses cop, we got:

Before:

1..limit(n)
        ^^^ Omit parentheses for method calls with arguments.

After:

1..limit(n)

馃憣

expect_no_offenses(<<-RUBY)
1..limit(n)
RUBY
end
Copy link
Member

Choose a reason for hiding this comment

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

Looks good! On the other hand, there is also erange in the similar case :-)

1...limit(n)

Would you please add along with the CHANGELOG entry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, didn't know that. Thanks for the catch!

@gsamokovarov gsamokovarov force-pushed the omit-parentheses-in-range-literals branch 3 times, most recently from 4611c7b to f2216da Compare February 14, 2019 08:43
@@ -259,13 +259,16 @@ def legitimate_call_with_parentheses?(node)
allowed_chained_call_with_parentheses?(node)
end

# rubocop:disable Metrics/CyclomaticComplexity
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole omit_parentheses style is based around the many edge-cases where we actually do need the parentheses. I have tried to cater to the Metrics/CyclomaticComplexity until now, but the extra helpers I introduce to hide the conditions blow up the line length cop. I think the code will be clearer if we don't measure the cyclomatic complexity for this whole cop and eventually inline the helpers where they are used.

Copy link
Member

@koic koic Feb 15, 2019

Choose a reason for hiding this comment

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

Introduction of range_type? meaning both irange_type? and erange_type? is proposed.
#6126 (comment)

We will be able to remove the disabling cop comment when replaced by this proposal . So I think this is fine.

Copy link
Member

Choose a reason for hiding this comment

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

#6773 will remove the disabling cop comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great!

Before:

```ruby
1..limit(n)
        ^^^ Omit parentheses for method calls with arguments.
```

After:

```ruby
1..limit(n)
```

馃憣
@gsamokovarov gsamokovarov force-pushed the omit-parentheses-in-range-literals branch from f2216da to 9fc434e Compare February 14, 2019 11:59
@koic koic merged commit d64cdb5 into rubocop:master Feb 15, 2019
@koic
Copy link
Member

koic commented Feb 15, 2019

Thanks!

@gsamokovarov gsamokovarov deleted the omit-parentheses-in-range-literals branch February 15, 2019 09:15
@Drenmi Drenmi mentioned this pull request Feb 19, 2019
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

3 participants