From 8307c6ab97ecce4fa353d22a6c1bb37035ed446f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Os=C3=B3rio?= Date: Wed, 22 Apr 2020 20:45:29 +0100 Subject: [PATCH] [Fix #238] Rails/IndexBy invalid syntax when .to_h is separated by a newline I believe that the test cases included in this commit express well the problem I'm trying to address, but running the auto-correction against the following snippet of code: ```rb x.map { |el| [el.to_sym, el] }. to_h ``` Would yield: ```rb x.index_by { |el| el.to_sym }. # notice the ending "dot" ``` The output isn't parseable/valid Ruby. I belive the problem has to do with the fact that correction code assumes that the `.to_h` statement is always grouped together (which might not be always true). The proposed solution was to try to compute the number of trailing characters to be stripped by subtracting the end position of the map block (the `}` character) from end position of the whole expression. --- CHANGELOG.md | 2 ++ lib/rubocop/cop/mixin/index_method.rb | 9 +++++++- spec/rubocop/cop/rails/index_by_spec.rb | 28 +++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6fd406349..429a470f44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Bug fixes * [#12](https://github.com/rubocop-hq/rubocop-rails/issues/12): Fix a false positive for `Rails/SkipsModelValidations` when passing a boolean literal to `touch`. ([@eugeneius][]) +* [#238](https://github.com/rubocop-hq/rubocop-rails/issues/238): Fix auto correction for `Rails/IndexBy` when the `.to_h` invocation is separated in multiple lines. ([@diogoosorio][]) ### Changes @@ -183,3 +184,4 @@ [@sunny]: https://github.com/sunny [@hoshinotsuyoshi]: https://github.com/hoshinotsuyoshi [@tejasbubane]: https://github.com/tejasbubane +[@diogoosorio]: https://github.com/diogoosorio diff --git a/lib/rubocop/cop/mixin/index_method.rb b/lib/rubocop/cop/mixin/index_method.rb index 2e8e2b5d5a..7659cb4679 100644 --- a/lib/rubocop/cop/mixin/index_method.rb +++ b/lib/rubocop/cop/mixin/index_method.rb @@ -112,7 +112,14 @@ def self.from_each_with_object(node, match) end def self.from_map_to_h(node, match) - strip_trailing_chars = node.parent&.block_type? ? 0 : '.to_h'.length + strip_trailing_chars = 0 + + unless node.parent&.block_type? + map_range = node.children.first.source_range + node_range = node.source_range + strip_trailing_chars = node_range.end_pos - map_range.end_pos + end + new(match, node.children.first, 0, strip_trailing_chars) end diff --git a/spec/rubocop/cop/rails/index_by_spec.rb b/spec/rubocop/cop/rails/index_by_spec.rb index 9566a5a1d3..13fa67190a 100644 --- a/spec/rubocop/cop/rails/index_by_spec.rb +++ b/spec/rubocop/cop/rails/index_by_spec.rb @@ -94,6 +94,34 @@ end end + context 'when `to_h` is on a different line' do + it 'registers an offense for `map { ... }.to_h`' do + expect_offense(<<~RUBY) + x.map { |el| [el.to_sym, el] }. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `map { ... }.to_h`. + to_h + RUBY + + expect_correction(<<~RUBY) + x.index_by { |el| el.to_sym } + RUBY + end + end + + context 'when `.to_h` is on a different line' do + it 'registers an offense for `map { ... }.to_h`' do + expect_offense(<<~RUBY) + x.map { |el| [el.to_sym, el] } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `map { ... }.to_h`. + .to_h + RUBY + + expect_correction(<<~RUBY) + x.index_by { |el| el.to_sym } + RUBY + end + end + context 'when to_h is not called on the result' do it 'does not register an offense for `map { ... }.to_h`' do expect_no_offenses('x.map { |el| [el.to_sym, el] }')