Add latest changes from gitlab-org/gitlab@master

5 files changed, 451 insertions, 268 deletions

diff --git a/spec/rubocop/cop/migration/add_limit_to_string_columns_spec.rb b/spec/rubocop/cop/migration/add_limit_to_string_columns_spec.rb
deleted file mode 100644
index 97a3ae8f2bc..00000000000
--- a/spec/rubocop/cop/migration/add_limit_to_string_columns_spec.rb
+++ /dev/null
@@ -1,268 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-require 'rubocop'
-require 'rubocop/rspec/support'
-
-require_relative '../../../../rubocop/cop/migration/add_limit_to_string_columns'
-
-describe RuboCop::Cop::Migration::AddLimitToStringColumns do
-  include CopHelper
-
-  subject(:cop) { described_class.new }
-
-  context 'in migration' do
-    before do
-      allow(cop).to receive(:in_migration?).and_return(true)
-
-      inspect_source(migration)
-    end
-
-    context 'when creating a table' do
-      context 'with string columns and limit' do
-        let(:migration) do
-          %q(
-           class CreateUsers < ActiveRecord::Migration[5.2]
-             DOWNTIME = false
-
-             def change
-               create_table :users do |t|
-                 t.string :username, null: false, limit: 255
-                 t.timestamps_with_timezone null: true
-               end
-             end
-           end
-          )
-        end
-
-        it 'register no offense' do
-          expect(cop.offenses.size).to eq(0)
-        end
-
-        context 'with limit in a different position' do
-          let(:migration) do
-            %q(
-             class CreateUsers < ActiveRecord::Migration[5.2]
-               DOWNTIME = false
-
-               def change
-                 create_table :users do |t|
-                   t.string :username, limit: 255, null: false
-                   t.timestamps_with_timezone null: true
-                 end
-               end
-             end
-            )
-          end
-
-          it 'registers an offense' do
-            expect(cop.offenses.size).to eq(0)
-          end
-        end
-      end
-
-      context 'with string columns and no limit' do
-        let(:migration) do
-          %q(
-           class CreateUsers < ActiveRecord::Migration[5.2]
-             DOWNTIME = false
-
-             def change
-               create_table :users do |t|
-                 t.string :username, null: false
-                 t.timestamps_with_timezone null: true
-               end
-             end
-           end
-          )
-        end
-
-        it 'registers an offense' do
-          expect(cop.offenses.size).to eq(1)
-          expect(cop.offenses.first.message)
-            .to eq('String columns should have a limit constraint. 255 is suggested')
-        end
-      end
-
-      context 'with no string columns' do
-        let(:migration) do
-          %q(
-            class CreateMilestoneReleases < ActiveRecord::Migration[5.2]
-              DOWNTIME = false
-
-              def change
-                create_table :milestone_releases do |t|
-                  t.integer :milestone_id
-                  t.integer :release_id
-                end
-              end
-            end
-          )
-        end
-
-        it 'register no offense' do
-          expect(cop.offenses.size).to eq(0)
-        end
-      end
-    end
-
-    context 'when adding columns' do
-      context 'with string columns with limit' do
-        let(:migration) do
-          %q(
-            class AddEmailToUsers < ActiveRecord::Migration[5.2]
-              DOWNTIME = false
-
-              def change
-                add_column :users, :email, :string, limit: 255
-              end
-            end
-          )
-        end
-
-        it 'registers no offense' do
-          expect(cop.offenses.size).to eq(0)
-        end
-
-        context 'with limit in a different position' do
-          let(:migration) do
-            %q(
-              class AddEmailToUsers < ActiveRecord::Migration[5.2]
-                DOWNTIME = false
-
-                def change
-                  add_column :users, :email, :string, limit: 255, default: 'example@email.com'
-                end
-              end
-            )
-          end
-
-          it 'registers no offense' do
-            expect(cop.offenses.size).to eq(0)
-          end
-        end
-      end
-
-      context 'with string columns with no limit' do
-        let(:migration) do
-          %q(
-            class AddEmailToUsers < ActiveRecord::Migration[5.2]
-              DOWNTIME = false
-
-              def change
-                add_column :users, :email, :string
-              end
-            end
-          )
-        end
-
-        it 'registers offense' do
-          expect(cop.offenses.size).to eq(1)
-          expect(cop.offenses.first.message)
-            .to eq('String columns should have a limit constraint. 255 is suggested')
-        end
-      end
-
-      context 'with no string columns' do
-        let(:migration) do
-          %q(
-            class AddEmailToUsers < ActiveRecord::Migration[5.2]
-              DOWNTIME = false
-
-              def change
-                add_column :users, :active, :boolean, default: false
-              end
-            end
-          )
-        end
-
-        it 'registers no offense' do
-          expect(cop.offenses.size).to eq(0)
-        end
-      end
-    end
-
-    context 'with add_column_with_default' do
-      context 'with a limit' do
-        let(:migration) do
-          %q(
-           class AddRuleTypeToApprovalMergeRequestRules < ActiveRecord::Migration[5.2]
-             DOWNTIME = false
-
-             def change
-               add_column_with_default(:approval_merge_request_rules, :rule_type, :string, limit: 2, default: 1)
-             end
-           end
-           )
-        end
-
-        it 'registers no offense' do
-          expect(cop.offenses.size).to eq(0)
-        end
-      end
-
-      context 'without a limit' do
-        let(:migration) do
-          %q(
-           class AddRuleTypeToApprovalMergeRequestRules < ActiveRecord::Migration[5.2]
-             DOWNTIME = false
-
-             def change
-               add_column_with_default(:approval_merge_request_rules, :rule_type, :string, default: 1)
-             end
-           end
-           )
-        end
-
-        it 'registers an offense' do
-          expect(cop.offenses.size).to eq(1)
-        end
-      end
-    end
-
-    context 'with methods' do
-      let(:migration) do
-        %q(
-          class AddEmailToUsers < ActiveRecord::Migration[5.2]
-            DOWNTIME = false
-
-            def change
-              add_column_if_table_not_exists :users, :first_name, :string, limit: 255
-              search_namespace(user_name)
-            end
-
-            def add_column_if_not_exists(table, name, *args)
-              add_column(table, name, *args) unless column_exists?(table, name)
-            end
-
-            def search_namespace(username)
-               Uniquify.new.string(username) do |str|
-                query = "SELECT id FROM namespaces WHERE parent_id IS NULL AND path='#{str}' LIMIT 1"
-                connection.exec_query(query)
-              end
-            end
-          end
-        )
-      end
-
-      it 'registers no offense' do
-        expect(cop.offenses.size).to eq(0)
-      end
-    end
-  end
-
-  context 'outside of migrations' do
-    let(:active_record_model) do
-      %q(
-      class User < ApplicationRecord
-      end
-      )
-    end
-
-    it 'registers no offense' do
-      inspect_source(active_record_model)
-
-      expect(cop.offenses.size).to eq(0)
-    end
-  end
-end
diff --git a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
new file mode 100644
index 00000000000..514260a4306
--- /dev/null
+++ b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
@@ -0,0 +1,160 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/migration/add_limit_to_text_columns'
+
+describe RuboCop::Cop::Migration::AddLimitToTextColumns do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'in migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    context 'when text columns are defined without a limit' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class TestTextLimits < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+            disable_ddl_transaction!
+
+            def up
+              create_table :test_text_limits, id: false do |t|
+                t.integer :test_id, null: false
+                t.text :name
+                  ^^^^ #{described_class::MSG}
+              end
+
+              add_column :test_text_limits, :email, :text
+              ^^^^^^^^^^ #{described_class::MSG}
+
+              add_column_with_default :test_text_limits, :role, :text, default: 'default'
+              ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+
+              change_column_type_concurrently :test_text_limits, :test_id, :text
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+            end
+          end
+        RUBY
+
+        expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/AddLimitToTextColumns'))
+      end
+    end
+
+    context 'when text columns are defined with a limit' do
+      it 'registers no offense' do
+        expect_no_offenses(<<~RUBY)
+          class TestTextLimits < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+            disable_ddl_transaction!
+
+            def up
+              create_table :test_text_limits, id: false do |t|
+                t.integer :test_id, null: false
+                t.text :name
+              end
+
+              add_column :test_text_limits, :email, :text
+              add_column_with_default :test_text_limits, :role, :text, default: 'default'
+              change_column_type_concurrently :test_text_limits, :test_id, :text
+
+              add_text_limit :test_text_limits, :name, 255
+              add_text_limit :test_text_limits, :email, 255
+              add_text_limit :test_text_limits, :role, 255
+              add_text_limit :test_text_limits, :test_id, 255
+            end
+          end
+        RUBY
+      end
+    end
+
+    # Make sure that the cop is properly checking for an `add_text_limit`
+    # over the same {table, attribute} as the one that triggered the offence
+    context 'when the limit is defined for a same name attribute but different table' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class TestTextLimits < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+            disable_ddl_transaction!
+
+            def up
+              create_table :test_text_limits, id: false do |t|
+                t.integer :test_id, null: false
+                t.text :name
+                  ^^^^ #{described_class::MSG}
+              end
+
+              add_column :test_text_limits, :email, :text
+              ^^^^^^^^^^ #{described_class::MSG}
+
+              add_column_with_default :test_text_limits, :role, :text, default: 'default'
+              ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+
+              change_column_type_concurrently :test_text_limits, :test_id, :text
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+
+              add_text_limit :wrong_table, :name, 255
+              add_text_limit :wrong_table, :email, 255
+              add_text_limit :wrong_table, :role, 255
+              add_text_limit :wrong_table, :test_id, 255
+            end
+          end
+        RUBY
+
+        expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/AddLimitToTextColumns'))
+      end
+    end
+
+    context 'on down' do
+      it 'registers no offense' do
+        expect_no_offenses(<<~RUBY)
+          class TestTextLimits < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+
+            def up
+              drop_table :no_offence_on_down
+            end
+
+            def down
+              create_table :no_offence_on_down, id: false do |t|
+                t.integer :test_id, null: false
+                t.text :name
+              end
+
+              add_column :no_offence_on_down, :email, :text
+
+              add_column_with_default :no_offence_on_down, :role, :text, default: 'default'
+            end
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'outside of migration' do
+    it 'registers no offense' do
+      expect_no_offenses(<<~RUBY)
+        class TestTextLimits < ActiveRecord::Migration[6.0]
+          DOWNTIME = false
+          disable_ddl_transaction!
+
+          def up
+            create_table :test_text_limits, id: false do |t|
+              t.integer :test_id, null: false
+              t.text :name
+            end
+
+            add_column :test_text_limits, :email, :text
+            add_column_with_default :test_text_limits, :role, :text, default: 'default'
+            change_column_type_concurrently :test_text_limits, :test_id, :text
+          end
+        end
+      RUBY
+    end
+  end
+end
diff --git a/spec/rubocop/cop/migration/prevent_strings_spec.rb b/spec/rubocop/cop/migration/prevent_strings_spec.rb
new file mode 100644
index 00000000000..2702ce1c090
--- /dev/null
+++ b/spec/rubocop/cop/migration/prevent_strings_spec.rb
@@ -0,0 +1,143 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/migration/prevent_strings'
+
+describe RuboCop::Cop::Migration::PreventStrings do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'in migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    context 'when the string data type is used' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class Users < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+
+            def up
+              create_table :users do |t|
+                t.string :username, null: false
+                  ^^^^^^ #{described_class::MSG}
+
+                t.timestamps_with_timezone null: true
+
+                t.string :password
+                  ^^^^^^ #{described_class::MSG}
+              end
+
+              add_column(:users, :bio, :string)
+              ^^^^^^^^^^ #{described_class::MSG}
+
+              add_column_with_default(:users, :url, :string, default: '/-/user', allow_null: false, limit: 255)
+              ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+
+              change_column_type_concurrently :users, :commit_id, :string
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+            end
+          end
+        RUBY
+
+        expect(cop.offenses.map(&:cop_name)).to all(eq('Migration/PreventStrings'))
+      end
+    end
+
+    context 'when the string data type is not used' do
+      it 'registers no offense' do
+        expect_no_offenses(<<~RUBY)
+          class Users < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+
+            def up
+              create_table :users do |t|
+                t.integer :not_a_string, null: false
+                t.timestamps_with_timezone null: true
+              end
+
+              add_column(:users, :not_a_string, :integer)
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when the text data type is used' do
+      it 'registers no offense' do
+        expect_no_offenses(<<~RUBY)
+          class Users < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+
+            def up
+              create_table :users do |t|
+                t.text :username, null: false
+                t.timestamps_with_timezone null: true
+                t.text :password
+              end
+
+              add_column(:users, :bio, :text)
+              add_column_with_default(:users, :url, :text, default: '/-/user', allow_null: false, limit: 255)
+              change_column_type_concurrently :users, :commit_id, :text
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'on down' do
+      it 'registers no offense' do
+        expect_no_offenses(<<~RUBY)
+          class Users < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+
+            def up
+              remove_column :users, :bio
+              remove_column :users, :url
+
+              drop_table :test_strings
+            end
+
+            def down
+              create_table :test_strings, id: false do |t|
+                t.integer :test_id, null: false
+                t.string :name
+              end
+
+              add_column(:users, :bio, :string)
+              add_column_with_default(:users, :url, :string, default: '/-/user', allow_null: false, limit: 255)
+              change_column_type_concurrently :users, :commit_id, :string
+            end
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'outside of migration' do
+    it 'registers no offense' do
+      expect_no_offenses(<<~RUBY)
+        class Users < ActiveRecord::Migration[6.0]
+          DOWNTIME = false
+
+          def up
+            create_table :users do |t|
+              t.string :username, null: false
+              t.timestamps_with_timezone null: true
+              t.string :password
+            end
+
+            add_column(:users, :bio, :string)
+            add_column_with_default(:users, :url, :string, default: '/-/user', allow_null: false, limit: 255)
+            change_column_type_concurrently :users, :commit_id, :string
+          end
+        end
+      RUBY
+    end
+  end
+end
diff --git a/spec/rubocop/cop/rspec/modify_sidekiq_middleware_spec.rb b/spec/rubocop/cop/rspec/modify_sidekiq_middleware_spec.rb
new file mode 100644
index 00000000000..938916d8d75
--- /dev/null
+++ b/spec/rubocop/cop/rspec/modify_sidekiq_middleware_spec.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require_relative '../../../support/helpers/expect_offense'
+require_relative '../../../../rubocop/cop/rspec/modify_sidekiq_middleware'
+
+describe RuboCop::Cop::RSpec::ModifySidekiqMiddleware do
+  include CopHelper
+  include ExpectOffense
+
+  subject(:cop) { described_class.new }
+
+  let(:source) do
+    <<~SRC
+    Sidekiq::Testing.server_middleware do |chain|
+      chain.add(MyCustomMiddleware)
+    end
+    SRC
+  end
+
+  let(:corrected) do
+    <<~SRC
+    with_sidekiq_server_middleware do |chain|
+      chain.add(MyCustomMiddleware)
+    end
+    SRC
+  end
+
+  it 'registers an offence' do
+    inspect_source(source)
+
+    expect(cop.offenses.size).to eq(1)
+  end
+
+  it 'can autocorrect the source' do
+    expect(autocorrect_source(source)).to eq(corrected)
+  end
+end
diff --git a/spec/rubocop/cop/static_translation_definition_spec.rb b/spec/rubocop/cop/static_translation_definition_spec.rb
new file mode 100644
index 00000000000..b85f9da9b4e
--- /dev/null
+++ b/spec/rubocop/cop/static_translation_definition_spec.rb
@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../rubocop/cop/static_translation_definition'
+
+describe RuboCop::Cop::StaticTranslationDefinition do
+  include CopHelper
+
+  using RSpec::Parameterized::TableSyntax
+
+  subject(:cop) { described_class.new }
+
+  shared_examples 'offense' do |code, highlight, line|
+    it 'registers an offense' do
+      inspect_source(code)
+
+      expect(cop.offenses.size).to eq(1)
+      expect(cop.offenses.map(&:line)).to eq([line])
+      expect(cop.highlights).to eq([highlight])
+    end
+  end
+
+  shared_examples 'no offense' do |code|
+    it 'does not register an offense' do
+      inspect_source(code)
+
+      expect(cop.offenses).to be_empty
+    end
+  end
+
+  describe 'offenses' do
+    where(:code, :highlight, :line) do
+      [
+        ['A = _("a")', '_("a")', 1],
+        ['B = s_("b")', 's_("b")', 1],
+        ['C = n_("c")', 'n_("c")', 1],
+        [
+          <<~CODE,
+            module MyModule
+              A = {
+                b: {
+                  c: _("a")
+                }
+              }
+            end
+          CODE
+          '_("a")',
+          4
+        ],
+        [
+          <<~CODE,
+            class MyClass
+              B = [
+                [
+                  s_("a")
+                ]
+              ]
+            end
+          CODE
+          's_("a")',
+          4
+        ]
+      ]
+    end
+
+    with_them do
+      include_examples 'offense', params[:code], params[:highlight], params[:line]
+    end
+  end
+
+  describe 'ignore' do
+    where(:code) do
+      [
+        'CONSTANT_1 = __("a")',
+        'CONSTANT_2 = s__("a")',
+        'CONSTANT_3 = n__("a")',
+        <<~CODE,
+          def method
+            s_('a')
+          end
+        CODE
+        <<~CODE,
+          class MyClass
+            VALID = -> {
+              s_('hi')
+            }
+          end
+        CODE
+        <<~CODE
+          class MyClass
+            def hello
+              {
+                a: _('hi')
+              }
+            end
+          end
+        CODE
+      ]
+    end
+
+    with_them do
+      include_examples 'no offense', params[:code]
+    end
+  end
+end