From b6fb70b230d598e39843b98c0c7a65a2070abb97 Mon Sep 17 00:00:00 2001 From: Jason Schweier Date: Sun, 4 Oct 2020 12:44:35 -0400 Subject: [PATCH] Add new cop --- CHANGELOG.md | 2 + config/default.yml | 8 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rails.adoc | 54 +++++++++++++ lib/rubocop/cop/rails/calendar_type_suffix.rb | 72 +++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/calendar_type_suffix_spec.rb | 78 +++++++++++++++++++ 7 files changed, 216 insertions(+) create mode 100644 lib/rubocop/cop/rails/calendar_type_suffix.rb create mode 100644 spec/rubocop/cop/rails/calendar_type_suffix_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b269f8b6c4..85d940e702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -298,3 +299,4 @@ [@bubaflub]: https://github.com/bubaflub [@dvandersluis]: https://github.com/dvandersluis [@Tietew]: https://github.com/Tietew +[@jmks]: https://github.com/jmks diff --git a/config/default.yml b/config/default.yml index b9d9d38571..864c91fe85 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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: diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 3f616650af..9919029c68 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index 1556dccf99..92462eec80 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -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 |=== diff --git a/lib/rubocop/cop/rails/calendar_type_suffix.rb b/lib/rubocop/cop/rails/calendar_type_suffix.rb new file mode 100644 index 0000000000..bdb891566a --- /dev/null +++ b/lib/rubocop/cop/rails/calendar_type_suffix.rb @@ -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 `%s` should be named with a `%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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index ea05106e55..1530d69415 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -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' diff --git a/spec/rubocop/cop/rails/calendar_type_suffix_spec.rb b/spec/rubocop/cop/rails/calendar_type_suffix_spec.rb new file mode 100644 index 0000000000..c2be79d25b --- /dev/null +++ b/spec/rubocop/cop/rails/calendar_type_suffix_spec.rb @@ -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