Skip to content

Commit

Permalink
Fix false-positives when constant is used with receiver
Browse files Browse the repository at this point in the history
  • Loading branch information
r7kamura committed Nov 14, 2022
1 parent c9abc2a commit ba588dc
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#866](https://github.com/rubocop/rubocop-rails/pull/866): Fix false-positives when constant is used with receiver on `Rails/DurationArithmetic`, `Rails/IndexBy`, `Rails/IndexWIth`, and `Rails/RequireDependency`. ([@r7kamura][])
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/index_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def self.from_map_to_h(node, match)
end

def self.from_hash_brackets_map(node, match)
new(match, node.children.last, 'Hash['.length, ']'.length)
new(match, node.children.last, "#{node.receiver.source}[".length, ']'.length)
end

def strip_prefix_and_suffix(node, corrector)
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/rails/duration_arithmetic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ class DurationArithmetic < Base
# @return [Boolean] true if matches
def_node_matcher :time_current?, <<~PATTERN
{
(send (const _ :Time) :current)
(send (send (const _ :Time) :zone) :now)
(send (const {nil? cbase} :Time) :current)
(send (send (const {nil? cbase} :Time) :zone) :now)
}
PATTERN

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/index_by.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class IndexBy < Base

def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
(send
(const _ :Hash)
(const {nil? cbase} :Hash)
:[]
(block
(call _ {:map :collect})
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/index_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class IndexWith < Base

def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
(send
(const _ :Hash)
(const {nil? cbase} :Hash)
:[]
(block
(call _ {:map :collect})
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/require_dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class RequireDependency < Base
RESTRICT_ON_SEND = %i[require_dependency].freeze

def_node_matcher :require_dependency_call?, <<~PATTERN
(send {nil? (const _ :Kernel)} :require_dependency _)
(send {nil? (const {nil? cbase} :Kernel)} :require_dependency _)
PATTERN

def on_send(node)
Expand Down
28 changes: 28 additions & 0 deletions spec/rubocop/cop/rails/duration_arithmetic_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,32 @@
created_at - 1.minute
RUBY
end

it 'does not register an offense for `Foo::Time.current`' do
expect_no_offenses(<<~RUBY)
Foo::Time.current + 1.hour
RUBY
end

it 'registers and correct an offense for `::Time.current`' do
expect_offense(<<~RUBY)
::Time.current + 1.hour
^^^^^^^^^^^^^^^^^^^^^^^ Do not add or subtract duration.
RUBY

expect_correction(<<~RUBY)
1.hour.from_now
RUBY
end

it 'registers and correct an offense for `::Time.zone.now`' do
expect_offense(<<~RUBY)
::Time.zone.now + 1.hour
^^^^^^^^^^^^^^^^^^^^^^^^ Do not add or subtract duration.
RUBY

expect_correction(<<~RUBY)
1.hour.from_now
RUBY
end
end
17 changes: 17 additions & 0 deletions spec/rubocop/cop/rails/index_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,23 @@
RUBY
end

it 'registers an offense for `::Hash[map { ... }]`' do
expect_offense(<<~RUBY)
::Hash[x.map { |el| [el.to_sym, el] }]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `Hash[map { ... }]`.
RUBY

expect_correction(<<~RUBY)
x.index_by { |el| el.to_sym }
RUBY
end

it 'does not register an offense for `Foo::Hash[map { ... }]`' do
expect_no_offenses(<<~RUBY)
Foo::Hash[x.map { |el| [el.to_sym, el] }]
RUBY
end

context 'when using Ruby 2.6 or newer', :ruby26 do
it 'registers an offense for `to_h { ... }`' do
expect_offense(<<~RUBY)
Expand Down
17 changes: 17 additions & 0 deletions spec/rubocop/cop/rails/index_with_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,23 @@
RUBY
end

it 'registers an offense for `::Hash[map { ... }]`' do
expect_offense(<<~RUBY)
::Hash[x.map { |el| [el, el.to_sym] }]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `Hash[map { ... }]`.
RUBY

expect_correction(<<~RUBY)
x.index_with { |el| el.to_sym }
RUBY
end

it 'does not register an offense for `Foo::Hash[map { ... }]`' do
expect_no_offenses(<<~RUBY)
Foo::Hash[x.map { |el| [el, el.to_sym] }]
RUBY
end

context 'when using Ruby 2.6 or newer', :ruby26 do
it 'registers an offense for `to_h { ... }`' do
expect_offense(<<~RUBY)
Expand Down
38 changes: 33 additions & 5 deletions spec/rubocop/cop/rails/require_dependency_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,39 @@

context 'when require_dependency' do
context 'when using Rails 6.0 or newer', :rails60 do
it 'registers an offense' do
expect_offense(<<~RUBY)
require_dependency 'foo'
^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `require_dependency` with Zeitwerk mode.
RUBY
context 'without receiver' do
it 'registers an offense' do
expect_offense(<<~RUBY)
require_dependency 'foo'
^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `require_dependency` with Zeitwerk mode.
RUBY
end
end

context 'with `Kernel` as receiver' do
it 'registers an offense' do
expect_offense(<<~RUBY)
Kernel.require_dependency 'foo'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `require_dependency` with Zeitwerk mode.
RUBY
end
end

context 'with `::Kernel` as receiver' do
it 'registers an offense' do
expect_offense(<<~RUBY)
::Kernel.require_dependency 'foo'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `require_dependency` with Zeitwerk mode.
RUBY
end
end

context 'with `Foo::Kernel` as receiver' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Foo::Kernel.require_dependency 'foo'
RUBY
end
end
end

Expand Down

0 comments on commit ba588dc

Please sign in to comment.