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)