summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2017-10-10 10:36:10 +0100
committerSean McGivern <sean@gitlab.com>2017-10-10 10:36:10 +0100
commita560291f143399b2c8d340fb0a02615dc72552c3 (patch)
tree5245ac6e4a7e48ece0f8b1efb1427073058bc6f0
parent43b692cb4b43a476862fb6e7bf4c09edd6035077 (diff)
downloadgitlab-ce-fix-timestampz-cop.tar.gz
Also warn on timestamp in datetime migration copfix-timestampz-cop
The types `timestamp` and `datetime` are aliases: https://github.com/rails/rails/blob/v4.2.10/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L362-L364
-rw-r--r--rubocop/cop/migration/datetime.rb20
-rw-r--r--spec/rubocop/cop/migration/datetime_spec.rb26
2 files changed, 39 insertions, 7 deletions
diff --git a/rubocop/cop/migration/datetime.rb b/rubocop/cop/migration/datetime.rb
index 651935dd53e..9cba3c35b26 100644
--- a/rubocop/cop/migration/datetime.rb
+++ b/rubocop/cop/migration/datetime.rb
@@ -7,14 +7,18 @@ module RuboCop
class Datetime < RuboCop::Cop::Cop
include MigrationHelpers
- MSG = 'Do not use the `datetime` data type, use `datetime_with_timezone` instead'.freeze
+ MSG = 'Do not use the `%s` data type, use `datetime_with_timezone` instead'.freeze
# Check methods in table creation.
def on_def(node)
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
- add_offense(send_node, :selector) if method_name(send_node) == :datetime
+ method_name = node.children[1]
+
+ if method_name == :datetime || method_name == :timestamp
+ add_offense(send_node, :selector, format(MSG, method_name))
+ end
end
end
@@ -23,12 +27,14 @@ module RuboCop
return unless in_migration?(node)
node.each_descendant do |descendant|
- add_offense(node, :expression) if descendant.type == :sym && descendant.children.last == :datetime
- end
- end
+ next unless descendant.type == :sym
- def method_name(node)
- node.children[1]
+ last_argument = descendant.children.last
+
+ if last_argument == :datetime || last_argument == :timestamp
+ add_offense(node, :expression, format(MSG, last_argument))
+ end
+ end
end
end
end
diff --git a/spec/rubocop/cop/migration/datetime_spec.rb b/spec/rubocop/cop/migration/datetime_spec.rb
index 388b086ce6a..b1dfcf1b048 100644
--- a/spec/rubocop/cop/migration/datetime_spec.rb
+++ b/spec/rubocop/cop/migration/datetime_spec.rb
@@ -9,6 +9,7 @@ describe RuboCop::Cop::Migration::Datetime do
include CopHelper
subject(:cop) { described_class.new }
+
let(:migration_with_datetime) do
%q(
class Users < ActiveRecord::Migration
@@ -22,6 +23,19 @@ describe RuboCop::Cop::Migration::Datetime do
)
end
+ let(:migration_with_timestamp) do
+ %q(
+ class Users < ActiveRecord::Migration
+ DOWNTIME = false
+
+ def change
+ add_column(:users, :username, :text)
+ add_column(:users, :last_sign_in, :timestamp)
+ end
+ end
+ )
+ end
+
let(:migration_without_datetime) do
%q(
class Users < ActiveRecord::Migration
@@ -58,6 +72,17 @@ describe RuboCop::Cop::Migration::Datetime do
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([7])
+ expect(cop.offenses.first.message).to include('datetime')
+ end
+ end
+
+ it 'registers an offense when the ":timestamp" data type is used' do
+ inspect_source(cop, migration_with_timestamp)
+
+ aggregate_failures do
+ expect(cop.offenses.size).to eq(1)
+ expect(cop.offenses.map(&:line)).to eq([7])
+ expect(cop.offenses.first.message).to include('timestamp')
end
end
@@ -81,6 +106,7 @@ describe RuboCop::Cop::Migration::Datetime do
context 'outside of migration' do
it 'registers no offense' do
inspect_source(cop, migration_with_datetime)
+ inspect_source(cop, migration_with_timestamp)
inspect_source(cop, migration_without_datetime)
inspect_source(cop, migration_with_datetime_with_timezone)