Commit c47b33ad authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'fix-timestampz-cop' into 'master'

Also warn on timestamp in datetime migration cop

See merge request gitlab-org/gitlab-ce!14784
parents 4261d07b 264be20d
# rubocop:disable Migration/Datetime
class AddArtifactsExpireDateToCiBuilds < ActiveRecord::Migration class AddArtifactsExpireDateToCiBuilds < ActiveRecord::Migration
def change def change
add_column :ci_builds, :artifacts_expire_at, :timestamp add_column :ci_builds, :artifacts_expire_at, :timestamp
......
# rubocop:disable Migration/Datetime
class AddQueuedAtToCiBuilds < ActiveRecord::Migration class AddQueuedAtToCiBuilds < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
# See http://doc.gitlab.com/ce/development/migration_style_guide.html # See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab. # for more information on how to write migrations for GitLab.
......
# rubocop:disable Migration/Datetime
# See http://doc.gitlab.com/ce/development/migration_style_guide.html # See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab. # for more information on how to write migrations for GitLab.
......
...@@ -7,14 +7,18 @@ module RuboCop ...@@ -7,14 +7,18 @@ module RuboCop
class Datetime < RuboCop::Cop::Cop class Datetime < RuboCop::Cop::Cop
include MigrationHelpers 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. # Check methods in table creation.
def on_def(node) def on_def(node)
return unless in_migration?(node) return unless in_migration?(node)
node.each_descendant(:send) do |send_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
end end
...@@ -23,12 +27,14 @@ module RuboCop ...@@ -23,12 +27,14 @@ module RuboCop
return unless in_migration?(node) return unless in_migration?(node)
node.each_descendant do |descendant| node.each_descendant do |descendant|
add_offense(node, :expression) if descendant.type == :sym && descendant.children.last == :datetime next unless descendant.type == :sym
end
end
def method_name(node) last_argument = descendant.children.last
node.children[1]
if last_argument == :datetime || last_argument == :timestamp
add_offense(node, :expression, format(MSG, last_argument))
end
end
end end
end end
end end
......
...@@ -9,6 +9,7 @@ describe RuboCop::Cop::Migration::Datetime do ...@@ -9,6 +9,7 @@ describe RuboCop::Cop::Migration::Datetime do
include CopHelper include CopHelper
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:migration_with_datetime) do let(:migration_with_datetime) do
%q( %q(
class Users < ActiveRecord::Migration class Users < ActiveRecord::Migration
...@@ -22,6 +23,19 @@ describe RuboCop::Cop::Migration::Datetime do ...@@ -22,6 +23,19 @@ describe RuboCop::Cop::Migration::Datetime do
) )
end 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 let(:migration_without_datetime) do
%q( %q(
class Users < ActiveRecord::Migration class Users < ActiveRecord::Migration
...@@ -58,6 +72,17 @@ describe RuboCop::Cop::Migration::Datetime do ...@@ -58,6 +72,17 @@ describe RuboCop::Cop::Migration::Datetime do
aggregate_failures do aggregate_failures do
expect(cop.offenses.size).to eq(1) expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([7]) 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
end end
...@@ -81,6 +106,7 @@ describe RuboCop::Cop::Migration::Datetime do ...@@ -81,6 +106,7 @@ describe RuboCop::Cop::Migration::Datetime do
context 'outside of migration' do context 'outside of migration' do
it 'registers no offense' do it 'registers no offense' do
inspect_source(cop, migration_with_datetime) inspect_source(cop, migration_with_datetime)
inspect_source(cop, migration_with_timestamp)
inspect_source(cop, migration_without_datetime) inspect_source(cop, migration_without_datetime)
inspect_source(cop, migration_with_datetime_with_timezone) inspect_source(cop, migration_with_datetime_with_timezone)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment