From 9cacaa109f8da6e606ac696cd06e466617d7b114 Mon Sep 17 00:00:00 2001 From: Tietew Date: Wed, 9 Sep 2020 12:42:18 +0900 Subject: [PATCH] Add `<>` operator to `Rails/WhereNot` cop. --- CHANGELOG.md | 1 + docs/modules/ROOT/pages/cops_rails.adoc | 2 + lib/rubocop/cop/rails/where_not.rb | 6 +- spec/rubocop/cop/rails/where_not_spec.rb | 78 +++++++++++++++++++++++- 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e3ac1d2f8..4ea105ada4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index feeac096b0..ef45ef39ff 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -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']) diff --git a/lib/rubocop/cop/rails/where_not.rb b/lib/rubocop/cop/rails/where_not.rb index 6c784d978d..4c406b6f9e 100644 --- a/lib/rubocop/cop/rails/where_not.rb +++ b/lib/rubocop/cop/rails/where_not.rb @@ -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']) @@ -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 diff --git a/spec/rubocop/cop/rails/where_not_spec.rb b/spec/rubocop/cop/rails/where_not_spec.rb index 965aacd8c0..14ebc45f73 100644 --- a/spec/rubocop/cop/rails/where_not_spec.rb +++ b/spec/rubocop/cop/rails/where_not_spec.rb @@ -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') @@ -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`. @@ -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) @@ -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']) @@ -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`. @@ -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 @@ -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