From b2bbf6f963b5fd4a4cabf81cab0e86bf37b7d49d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 8 Nov 2024 15:39:33 +0300 Subject: [PATCH] Introduce `AllCops: MigratedSchemaVersion` config This PR introduces `AllCops: MigratedSchemaVersion` config. By specifying `MigratedSchemaVersion` option, migration files that have been migrated can be ignored. When `MigratedSchemaVersion: '20241231000000'` is set. Migration files lower than or equal to '20250101000000' will be ignored. For example, this is the timestamp in db/migrate/20250101000000_create_articles.rb. ```yaml AllCops MigratedSchemaVersion: 20250101000000 ``` This prevents inspecting schema settings for already applied migration files and avoids having different database schemas depending on when `bin/rails db:migrate` is executed. --- README.md | 14 +++++++ .../new_intro_migrated_schema_version.md | 1 + config/default.yml | 4 ++ docs/modules/ROOT/pages/usage.adoc | 15 ++++++++ lib/rubocop/cop/mixin/migrations_helper.rb | 29 ++++++++++++++ lib/rubocop/cop/rails/add_column_index.rb | 1 + lib/rubocop/cop/rails/bulk_change_table.rb | 1 + .../cop/rails/dangerous_column_names.rb | 2 + lib/rubocop/cop/rails/migration_class_name.rb | 2 +- lib/rubocop/cop/rails/not_null_column.rb | 1 + lib/rubocop/cop/rails/reversible_migration.rb | 2 +- .../reversible_migration_method_definition.rb | 2 +- lib/rubocop/cop/rails/schema_comment.rb | 1 + .../cop/rails/three_state_boolean_column.rb | 3 +- .../cop/rails/add_column_index_spec.rb | 34 ++++++++++++++--- .../cop/rails/dangerous_column_names_spec.rb | 16 +++++++- .../cop/rails/migration_class_name_spec.rb | 7 +++- .../rubocop/cop/rails/not_null_column_spec.rb | 38 +++++++++++-------- ...rsible_migration_method_definition_spec.rb | 26 ++++++++++--- .../cop/rails/reversible_migration_spec.rb | 21 ++++++++-- spec/rubocop/cop/rails/schema_comment_spec.rb | 32 ++++++++++------ .../rails/three_state_boolean_column_spec.rb | 22 ++++++++--- 22 files changed, 220 insertions(+), 54 deletions(-) create mode 100644 changelog/new_intro_migrated_schema_version.md diff --git a/README.md b/README.md index 72a264c7b3..3c1e74b740 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,20 @@ gems.locked file to find the version of Rails that has been bound to the application. If neither of those files exist, RuboCop will use Rails 5.0 as the default. +### `AllCops: MigratedSchemaVersion` + +By specifying `MigratedSchemaVersion` option, migration files that have been migrated can be ignored. +When `MigratedSchemaVersion: '20241231000000'` is set. Migration files lower than or equal to '20250101000000' will be ignored. +For example, this is the timestamp in db/migrate/20250101000000_create_articles.rb. + +```yaml +AllCops + MigratedSchemaVersion: '20250101000000' +``` + +This prevents inspecting schema settings for already applied migration files and avoids having different database schemas +depending on when `bin/rails db:migrate` is executed. + ## Rails configuration tip In Rails 6.1+, add the following `config.generators.after_generate` setting to diff --git a/changelog/new_intro_migrated_schema_version.md b/changelog/new_intro_migrated_schema_version.md new file mode 100644 index 0000000000..155ca86419 --- /dev/null +++ b/changelog/new_intro_migrated_schema_version.md @@ -0,0 +1 @@ +* [#1383](https://github.com/rubocop/rubocop-rails/pull/1383): Introduce `AllCops: MigratedSchemaVersion` config. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 0e7d183460..7e64608a9c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -25,6 +25,10 @@ AllCops: # application. If neither of those files exist, RuboCop will use Rails 5.0 # as the default. TargetRailsVersion: ~ + # By specifying `MigratedSchemaVersion` option, migration files that have been migrated can be ignored. + # When `MigratedSchemaVersion: '20241231000000'` is set. Migration files lower than or equal to '20250101000000' will be ignored. + # For example, this is the timestamp in db/migrate/20250101000000_create_articles.rb. + MigratedSchemaVersion: ~ Lint/NumberConversion: # Add Rails' duration methods to the ignore list for `Lint/NumberConversion` diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index c4aa17b024..c1848aefcd 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -52,6 +52,21 @@ gems.locked file to find the version of Rails that has been bound to the application. If neither of those files exist, RuboCop will use Rails 5.0 as the default. +=== `AllCops: MigratedSchemaVersion` + +By specifying `MigratedSchemaVersion` option, migration files that have been migrated can be ignored. +When `MigratedSchemaVersion: '20241231000000'` is set. Migration files lower than or equal to '20250101000000' will be ignored. +For example, this is the timestamp in db/migrate/20250101000000_create_articles.rb. + +[source,yaml] +---- +AllCops + MigratedSchemaVersion: '20250101000000' +---- + +This prevents inspecting schema settings for already applied migration files and avoids having different database schemas +depending on when `bin/rails db:migrate` is executed. + == Rails configuration tip In Rails 6.1+, add the following `config.generators.after_generate` setting to diff --git a/lib/rubocop/cop/mixin/migrations_helper.rb b/lib/rubocop/cop/mixin/migrations_helper.rb index 76dedda41f..ecdb3e146a 100644 --- a/lib/rubocop/cop/mixin/migrations_helper.rb +++ b/lib/rubocop/cop/mixin/migrations_helper.rb @@ -21,6 +21,35 @@ def in_migration?(node) migration_class?(class_node) end end + + # rubocop:disable Style/DocumentDynamicEvalDefinition + %i[on_send on_csend on_block on_numblock on_class].each do |method| + class_eval(<<~RUBY, __FILE__, __LINE__ + 1) + def #{method}(node) + return if already_migrated_file? + + super if method(__method__).super_method + end + RUBY + end + # rubocop:enable Style/DocumentDynamicEvalDefinition + + private + + def already_migrated_file? + return false unless migrated_schema_version + + match_data = File.basename(processed_source.file_path).match(/(?\d{14})/) + schema_version = match_data['timestamp'] if match_data + + return false unless schema_version + + schema_version <= migrated_schema_version.to_s # Ignore applied migration files. + end + + def migrated_schema_version + config.for_all_cops.fetch('MigratedSchemaVersion', nil) + end end end end diff --git a/lib/rubocop/cop/rails/add_column_index.rb b/lib/rubocop/cop/rails/add_column_index.rb index 6f58055695..480a1e84c9 100644 --- a/lib/rubocop/cop/rails/add_column_index.rb +++ b/lib/rubocop/cop/rails/add_column_index.rb @@ -19,6 +19,7 @@ module Rails class AddColumnIndex < Base extend AutoCorrector include RangeHelp + prepend MigrationsHelper MSG = '`add_column` does not accept an `index` key, use `add_index` instead.' RESTRICT_ON_SEND = %i[add_column].freeze diff --git a/lib/rubocop/cop/rails/bulk_change_table.rb b/lib/rubocop/cop/rails/bulk_change_table.rb index 83ebb9b70e..62ec19c5df 100644 --- a/lib/rubocop/cop/rails/bulk_change_table.rb +++ b/lib/rubocop/cop/rails/bulk_change_table.rb @@ -65,6 +65,7 @@ module Rails # end class BulkChangeTable < Base include DatabaseTypeResolvable + prepend MigrationsHelper MSG_FOR_CHANGE_TABLE = <<~MSG.chomp You can combine alter queries using `bulk: true` options. diff --git a/lib/rubocop/cop/rails/dangerous_column_names.rb b/lib/rubocop/cop/rails/dangerous_column_names.rb index 3961e3b40b..1d24e8c526 100644 --- a/lib/rubocop/cop/rails/dangerous_column_names.rb +++ b/lib/rubocop/cop/rails/dangerous_column_names.rb @@ -14,6 +14,8 @@ module Rails # # good # add_column :users, :saved class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength + prepend MigrationsHelper + COLUMN_TYPE_METHOD_NAMES = %i[ bigint binary diff --git a/lib/rubocop/cop/rails/migration_class_name.rb b/lib/rubocop/cop/rails/migration_class_name.rb index 1e895accde..ecb05a8f00 100644 --- a/lib/rubocop/cop/rails/migration_class_name.rb +++ b/lib/rubocop/cop/rails/migration_class_name.rb @@ -20,7 +20,7 @@ module Rails # class MigrationClassName < Base extend AutoCorrector - include MigrationsHelper + prepend MigrationsHelper MSG = 'Replace with `%s` that matches the file name.' diff --git a/lib/rubocop/cop/rails/not_null_column.rb b/lib/rubocop/cop/rails/not_null_column.rb index 3395e41265..d3307f50f2 100644 --- a/lib/rubocop/cop/rails/not_null_column.rb +++ b/lib/rubocop/cop/rails/not_null_column.rb @@ -41,6 +41,7 @@ module Rails # change_column_null :products, :category_id, false class NotNullColumn < Base include DatabaseTypeResolvable + prepend MigrationsHelper MSG = 'Do not add a NOT NULL column without a default value.' RESTRICT_ON_SEND = %i[add_column add_reference].freeze diff --git a/lib/rubocop/cop/rails/reversible_migration.rb b/lib/rubocop/cop/rails/reversible_migration.rb index 7f442a1f98..899de2707d 100644 --- a/lib/rubocop/cop/rails/reversible_migration.rb +++ b/lib/rubocop/cop/rails/reversible_migration.rb @@ -151,7 +151,7 @@ module Rails # remove_index :users, column: :email # end class ReversibleMigration < Base - include MigrationsHelper + prepend MigrationsHelper MSG = '%s is not reversible.' diff --git a/lib/rubocop/cop/rails/reversible_migration_method_definition.rb b/lib/rubocop/cop/rails/reversible_migration_method_definition.rb index fbf2b1d87d..979f066a6b 100644 --- a/lib/rubocop/cop/rails/reversible_migration_method_definition.rb +++ b/lib/rubocop/cop/rails/reversible_migration_method_definition.rb @@ -43,7 +43,7 @@ module Rails # end # end class ReversibleMigrationMethodDefinition < Base - include MigrationsHelper + prepend MigrationsHelper MSG = 'Migrations must contain either a `change` method, or both an `up` and a `down` method.' diff --git a/lib/rubocop/cop/rails/schema_comment.rb b/lib/rubocop/cop/rails/schema_comment.rb index 0e98e2eca9..bf40899dc8 100644 --- a/lib/rubocop/cop/rails/schema_comment.rb +++ b/lib/rubocop/cop/rails/schema_comment.rb @@ -23,6 +23,7 @@ module Rails # class SchemaComment < Base include ActiveRecordMigrationsHelper + prepend MigrationsHelper COLUMN_MSG = 'New database column without `comment`.' TABLE_MSG = 'New database table without `comment`.' diff --git a/lib/rubocop/cop/rails/three_state_boolean_column.rb b/lib/rubocop/cop/rails/three_state_boolean_column.rb index 50351910db..1bdbebe248 100644 --- a/lib/rubocop/cop/rails/three_state_boolean_column.rb +++ b/lib/rubocop/cop/rails/three_state_boolean_column.rb @@ -18,8 +18,9 @@ module Rails # t.boolean :active, default: true, null: false # class ThreeStateBooleanColumn < Base - MSG = 'Boolean columns should always have a default value and a `NOT NULL` constraint.' + prepend MigrationsHelper + MSG = 'Boolean columns should always have a default value and a `NOT NULL` constraint.' RESTRICT_ON_SEND = %i[add_column column boolean].freeze def_node_matcher :three_state_boolean?, <<~PATTERN diff --git a/spec/rubocop/cop/rails/add_column_index_spec.rb b/spec/rubocop/cop/rails/add_column_index_spec.rb index c29c34cebe..575de16c86 100644 --- a/spec/rubocop/cop/rails/add_column_index_spec.rb +++ b/spec/rubocop/cop/rails/add_column_index_spec.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::AddColumnIndex, :config do + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }) + end + it 'registers an offense and corrects when an `add_column` call has `index: true`' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, default: 0, index: true ^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. RUBY @@ -14,7 +18,7 @@ end it 'registers an offense and corrects when an `add_column` call has `index:` with a hash' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, default: 0, index: { unique: true, name: 'my_unique_index' } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. RUBY @@ -26,7 +30,7 @@ end it 'registers an offense and corrects with there is another hash key after `index`' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, index: true, default: 0 ^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. RUBY @@ -38,7 +42,7 @@ end it 'registers an offense and corrects with string keys' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, 'index' => true, default: 0 ^^^^^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. RUBY @@ -50,7 +54,7 @@ end it 'registers an offense and corrects when on multiple lines' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, index: true, ^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. @@ -65,7 +69,7 @@ end it 'can correct multiple `add_column` calls' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, default: 0, index: true ^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. add_column :table, :column2, :integer, default: 0, index: true @@ -85,4 +89,22 @@ add_column :table, :column, :integer, default: 0 RUBY end + + context '`MigratedSchemaVersion` is an integer' do + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => 20240101010101 }) # rubocop:disable Style/NumericLiterals + end + + it 'registers an offense and corrects when an `add_column` call has `index: true`' do + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') + add_column :table, :column, :integer, default: 0, index: true + ^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead. + RUBY + + expect_correction(<<~RUBY) + add_column :table, :column, :integer, default: 0 + add_index :table, :column + RUBY + end + end end diff --git a/spec/rubocop/cop/rails/dangerous_column_names_spec.rb b/spec/rubocop/cop/rails/dangerous_column_names_spec.rb index f6954959b8..8be00eb5f7 100644 --- a/spec/rubocop/cop/rails/dangerous_column_names_spec.rb +++ b/spec/rubocop/cop/rails/dangerous_column_names_spec.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::DangerousColumnNames, :config do + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }) + end + context 'with non-dangerous column name' do it 'does not register an offense' do expect_no_offenses(<<~RUBY) @@ -18,6 +22,14 @@ end end + context 'with dangerous column name on `add_column` when migration file was migrated' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY, '20190101010101_add_save_to_users.rb') + add_column :users, :save, :string + RUBY + end + end + context 'with dangerous column name on `rename_column`' do it 'registers an offense' do expect_offense(<<~RUBY) @@ -29,7 +41,7 @@ context 'with dangerous column name on `t.string`' do it 'registers an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users do |t| t.string :save ^^^^^ Avoid dangerous column names. @@ -40,7 +52,7 @@ context 'with dangerous column name on `t.rename`' do it 'registers an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users do |t| t.rename :name, :save ^^^^^ Avoid dangerous column names. diff --git a/spec/rubocop/cop/rails/migration_class_name_spec.rb b/spec/rubocop/cop/rails/migration_class_name_spec.rb index a0a7c23302..9fb60392da 100644 --- a/spec/rubocop/cop/rails/migration_class_name_spec.rb +++ b/spec/rubocop/cop/rails/migration_class_name_spec.rb @@ -1,7 +1,10 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::MigrationClassName, :config do - let(:filename) { 'db/migrate/20220101050505_create_users.rb' } + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }) + end + let(:filename) { 'db/migrate/20250101010101_create_users.rb' } context 'when the class name matches its file name' do it 'does not register an offense' do @@ -85,7 +88,7 @@ class AddBlobs < ActiveRecord::Migration[7.0] # end # context 'when `ActiveSupport::Inflector` is applied to the class name and the case is different' do - let(:filename) { 'db/migrate/20210623095243_remove_unused_oauth_scope_grants.rb' } + let(:filename) { 'db/migrate/20250101010101_remove_unused_oauth_scope_grants.rb' } it 'does not register an offense' do expect_no_offenses(<<~RUBY, filename) diff --git a/spec/rubocop/cop/rails/not_null_column_spec.rb b/spec/rubocop/cop/rails/not_null_column_spec.rb index 580f998026..af2693038a 100644 --- a/spec/rubocop/cop/rails/not_null_column_spec.rb +++ b/spec/rubocop/cop/rails/not_null_column_spec.rb @@ -1,16 +1,28 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::NotNullColumn, :config do - let(:cop_config) { { 'Include' => nil } } + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }, + 'Rails/NotNullColumn' => { 'Database' => database, 'Include' => nil } + ) + end + let(:database) { 'sqlite3' } context 'with add_column call' do context 'with null: false' do it 'reports an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_name_to_users.rb') add_column :users, :name, :string, null: false ^^^^^^^^^^^ Do not add a NOT NULL column without a default value. RUBY end + + it 'does not register an offense when migration file was migrated' do + expect_no_offenses(<<~RUBY, '20190101010101_add_name_to_users.rb') + add_column :users, :name, :string, null: false + RUBY + end end context 'with null: false and default' do @@ -31,7 +43,7 @@ context 'with null: false and default: nil' do it 'reports an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_name_to_users.rb') add_column :users, :name, :string, null: false, default: nil ^^^^^^^^^^^ Do not add a NOT NULL column without a default value. RUBY @@ -102,7 +114,7 @@ def change context 'with shortcut column call' do context 'with null: false' do it 'reports an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') def change change_table :users do |t| t.string :name, null: false @@ -113,7 +125,7 @@ def change end it 'reports multiple offenses' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') def change change_table :users do |t| t.string :name, null: false @@ -154,7 +166,7 @@ def change context 'with column call' do context 'with null: false' do it 'reports an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') def change change_table :users do |t| t.column :name, :string, null: false @@ -193,7 +205,7 @@ def change context 'with reference call' do context 'with null: false' do it 'reports an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') def change change_table :users do |t| t.references :address, null: false @@ -221,7 +233,7 @@ def change context 'with add_reference call' do context 'with null: false' do it 'reports an offense' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_products.rb') add_reference :products, :category, null: false ^^^^^^^^^^^ Do not add a NOT NULL column without a default value. RUBY @@ -246,9 +258,7 @@ def change end context 'when database is MySQL' do - let(:cop_config) do - { 'Database' => 'mysql' } - end + let(:database) { 'mysql' } it 'does not register an offense when using `null: false` for `:text` type' do expect_no_offenses(<<~RUBY) @@ -268,12 +278,10 @@ def change end context 'when database is PostgreSQL' do - let(:cop_config) do - { 'Database' => 'postgresql' } - end + let(:database) { 'postgresql' } it 'registers an offense when using `null: false` for `:text` type' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_content_to_articles.rb') def change add_column :articles, :content, :text, null: false ^^^^^^^^^^^ Do not add a NOT NULL column without a default value. diff --git a/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb b/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb index ebace61127..a5dcd455c6 100644 --- a/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb +++ b/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::ReversibleMigrationMethodDefinition, :config do + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }) + end + it 'does not register an offense with a change method' do expect_no_offenses(<<~RUBY) class SomeMigration < ActiveRecord::Migration[6.0] @@ -12,7 +16,7 @@ def change end it 'registers an offense with only an up method' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_some_migration.rb') class SomeMigration < ActiveRecord::Migration[6.0] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. @@ -23,8 +27,18 @@ def up RUBY end + it 'does not register an offense with only an up method when migration file was migrated' do + expect_no_offenses(<<~RUBY, '20190101010101_some_migration.rb') + class SomeMigration < ActiveRecord::Migration[6.0] + def up + add_column :users, :email, :text, null: false + end + end + RUBY + end + it 'registers an offense with only an up method and `::` prefixed class name' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_some_migration.rb') class ::SomeMigration < ActiveRecord::Migration[6.0] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. @@ -36,7 +50,7 @@ def up end it 'registers an offense with only a down method' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_some_migration.rb') class SomeMigration < ActiveRecord::Migration[6.0] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. @@ -62,7 +76,7 @@ def down end it "registers an offense with a typo'd change method" do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_some_migration.rb') class SomeMigration < ActiveRecord::Migration[6.0] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. def chance @@ -102,7 +116,7 @@ def change end it 'registers offenses correctly with any migration class' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_some_migration.rb') class SomeMigration < ActiveRecord::Migration[5.2] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. def chance @@ -144,7 +158,7 @@ def change end it 'registers an offense with only an up method' do - expect_offense(<<~RUBY, 'db/animals_migrate/20211007000002_add_nice_to_animals.rb') + expect_offense(<<~RUBY, 'db/animals_migrate/20250101010101_add_nice_to_animals.rb') class AddNiceToAnimals < ActiveRecord::Migration[7.0] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. diff --git a/spec/rubocop/cop/rails/reversible_migration_spec.rb b/spec/rubocop/cop/rails/reversible_migration_spec.rb index 445afd42e0..0962c05b85 100644 --- a/spec/rubocop/cop/rails/reversible_migration_spec.rb +++ b/spec/rubocop/cop/rails/reversible_migration_spec.rb @@ -1,6 +1,11 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::ReversibleMigration, :config do + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'MigratedSchemaVersion' => '20240101010101', 'TargetRailsVersion' => rails_version } + ) + end let(:source) do <<~RUBY class ExampleMigration < ActiveRecord::Migration[7.0] @@ -312,7 +317,7 @@ def change context 'when multiple databases' do it 'does not register an offense for reversible operation' do - expect_no_offenses(<<~RUBY, 'db/animals_migrate/20211007000002_create_animals.rb') + expect_no_offenses(<<~RUBY, 'db/animals_migrate/20250101010101_create_animals.rb') class CreateAnimals < ActiveRecord::Migration[7.0] def change create_table :animals @@ -322,7 +327,7 @@ def change end it 'registers an offense for irreversible operation' do - expect_offense(<<~RUBY, 'db/animals_migrate/20211007000002_remove_animals.rb') + expect_offense(<<~RUBY, 'db/animals_migrate/20250101010101_remove_animals.rb') class RemoveAnimals < ActiveRecord::Migration[7.0] def change drop_table :animals @@ -331,11 +336,21 @@ def change end RUBY end + + it 'does not register an offense for irreversible operation when migration file was migrated' do + expect_no_offenses(<<~RUBY, 'db/animals_migrate/20190101010101_remove_animals.rb') + class RemoveAnimals < ActiveRecord::Migration[7.0] + def change + drop_table :animals + end + end + RUBY + end end context 'when irreversible operation is used in `::` prefixed class definition' do it 'registers an offense' do - expect_offense(<<~RUBY, 'db/migrate/20211007000002_remove_animals.rb') + expect_offense(<<~RUBY, 'db/migrate/20250101010101_remove_animals.rb') class ::RemoveAnimals < ActiveRecord::Migration[7.0] def change drop_table :animals diff --git a/spec/rubocop/cop/rails/schema_comment_spec.rb b/spec/rubocop/cop/rails/schema_comment_spec.rb index e7ee07aecb..aa7ab81637 100644 --- a/spec/rubocop/cop/rails/schema_comment_spec.rb +++ b/spec/rubocop/cop/rails/schema_comment_spec.rb @@ -1,30 +1,40 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::SchemaComment, :config do + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }) + end + context 'when send add_column' do it 'registers an offense when `add_column` has no `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`. RUBY end + it 'does not register an offense when `add_column` has no `comment` option when migration file was migrated' do + expect_no_offenses(<<~RUBY, '20190101010101_add_column_to_table.rb') + add_column :table, :column, :integer + RUBY + end + it 'registers an offense when `add_column` has no `comment` option, but other options' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') 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) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') 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) + expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb') add_column :table, :column, :integer, comment: '' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`. RUBY @@ -45,7 +55,7 @@ context 'when send create_table' do it 'registers an offense when `create_table` has no `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users do |t| ^^^^^^^^^^^^^^^^^^^ New database table without `comment`. end @@ -53,7 +63,7 @@ end it 'registers an offense when `create_table` has a nil `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users, comment: nil do |t| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database table without `comment`. end @@ -61,7 +71,7 @@ end it 'registers an offense when `create_table` has a empty `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users, comment: '' do |t| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database table without `comment`. @@ -70,7 +80,7 @@ end it 'registers an offense when `t.column` has no `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users, comment: 'Table' do |t| t.column :column, :integer ^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`. @@ -79,7 +89,7 @@ end it 'registers an offense when `t.integer` has no `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users, comment: 'Table' do |t| t.integer :column ^^^^^^^^^^^^^^^^^ New database column without `comment`. @@ -88,7 +98,7 @@ end it 'registers two offenses when two `t.column` have no `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users, comment: 'Table' do |t| t.column :column1, :integer ^^^^^^^^^^^^^^^^^^^^^^^^^^^ New database column without `comment`. @@ -99,7 +109,7 @@ end it 'registers two offenses when two `t.integer` have no `comment` option' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_create_users.rb') create_table :users, comment: 'Table' do |t| t.integer :column1 ^^^^^^^^^^^^^^^^^^ New database column without `comment`. diff --git a/spec/rubocop/cop/rails/three_state_boolean_column_spec.rb b/spec/rubocop/cop/rails/three_state_boolean_column_spec.rb index cada92291c..7cf3bc45c6 100644 --- a/spec/rubocop/cop/rails/three_state_boolean_column_spec.rb +++ b/spec/rubocop/cop/rails/three_state_boolean_column_spec.rb @@ -1,9 +1,13 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::ThreeStateBooleanColumn, :config do + let(:config) do + RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' }) + end + describe '#add_column' do it 'registers an offense with three state boolean' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_active_to_users.rb') add_column :users, :active, :boolean ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Boolean columns should always have a default value and a `NOT NULL` constraint. add_column :users, :active, :boolean, default: true @@ -13,6 +17,14 @@ RUBY end + it 'does not register an offense with three state boolean when migration file was migrated' do + expect_no_offenses(<<~RUBY, '20190101010101_add_active_to_users.rb') + add_column :users, :active, :boolean + add_column :users, :active, :boolean, default: true + add_column :users, :active, :boolean, default: true, null: true + RUBY + end + it 'does not register an offense with non boolean' do expect_no_offenses(<<~RUBY) add_column :users, :email, :string @@ -44,7 +56,7 @@ def change end it 'registers an offense when using `#change_column_null` for other table or column' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_active_to_users.rb') def change add_column :users, :active, :boolean ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Boolean columns should always have a default value and a `NOT NULL` constraint. @@ -58,7 +70,7 @@ def change describe '#column' do it 'registers an offense with three state boolean' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_active_to_users.rb') t.column :active, :boolean ^^^^^^^^^^^^^^^^^^^^^^^^^^ Boolean columns should always have a default value and a `NOT NULL` constraint. t.column :active, :boolean, default: true @@ -119,7 +131,7 @@ def change describe '#boolean' do it 'registers an offense for three state boolean' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_active_to_users.rb') t.boolean :active ^^^^^^^^^^^^^^^^^ Boolean columns should always have a default value and a `NOT NULL` constraint. t.boolean :active, default: true @@ -168,7 +180,7 @@ def change end it 'registers an offense when using `#change_column_null` for other table or column' do - expect_offense(<<~RUBY) + expect_offense(<<~RUBY, '20250101010101_add_active_to_users.rb') def change create_table(:users) do |t| t.boolean :active