Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new rails Schema Comment cop #568

Merged
merged 1 commit into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -483,3 +483,4 @@
[@johnsyweb]: https://github.com/johnsyweb
[@theunraveler]: https://github.com/theunraveler
[@pirj]: https://github.com/pirj
[@vitormd]: https://github.com/vitormd
1 change: 1 addition & 0 deletions changelog/new_schema_comment_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#568](https://github.com/rubocop/rubocop-rails/issues/568): Add `Rails/SchemaComment` cop. ([@vitormd][])
7 changes: 7 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,13 @@ Rails/SaveBang:
AllowedReceivers: []
SafeAutoCorrect: false

Rails/SchemaComment:
Description: >-
This cop enforces the use of the `comment` option when adding a new table or column
to the database during a migration.
Enabled: false
VersionAdded: '2.13'

Rails/ScopeArgs:
Description: 'Checks the arguments of ActiveRecord scopes.'
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 @@ -97,6 +97,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
* xref:cops_rails.adoc#railssafenavigation[Rails/SafeNavigation]
* xref:cops_rails.adoc#railssafenavigationwithblank[Rails/SafeNavigationWithBlank]
* xref:cops_rails.adoc#railssavebang[Rails/SaveBang]
* xref:cops_rails.adoc#railsschemacomment[Rails/SchemaComment]
* xref:cops_rails.adoc#railsscopeargs[Rails/ScopeArgs]
* xref:cops_rails.adoc#railsshorti18n[Rails/ShortI18n]
* xref:cops_rails.adoc#railsskipsmodelvalidations[Rails/SkipsModelValidations]
Expand Down
34 changes: 34 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4503,6 +4503,40 @@ Service::Mailer::update

* https://rails.rubystyle.guide#save-bang

== Rails/SchemaComment

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Disabled
| Yes
| No
| 2.13
| -
|===

This cop enforces the use of the `comment` option when adding a new table or column
to the database during a migration.

=== Examples

[source,ruby]
----
# bad (no comment for a new column or table)
add_column :table, :column, :integer

create_table :table do |t|
t.type :column
end

# good
add_column :table, :column, :integer, comment: 'Number of offenses'

create_table :table, comment: 'Table of offenses data' do |t|
t.type :column, comment: 'Number of offenses'
end
----

== Rails/ScopeArgs

|===
Expand Down
34 changes: 34 additions & 0 deletions lib/rubocop/cop/mixin/active_record_migrations_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

module RuboCop
module Cop
# A mixin to extend cops for Active Record features
module ActiveRecordMigrationsHelper
extend NodePattern::Macros

RAILS_ABSTRACT_SCHEMA_DEFINITIONS = %i[
bigint binary boolean date datetime decimal float integer json string
text time timestamp virtual
].freeze
RAILS_ABSTRACT_SCHEMA_DEFINITIONS_HELPERS = %i[
column references belongs_to primary_key numeric
].freeze
POSTGRES_SCHEMA_DEFINITIONS = %i[
bigserial bit bit_varying cidr citext daterange hstore inet interval
int4range int8range jsonb ltree macaddr money numrange oid point line
lseg box path polygon circle serial tsrange tstzrange tsvector uuid xml
].freeze
MYSQL_SCHEMA_DEFINITIONS = %i[
blob tinyblob mediumblob longblob tinytext mediumtext longtext
unsigned_integer unsigned_bigint unsigned_float unsigned_decimal
].freeze

def_node_matcher :create_table_with_block?, <<~PATTERN
(block
(send nil? :create_table ...)
(args (arg _var))
_)
PATTERN
end
end
end
9 changes: 2 additions & 7 deletions lib/rubocop/cop/rails/create_table_with_timestamps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,11 @@ module Rails
# t.datetime :updated_at, default: -> { 'CURRENT_TIMESTAMP' }
# end
class CreateTableWithTimestamps < Base
include ActiveRecordMigrationsHelper

MSG = 'Add timestamps when creating a new table.'
RESTRICT_ON_SEND = %i[create_table].freeze

def_node_matcher :create_table_with_block?, <<~PATTERN
(block
(send nil? :create_table ...)
(args (arg _var))
_)
PATTERN

def_node_matcher :create_table_with_timestamps_proc?, <<~PATTERN
(send nil? :create_table (sym _) ... (block-pass (sym :timestamps)))
PATTERN
Expand Down
104 changes: 104 additions & 0 deletions lib/rubocop/cop/rails/schema_comment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop enforces the use of the `comment` option when adding a new table or column
# to the database during a migration.
#
# @example
# # bad (no comment for a new column or table)
# add_column :table, :column, :integer
#
# create_table :table do |t|
# t.type :column
# end
#
# # good
# add_column :table, :column, :integer, comment: 'Number of offenses'
#
# create_table :table, comment: 'Table of offenses data' do |t|
# t.type :column, comment: 'Number of offenses'
# end
#
class SchemaComment < Base
include ActiveRecordMigrationsHelper

COLUMN_MSG = 'New database column without `comment`.'
TABLE_MSG = 'New database table without `comment`.'
RESTRICT_ON_SEND = %i[add_column create_table].freeze
CREATE_TABLE_COLUMN_METHODS = Set[
*(
RAILS_ABSTRACT_SCHEMA_DEFINITIONS |
RAILS_ABSTRACT_SCHEMA_DEFINITIONS_HELPERS |
POSTGRES_SCHEMA_DEFINITIONS |
MYSQL_SCHEMA_DEFINITIONS
)
].freeze

# @!method comment_present?(node)
def_node_matcher :comment_present?, <<~PATTERN
(hash <(pair {(sym :comment) (str "comment")} (_ [present?])) ...>)
PATTERN

