Skip to content

Commit

Permalink
Add new cop
Browse files Browse the repository at this point in the history
  • Loading branch information
jmks committed Oct 4, 2020
1 parent 6c9b6e9 commit b6fb70b
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### New features

* [#362](https://github.com/rubocop-hq/rubocop-rails/pull/362): Add new `Rails/WhereEquals` cop. ([@eugeneius][])
* [TODO](https://github.com/rubocop-hq/rubocop-rails/pull/TODO): Add new `Rails/CalendarTypeSuffix` cop. ([@jmks][])

### Bug fixes

Expand Down Expand Up @@ -298,3 +299,4 @@
[@bubaflub]: https://github.com/bubaflub
[@dvandersluis]: https://github.com/dvandersluis
[@Tietew]: https://github.com/Tietew
[@jmks]: https://github.com/jmks
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ Rails/BulkChangeTable:
Include:
- db/migrate/*.rb

Rails/CalendarTypeSuffix:
Description: 'Check whether database columns for calendar types follow a naming convention.'
Enabled: false
DateSuffix: "on"
DateTimeSuffix: "at"
TimeSuffix: "time"
VersionAdded: '2.9'

Rails/ContentTag:
Description: 'Use `tag` instead of `content_tag`.'
Reference:
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 @@ -30,6 +30,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
* xref:cops_rails.adoc#railsbelongsto[Rails/BelongsTo]
* xref:cops_rails.adoc#railsblank[Rails/Blank]
* xref:cops_rails.adoc#railsbulkchangetable[Rails/BulkChangeTable]
* xref:cops_rails.adoc#railscalendartypesuffix[Rails/CalendarTypeSuffix]
* xref:cops_rails.adoc#railscontenttag[Rails/ContentTag]
* xref:cops_rails.adoc#railscreatetablewithtimestamps[Rails/CreateTableWithTimestamps]
* xref:cops_rails.adoc#railsdate[Rails/Date]
Expand Down
54 changes: 54 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,60 @@ end
| Array
|===

== Rails/CalendarTypeSuffix

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Disabled
| Yes
| No
| 2.9
| -
|===

It is convenient to follow a convention when naming database columns.
For calendar-type columns, a reasonable convention is:

date => * _on
datetime => *_at (e.g. timestamps)
time => *_time

"Inspired" by https://github.com/thoughtbot/guides/tree/master/rails

=== Examples

[source,ruby]
----
# bad
t.date "started_at"
t.datetime "created_datetime"
t.time "end_at"
# good
t.date "started_on"
t.datetime "created_at"
t.time "end_time"
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| DateSuffix
| `on`
| String

| DateTimeSuffix
| `at`
| String

| TimeSuffix
| `time`
| String
|===

== Rails/ContentTag

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

module RuboCop
module Cop
module Rails
# It is convenient to follow a convention when naming database columns.
# For calendar-type columns, a reasonable convention is:
#
# date => * _on
# datetime => *_at (e.g. timestamps)
# time => *_time
#
# @example
# # bad
# t.date "started_at"
# t.datetime "created_datetime"
# t.time "end_at"
#
# # good
# t.date "started_on"
# t.datetime "created_at"
# t.time "end_time"
#
# "Inspired" by https://github.com/thoughtbot/guides/tree/master/rails
class CalendarTypeSuffix < Cop
MSG = 'Columns of type `%<type>s` should be named with a `%<suffix>s` suffix.'

def_node_matcher :calendar_type?, '(send _ ${:date :datetime :time} (str $_) ...)'

def on_send(node)
calendar_type?(node) do |type, name|
unless follows_convention?(type, name)
add_offense(
node,
location: highlight_range(node),
message: format(MSG, type: type, suffix: suffix_for(type))
)
end
end
end

def relevant_file?(file)
schema_file?(file) && super
end

private

def schema_file?(file)
File.basename(file) == 'schema.rb'
end

def follows_convention?(type, name)
name.end_with?("_#{suffix_for(type)}")
end

def suffix_for(type)
case type
when :date then cop_config['DateSuffix']
when :datetime then cop_config['DateTimeSuffix']
when :time then cop_config['TimeSuffix']
end
end

def highlight_range(node)
node.loc.expression.with(
end_pos: node.children[2].loc.expression.end_pos
)
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 @@ -18,6 +18,7 @@
require_relative 'rails/belongs_to'
require_relative 'rails/blank'
require_relative 'rails/bulk_change_table'
require_relative 'rails/calendar_type_suffix'
require_relative 'rails/content_tag'
require_relative 'rails/create_table_with_timestamps'
require_relative 'rails/date'
Expand Down
78 changes: 78 additions & 0 deletions spec/rubocop/cop/rails/calendar_type_suffix_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::CalendarTypeSuffix, :config do
subject(:cop) do
cop = described_class.new(config)
allow(cop).to receive(:relevant_file?).and_return(true)
cop
end

let(:schema_path) do
f = Tempfile.create('rubocop-rails-CalendarTypeSuffix-test-')
f.close
Pathname(f.path)
end

let(:cop_config) do
{
'DateSuffix' => 'on',
'DateTimeSuffix' => 'at',
'TimeSuffix' => 'time'
}
end

before do
RuboCop::Rails::SchemaLoader.reset!
schema_path.write(schema)
allow(RuboCop::Rails::SchemaLoader).to receive(:db_schema_path)
.and_return(schema_path)
end

after do
RuboCop::Rails::SchemaLoader.reset!
schema_path.unlink
end

context 'when calendar type columns do no follow the conventional suffixes' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table "users", force: :cascade do |t|
t.date "sign_up", null: false
t.datetime "last_login"
t.time "lock_until"
end
end
RUBY

it 'registers an offense' do
expect_offense(<<~RUBY)
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table "users", force: :cascade do |t|
t.date "sign_up", null: false
^^^^^^^^^^^^^^^^ Columns of type `date` should be named with a `on` suffix.
t.datetime "last_login"
^^^^^^^^^^^^^^^^^^^^^^^ Columns of type `datetime` should be named with a `at` suffix.
t.time "lock_until"
^^^^^^^^^^^^^^^^^^^ Columns of type `time` should be named with a `time` suffix.
end
end
RUBY
end
end

context 'when calendar type columns follow the conventional suffixes' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table "users", force: :cascade do |t|
t.date "signed_up_on", null: false
t.datetime "last_login_at"
t.time "locked_until_time"
end
end
RUBY

it 'does not register an offense' do
expect_no_offenses(schema)
end
end
end

0 comments on commit b6fb70b

Please sign in to comment.