From 94b8dbf6aa31c091526d8f7012c536cad27ddd63 Mon Sep 17 00:00:00 2001 From: Ryo Nakamura Date: Sat, 18 Jun 2022 11:44:19 +0900 Subject: [PATCH] Add `Sevencop/OrderField` cop --- CHANGELOG.md | 4 ++ README.md | 22 ++++++- config/default.yml | 6 ++ lib/rubocop/cop/sevencop/order_field.rb | 62 +++++++++++++++++++ lib/sevencop.rb | 1 + .../cop/rails_deprecation/order_field_spec.rb | 58 +++++++++++++++++ 6 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/cop/sevencop/order_field.rb create mode 100644 spec/rubocop/cop/rails_deprecation/order_field_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 7213fb8..04ab3e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Added + +- Add `Sevencop/OrderField` cop. + ### Changed - Improve performance of `Sevencop/UniquenessValidatorExplicitCaseSensitivity` cop. diff --git a/README.md b/README.md index 7e6d98b..3d2158a 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,26 @@ require: ## Cops +### Sevencop/OrderField + +Identifies a String including "field" is passed to `order` or `reorder`. + +```ruby +# bad +articles.order('field(id, ?)', a) + +# good +articles.order(Arel.sql('field(id, ?)'), a) + +# bad +reorder('field(id, ?)', a) + +# good +reorder(Arel.sql('field(id, ?)'), a) +``` + +`Enabled: false` by default. + ### `Sevencop/RedundantExistenceCheck` Identifies redundant existent check before file operation. @@ -76,4 +96,4 @@ validates :name, uniqueness: { allow_nil: true, scope: :user_id, case_sensitive: Useful to keep the same behavior between Rails 6.0 and 6.1 where case insensitive collation is used in MySQL. -Note that this cop is `Enabled: false` by default. +`Enabled: false` by default. diff --git a/config/default.yml b/config/default.yml index e2d4b6c..59fbb42 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1,3 +1,9 @@ +Sevencop/UniquenessValidatorExplicitCaseSensitivity: + Description: Wrap safe SQL String by `Arel.sql`. + Enabled: false + Safe: false + VersionAdded: '0.4' + Sevencop/RedundantExistenceCheck: Description: Avoid redundant existent check before file operation. Enabled: true diff --git a/lib/rubocop/cop/sevencop/order_field.rb b/lib/rubocop/cop/sevencop/order_field.rb new file mode 100644 index 0000000..eb47261 --- /dev/null +++ b/lib/rubocop/cop/sevencop/order_field.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Sevencop + # Identifies a String including "field" is passed to `order` or `reorder`. + # + # @safety + # This cop is unsafe because it can register a false positive. + # + # @example + # + # # bad + # articles.order('field(id, ?)', a) + # + # # good + # articles.order(Arel.sql('field(id, ?)'), a) + # + # # bad + # reorder('field(id, ?)', a) + # + # # good + # reorder(Arel.sql('field(id, ?)'), a) + # + class OrderField < Base + extend AutoCorrector + + MSG = 'Wrap safe SQL String by `Arel.sql`.' + + RESTRICT_ON_SEND = %i[ + order + reorder + ].freeze + + ORDER_METHOD_NAMES = ::Set[ + *RESTRICT_ON_SEND + ] + + def_node_matcher :order_with_field?, <<~PATTERN + (send + _ ORDER_METHOD_NAMES + (str /field\(.+\)/) + ... + ) + PATTERN + + # @param [RuboCop::AST::SendNode] node + def on_send(node) + return unless order_with_field?(node) + + first_argument_node = node.arguments.first + add_offense(first_argument_node) do |corrector| + corrector.replace( + node.arguments.first, + "Arel.sql(#{first_argument_node.source})" + ) + end + end + end + end + end +end diff --git a/lib/sevencop.rb b/lib/sevencop.rb index 3461155..902ec4c 100644 --- a/lib/sevencop.rb +++ b/lib/sevencop.rb @@ -6,6 +6,7 @@ require_relative 'sevencop/inject' require_relative 'sevencop/version' +require_relative 'rubocop/cop/sevencop/order_field' require_relative 'rubocop/cop/sevencop/redundant_existence_check' require_relative 'rubocop/cop/sevencop/uniqueness_validator_explicit_case_sensitivity' diff --git a/spec/rubocop/cop/rails_deprecation/order_field_spec.rb b/spec/rubocop/cop/rails_deprecation/order_field_spec.rb new file mode 100644 index 0000000..8ee6674 --- /dev/null +++ b/spec/rubocop/cop/rails_deprecation/order_field_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Sevencop::OrderField, :config do + context 'without field' do + it 'registers no offense' do + expect_no_offenses(<<~TEXT) + articles.order('id DESC') + TEXT + end + end + + context 'with Hash' do + it 'registers no offense' do + expect_no_offenses(<<~TEXT) + articles.order(id: :desc) + TEXT + end + end + + context 'with receiver' do + it 'autocorrects offense' do + expect_offense(<<~TEXT) + articles.order('field(id, ?)', a) + ^^^^^^^^^^^^^^ Wrap safe SQL String by `Arel.sql`. + TEXT + + expect_correction(<<~RUBY) + articles.order(Arel.sql('field(id, ?)'), a) + RUBY + end + end + + context 'without receiver' do + it 'autocorrects offense' do + expect_offense(<<~TEXT) + order('field(id, ?)', a) + ^^^^^^^^^^^^^^ Wrap safe SQL String by `Arel.sql`. + TEXT + + expect_correction(<<~RUBY) + order(Arel.sql('field(id, ?)'), a) + RUBY + end + end + + context 'with reorder' do + it 'autocorrects offense' do + expect_offense(<<~TEXT) + reorder('field(id, ?)', a) + ^^^^^^^^^^^^^^ Wrap safe SQL String by `Arel.sql`. + TEXT + + expect_correction(<<~RUBY) + reorder(Arel.sql('field(id, ?)'), a) + RUBY + end + end +end