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

Style/GuardClause false positive #7928

Closed
baelter opened this issue May 4, 2020 · 4 comments · Fixed by #7932 or #7935
Closed

Style/GuardClause false positive #7928

baelter opened this issue May 4, 2020 · 4 comments · Fixed by #7932 or #7935
Labels

Comments

@baelter
Copy link

baelter commented May 4, 2020

Style/GuardClause false positive for code like this:

b = nil
a = 
  if b
	true || raise "err"
  else
	puts "else"
  end

Expected behavior

No trigger

Actual behavior

Cop triggers on if statement

Steps to reproduce the problem

rubocop code above

RuboCop version

$ [bundle exec] rubocop -V
0.82.0 (using Parser 2.7.1.2, running on ruby 2.6.6 x86_64-darwin19)
@koic
Copy link
Member

koic commented May 5, 2020

I think this is not a false positive, so it is an expected behavior:

# bad
if cond
  foo || raise('hi')
else
  bar
end

# good
foo || raise('hi') if cond

bar

OTOH, the offense message for that would be incorrect. I have opened a PR #7932. Thank you.

@baelter
Copy link
Author

baelter commented May 5, 2020

So:

# bad
a = 
  if cond
    foo || raise('hi')
  else
    bar
  end

# good	
a = foo || raise('hi') if cond
a ||= bar

?

@koic
Copy link
Member

koic commented May 6, 2020

Ah, that case is a false positive. If assigned, this cop should be accept it. Seems to have multiple issues. I will open the patch for it.

@koic
Copy link
Member

koic commented May 6, 2020

I opened a PR #7935 to solve this issue.

koic added a commit to koic/rubocop that referenced this issue May 10, 2020
Fixes rubocop#7928.

This PR fixes a false message for `Style/GuardClause` when using
`and` or `or` operators for guard clause in `then` or `else` branches.

The following is an example:

```console
% cat example.rb
if cond
  foo || raise('hi')
else
  bar
end
```

## Before

```console
% bundle exec rubocop --only Style/GuardClause
(snip)
Offenses:

example.rb:1:1: C: Style/GuardClause: Use a guard clause (raise('hi') if
cond) instead of wrapping the code inside a conditional expression.
if cond
^^

1 file inspected, 1 offense detected
```

## After

```console
% bundle exec rubocop --only Style/GuardClause
(snip)

Offenses:

example.rb:1:1: C: Style/GuardClause: Use a guard clause (foo ||
raise('hi') if cond) instead of wrapping the code inside a conditional
expression.
if cond
^^

1 file inspected, 1 offense detected
```
koic added a commit that referenced this issue May 10, 2020
[Fix #7928] Fix a false message for `Style/GuardClause`
koic added a commit to koic/rubocop that referenced this issue May 10, 2020
Fixes rubocop#7928.

This PR fixes a false positive for `Style/GuardClause`
when assigning the result of a guard condition with `else`.
koic added a commit that referenced this issue May 10, 2020
…_clause

[Fix #7928] Fix a false positive for `Style/GuardClause`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants