Skip to content

Commit

Permalink
Merge pull request rubocop#351 from Tietew/add_stdneq_wherenot
Browse files Browse the repository at this point in the history
Add `<>` operator to `Rails/WhereNot` cop.
  • Loading branch information
koic authored Sep 10, 2020
2 parents de09957 + 9cacaa1 commit 119bbaa
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#345](https://github.com/rubocop-hq/rubocop-rails/issues/345): Fix error of `Rails/AfterCommitOverride` on `after_commit` with a lambda. ([@pocke][])
* [#349](https://github.com/rubocop-hq/rubocop-rails/pull/349): Fix errors of `Rails/UniqueValidationWithoutIndex`. ([@Tietew][])
* [#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][])

## 2.8.0 (2020-09-04)

Expand Down
2 changes: 2 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4372,6 +4372,8 @@ in `where` can be replaced with `where.not(...)`.
# bad
User.where('name != ?', 'Gabe')
User.where('name != :name', name: 'Gabe')
User.where('name <> ?', 'Gabe')
User.where('name <> :name', name: 'Gabe')
User.where('name IS NOT NULL')
User.where('name NOT IN (?)', ['john', 'jane'])
User.where('name NOT IN (:names)', names: ['john', 'jane'])
Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/cop/rails/where_not.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ module Rails
# # bad
# User.where('name != ?', 'Gabe')
# User.where('name != :name', name: 'Gabe')
# User.where('name <> ?', 'Gabe')
# User.where('name <> :name', name: 'Gabe')
# User.where('name IS NOT NULL')
# User.where('name NOT IN (?)', ['john', 'jane'])
# User.where('name NOT IN (:names)', names: ['john', 'jane'])
Expand Down Expand Up @@ -62,9 +64,9 @@ def autocorrect(node)
end
end

NOT_EQ_ANONYMOUS_RE = /\A([\w.]+)\s+!=\s+\?\z/.freeze # column != ?
NOT_EQ_ANONYMOUS_RE = /\A([\w.]+)\s+(?:!=|<>)\s+\?\z/.freeze # column != ?, column <> ?
NOT_IN_ANONYMOUS_RE = /\A([\w.]+)\s+NOT\s+IN\s+\(\?\)\z/i.freeze # column NOT IN (?)
NOT_EQ_NAMED_RE = /\A([\w.]+)\s+!=\s+:(\w+)\z/.freeze # column != :column
NOT_EQ_NAMED_RE = /\A([\w.]+)\s+(?:!=|<>)\s+:(\w+)\z/.freeze # column != :column, column <> :column
NOT_IN_NAMED_RE = /\A([\w.]+)\s+NOT\s+IN\s+\(:(\w+)\)\z/i.freeze # column NOT IN (:column)
IS_NOT_NULL_RE = /\A([\w.]+)\s+IS\s+NOT\s+NULL\z/i.freeze # column IS NOT NULL

Expand Down
78 changes: 75 additions & 3 deletions spec/rubocop/cop/rails/where_not_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,28 @@
RUBY
end

it 'registers an offense and corrects when using `<>` and anonymous placeholder' do
expect_offense(<<~RUBY)
User.where('name <> ?', 'Gabe')
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
RUBY

expect_correction(<<~RUBY)
User.where.not(name: 'Gabe')
RUBY
end

it 'registers an offense and corrects when using `<>` and named placeholder' do
expect_offense(<<~RUBY)
User.where('name <> :name', name: 'Gabe')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
RUBY

expect_correction(<<~RUBY)
User.where.not(name: 'Gabe')
RUBY
end

it 'registers an offense and corrects when using `IS NOT NULL`' do
expect_offense(<<~RUBY)
User.where('name IS NOT NULL')
Expand Down Expand Up @@ -58,7 +80,7 @@
RUBY
end

it 'registers an offense and corrects when using namespaced columns' do
it 'registers an offense and corrects when using `!=` and namespaced columns' do
expect_offense(<<~RUBY)
Course.where('enrollments.student_id != ?', student.id)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not('enrollments.student_id' => student.id)` instead of manually constructing negated SQL in `where`.
Expand All @@ -69,6 +91,17 @@
RUBY
end

it 'registers an offense and corrects when using `<>` and namespaced columns' do
expect_offense(<<~RUBY)
Course.where('enrollments.student_id <> ?', student.id)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not('enrollments.student_id' => student.id)` instead of manually constructing negated SQL in `where`.
RUBY

expect_correction(<<~RUBY)
Course.where.not('enrollments.student_id' => student.id)
RUBY
end

context 'with array arguments' do
it 'registers an offense and corrects when using `!=` and anonymous placeholder' do
expect_offense(<<~RUBY)
Expand All @@ -92,6 +125,28 @@
RUBY
end

it 'registers an offense and corrects when using `<>` and anonymous placeholder' do
expect_offense(<<~RUBY)
User.where(['name <> ?', 'Gabe'])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
RUBY

expect_correction(<<~RUBY)
User.where.not(name: 'Gabe')
RUBY
end

it 'registers an offense and corrects when using `<>` and named placeholder' do
expect_offense(<<~RUBY)
User.where(['name <> :name', { name: 'Gabe' }])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
RUBY

expect_correction(<<~RUBY)
User.where.not(name: 'Gabe')
RUBY
end

it 'registers an offense and corrects when using `IS NOT NULL`' do
expect_offense(<<~RUBY)
User.where(['name IS NOT NULL'])
Expand Down Expand Up @@ -125,7 +180,7 @@
RUBY
end

it 'registers an offense and corrects when using namespaced columns' do
it 'registers an offense and corrects when using `!=` and namespaced columns' do
expect_offense(<<~RUBY)
Course.where(['enrollments.student_id != ?', student.id])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not('enrollments.student_id' => student.id)` instead of manually constructing negated SQL in `where`.
Expand All @@ -135,6 +190,17 @@
Course.where.not('enrollments.student_id' => student.id)
RUBY
end

it 'registers an offense and corrects when using `<>` and namespaced columns' do
expect_offense(<<~RUBY)
Course.where(['enrollments.student_id <> ?', student.id])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not('enrollments.student_id' => student.id)` instead of manually constructing negated SQL in `where`.
RUBY

expect_correction(<<~RUBY)
Course.where.not('enrollments.student_id' => student.id)
RUBY
end
end

it 'does not register an offense when using `where.not`' do
Expand All @@ -155,9 +221,15 @@
RUBY
end

it 'does not register an offense when template string contains negation and additional boolean logic' do
it 'does not register an offense when template string contains `!=` and additional boolean logic' do
expect_no_offenses(<<~RUBY)
User.where('name != ? AND age != ?', 'john', 19)
RUBY
end

it 'does not register an offense when template string contains `<>` and additional boolean logic' do
expect_no_offenses(<<~RUBY)
User.where('name <> ? AND age <> ?', 'john', 19)
RUBY
end
end

0 comments on commit 119bbaa

Please sign in to comment.