Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a false positive for Rails/DynamicFindBy #346

Merged
merged 1 commit into from
Sep 14, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix a false positive for Rails/DynamicFindBy
This PR fixes a false positive for `Rails/DynamicFindBy`
when any of the arguments are splat argument.

```console
% cat example.rb
find_by_scan(*args)

% bundle exec rubocop -a example.rb
(snip)

Inspecting 1 file
E

Offenses:

example.rb:1:1: C: [Corrected] Rails/DynamicFindBy: Use find_by instead
of dynamic find_by_scan.
find_by_scan(*args)
^^^^^^^^^^^^^^^^^^^
example.rb:1:15: E: Lint/Syntax: unexpected token tSTAR
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter,
under AllCops)
find_by(scan: *args)
              ^

1 file inspected, 2 offenses detected, 1 offense corrected

% cat example.rb
find_by(scan: *args)

% ruby -c example.rb
example.rb:1: syntax error, unexpected *
find_by(scan: *args)
```
  • Loading branch information
koic committed Sep 13, 2020
commit 01036ab979b94d9ac71ffa7b5be3fe02d3a722dc
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@
* [#338](https://github.com/rubocop-hq/rubocop-rails/issues/338): Fix a false positive for `Rails/IndexBy` and `Rails/IndexWith` when the `each_with_object` hash is used in the transformed key or value. ([@eugeneius][])
* [#351](https://github.com/rubocop-hq/rubocop-rails/pull/351): Add `<>` operator to `Rails/WhereNot` cop. ([@Tietew][])
* [#352](https://github.com/rubocop-hq/rubocop-rails/pull/352): Do not register offense if given a splatted hash. ([@dvandersluis][])
* [#346](https://github.com/rubocop-hq/rubocop-rails/pull/346): Fix a false positive for `Rails/DynamicFindBy` when any of the arguments are splat argument. ([@koic][])

## 2.8.0 (2020-09-04)

1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/dynamic_find_by.rb
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ def on_send(node)
method_name = node.method_name
static_name = static_method_name(method_name)
return unless static_name
return if node.arguments.any?(&:splat_type?)

add_offense(node,
message: format(MSG, static_name: static_name,
8 changes: 8 additions & 0 deletions spec/rubocop/cop/rails/dynamic_find_by_spec.rb
Original file line number Diff line number Diff line change
@@ -133,6 +133,14 @@
expect_no_offenses('User.find_by(name: name)')
end

it 'accepts splat argument' do
expect_no_offenses('User.find_by_scan(*args)')
end

it 'accepts any of the arguments are splat argument' do
expect_no_offenses('User.find_by_foo_and_bar(arg, *args)')
end

it 'accepts method in whitelist' do
expect_no_offenses(<<~RUBY)
User.find_by_sql(["select * from users where name = ?", name])