--auto-gen-config sets a value to "-Infinity" for WordArray #2740

maxjacobson · 2016-01-29T14:37:14Z

I haven't investigated exactly why this is occurring yet, but running rubocop --auto-gen-config with Rubocop 0.36 on a codebase generated this config:

Style/WordArray:
  EnforcedStyle: percent
  MinSize: -Infinity

And this config is invalid and leads to errors later when it's loaded. I don't think our word arrays are particularly unusual -- we have 4 of them, ranging in length from 2 to 3 elements.

The text was updated successfully, but these errors were encountered:

jonas054 · 2016-02-01T18:50:41Z

I think we need more help with reproducing. Do you use square brackets or %w in the word arrays you have? What's your configured style before running rubocop --auto-gen-config?

maxjacobson · 2016-02-01T19:21:33Z

Using square brackets before

The earlier style (from an earlier run of rubocop --auto-gen-config) was:

# Offense count: 2                                                                  
# Cop supports --auto-correct.                                                      
# Configuration parameters: WordRegex.                                              
Style/WordArray:                                                                    
  MinSize: 3

jonas054 · 2016-02-03T05:25:36Z

Thanks. I still haven't been able to reproduce the problem, though.

The values you see in the generated configuration must come from these assignments, but I can't figure out what sequence of calls to the style_detected method could lead to this outcome.

@maxjacobson Can you add p style, ary_size at the top of style_detected and then reproduce the problem? That might bring some clarity.

@alexdowad Do you have any better ideas?

madwort · 2016-02-03T10:30:01Z

Is there any chance that https://github.com/bbatsov/rubocop/blob/v0.36.0/lib/rubocop/cop/style/word_array.rb#L25 node.children might not be an array & be some other object that also implements .size but gives a weird result (if was an object whose .size returned Float::NAN that might do it)?

alexdowad · 2016-02-03T13:18:30Z

@jonas054 @largest_brackets is initialized to negative infinity on line 104. That is so that any value found in any source file will always be larger. The intention was for that value to always be overwritten on the very first call to this method, but apparently it's not being overwritten.

alexdowad · 2016-02-03T13:20:34Z

The only reason why the initial value would never be overwritten is if brackets style is never detected, i.e. no ['words', 'array'] is ever seen.

alexdowad · 2016-02-03T13:25:08Z

@maxjacobson Please edit word_array.rb and add the following line after line 102:

puts "**** detected style: #{style} array size: #{ary_size} config_to_allow_offenses: #{cfg.inspect}"

Then try again and post the console output here.

maxjacobson · 2016-02-03T13:25:39Z

was just adding some logging 😄

it looks like it transitions from brackets to percent and that's when it breaks...

alexdowad · 2016-02-03T13:30:22Z

The issue is that a new cop object is instantiated for each inspected file, but the config_to_allow_offenses persists.

alexdowad · 2016-02-03T13:31:02Z

Let me see what I can do about this. @jonas054 you could assign this issue to me if you like.

maxjacobson · 2016-02-03T13:31:27Z

(Sounds like you know what's up, but here's the output in the requested format anyway)

