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/ConditionalAssignment cop stop working with advanced assignment #3750

Closed
Bounga opened this issue Nov 30, 2016 · 2 comments · Fixed by #3762
Closed

Style/ConditionalAssignment cop stop working with advanced assignment #3750

Bounga opened this issue Nov 30, 2016 · 2 comments · Fixed by #3762

Comments

@Bounga
Copy link

Bounga commented Nov 30, 2016

Hello,

I'm an happy user of Rubocop. We use it at work to standardize the way we write our code and today I discovered an edge case where Rubocop fail to report an invalid style.

We like to have assignments before the conditional branches rather than duplicating it in each branch, this is the default behavior of Rubocop.


Expected behavior

Rubocop should report an Style/ConditionalAssignment offense for this code:

if foo
  conditions = {
    id: entity_id,
    name: { some: "%#{attributes[:thing]}%" },
    or: {
      id: entity_id
    }
  }
else
  conditions = { id: entity_ids }
end

if conditions
end

because conditions is assigned in the branches rather than before the if.

Actual behavior

Everything is ok for Rubocop:

$ rubocop toto.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

Odd behavior

Rubocop catches the offense if you change unrelated stuff:

if foo
  conditions = {
    id: foo_ids,
    name: { some: "%#{attributes[:thing]}%" },
    or: {
      id: foo_ids
    }
  }
else
  conditions = { id: entity_ids }
end

if conditions
end

Just changed entity_ids to foo_ids then:

$ rubocop toto.rb
Inspecting 1 file
C

Offenses:

toto.rb:1:1: C: Use the return of the conditional for variable assignment and comparison.
if foo ...
^^^^^^

1 file inspected, 1 offense detected

There's other ways to get accurate detection:

if foo
  conditions = {
    id: entity_ids,
    name: { some: "thing" },
    or: {
      id: entity_ids
    }
  }
else
  conditions = { id: entity_ids }
end

if conditions
end

Now I changed the interpolated string to a raw string:

$ rubocop toto.rb
Inspecting 1 file
C

Offenses:

toto.rb:1:1: C: Use the return of the conditional for variable assignment and comparison.
if foo ...
^^^^^^

1 file inspected, 1 offense detected

So it seems like a parsing bug or something but it's really odd.

Steps to reproduce the problem

To reproduce the problem just create a file, paste the provided code in it then run rubocop command on it.

RuboCop version

 $ rubocop -v
0.46.0
$ ruby -v
ruby 2.2.0p0 (2014-12-25 revision 49005) [x86_64-darwin15]

Same things happen with recent version of Ruby:

 $ ruby -v
ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]

Hope you'll have more clue than I have since it seems to be a really tricky bug to track.

@rrosenblum
Copy link
Contributor

Digging into this example a little bit. Using a default configuration, the example is not registering an offense because the cop thinks that a correction would exceed the maximum configured line length. This appears to be a bug to me because as far as I can tell, a correction would not exceed 80 characters. I will have to do a bit more digging to figure out a solution.

@rrosenblum
Copy link
Contributor

It looks like the bug is related to the hash being defined across multiple lines. The code we are using to check on the line length isn't properly checking for code across multiple lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants