From 12cba4a98fe65673fbdc261cb160443c1cd8ae70 Mon Sep 17 00:00:00 2001 From: David Shaffer Date: Fri, 4 Sep 2020 13:56:32 -0400 Subject: [PATCH] Add new Rails/ArelStar cop --- CHANGELOG.md | 3 ++ config/default.yml | 8 ++++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rails.adoc | 40 ++++++++++++++++++ lib/rubocop/cop/rails/arel_star.rb | 41 ++++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + spec/rubocop/cop/rails/arel_star_spec.rb | 53 ++++++++++++++++++++++++ 7 files changed, 147 insertions(+) create mode 100644 lib/rubocop/cop/rails/arel_star.rb create mode 100644 spec/rubocop/cop/rails/arel_star_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ee83edaaf..a2426a45e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ * [#345](https://github.com/rubocop-hq/rubocop-rails/issues/345): Fix error of `Rails/AfterCommitOverride` on `after_commit` with a lambda. ([@pocke][]) +* [#344](https://github.com/rubocop-hq/rubocop-rails/pull/344): Add new `Rails/ArelStar` cop which checks for quoted literal asterisks in `arel_table` calls. ([@flanger001][]) + ## 2.8.0 (2020-09-04) ### New features @@ -281,3 +283,4 @@ [@mobilutz]: https://github.com/mobilutz [@bubaflub]: https://github.com/bubaflub [@dvandersluis]: https://github.com/dvandersluis +[@flanger001]: https://github.com/flanger001 diff --git a/config/default.yml b/config/default.yml index 02abf38546..78c1898ffa 100644 --- a/config/default.yml +++ b/config/default.yml @@ -98,6 +98,14 @@ Rails/ApplicationRecord: VersionAdded: '0.49' VersionChanged: '2.5' +Rails/ArelStar: + Description: 'Enforces `Arel.star` instead of `"*"` for expanded columns.' + Enabled: true + Safe: false + SafeAutoCorrect: false + AutoCorrect: false + VersionAdded: '2.9' + Rails/AssertNot: Description: 'Use `assert_not` instead of `assert !`.' Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 7b11177ea9..a99c20c938 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -26,6 +26,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide]. * xref:cops_rails.adoc#railsapplicationjob[Rails/ApplicationJob] * xref:cops_rails.adoc#railsapplicationmailer[Rails/ApplicationMailer] * xref:cops_rails.adoc#railsapplicationrecord[Rails/ApplicationRecord] +* xref:cops_rails.adoc#railsarelstar[Rails/ArelStar] * xref:cops_rails.adoc#railsassertnot[Rails/AssertNot] * xref:cops_rails.adoc#railsbelongsto[Rails/BelongsTo] * xref:cops_rails.adoc#railsblank[Rails/Blank] diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index feeac096b0..39d46310db 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -374,6 +374,46 @@ class Rails4Model < ActiveRecord::Base end ---- +== Rails/ArelStar + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Enabled +| No +| Yes (Unsafe) +| 2.9 +| - +|=== + +This cop prevents usage of `"*"` on an Arel::Table column reference. + +Using `arel_table["*"]` causes the outputted string to be a literal +quoted asterisk (e.g. `my_model`.`*`). This causes the +database to look for a column named `*` (or `"*"`) as opposed +to expanding the column list as one would likely expect. + +=== Examples + +[source,ruby] +---- +# bad + MyTable.arel_table["*"] + +# good + MyTable.arel_table[Arel.star] +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| AutoCorrect +| `false` +| Boolean +|=== + == Rails/AssertNot |=== diff --git a/lib/rubocop/cop/rails/arel_star.rb b/lib/rubocop/cop/rails/arel_star.rb new file mode 100644 index 0000000000..387eeb023a --- /dev/null +++ b/lib/rubocop/cop/rails/arel_star.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # This cop prevents usage of `"*"` on an Arel::Table column reference. + # + # Using `arel_table["*"]` causes the outputted string to be a literal + # quoted asterisk (e.g. `my_model`.`*`). This causes the + # database to look for a column named `*` (or `"*"`) as opposed + # to expanding the column list as one would likely expect. + # + # @example + # # bad + # MyTable.arel_table["*"] + # + # # good + # MyTable.arel_table[Arel.star] + # + class ArelStar < Cop + MSG = 'Use `Arel.star` instead of `"*"` for expanded column lists.' + + RESTRICT_ON_SEND = %i[[]].freeze + + def_node_matcher :star_bracket?, <<~PATTERN + (send {const {_ :arel_table}} :[] $(str "*")) + PATTERN + + def on_send(node) + return unless (star = star_bracket?(node)) + + add_offense(star) + end + + def autocorrect(node) + ->(corrector) { corrector.replace(node.loc.expression, 'Arel.star') } + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 8531298c27..e11ddbe497 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -14,6 +14,7 @@ require_relative 'rails/application_job' require_relative 'rails/application_mailer' require_relative 'rails/application_record' +require_relative 'rails/arel_star' require_relative 'rails/assert_not' require_relative 'rails/belongs_to' require_relative 'rails/blank' diff --git a/spec/rubocop/cop/rails/arel_star_spec.rb b/spec/rubocop/cop/rails/arel_star_spec.rb new file mode 100644 index 0000000000..d50a3f1da0 --- /dev/null +++ b/spec/rubocop/cop/rails/arel_star_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::ArelStar do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when an asterisk is used on an Arel::Table column reference' do + expect_offense(<<~RUBY) + class MyModel < ApplicationRecord + scope :my_scope, -> { select(arel_table["*"]) } + ^^^ Use `Arel.star` instead of `"*"` for expanded column lists. + end + RUBY + + expect_correction(<<~RUBY) + class MyModel < ApplicationRecord + scope :my_scope, -> { select(arel_table[Arel.star]) } + end + RUBY + end + + it 'registers an offense on the `arel_table` object in a void' do + expect_offense(<<~RUBY) + arel_table["*"] + ^^^ Use `Arel.star` instead of `"*"` for expanded column lists. + RUBY + + expect_correction(<<~RUBY) + arel_table[Arel.star] + RUBY + end + + it 'registers an offense on the `arel_table` object on a model' do + expect_offense(<<~RUBY) + MyModel.arel_table["*"] + ^^^ Use `Arel.star` instead of `"*"` for expanded column lists. + RUBY + + expect_correction(<<~RUBY) + MyModel.arel_table[Arel.star] + RUBY + end + + it 'registers an offense for ArelExtensions asterisks' do + expect_offense(<<~RUBY) + MyModel["*"] + ^^^ Use `Arel.star` instead of `"*"` for expanded column lists. + RUBY + + expect_correction(<<~RUBY) + MyModel[Arel.star] + RUBY + end +end