Skip to content

Commit

Permalink
Add new Rails/ArelStar cop
Browse files Browse the repository at this point in the history
  • Loading branch information
flanger001 committed Sep 7, 2020
1 parent 1bda5d2 commit 12cba4a
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -281,3 +283,4 @@
[@mobilutz]: https://github.com/mobilutz
[@bubaflub]: https://github.com/bubaflub
[@dvandersluis]: https://github.com/dvandersluis
[@flanger001]: https://github.com/flanger001
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
40 changes: 40 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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. <tt>`my_model`.`*`</tt>). This causes the
database to look for a column named <tt>`*`</tt> (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

|===
Expand Down
41 changes: 41 additions & 0 deletions lib/rubocop/cop/rails/arel_star.rb
Original file line number Diff line number Diff line change
@@ -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. <tt>`my_model`.`*`</tt>). This causes the
# database to look for a column named <tt>`*`</tt> (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
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
53 changes: 53 additions & 0 deletions spec/rubocop/cop/rails/arel_star_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 12cba4a

Please sign in to comment.