{"action":"created","issue":{"url":"https://api.github.com/repos/bbatsov/rubocop/issues/2109","labels_url":"https://api.github.com/repos/bbatsov/rubocop/issues/2109/labels{/name}","comments_url":"https://api.github.com/repos/bbatsov/rubocop/issues/2109/comments","events_url":"https://api.github.com/repos/bbatsov/rubocop/issues/2109/events","html_url":"https://github.com/bbatsov/rubocop/issues/2109","id":99391004,"number":2109,"title":"ExtraSpacing Double Checks","user":{"login":"jfelchner","id":285582,"avatar_url":"https://avatars.githubusercontent.com/u/285582?v=3","gravatar_id":"","url":"https://api.github.com/users/jfelchner","html_url":"https://github.com/jfelchner","followers_url":"https://api.github.com/users/jfelchner/followers","following_url":"https://api.github.com/users/jfelchner/following{/other_user}","gists_url":"https://api.github.com/users/jfelchner/gists{/gist_id}","starred_url":"https://api.github.com/users/jfelchner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jfelchner/subscriptions","organizations_url":"https://api.github.com/users/jfelchner/orgs","repos_url":"https://api.github.com/users/jfelchner/repos","events_url":"https://api.github.com/users/jfelchner/events{/privacy}","received_events_url":"https://api.github.com/users/jfelchner/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"jonas054","id":336720,"avatar_url":"https://avatars.githubusercontent.com/u/336720?v=3","gravatar_id":"","url":"https://api.github.com/users/jonas054","html_url":"https://github.com/jonas054","followers_url":"https://api.github.com/users/jonas054/followers","following_url":"https://api.github.com/users/jonas054/following{/other_user}","gists_url":"https://api.github.com/users/jonas054/gists{/gist_id}","starred_url":"https://api.github.com/users/jonas054/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonas054/subscriptions","organizations_url":"https://api.github.com/users/jonas054/orgs","repos_url":"https://api.github.com/users/jonas054/repos","events_url":"https://api.github.com/users/jonas054/events{/privacy}","received_events_url":"https://api.github.com/users/jonas054/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2015-08-06T09:03:09Z","updated_at":"2015-08-06T19:12:47Z","closed_at":null,"body":"@jonas054 I wanted to throw out a few `ExtraSpacing` offenses and possible false negatives and make sure the cop is working as intended.\r\n\r\nBefore I get started, I just wanted to say that this cop is 10,000 times better now and caught a _bunch_ of errors.  As it stands, in our project, we still can't currently enable it by default, but we can run it selectively to catch the correct misspacings.\r\n\r\n#### Alignment with Previous Line\r\n\r\nIn the majority of cases, the current behavior is what I would want.  So that if you had:\r\n\r\n```ruby\r\nsomething = 'something else'\r\n\r\nanother   = 'another thing'\r\n```\r\n\r\nit would consider those to not have any extra spacing.\r\n\r\nHowever, it causes a wrinkle with something like:\r\n\r\n```ruby\r\n    base.class_eval do\r\n      index_name    \"index name\"\r\n      document_type name\r\n\r\n      after_commit  on: [:create] do\r\n        Workers::IndexUpdate.perform_async(:index, 'Thingy', id)\r\n      end\r\n\r\n      after_commit  on: [:update] do\r\n        Workers::IndexUpdate.perform_async(:index, 'Thingy', id)\r\n      end\r\n    end\r\n```\r\n\r\nIn this example, the extra spacing before the `on` in the `:update` line is caught, but the space before the `on` on the `:create` line is not because (I'm assuming) it's looking at how `name` is aligned above it.\r\n\r\n#### Alignment with Method Calls on the Previous Line\r\n\r\n```ruby\r\n    self.method_call_one_with_a_really_long_name   = config(something,\r\n                                                            'whatever',\r\n                                                            another_thing).\r\n                                                     to_f\r\n    self.method_call_two                           = config(something,\r\n                                                            'blah',\r\n                                                            yet_another_thing).\r\n                                                     to_f\r\n    self.method_call_three                         = config(something,\r\n                                                            'nice_going',\r\n                                                            rubocop_rules).\r\n                                                     to_f\r\n```\r\n\r\nIn this case I want to keep all the method calls lined up but `ExtraSpacing` wants to collapse the first lines.\r\n\r\n```ruby\r\n    self.method_call_one_with_a_really_long_name = config(something,\r\n                                                            'whatever',\r\n                                                            another_thing).\r\n                                                     to_f\r\n    self.method_call_two = config(something,\r\n                                                            'blah',\r\n                                                            yet_another_thing).\r\n                                                     to_f\r\n    self.method_call_three = config(something,\r\n                                                            'nice_going',\r\n                                                            rubocop_rules).\r\n                                                     to_f\r\n```\r\n\r\n#### Multiline Alignment ####\r\n\r\nA lot of the alignment I do is that which spans multiple lines.  It makes the code much easier to scan.\r\n\r\nCurrently when `ExtraSpacing` sees the following code:\r\n\r\n```ruby\r\n    first_product          = FactoryGirl.create(:product,\r\n                                                :with_trait)\r\n    second_product         = FactoryGirl.create(:product,\r\n                                                base_model_id: first_product.base_model_id)\r\n    third_product          = FactoryGirl.create(:product,\r\n                                                :with_trait)\r\n    color_attribute_name   = FactoryGirl.create(:attribute,\r\n                                                name:   'Color',\r\n                                                values: %w{red})\r\n    red_value              = color_attribute_name.values.first\r\n    size_attribute_name    = FactoryGirl.create(:attribute,\r\n                                                name:   'Size',\r\n                                                values: %w{small large})\r\n    carrier_attribute_name = FactoryGirl.create(:attribute,\r\n                                                name:   'Carrier',\r\n                                                values: %w{verizon})\r\n```\r\n\r\nit wants to turn it into this:\r\n\r\n```ruby\r\n    first_product = FactoryGirl.create(:product,\r\n                                       :with_trait)\r\n    second_product = FactoryGirl.create(:product,\r\n                                        base_model_id: first_product.base_model_id)\r\n    third_product = FactoryGirl.create(:product,\r\n                                       :with_trait)\r\n    color_attribute_name = FactoryGirl.create(:attribute,\r\n                                              name:   'Color',\r\n                                              values: %w{red})\r\n    red_value            = color_attribute_name.values.first\r\n    size_attribute_name  = FactoryGirl.create(:attribute,\r\n                                              name:   'Size',\r\n                                              values: %w{small large})\r\n    carrier_attribute_name = FactoryGirl.create(:attribute,\r\n                                                name:   'Carrier',\r\n                                                values: %w{verizon})\r\n```\r\n\r\n#### Alignment of Hash Values ####\r\n\r\nCurrently it appears that `ExtraSpacing` conflicts (possibly?) with the `AlignHash` cop's autocorrection.  It seems as though `ExtraSpacing` is trying to correct spacing on hash values which I believe it should be prohibited from doing since that's the job of the `AlignHash` cop.\r\n\r\nWhen I attempt to run it, I get:\r\n\r\n```\r\n/usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:126:in `check_for_infinite_loop': Infinite loop detected in /Users/jfelchner/Projects/my_project/my_file.rb. (RuboCop::Runner::InfiniteCorrectionLoop)\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:101:in `block in do_inspection_loop'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:100:in `loop'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:100:in `do_inspection_loop'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:78:in `process_file'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:53:in `block in inspect_files'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:51:in `each'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:51:in `inspect_files'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:30:in `run'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/cli.rb:26:in `run'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/bin/rubocop:13:in `block in <top (required)>'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/2.2.0/benchmark.rb:303:in `realtime'\r\n        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/bin/rubocop:12:in `<top (required)>'\r\n        from /usr/local/var/rbenv/versions/2.2.2/bin/rubocop:23:in `load'\r\n        from /usr/local/var/rbenv/versions/2.2.2/bin/rubocop:23:in `<main>'\r\n```\r\n\r\n#### Proposed Solution ####\r\n\r\nSo even though it looks like a lot, I think that the solution is actually pretty simple.  I think that, when `ExtraSpacing` is trying to determine whether alignment is being attempted, rather than looking on the previous line, it needs to look at the previous line _with the same indentation as the current line._\r\n\r\nIf it does that, I think it will solve all of my issues (with the exception of the first one which is not that big of a deal IMO, but I wanted to make you aware).\r\n\r\n#### Thank You ####\r\n\r\nThanks for making this cop so awesome @jonas054.  It caught a _ton_ of errors in our code. :grinning:"},"comment":{"url":"https://api.github.com/repos/bbatsov/rubocop/issues/comments/128477676","html_url":"https://github.com/bbatsov/rubocop/issues/2109#issuecomment-128477676","issue_url":"https://api.github.com/repos/bbatsov/rubocop/issues/2109","id":128477676,"user":{"login":"jonas054","id":336720,"avatar_url":"https://avatars.githubusercontent.com/u/336720?v=3","gravatar_id":"","url":"https://api.github.com/users/jonas054","html_url":"https://github.com/jonas054","followers_url":"https://api.github.com/users/jonas054/followers","following_url":"https://api.github.com/users/jonas054/following{/other_user}","gists_url":"https://api.github.com/users/jonas054/gists{/gist_id}","starred_url":"https://api.github.com/users/jonas054/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonas054/subscriptions","organizations_url":"https://api.github.com/users/jonas054/orgs","repos_url":"https://api.github.com/users/jonas054/repos","events_url":"https://api.github.com/users/jonas054/events{/privacy}","received_events_url":"https://api.github.com/users/jonas054/received_events","type":"User","site_admin":false},"created_at":"2015-08-06T19:12:47Z","updated_at":"2015-08-06T19:12:47Z","body":"@jfelchner Thanks for the detailed description. The infinite correction loop is of course a bug. Your suggested correction for the other false positive problems sounds reasonable. I'll try it out. The false negative is less of a problem, I agree."}}