# Logfile created on 2016-02-03 08:30:27 -0500 by logger.rb/41954
I, [2016-02-03T08:30:27.811472 #30703]  INFO -- : **** detected style: brackets array size: 3 config_to_allow_offenses: {}
I, [2016-02-03T08:30:29.099091 #30703]  INFO -- : **** detected style: brackets array size: 2 config_to_allow_offenses: {"EnforcedStyle"=>"brackets"}
I, [2016-02-03T08:30:29.541292 #30703]  INFO -- : **** detected style: brackets array size: 3 config_to_allow_offenses: {"EnforcedStyle"=>"brackets"}
I, [2016-02-03T08:30:30.045335 #30703]  INFO -- : **** detected style: brackets array size: 3 config_to_allow_offenses: {"EnforcedStyle"=>"brackets"}
I, [2016-02-03T08:30:30.177833 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"brackets"}
I, [2016-02-03T08:30:30.179034 #30703]  INFO -- : **** detected style: percent array size: 4 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}
I, [2016-02-03T08:30:30.215578 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}
I, [2016-02-03T08:30:30.216600 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}
I, [2016-02-03T08:30:30.230177 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}
I, [2016-02-03T08:30:30.231207 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}
I, [2016-02-03T08:30:30.243237 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}
I, [2016-02-03T08:30:30.244243 #30703]  INFO -- : **** detected style: percent array size: 1 config_to_allow_offenses: {"EnforcedStyle"=>"percent", "MinSize"=>-Infinity}

jonas054 · 2016-02-03T19:56:02Z

@alexdowad Thanks for stepping up! I can't find a way to assign it to you in the GitHub UI (only to the four people who are collaborators), but let it be known that I hereby hand it over to you.

jaredbeck · 2016-08-15T02:07:06Z

If it helps, I can confirm that --auto-gen-config still sets -Infinity in 0.42.0.

backus · 2017-01-25T03:11:17Z

Here is a minimal reproduction:

$ cat a.rb
['g', 'h']

$ cat c.rb
%w(a b)

$ ls -la
total 16
drwxr-xr-x   4 johnbackus  staff   136B Jan 24 19:09 .
drwxr-xr-x@ 52 johnbackus  staff   1.7K Jan 24 19:03 ..
-rw-r--r--   1 johnbackus  staff    11B Jan 24 19:07 a.rb
-rw-r--r--   1 johnbackus  staff     8B Jan 24 19:07 c.rb

$ rubocop -V
0.47.1 (using Parser 2.3.3.1, running on ruby 2.3.1 x86_64-darwin16)

$ rubocop --auto-gen-config a.rb c.rb
Inspecting 2 files
C.

Offenses:

a.rb:1:1: C: Use %w or %W for an array of words.
['g', 'h']
^^^^^^^^^^

2 files inspected, 1 offense detected
Created .rubocop_todo.yml.
Run `rubocop --config .rubocop_todo.yml`, or add `inherit_from: .rubocop_todo.yml` in a .rubocop.yml file.

$ cat .rubocop_todo.yml
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2017-01-24 19:10:11 -0800 using RuboCop version 0.47.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: SupportedStyles, WordRegex.
# SupportedStyles: percent, brackets
Style/WordArray:
  EnforcedStyle: percent
  MinSize: -Infinity

mikegee · 2017-01-25T13:32:44Z

I successfully duplicated this following @backus's procedure, but I'm having trouble translating that into a failing spec. Here's my attempt so far. I hacked in some helpers that were supposed to simulate processing multiple files. I wonder if there are any existing specs that process multiple files I could use for guidance.

Those specs are giving "Enabled" => false rather than "MinSize" => -Infinity.

…ulousness WordArray has an ill-advised feature whereby one can set a 'MinSize' config parameter, meaning 'only enforce this rule on arrays bigger than N'. As is common in poorly-thought-out code, thrown together without rhyme or reason, it was not implemented correctly; the 'MinSize' rule only applies when the enforced style is 'percent', not when it is 'brackets'. This was never noticed, probably because the number of people using this most questionable of features is approximately zero. But this is just a footnote to our story of woe. No, the problem is not that 'MinSize' does not apply to 'brackets' style. Take good heed, gentle reader of commit logs: A feature which almost nobody wants or needs can still break your program, even for users who don't intentionally invoke it. Those forgotten and abandoned features are veritable breeding grounds for bugs, legions of bugs, bugs which spill out and infect otherwise healthy code. In this case, when WordArray was amended to support auto-config generation, it was made to generate a 'MinSize' config when this would help to suppress style warnings. Why was this done? Why bother with 'MinSize'? Why not just set the auto-generated config to {'Enabled' => false}, and be done with it? Really, why? Mark Twain answers: 'To a man with a hammer, everything looks like a nail.' The auto-config implementer used 'MinSize' because it was there, not because it really helped anything. The auto-config code had to track the size of the largest bracketed array seen and that of the smallest percent-word array seen, to generate the 'optimal' value of 'MinSize', if anything about this sordid hatchet job can be called 'optimal'. But it kept these values in instance variables, and RuboCop uses a different Cop instance when checking each file. While all the instances of WordArray shared the same 'config_to_allow_offenses' hash, they did not share their largest/smallest array size values. So situations arose where one WordArray instance would inspect a file and determine that 'brackets' style should be used. Another would come along and see a percent-word array, meaning that 'brackets' style won't do. Since the 'EnforcedStyle' was already set to 'brackets', it would expect that a valid array size should be stored in '@largest_brackets', but that was not the case, because those values weren't shared between instances. The ugly and bad fix for this bug is to make all instances of WordArray share their 'largest_brackets' value. Then everything "works"... but we have more insidious global state hiding below the surface, making it ever less likely that RuboCop can ever be properly refactored into an independent backend library and frontend interface. (The good and virtuous fix is to rip out the 'MinSize' parameter and consign it to the junk heap.)

jaredbeck · 2017-01-26T16:57:25Z

I highly recommend reading the commit message on 9a96a58 lol

mikegee mentioned this issue Jan 25, 2017

Style/WordArray triggers invalid YAML serialization in --auto-gen-config #2979

Closed

bbatsov closed this as completed in 9a96a58 Jan 26, 2017

backus mentioned this issue Mar 28, 2017

Error with Style/WordArray cop #4141

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

--auto-gen-config sets a value to "-Infinity" for WordArray #2740

--auto-gen-config sets a value to "-Infinity" for WordArray #2740

maxjacobson commented Jan 29, 2016

jonas054 commented Feb 1, 2016

maxjacobson commented Feb 1, 2016

jonas054 commented Feb 3, 2016

madwort commented Feb 3, 2016

alexdowad commented Feb 3, 2016

alexdowad commented Feb 3, 2016

alexdowad commented Feb 3, 2016

maxjacobson commented Feb 3, 2016

alexdowad commented Feb 3, 2016

alexdowad commented Feb 3, 2016

maxjacobson commented Feb 3, 2016

jonas054 commented Feb 3, 2016

jaredbeck commented Aug 15, 2016

backus commented Jan 25, 2017

mikegee commented Jan 25, 2017

jaredbeck commented Jan 26, 2017

--auto-gen-config sets a value to "-Infinity" for WordArray #2740

--auto-gen-config sets a value to "-Infinity" for WordArray #2740

Comments

maxjacobson commented Jan 29, 2016

jonas054 commented Feb 1, 2016

maxjacobson commented Feb 1, 2016

jonas054 commented Feb 3, 2016

madwort commented Feb 3, 2016

alexdowad commented Feb 3, 2016

alexdowad commented Feb 3, 2016

alexdowad commented Feb 3, 2016

maxjacobson commented Feb 3, 2016

alexdowad commented Feb 3, 2016

alexdowad commented Feb 3, 2016

maxjacobson commented Feb 3, 2016

jonas054 commented Feb 3, 2016

jaredbeck commented Aug 15, 2016

backus commented Jan 25, 2017

mikegee commented Jan 25, 2017

jaredbeck commented Jan 26, 2017