Style/ConditionalAssignment does not report when Metrics/LineLength set to less than 86

[bunufi]

Style/ConditionalAssignment does not report an offense if Metrics/LineLength is set to less than 96.

---

Expected behavior

Changing settings to Metrics/LineLength should not affect outcome of other cops.

Actual behavior

Offense is not reported when it should be.

Steps to reproduce the problem

Have some code that offends Style/ConditionalAssignment cop. Such as:

def do_this_do_that(field, value, filter)
  if filter
    @i_do_not_believe_in_unicorns << { field => value }
  else
    @i_do_not_believe_in_unicorns << { field => 'i drink bear' }
  end
end

Set following rubocop settings:

Metrics/LineLength:
  Max: 95

Style/ConditionalAssignment:
  Enabled: true

Outcome:
No offenses

Change settings to (line length more than 95):

Metrics/LineLength:
  Max: 96

Style/ConditionalAssignment:
  Enabled: true

Outcome:
Style/ConditionalAssignment reports an offense

Note:
Specified cops are the only cops that are defined. The rest are defaults
I have noticed that line length changes depending on the code. Initially line lengths were 85 and 86, but after changing to my preferred method names it increased to 95, 96.

RuboCop version

Include the output of rubocop -V. Here's an example:

$ rubocop -V
0.46.0
0.48.1
0.49.0

same behavior with all version listed above.

[bbatsov]

@rrosenblum Can you look into this?

[Drenmi]

Some cops will not suggest a change that would violate the maximum line length. Sounds like this could be one of them.

[bbatsov]

Ah, yeah - because by default moving the assignment out makes the first line longer. Guess we should really mention this in the documentation of those cops. Even I forgot about this. :-)

[rrosenblum]

@Drenmi's conclusion is accurate of the functionality. Based on the example provided, the longest line length is 64 characters which should fall within the given range. @DAneas is the example that you provided nested in modules or classes that would cause it exceed the max line length?

[bunufi]

@Drenmi for the example and replication I have taken the code from the codebase and rubocoped it in its own file. Were you able to replicate it on your side?
update: ups, mentioned the wrong person, I meant @rrosenblum

[bunufi]

@Drenmi I dont really understand Some cops will not suggest a change that would violate the maximum line length.
If I would take away the assignment from within if statement, I would get something like the following.

def do_this_do_that(field, value, filter)
  @i_do_not_believe_in_unicorns <<
    if filter
      { field => value }
    else
      { field => 'i drink bear' }
    end
end

That does not violate the maximum line length. What am I missing?

[artmotion]

Hi.
Can you explain the statement

Some cops will not suggest a change that would violate the maximum line length.

more in detail?
Does that mean, Rubocop will not flag some problems, if the fix/solution/rewrite would indeed violate e.g. the line length?

[bbatsov]

Does that mean, Rubocop will not flag some problems, if the fix/solution/rewrite would indeed violate e.g. the line length?

Exactly. That's why I think:

• We need to add this to their documentation
• Add some config to enabled/disable this behaviour for every cop that does it

[artmotion]

Thanks for the feedback on this!

[rrosenblum]

or the example and replication I have taken the code from the codebase and rubocoped it in its own file. Were you able to replicate it on your side?

@DAneas I have been able to replicate this. It looks like there is a bug in the code that is checking for the longest line length. We are double counting the assignment portion so this should have registered an offense.

If I would take away the assignment from within if statement, I would get something like the following.

def do_this_do_that(field, value, filter)
  @i_do_not_believe_in_unicorns <<
    if filter
      { field => value }
    else
      { field => 'i drink bear' }
    end
end

You are correct that would be short enough. The cop doesn't split the assignment from the condition (maybe this could be advanced behavior to try to register an offense for as much as possible). The cop looks to make the correction in the following way

def do_this_do_that(field, value, filter)
  @i_do_not_believe_in_unicorns << if filter
                                     { field => value }
                                   else
                                     { field => 'i drink bear' }
                                   end
end

• We need to add this to their documentation
• Add some config to enabled/disable this behavior for every cop that does it

I completely agree. I realize that some people may find the correction more important than violating max line length.

[bunufi]

@rrosenblum thanks for taking time to explain this.

From conditional_assignment.rb

        # If `Metrics/LineLength` is enabled, we do not want to introduce an
        # offense by auto-correcting this cop. Find the max configured line
        # length. Find the longest line of condition.

Would it make sense to have correction_exceeds_line_limit? invoked only if auto-correct option is passed to rubocop? The use case for this method is clear, but I don't think it should hide offenses by default.

[rrosenblum]

Would it make sense to have correction_exceeds_line_limit? invoked only if auto-correct option is passed to rubocop? The use case for this method is clear, but I don't think it should hide offenses by default.

I have been on the fence about this for some time now. In general, we try to make it so that auto-correction of one cop does not introduce an offense in another cop. Maybe the answer is as you said, always register an offense, but don't automatically fix it. Regardless, it seems like this should at least be configurable and apply consistently to all cops that make use of it.

[bunufi]

Great work @rrosenblum!
Thanks for following up on that one. Looking forward for the release.