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

Lint/EmptyWhen: false positive with range #10789

Closed
kazzix14 opened this issue Jul 5, 2022 · 4 comments
Closed

Lint/EmptyWhen: false positive with range #10789

kazzix14 opened this issue Jul 5, 2022 · 4 comments

Comments

@kazzix14
Copy link

kazzix14 commented Jul 5, 2022

Expected behavior

RuboCop does not report an error.

Actual behavior

RuboCop does report an error that seemed to be invalid.

root@5654c24326e8:/tmp/repro# bundle exec rubocop
The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file.

Please also note that you can opt-in to new cops by default by adding this to your config:
  AllCops:
    NewCops: enable

Gemspec/DeprecatedAttributeAssignment: # new in 1.30
  Enabled: true
Gemspec/RequireMFA: # new in 1.23
  Enabled: true
Layout/LineContinuationLeadingSpace: # new in 1.31
  Enabled: true
Layout/LineContinuationSpacing: # new in 1.31
  Enabled: true
Layout/LineEndStringConcatenationIndentation: # new in 1.18
  Enabled: true
Layout/SpaceBeforeBrackets: # new in 1.7
  Enabled: true
Lint/AmbiguousAssignment: # new in 1.7
  Enabled: true
Lint/AmbiguousOperatorPrecedence: # new in 1.21
  Enabled: true
Lint/AmbiguousRange: # new in 1.19
  Enabled: true
Lint/ConstantOverwrittenInRescue: # new in 1.31
  Enabled: true
Lint/DeprecatedConstants: # new in 1.8
  Enabled: true
Lint/DuplicateBranch: # new in 1.3
  Enabled: true
Lint/DuplicateRegexpCharacterClassElement: # new in 1.1
  Enabled: true
Lint/EmptyBlock: # new in 1.1
  Enabled: true
Lint/EmptyClass: # new in 1.3
  Enabled: true
Lint/EmptyInPattern: # new in 1.16
  Enabled: true
Lint/IncompatibleIoSelectWithFiberScheduler: # new in 1.21
  Enabled: true
Lint/LambdaWithoutLiteralBlock: # new in 1.8
  Enabled: true
Lint/NoReturnInBeginEndBlocks: # new in 1.2
  Enabled: true
Lint/NonAtomicFileOperation: # new in 1.31
  Enabled: true
Lint/NumberedParameterAssignment: # new in 1.9
  Enabled: true
Lint/OrAssignmentToConstant: # new in 1.9
  Enabled: true
Lint/RedundantDirGlobSort: # new in 1.8
  Enabled: true
Lint/RefinementImportMethods: # new in 1.27
  Enabled: true
Lint/RequireRelativeSelfPath: # new in 1.22
  Enabled: true
Lint/SymbolConversion: # new in 1.9
  Enabled: true
Lint/ToEnumArguments: # new in 1.1
  Enabled: true
Lint/TripleQuotes: # new in 1.9
  Enabled: true
Lint/UnexpectedBlockArity: # new in 1.5
  Enabled: true
Lint/UnmodifiedReduceAccumulator: # new in 1.1
  Enabled: true
Lint/UselessRuby2Keywords: # new in 1.23
  Enabled: true
Naming/BlockForwarding: # new in 1.24
  Enabled: true
Security/CompoundHash: # new in 1.28
  Enabled: true
Security/IoMethods: # new in 1.22
  Enabled: true
Style/ArgumentsForwarding: # new in 1.1
  Enabled: true
Style/CollectionCompact: # new in 1.2
  Enabled: true
Style/DocumentDynamicEvalDefinition: # new in 1.1
  Enabled: true
Style/EndlessMethod: # new in 1.8
  Enabled: true
Style/EnvHome: # new in 1.29
  Enabled: true
Style/FetchEnvVar: # new in 1.28
  Enabled: true
Style/FileRead: # new in 1.24
  Enabled: true
Style/FileWrite: # new in 1.24
  Enabled: true
Style/HashConversion: # new in 1.10
  Enabled: true
Style/HashExcept: # new in 1.7
  Enabled: true
Style/IfWithBooleanLiteralBranches: # new in 1.9
  Enabled: true
Style/InPatternThen: # new in 1.16
  Enabled: true
Style/MapCompactWithConditionalBlock: # new in 1.30
  Enabled: true
Style/MapToHash: # new in 1.24
  Enabled: true
Style/MultilineInPatternThen: # new in 1.16
  Enabled: true
Style/NegatedIfElseCondition: # new in 1.2
  Enabled: true
Style/NestedFileDirname: # new in 1.26
  Enabled: true
Style/NilLambda: # new in 1.3
  Enabled: true
Style/NumberedParameters: # new in 1.22
  Enabled: true
Style/NumberedParametersLimit: # new in 1.22
  Enabled: true
Style/ObjectThen: # new in 1.28
  Enabled: true
Style/OpenStructUse: # new in 1.23
  Enabled: true
Style/QuotedSymbols: # new in 1.16
  Enabled: true
Style/RedundantArgument: # new in 1.4
  Enabled: true
Style/RedundantInitialize: # new in 1.27
  Enabled: true
Style/RedundantSelfAssignmentBranch: # new in 1.19
  Enabled: true
Style/SelectByRegexp: # new in 1.22
  Enabled: true
Style/StringChars: # new in 1.12
  Enabled: true
Style/SwapValues: # new in 1.1
  Enabled: true
For more information: https://docs.rubocop.org/rubocop/versioning.html
Inspecting 2 files
.W

Offenses:

repro.rb:8:1: W: Lint/EmptyWhen: Avoid when branches without a body.
when 50... ...
^^^^^^^^^^

2 files inspected, 1 offense detected

Steps to reproduce the problem

place these files then bundle install && bundle exec rubocop

root@5654c24326e8:/tmp/repro# tree -a 
.
├── .rubocop.yml
├── Gemfile
├── Gemfile.lock
└── repro.rb

0 directories, 4 files
root@5654c24326e8:/tmp/repro# cat Gemfile
# frozen_string_literal: true

source 'https://rubygems.org'

gem 'rubocop'
root@5654c24326e8:/tmp/repro# cat Gemfile.lock 
GEM
  remote: https://rubygems.org/
  specs:
    ast (2.4.2)
    json (2.6.2)
    parallel (1.22.1)
    parser (3.1.2.0)
      ast (~> 2.4.1)
    rainbow (3.1.1)
    regexp_parser (2.5.0)
    rexml (3.2.5)
    rubocop (1.31.1)
      json (~> 2.3)
      parallel (~> 1.10)
      parser (>= 3.1.0.0)
      rainbow (>= 2.2.2, < 4.0)
      regexp_parser (>= 1.8, < 3.0)
      rexml (>= 3.2.5, < 4.0)
      rubocop-ast (>= 1.18.0, < 2.0)
      ruby-progressbar (~> 1.7)
      unicode-display_width (>= 1.4.0, < 3.0)
    rubocop-ast (1.18.0)
      parser (>= 3.1.1.0)
    ruby-progressbar (1.11.0)
    unicode-display_width (2.2.0)

PLATFORMS
  x86_64-linux

DEPENDENCIES
  rubocop

BUNDLED WITH
   2.3.7
root@5654c24326e8:/tmp/repro# cat repro.rb 
# frozen_string_literal: true

var = 12

case var
when 10
  :a
when 50...
  :c
end
root@5654c24326e8:/tmp/repro# cat .rubocop.yml 
AllCops:
  TargetRubyVersion: 3.1

RuboCop version

root@5654c24326e8:/tmp/repro# bundle exec rubocop -V
1.31.1 (using Parser 3.1.2.0, rubocop-ast 1.18.0, running on ruby 3.1.2 x86_64-linux)
@ydah
Copy link
Member

ydah commented Jul 5, 2022

Thank you for your feedback.

# frozen_string_literal: true

var = 12

case var
when 10
  :a
when 50...
  :c
end

The above code appears to generate an ArgumentError when executed.

bundle exec ruby test.rb
test.rb:8: warning: ... at EOL, should be parenthesized?
test.rb:8:in `<main>': bad value for range (ArgumentError)

ruby --version                                                        
ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin21]

I think the following rewrite would result in the intended behavior. How about it?

# frozen_string_literal: true

var = 12

case var
when 10
  :a
when (50...)
  :c
end

@koic
Copy link
Member

koic commented Jul 6, 2022

Yep. This is a range's specification. The following is the behavior when line break is used on the endless range without parentheses.

% cat range.rb
p 1...
42
% ruby range.rb
range.rb:1: warning: ... at EOL, should be parenthesized?
1...42

In other words, the reproduction code is interpreted as follows.

# frozen_string_literal: true

var = 12

case var
when 10
  :a
when (50...
  :c)
end

So, this issue case should add parentheses to the expected endless range, not false positives.

when (50...)
  :c

@koic koic closed this as completed Jul 6, 2022
@kazzix14
Copy link
Author

kazzix14 commented Jul 6, 2022

thanks for your responses.

Yes, that code generates an ArgumentError.
I didn't notice that, cause the code was working before rubocop -a.
The original was like this.

This is what happens when I do rubocop -a

I'm not sure if this is expected behavior or if I should create another issue.
but it should make my code like this I think?

I thought this can be another issue after rethinking, so I created #10791 for this.

@koic
Copy link
Member

koic commented Jul 6, 2022

First, I've opened #10792 to detect this issue case.

koic added a commit to koic/rubocop that referenced this issue Jul 7, 2022
## 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
```
bbatsov pushed a commit that referenced this issue Jul 7, 2022
## 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 #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
```
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

No branches or pull requests

3 participants