diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-10-10 12:26:47 +0000 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-10-10 12:26:47 +0000 |
commit | c47b33ad33fbb9f95e88ad688261aedf7eeb580f (patch) | |
tree | 610a7367bc90669d10516d0999f112c08f6529f1 | |
parent | 4261d07bf09cdc98ac4f0a801a87a59915e909da (diff) | |
parent | 264be20d9449f38f57f2f87a267ea10f5797eabd (diff) | |
download | gitlab-ce-c47b33ad33fbb9f95e88ad688261aedf7eeb580f.tar.gz |
Merge branch 'fix-timestampz-cop' into 'master'
Also warn on timestamp in datetime migration cop
See merge request gitlab-org/gitlab-ce!14784
6 files changed, 43 insertions, 7 deletions
diff --git a/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb b/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb index 915167b038d..8e9ab3f8acc 100644 --- a/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb +++ b/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddArtifactsExpireDateToCiBuilds < ActiveRecord::Migration def change add_column :ci_builds, :artifacts_expire_at, :timestamp diff --git a/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb b/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb index 756910a1fa0..fd7a48d881e 100644 --- a/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb +++ b/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddQueuedAtToCiBuilds < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb b/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb index 6ac10723c82..a5d1eca82bb 100644 --- a/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb +++ b/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb b/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb index 7a1acdcbf69..47ba6bde856 100644 --- a/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb +++ b/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. 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) |