# @!method add_column?(node)
def_node_matcher :add_column?, <<~PATTERN
(send nil? :add_column _table _column _type _?)
PATTERN

# @!method add_column_with_comment?(node)
def_node_matcher :add_column_with_comment?, <<~PATTERN
(send nil? :add_column _table _column _type #comment_present?)
PATTERN

# @!method create_table?(node)
def_node_matcher :create_table?, <<~PATTERN
(send nil? :create_table _table _?)
PATTERN

# @!method create_table?(node)
def_node_matcher :create_table_with_comment?, <<~PATTERN
(send nil? :create_table _table #comment_present? ...)
PATTERN

# @!method t_column?(node)
def_node_matcher :t_column?, <<~PATTERN
(send _var CREATE_TABLE_COLUMN_METHODS ...)
PATTERN

# @!method t_column_with_comment?(node)
def_node_matcher :t_column_with_comment?, <<~PATTERN
(send _var CREATE_TABLE_COLUMN_METHODS _column _type? #comment_present?)
PATTERN

def on_send(node)
if add_column_without_comment?(node)
add_offense(node, message: COLUMN_MSG)
elsif create_table?(node)
if create_table_without_comment?(node)
add_offense(node, message: TABLE_MSG)
elsif create_table_column_call_without_comment?(node)
add_offense(node.parent.body, message: COLUMN_MSG)
end
vitormd marked this conversation as resolved.
Show resolved Hide resolved
end
end

private

def add_column_without_comment?(node)
add_column?(node) && !add_column_with_comment?(node)
end
vitormd marked this conversation as resolved.
Show resolved Hide resolved

def create_table_without_comment?(node)
create_table?(node) && !create_table_with_comment?(node)
end

def create_table_column_call_without_comment?(node)
create_table_with_block?(node.parent) &&
t_column?(node.parent.body) &&
!t_column_with_comment?(node.parent.body)
end
end
end
end
end
2 changes: 2 additions & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative 'mixin/active_record_helper'
require_relative 'mixin/active_record_migrations_helper'
require_relative 'mixin/enforce_superclass'
require_relative 'mixin/index_method'
require_relative 'mixin/target_rails_version'
Expand All @@ -10,6 +11,7 @@
require_relative 'rails/active_record_callbacks_order'
require_relative 'rails/active_record_override'
require_relative 'rails/active_support_aliases'
require_relative 'rails/schema_comment'
require_relative 'rails/add_column_index'
require_relative 'rails/after_commit_override'
require_relative 'rails/application_controller'
Expand Down
117 changes: 117 additions & 0 deletions spec/rubocop/cop/rails/schema_comment_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::SchemaComment, :config do
context 'when send add_column' do
it 'registers an offense when `add_column` has no `comment` option' do
expect_offense(<<~RUBY)
add_column :table, :column, :integer
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`.
RUBY
end

it 'registers an offense when `add_column` has no `comment` option, but other options' do
expect_offense(<<~RUBY)
add_column :table, :column, :integer, default: 0
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`.
RUBY
end

it 'registers an offense when `add_column` has a nil `comment` option' do
expect_offense(<<~RUBY)
add_column :table, :column, :integer, comment: nil
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`.
RUBY
end

it 'registers an offense when `add_column` has an empty `comment` option' do
expect_offense(<<~RUBY)
add_column :table, :column, :integer, comment: ''
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`.
RUBY
end

it 'does not register an offense when `add_column` has `comment` option' do
expect_no_offenses(<<~RUBY)
add_column :table, :column, :integer, comment: 'An integer field'
RUBY
end

it 'does not register an offense when `add_column` has `comment` option'\
'among other options' do
expect_no_offenses(<<~RUBY)
add_column :table, :column, :integer, null: false, comment: 'An integer field', default: 0
RUBY
end
end

context 'when send create_table' do
it 'registers an offense when `create_table` has no `comment` option' do
expect_offense(<<~RUBY)
create_table :users do |t|
^^^^^^^^^^^^^^^^^^^ New database table without `comment`.
end
RUBY
end

it 'registers an offense when `create_table` has a nil `comment` option' do
expect_offense(<<~RUBY)
create_table :users, comment: nil do |t|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database table without `comment`.
end
RUBY
end

it 'registers an offense when `create_table` has a empty `comment` option' do
expect_offense(<<~RUBY)
create_table :users, comment: '' do |t|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database table without `comment`.

end
RUBY
end

it 'registers an offense when `t.column` has no `comment` option' do
expect_offense(<<~RUBY)
create_table :users, comment: 'Table' do |t|
t.column :column, :integer
^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`.
end
RUBY
end

it 'registers an offense when `t.integer` has no `comment` option' do
expect_offense(<<~RUBY)
create_table :users, comment: 'Table' do |t|
t.integer :column
^^^^^^^^^^^^^^^^^ New database column without `comment`.
end
RUBY
end

it 'does not register an offense when `t.column` has `comment` option' do
expect_no_offenses(<<~RUBY)
create_table :users, comment: 'Table' do |t|
t.column :column, :integer, comment: 'I am a column'
end
RUBY
end

it 'does not register an offense when `t.column` has `comment` option' \
'among other options' do
expect_no_offenses(<<~RUBY)
create_table :users, comment: 'Table' do |t|
t.column :column, :integer, default: nil, comment: 'I am a column', null: true
end
RUBY
end

it 'does not register an offense when `t.integer` has `comment` option'\
'among other options' do
expect_no_offenses(<<~RUBY)
create_table :users, comment: 'Table' do |t|
t.integer :column, default: nil, comment: 'I am a column', null: true
vitormd marked this conversation as resolved.
Show resolved Hide resolved
end
RUBY
end
end
end