From 9c27c90b4a8a9ea179af9588a7dbd2037829905f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 24 Apr 2017 12:16:33 -0500 Subject: Rename AddColumnWithDefault to ReversibleAddColumnWithDefault We're going to add another cop that deals with another aspect of `add_column_with_default`, so we need to separate them. --- rubocop/cop/migration/add_column_with_default.rb | 35 ------------------ .../reversible_add_column_with_default.rb | 35 ++++++++++++++++++ rubocop/rubocop.rb | 2 +- .../cop/migration/add_column_with_default_spec.rb | 41 ---------------------- .../reversible_add_column_with_default_spec.rb | 41 ++++++++++++++++++++++ 5 files changed, 77 insertions(+), 77 deletions(-) delete mode 100644 rubocop/cop/migration/add_column_with_default.rb create mode 100644 rubocop/cop/migration/reversible_add_column_with_default.rb delete mode 100644 spec/rubocop/cop/migration/add_column_with_default_spec.rb create mode 100644 spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb deleted file mode 100644 index e04af8fc096..00000000000 --- a/rubocop/cop/migration/add_column_with_default.rb +++ /dev/null @@ -1,35 +0,0 @@ -require_relative '../../migration_helpers' - -module RuboCop - module Cop - module Migration - # Cop that checks if `add_column_with_default` is used with `up`/`down` methods - # and not `change`. - class AddColumnWithDefault < RuboCop::Cop::Cop - include MigrationHelpers - - def_node_matcher :add_column_with_default?, <<~PATTERN - (send nil :add_column_with_default $...) - PATTERN - - def_node_matcher :defines_change?, <<~PATTERN - (def :change ...) - PATTERN - - MSG = '`add_column_with_default` is not reversible so you must manually define ' \ - 'the `up` and `down` methods in your migration class, using `remove_column` in `down`'.freeze - - def on_send(node) - return unless in_migration?(node) - return unless add_column_with_default?(node) - - node.each_ancestor(:def) do |def_node| - next unless defines_change?(def_node) - - add_offense(def_node, :name) - end - end - end - end - end -end diff --git a/rubocop/cop/migration/reversible_add_column_with_default.rb b/rubocop/cop/migration/reversible_add_column_with_default.rb new file mode 100644 index 00000000000..f413f06f39b --- /dev/null +++ b/rubocop/cop/migration/reversible_add_column_with_default.rb @@ -0,0 +1,35 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if `add_column_with_default` is used with `up`/`down` methods + # and not `change`. + class ReversibleAddColumnWithDefault < RuboCop::Cop::Cop + include MigrationHelpers + + def_node_matcher :add_column_with_default?, <<~PATTERN + (send nil :add_column_with_default $...) + PATTERN + + def_node_matcher :defines_change?, <<~PATTERN + (def :change ...) + PATTERN + + MSG = '`add_column_with_default` is not reversible so you must manually define ' \ + 'the `up` and `down` methods in your migration class, using `remove_column` in `down`'.freeze + + def on_send(node) + return unless in_migration?(node) + return unless add_column_with_default?(node) + + node.each_ancestor(:def) do |def_node| + next unless defines_change?(def_node) + + add_offense(def_node, :name) + end + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index d580aa6857a..cdc0fbfd6a8 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,9 +1,9 @@ require_relative 'cop/custom_error_class' require_relative 'cop/gem_fetcher' require_relative 'cop/migration/add_column' -require_relative 'cop/migration/add_column_with_default' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' require_relative 'cop/migration/remove_concurrent_index' require_relative 'cop/migration/remove_index' +require_relative 'cop/migration/reversible_add_column_with_default' diff --git a/spec/rubocop/cop/migration/add_column_with_default_spec.rb b/spec/rubocop/cop/migration/add_column_with_default_spec.rb deleted file mode 100644 index 6b9b6b19650..00000000000 --- a/spec/rubocop/cop/migration/add_column_with_default_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'spec_helper' - -require 'rubocop' -require 'rubocop/rspec/support' - -require_relative '../../../../rubocop/cop/migration/add_column_with_default' - -describe RuboCop::Cop::Migration::AddColumnWithDefault do - include CopHelper - - subject(:cop) { described_class.new } - - context 'in migration' do - before do - allow(cop).to receive(:in_migration?).and_return(true) - end - - it 'registers an offense when add_column_with_default is used inside a change method' do - inspect_source(cop, 'def change; add_column_with_default :table, :column, default: false; end') - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end - end - - it 'registers no offense when add_column_with_default is used inside an up method' do - inspect_source(cop, 'def up; add_column_with_default :table, :column, default: false; end') - - expect(cop.offenses.size).to eq(0) - end - end - - context 'outside of migration' do - it 'registers no offense' do - inspect_source(cop, 'def change; add_column_with_default :table, :column, default: false; end') - - expect(cop.offenses.size).to eq(0) - end - end -end diff --git a/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb b/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb new file mode 100644 index 00000000000..3723d635083 --- /dev/null +++ b/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/reversible_add_column_with_default' + +describe RuboCop::Cop::Migration::ReversibleAddColumnWithDefault do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when add_column_with_default is used inside a change method' do + inspect_source(cop, 'def change; add_column_with_default :table, :column, default: false; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers no offense when add_column_with_default is used inside an up method' do + inspect_source(cop, 'def up; add_column_with_default :table, :column, default: false; end') + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, 'def change; add_column_with_default :table, :column, default: false; end') + + expect(cop.offenses.size).to eq(0) + end + end +end -- cgit v1.2.1