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

FavorUnlessOverNegatedIf triggered when using elsifs #427

Closed
jurriaan opened this issue Aug 10, 2013 · 8 comments
Closed

FavorUnlessOverNegatedIf triggered when using elsifs #427

jurriaan opened this issue Aug 10, 2013 · 8 comments

Comments

@jurriaan
Copy link
Contributor

Here's an example:

test = 'String'

a = if test.is_a?(String)
  3
elsif test.is_a?(Array)
  2
elsif !test.nil?
  1
end

p a

which gives the following output:

Offences:

test.rb:7:1: C: FavorUnlessOverNegatedIf: Favor unless (or control flow or) over if for negative conditions.
elsif !test.nil?
^^^
test.rb:9:1: W: EndAlignment: end at 9, 0 is not aligned with if at 3, 4
end
^^^

1 file inspected, 2 offences detected

How to fix this? I think it FavorUnlessOverNegatedIf shouldn't trigger in this case.
I also think the EndAlignment shouldn't trigger either.

@markijbema
Copy link
Contributor

Shouldn't elsif be ignored altogether by this cop? There's no elsunless you can use in this case.

@jurriaan
Copy link
Contributor Author

I think so. The comment in the cop says it ignores if's with else statements. But it doesn't work for elsifs..

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 10, 2013

Internally the parser represents elsif as a nested if, which is probably causing the bug.

@jurriaan
Copy link
Contributor Author

Thanks! What about the EndAlignment cop?

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 10, 2013

This is intentional - most IDEs/editors align if/else/elsif/end even for assignments, so we've decided to discourage other indentation practices. You'll notice that this indentation style is fine for blocks, though.

@jurriaan
Copy link
Contributor Author

Your latest change breaks ternary operators:

letme = crash ? :rubocop : ':('

=>

An error occurred while FavorUnlessOverNegatedIf cop was inspecting test.rb.
undefined method `keyword' for #<Parser::Source::Map::Ternary:0x007ff2bfac2948>
.../gems/rubocop-040b17a66d89/lib/rubocop/cop/style/favor_unless_over_negated_if.rb:30:in `on_if'

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 10, 2013

That is no longer my latest change - this is already fixed :-)

@jurriaan
Copy link
Contributor Author

👍

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