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

Add new Lint/RequireRangeParentheses cop #10792

Merged

Conversation

koic
Copy link
Member

@koic koic commented Jul 6, 2022

Context

It emulates the following Ruby warning.

% cat example.rb
1...
2
% ruby example.rb
example.rb:1: warning: ... at EOL, should be parenthesized?

So, this cop will detect case like #10789.

Summary

It checks that a range literal is enclosed in parentheses when the end of the range is at a line break.

example

# bad - Represents `(1..42)`, not endless range.
1..
42

# good - It's incompatible, but your intentions when using endless range may be:
(1..)
42

# good
1..42

# good
(1..42)

# good
(1..
42)

NOTE: The following is maybe intended for (42..). But, compatible is 42..do_something. So, this cop does not provide autocorrection because it is left to user.

case condition
when 42..
  do_something
end

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • 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.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@koic koic force-pushed the add_new_lint_require_range_parentheses_cop branch 4 times, most recently from a263284 to 9911536 Compare July 6, 2022 09:07
# Checks that the range is enclosed in parentheses when the end of the range is line break.
#
# NOTE: The following is maybe intended for `(42..)`. But, compatible is `42..do_something`.
# So, this cop does not provide autocollection because it is left to user.
Copy link
Collaborator

Choose a reason for hiding this comment

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

autocollection -> autocorrection

#
# @example
#
# # bad
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might also add comments for the bad/good cases so it's clearer what they mean.

module RuboCop
module Cop
module Lint
# Checks that the range is enclosed in parentheses when the end of the range is line break.
Copy link
Collaborator

Choose a reason for hiding this comment

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

that a range literal
is at a line break

# 42
#
class RequireRangeParentheses < Base
MSG = 'Use parentheses in the endless range to avoid confusion about precedence.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrap the endless range literal %s to avoid precedence ambiguity.

@koic koic force-pushed the add_new_lint_require_range_parentheses_cop branch from 9911536 to 8aed359 Compare July 7, 2022 08:02
@koic
Copy link
Member Author

koic commented Jul 7, 2022

I've updated this PR. Thank you!

## Context

It emulates the following Ruby warning.

```console
% cat example.rb
1...
2
```

```consle
% ruby example.rb
example.rb:1: warning: ... at EOL, should be parenthesized?
```

So, this cop will detect case like rubocop#10789.

## Summary

It checks that a range literal is enclosed in parentheses when the end of the range is
at a line break.

### example

```ruby
# bad - Represents `(1..42)`, not endless range.
1..
42

# good - It's incompatible, but your intentions when using endless range may be:
(1..)
42

# good
1..42

# good
(1..42)

# good
(1..
42)
```

NOTE: The following is maybe intended for `(42..)`. But, compatible is `42..do_something`.
So, this cop does not provide autocorrection because it is left to user.

```ruby
case condition
when 42..
  do_something
end
```
@koic koic force-pushed the add_new_lint_require_range_parentheses_cop branch from 8aed359 to 3c96b2a Compare July 7, 2022 08:34
@bbatsov bbatsov merged commit 0ad09e7 into rubocop:master Jul 7, 2022
koic added a commit that referenced this pull request Jul 7, 2022
Follow up #10792.

This commit suppresses the following `Lint/RequireRangeParentheses` offense.

```console
% bundle exec rubocop
(snip)

Offenses:

lib/rubocop/cop/mixin/percent_array.rb:101:13: W:
Lint/RequireRangeParentheses: Wrap the endless range literal
node.children[0].location.expression.end_pos... to avoid precedence
ambiguity.
            node.children[0].location.expression.end_pos... ...
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```
@koic koic deleted the add_new_lint_require_range_parentheses_cop branch July 7, 2022 15:27
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