From 5a42999bbabc013014136da0f36bbd9e4d82509c Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Tue, 4 Jul 2023 14:15:48 -0400 Subject: [PATCH] Add new `Rails/MigrationTimestamp` cop This cop enforces that migration file names start with a valid timestamp in the past. --- ...w_add_new_rails_migration_timestamp_cop.md | 1 + config/default.yml | 7 ++ lib/rubocop/cop/rails/migration_timestamp.rb | 63 +++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/migration_timestamp_spec.rb | 80 +++++++++++++++++++ 5 files changed, 152 insertions(+) create mode 100644 changelog/new_add_new_rails_migration_timestamp_cop.md create mode 100644 lib/rubocop/cop/rails/migration_timestamp.rb create mode 100644 spec/rubocop/cop/rails/migration_timestamp_spec.rb diff --git a/changelog/new_add_new_rails_migration_timestamp_cop.md b/changelog/new_add_new_rails_migration_timestamp_cop.md new file mode 100644 index 0000000000..13656692c0 --- /dev/null +++ b/changelog/new_add_new_rails_migration_timestamp_cop.md @@ -0,0 +1 @@ +* [#1044](https://github.com/rubocop/rubocop-rails/pull/1044): Add new `Rails/MigrationTimestamp` cop. ([@sambostock][]) diff --git a/config/default.yml b/config/default.yml index 0fef0c30d5..286c5d8c01 100644 --- a/config/default.yml +++ b/config/default.yml @@ -656,6 +656,13 @@ Rails/MigrationClassName: Include: - db/**/*.rb +Rails/MigrationTimestamp: + Description: 'Checks that migration filenames start with a valid timestamp in the past.' + Enabled: pending + VersionAdded: '<>' + Include: + - db/migrate/**/*.rb + Rails/NegateInclude: Description: 'Prefer `collection.exclude?(obj)` over `!collection.include?(obj)`.' StyleGuide: 'https://rails.rubystyle.guide#exclude' diff --git a/lib/rubocop/cop/rails/migration_timestamp.rb b/lib/rubocop/cop/rails/migration_timestamp.rb new file mode 100644 index 0000000000..c4ac6ef7a4 --- /dev/null +++ b/lib/rubocop/cop/rails/migration_timestamp.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'time' + +module RuboCop + module Cop + module Rails + # Checks that migration file names start with a valid timestamp. + # + # @example + # # bad + # # db/migrate/bad.rb + + # # bad + # # db/migrate/123_bad.rb + + # # bad + # # db/migrate/20171301000000_bad.rb + # + # # good + # # db/migrate/20170101000000_good.rb + # + class MigrationTimestamp < Base + include RangeHelp + + MSG = 'Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past.' + + def on_new_investigation + file_path = processed_source.file_path + return unless file_path.include?('db/migrate') + + timestamp = File.basename(file_path).split('_', 2).first + return if valid_timestamp?(timestamp) + + add_offense(source_range(processed_source.buffer, 1, 0)) + end + + private + + def valid_timestamp?(timestamp) + format = '%Y%m%d%H%M%S' + format_with_utc_suffix = '%Y%m%d%H%M%S %Z' + timestamp_with_utc_suffix = "#{timestamp} UTC" + + timestamp && + # Time.strptime has no way to externally declare what timezone the string is in, so we append it. + (time = Time.strptime(timestamp_with_utc_suffix, format_with_utc_suffix)) && + # Time.strptime fuzzily accepts invalid dates around boundaries + # | Wrong Days per Month | 24th Hour | 60th Minute | 60th Second + # ---------+----------------------+----------------+----------------+---------------- + # Actual | 20000231000000 | 20000101240000 | 20000101006000 | 20000101000060 + # Expected | 20000302000000 | 20000102000000 | 20000101010000 | 20000101000100 + # We want normalized values, so we can check if Time#strftime matches the original. + time.strftime(format) == timestamp && + # No timestamps in the future + time <= Time.now.utc + rescue ArgumentError + false + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 5e0db983d6..1a6e35a087 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -73,6 +73,7 @@ require_relative 'rails/mailer_name' require_relative 'rails/match_route' require_relative 'rails/migration_class_name' +require_relative 'rails/migration_timestamp' require_relative 'rails/negate_include' require_relative 'rails/not_null_column' require_relative 'rails/order_by_id' diff --git a/spec/rubocop/cop/rails/migration_timestamp_spec.rb b/spec/rubocop/cop/rails/migration_timestamp_spec.rb new file mode 100644 index 0000000000..9b20d507fc --- /dev/null +++ b/spec/rubocop/cop/rails/migration_timestamp_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::MigrationTimestamp, :config do + it 'registers no offenses if timestamp is valid' do + expect_no_offenses(<<~RUBY, 'db/migrate/20170101000000_good.rb') + # ... + RUBY + end + + it 'registers an offense if timestamp is impossible' do + expect_offense(<<~RUBY, 'db/migrate/20002222222222_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if timestamp swaps month and day' do + expect_offense(<<~RUBY, 'db/migrate/20003112000000_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if timestamp day is wrong' do + expect_offense(<<~RUBY, 'db/migrate/20000231000000_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if timestamp hours are invalid' do + expect_offense(<<~RUBY, 'db/migrate/20000101240000_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if timestamp minutes are invalid' do + expect_offense(<<~RUBY, 'db/migrate/20000101006000_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if timestamp seconds are invalid' do + expect_offense(<<~RUBY, 'db/migrate/20000101000060_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if timestamp is invalid' do + expect_offense(<<~RUBY, 'db/migrate/123_bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if no timestamp at all' do + expect_offense(<<~RUBY, 'db/migrate/bad.rb') + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers an offense if the timestamp is in the future' do + timestamp = (Time.now.utc + 5).strftime('%Y%m%d%H%M%S') + expect_offense(<<~RUBY, "db/migrate/#{timestamp}_bad.rb") + # ... + ^ Migration file name must start with a valid `YYYYmmddHHMMSS_` timestamp in the past. + RUBY + end + + it 'registers no offense if the timestamp is in the past' do + timestamp = (Time.now.utc - 5).strftime('%Y%m%d%H%M%S') + expect_no_offenses(<<~RUBY, "db/migrate/#{timestamp}_good.rb") + # ... + RUBY + end +end