Commit 9fc799a5 authored by Doug Stull's avatar Doug Stull

Resolve cop helper cop exceptions

- follow standards.
parent 1ae7050e
......@@ -31,38 +31,6 @@ FactoryBot/InlineAssociation:
InternalAffairs/DeprecateCopHelper:
Exclude:
- 'spec/rubocop/cop/group_public_or_visible_to_user_spec.rb'
- 'spec/rubocop/cop/static_translation_definition_spec.rb'
- 'spec/rubocop/cop/lint/last_keyword_argument_spec.rb'
- 'spec/rubocop/cop/usage_data/large_table_spec.rb'
- 'spec/rubocop/cop/usage_data/distinct_count_by_large_foreign_key_spec.rb'
- 'spec/rubocop/cop/filename_length_spec.rb'
- 'spec/rubocop/cop/put_project_routes_under_scope_spec.rb'
- 'spec/rubocop/cop/gitlab/rails_logger_spec.rb'
- 'spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb'
- 'spec/rubocop/cop/gitlab/avoid_uploaded_file_from_params_spec.rb'
- 'spec/rubocop/cop/gitlab/duplicate_spec_location_spec.rb'
- 'spec/rubocop/cop/gitlab/bulk_insert_spec.rb'
- 'spec/rubocop/cop/gitlab/intersect_spec.rb'
- 'spec/rubocop/cop/gitlab/json_spec.rb'
- 'spec/rubocop/cop/gitlab/httparty_spec.rb'
- 'spec/rubocop/cop/gitlab/policy_rule_boolean_spec.rb'
- 'spec/rubocop/cop/gitlab/except_spec.rb'
- 'spec/rubocop/cop/gitlab/const_get_inherit_false_spec.rb'
- 'spec/rubocop/cop/gitlab/change_timezone_spec.rb'
- 'spec/rubocop/cop/gitlab/predicate_memoization_spec.rb'
- 'spec/rubocop/cop/gitlab/union_spec.rb'
- 'spec/rubocop/cop/gitlab/finder_with_find_by_spec.rb'
- 'spec/rubocop/cop/ruby_interpolation_in_translation_spec.rb'
- 'spec/rubocop/cop/active_record_association_reload_spec.rb'
- 'spec/rubocop/cop/ban_catch_throw_spec.rb'
- 'spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb'
- 'spec/rubocop/cop/avoid_becomes_spec.rb'
- 'spec/rubocop/cop/qa/ambiguous_page_object_name_spec.rb'
- 'spec/rubocop/cop/qa/element_with_pattern_spec.rb'
- 'spec/rubocop/cop/inject_enterprise_edition_module_spec.rb'
- 'spec/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers_spec.rb'
- 'spec/rubocop/cop/default_scope_spec.rb'
- 'spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb'
- 'spec/rubocop/cop/scalability/idempotent_worker_spec.rb'
- 'spec/rubocop/cop/scalability/cron_worker_context_spec.rb'
......
......@@ -4,13 +4,13 @@ module RuboCop
module Cop
module Gitlab
class HTTParty < RuboCop::Cop::Cop
MSG_SEND = <<~EOL.freeze
MSG_SEND = <<~EOL
Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP
wrapper. To allow request to localhost or the private network set
the option :allow_local_requests in the request call.
EOL
MSG_INCLUDE = <<~EOL.freeze
MSG_INCLUDE = <<~EOL
Avoid including `HTTParty` directly. Instead, use the Gitlab::HTTP
wrapper. To allow request to localhost or the private network set
the option :allow_local_requests in the request call.
......
......@@ -4,7 +4,7 @@ module RuboCop
module Cop
module Gitlab
class Json < RuboCop::Cop::Cop
MSG_SEND = <<~EOL.freeze
MSG = <<~EOL
Avoid calling `JSON` directly. Instead, use the `Gitlab::Json`
wrapper. This allows us to alter the JSON parser being used.
EOL
......@@ -14,7 +14,7 @@ module RuboCop
PATTERN
def on_send(node)
add_offense(node, location: :expression, message: MSG_SEND) if json_node?(node)
add_offense(node) if json_node?(node)
end
def autocorrect(node)
......
# frozen_string_literal: true
module RuboCop
module Cop
module Gitlab
class ModuleWithInstanceVariables < RuboCop::Cop::Cop
MSG = <<~EOL.freeze
MSG = <<~EOL
Do not use instance variables in a module. Please read this
for the rationale behind it:
......
# frozen_string_literal: true
module RuboCop
module Cop
module Gitlab
class PredicateMemoization < RuboCop::Cop::Cop
MSG = <<~EOL.freeze
MSG = <<~EOL
Avoid using `@value ||= query` inside predicate methods in order to
properly memoize `false` or `nil` values.
https://docs.gitlab.com/ee/development/utilities.html#strongmemoize
......@@ -12,7 +14,7 @@ module RuboCop
return unless predicate_method?(node)
select_offenses(node).each do |offense|
add_offense(offense, location: :expression)
add_offense(offense)
end
end
......
......@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../rubocop/cop/active_record_association_reload'
RSpec.describe RuboCop::Cop::ActiveRecordAssociationReload do
include CopHelper
subject(:cop) { described_class.new }
context 'when using ActiveRecord::Base' do
......
......@@ -2,33 +2,31 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/avoid_becomes'
RSpec.describe RuboCop::Cop::AvoidBecomes do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of becomes with a constant parameter' do
inspect_source('foo.becomes(Project)')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~CODE)
foo.becomes(Project)
^^^^^^^^^^^^^^^^^^^^ Avoid the use of becomes(SomeConstant), [...]
CODE
end
it 'flags the use of becomes with a namespaced constant parameter' do
inspect_source('foo.becomes(Namespace::Group)')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~CODE)
foo.becomes(Namespace::Group)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid the use of becomes(SomeConstant), [...]
CODE
end
it 'flags the use of becomes with a dynamic parameter' do
inspect_source(<<~RUBY)
model = Namespace
project = Project.first
project.becomes(model)
RUBY
expect(cop.offenses.size).to eq(1)
expect_offense(<<~CODE)
model = Namespace
project = Project.first
project.becomes(model)
^^^^^^^^^^^^^^^^^^^^^^ Avoid the use of becomes(SomeConstant), [...]
CODE
end
end
......@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../rubocop/cop/avoid_break_from_strong_memoize'
RSpec.describe RuboCop::Cop::AvoidBreakFromStrongMemoize do
include CopHelper
subject(:cop) { described_class.new }
it 'flags violation for break inside strong_memoize' do
......@@ -56,7 +54,7 @@ RSpec.describe RuboCop::Cop::AvoidBreakFromStrongMemoize do
call do
strong_memoize(:result) do
break if something
^^^^^ Do not use break inside strong_memoize, use next instead.
do_an_heavy_calculation
end
end
......@@ -65,7 +63,7 @@ RSpec.describe RuboCop::Cop::AvoidBreakFromStrongMemoize do
expect(instance).to receive(:add_offense).once
end
inspect_source(source)
expect_offense(source)
end
it "doesn't check when block is empty" do
......
......@@ -2,18 +2,15 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers'
RSpec.describe RuboCop::Cop::AvoidKeywordArgumentsInSidekiqWorkers do
include CopHelper
subject(:cop) { described_class.new }
it 'flags violation for keyword arguments usage in perform method signature' do
expect_offense(<<~RUBY)
def perform(id:)
^^^^^^^^^^^^^^^^ Do not use keyword arguments in Sidekiq workers. For details, check https://github.com/mperham/sidekiq/issues/2372
^^^^^^^^^^^^^^^^ Do not use keyword arguments in Sidekiq workers. For details, [...]
end
RUBY
end
......@@ -21,7 +18,7 @@ RSpec.describe RuboCop::Cop::AvoidKeywordArgumentsInSidekiqWorkers do
it 'flags violation for optional keyword arguments usage in perform method signature' do
expect_offense(<<~RUBY)
def perform(id: nil)
^^^^^^^^^^^^^^^^^^^^ Do not use keyword arguments in Sidekiq workers. For details, check https://github.com/mperham/sidekiq/issues/2372
^^^^^^^^^^^^^^^^^^^^ Do not use keyword arguments in Sidekiq workers. For details, [...]
end
RUBY
end
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/ban_catch_throw'
RSpec.describe RuboCop::Cop::BanCatchThrow do
include CopHelper
subject(:cop) { described_class.new }
it 'registers an offense when `catch` or `throw` are used' do
inspect_source("catch(:foo) {\n throw(:foo)\n}")
aggregate_failures do
expect(cop.offenses.size).to eq(2)
expect(cop.offenses.map(&:line)).to eq([1, 2])
expect(cop.highlights).to eq(['catch(:foo)', 'throw(:foo)'])
end
expect_offense(<<~CODE)
catch(:foo) {
^^^^^^^^^^^ Do not use catch or throw unless a gem's API demands it.
throw(:foo)
^^^^^^^^^^^ Do not use catch or throw unless a gem's API demands it.
}
CODE
end
it 'does not register an offense for a method called catch or throw' do
inspect_source("foo.catch(:foo) {\n foo.throw(:foo)\n}")
expect(cop.offenses).to be_empty
expect_no_offenses(<<~CODE)
foo.catch(:foo) {
foo.throw(:foo)
}
CODE
end
end
......@@ -2,47 +2,44 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/default_scope'
RSpec.describe RuboCop::Cop::DefaultScope do
include CopHelper
subject(:cop) { described_class.new }
it 'does not flag the use of default_scope with a send receiver' do
inspect_source('foo.default_scope')
expect(cop.offenses.size).to eq(0)
expect_no_offenses('foo.default_scope')
end
it 'flags the use of default_scope with a constant receiver' do
inspect_source('User.default_scope')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~SOURCE)
User.default_scope
^^^^^^^^^^^^^^^^^^ Do not use `default_scope`, [...]
SOURCE
end
it 'flags the use of default_scope with a nil receiver' do
inspect_source('class Foo ; default_scope ; end')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~SOURCE)
class Foo ; default_scope ; end
^^^^^^^^^^^^^ Do not use `default_scope`, [...]
SOURCE
end
it 'flags the use of default_scope when passing arguments' do
inspect_source('class Foo ; default_scope(:foo) ; end')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~SOURCE)
class Foo ; default_scope(:foo) ; end
^^^^^^^^^^^^^^^^^^^ Do not use `default_scope`, [...]
SOURCE
end
it 'flags the use of default_scope when passing a block' do
inspect_source('class Foo ; default_scope { :foo } ; end')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~SOURCE)
class Foo ; default_scope { :foo } ; end
^^^^^^^^^^^^^ Do not use `default_scope`, [...]
SOURCE
end
it 'ignores the use of default_scope with a local variable receiver' do
inspect_source('users = User.all ; users.default_scope')
expect(cop.offenses.size).to eq(0)
expect_no_offenses('users = User.all ; users.default_scope')
end
end
......@@ -2,19 +2,16 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/avoid_uploaded_file_from_params'
RSpec.describe RuboCop::Cop::Gitlab::AvoidUploadedFileFromParams do
include CopHelper
subject(:cop) { described_class.new }
context 'UploadedFile.from_params' do
context 'when using UploadedFile.from_params' do
it 'flags its call' do
expect_offense(<<~SOURCE)
UploadedFile.from_params(params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `UploadedFile` set by `multipart.rb` instead of calling `UploadedFile.from_params` directly. See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route
UploadedFile.from_params(params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `UploadedFile` set by `multipart.rb` instead of calling [...]
SOURCE
end
end
......
......@@ -2,25 +2,22 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/bulk_insert'
RSpec.describe RuboCop::Cop::Gitlab::BulkInsert do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of Gitlab::Database.bulk_insert' do
expect_offense(<<~SOURCE)
Gitlab::Database.bulk_insert('merge_request_diff_files', rows)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{RuboCop::Cop::Gitlab::BulkInsert::MSG}
Gitlab::Database.bulk_insert('merge_request_diff_files', rows)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `BulkInsertSafe` concern, [...]
SOURCE
end
it 'flags the use of ::Gitlab::Database.bulk_insert' do
expect_offense(<<~SOURCE)
::Gitlab::Database.bulk_insert('merge_request_diff_files', rows)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{RuboCop::Cop::Gitlab::BulkInsert::MSG}
::Gitlab::Database.bulk_insert('merge_request_diff_files', rows)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `BulkInsertSafe` concern, [...]
SOURCE
end
end
......@@ -2,19 +2,16 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/change_timzone'
RSpec.describe RuboCop::Cop::Gitlab::ChangeTimezone do
include CopHelper
subject(:cop) { described_class.new }
context 'Time.zone=' do
it 'registers an offense with no 2nd argument' do
expect_offense(<<~PATTERN)
Time.zone = 'Awkland'
^^^^^^^^^^^^^^^^^^^^^ Do not change timezone in the runtime (application or rspec), it could result in silently modifying other behavior.
^^^^^^^^^^^^^^^^^^^^^ Do not change timezone in the runtime (application or rspec), it could result [...]
PATTERN
end
end
......
......@@ -2,78 +2,75 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/const_get_inherit_false'
RSpec.describe RuboCop::Cop::Gitlab::ConstGetInheritFalse do
include CopHelper
subject(:cop) { described_class.new }
context 'Object.const_get' do
it 'registers an offense with no 2nd argument' do
it 'registers an offense with no 2nd argument and corrects' do
expect_offense(<<~PATTERN)
Object.const_get(:CONSTANT)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Object.const_get(:CONSTANT)')).to eq('Object.const_get(:CONSTANT, false)')
expect_correction(<<~PATTERN)
Object.const_get(:CONSTANT, false)
PATTERN
end
context 'inherit=false' do
it 'does not register an offense' do
expect_no_offenses(<<~PATTERN)
Object.const_get(:CONSTANT, false)
Object.const_get(:CONSTANT, false)
PATTERN
end
end
context 'inherit=true' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~PATTERN)
Object.const_get(:CONSTANT, true)
^^^^^^^^^ Use inherit=false when using const_get.
Object.const_get(:CONSTANT, true)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Object.const_get(:CONSTANT, true)')).to eq('Object.const_get(:CONSTANT, false)')
expect_correction(<<~PATTERN)
Object.const_get(:CONSTANT, false)
PATTERN
end
end
end
context 'const_get for a nested class' do
it 'registers an offense on reload usage' do
it 'registers an offense on reload usage and corrects' do
expect_offense(<<~PATTERN)
Nested::Blog.const_get(:CONSTANT)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Nested::Blag.const_get(:CONSTANT)')).to eq('Nested::Blag.const_get(:CONSTANT, false)')
expect_correction(<<~PATTERN)
Nested::Blog.const_get(:CONSTANT, false)
PATTERN
end
context 'inherit=false' do
it 'does not register an offense' do
expect_no_offenses(<<~PATTERN)
Nested::Blog.const_get(:CONSTANT, false)
Nested::Blog.const_get(:CONSTANT, false)
PATTERN
end
end
context 'inherit=true' do
it 'registers an offense if inherit is true' do
it 'registers an offense if inherit is true and corrects' do
expect_offense(<<~PATTERN)
Nested::Blog.const_get(:CONSTANT, true)
^^^^^^^^^ Use inherit=false when using const_get.
Nested::Blog.const_get(:CONSTANT, true)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Nested::Blag.const_get(:CONSTANT, true)')).to eq('Nested::Blag.const_get(:CONSTANT, false)')
expect_correction(<<~PATTERN)
Nested::Blog.const_get(:CONSTANT, false)
PATTERN
end
end
end
......
......@@ -6,8 +6,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/gitlab/duplicate_spec_location'
RSpec.describe RuboCop::Cop::Gitlab::DuplicateSpecLocation do
include CopHelper
subject(:cop) { described_class.new }
let(:rails_root) { '../../../../' }
......
......@@ -2,12 +2,9 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/except'
RSpec.describe RuboCop::Cop::Gitlab::Except do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of Gitlab::SQL::Except.new' do
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/finder_with_find_by'
RSpec.describe RuboCop::Cop::Gitlab::FinderWithFindBy do
include CopHelper
subject(:cop) { described_class.new }
context 'when calling execute.find' do
let(:source) do
<<~SRC
DummyFinder.new(some_args)
.execute
.find_by!(1)
SRC
end
let(:corrected_source) do
<<~SRC
DummyFinder.new(some_args)
.find_by!(1)
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_source)
it 'registers an offense and corrects' do
expect_offense(<<~CODE)
DummyFinder.new(some_args)
.execute
.find_by!(1)
^^^^^^^^ Don't chain finders `#execute` method with [...]
CODE
expect_correction(<<~CODE)
DummyFinder.new(some_args)
.find_by!(1)
CODE
end
context 'when called within the `FinderMethods` module' do
let(:source) do
<<~SRC
it 'does not register an offense' do
expect_no_offenses(<<~SRC)
module FinderMethods
def find_by!(*args)
execute.find_by!(args)
......@@ -48,12 +33,6 @@ RSpec.describe RuboCop::Cop::Gitlab::FinderWithFindBy do
end
SRC
end
it 'does not register an offence' do
inspect_source(source)
expect(cop.offenses).to be_empty
end
end
end
end
......@@ -2,46 +2,30 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/httparty'
RSpec.describe RuboCop::Cop::Gitlab::HTTParty do # rubocop:disable RSpec/FilePath
include CopHelper
subject(:cop) { described_class.new }
shared_examples('registering include offense') do |options|
let(:offending_lines) { options[:offending_lines] }
shared_examples('registering include offense') do
it 'registers an offense when the class includes HTTParty' do
inspect_source(source)
aggregate_failures do
expect(cop.offenses.size).to eq(offending_lines.size)
expect(cop.offenses.map(&:line)).to eq(offending_lines)
end
expect_offense(source)
end
end
shared_examples('registering call offense') do |options|
let(:offending_lines) { options[:offending_lines] }
shared_examples('registering call offense') do
it 'registers an offense when the class calls HTTParty' do
inspect_source(source)
aggregate_failures do
expect(cop.offenses.size).to eq(offending_lines.size)
expect(cop.offenses.map(&:line)).to eq(offending_lines)
end
expect_offense(source)
end
end
context 'when source is a regular module' do
it_behaves_like 'registering include offense', offending_lines: [2] do
it_behaves_like 'registering include offense' do
let(:source) do
<<~RUBY
module M
include HTTParty
^^^^^^^^^^^^^^^^ Avoid including `HTTParty` directly. [...]
end
RUBY
end
......@@ -49,11 +33,12 @@ RSpec.describe RuboCop::Cop::Gitlab::HTTParty do # rubocop:disable RSpec/FilePat
end
context 'when source is a regular class' do
it_behaves_like 'registering include offense', offending_lines: [2] do
it_behaves_like 'registering include offense' do
let(:source) do
<<~RUBY
class Foo
include HTTParty
^^^^^^^^^^^^^^^^ Avoid including `HTTParty` directly. [...]
end
RUBY
end
......@@ -61,12 +46,13 @@ RSpec.describe RuboCop::Cop::Gitlab::HTTParty do # rubocop:disable RSpec/FilePat
end
context 'when HTTParty is called' do
it_behaves_like 'registering call offense', offending_lines: [3] do
it_behaves_like 'registering call offense' do
let(:source) do
<<~RUBY
class Foo
def bar
HTTParty.get('http://example.com')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `HTTParty` directly. [...]
end
end
RUBY
......
......@@ -2,12 +2,9 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/intersect'
RSpec.describe RuboCop::Cop::Gitlab::Intersect do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of Gitlab::SQL::Intersect.new' do
......
......@@ -2,38 +2,21 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/json'
RSpec.describe RuboCop::Cop::Gitlab::Json do
include CopHelper
subject(:cop) { described_class.new }
shared_examples('registering call offense') do |options|
let(:offending_lines) { options[:offending_lines] }
it 'registers an offense when the class calls JSON' do
inspect_source(source)
aggregate_failures do
expect(cop.offenses.size).to eq(offending_lines.size)
expect(cop.offenses.map(&:line)).to eq(offending_lines)
end
end
end
context 'when JSON is called' do
it_behaves_like 'registering call offense', offending_lines: [3] do
let(:source) do
<<~RUBY
class Foo
def bar
JSON.parse('{ "foo": "bar" }')
end
it 'registers an offense' do
expect_offense(<<~RUBY)
class Foo
def bar
JSON.parse('{ "foo": "bar" }')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `JSON` directly. [...]
end
RUBY
end
end
RUBY
end
end
end
......@@ -2,42 +2,33 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/module_with_instance_variables'
RSpec.describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do
include CopHelper
let(:msg) { "Do not use instance variables in a module. [...]" }
subject(:cop) { described_class.new }
shared_examples('registering offense') do |options|
let(:offending_lines) { options[:offending_lines] }
shared_examples('registering offense') do
it 'registers an offense when instance variable is used in a module' do
inspect_source(source)
aggregate_failures do
expect(cop.offenses.size).to eq(offending_lines.size)
expect(cop.offenses.map(&:line)).to eq(offending_lines)
end
expect_offense(source)
end
end
shared_examples('not registering offense') do
it 'does not register offenses' do
inspect_source(source)
expect(cop.offenses).to be_empty
expect_no_offenses(source)
end
end
context 'when source is a regular module' do
it_behaves_like 'registering offense', offending_lines: [3] do
it_behaves_like 'registering offense' do
let(:source) do
<<~RUBY
module M
def f
@f = true
^^^^^^^^^ #{msg}
end
end
RUBY
......@@ -46,13 +37,14 @@ RSpec.describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do
end
context 'when source is a nested module' do
it_behaves_like 'registering offense', offending_lines: [4] do
it_behaves_like 'registering offense' do
let(:source) do
<<~RUBY
module N
module M
def f
@f = true
^^^^^^^^^ #{msg}
end
end
end
......@@ -62,13 +54,14 @@ RSpec.describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do
end
context 'when source is a nested module with multiple offenses' do
it_behaves_like 'registering offense', offending_lines: [4, 12] do
it_behaves_like 'registering offense' do
let(:source) do
<<~RUBY
module N
module M
def f
@f = true
^^^^^^^^^ #{msg}
end
def g
......@@ -77,6 +70,7 @@ RSpec.describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do
def h
@h = true
^^^^^^^^^ #{msg}
end
end
end
......@@ -129,12 +123,13 @@ RSpec.describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do
end
context 'when source is using simple or ivar assignment with other ivar' do
it_behaves_like 'registering offense', offending_lines: [3] do
it_behaves_like 'registering offense' do
let(:source) do
<<~RUBY
module M
def f
@f ||= g(@g)
^^ #{msg}
end
end
RUBY
......@@ -143,13 +138,15 @@ RSpec.describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do
end
context 'when source is using or ivar assignment with something else' do
it_behaves_like 'registering offense', offending_lines: [3, 4] do
it_behaves_like 'registering offense' do
let(:source) do
<<~RUBY
module M
def f
@f ||= true
^^ #{msg}
@f.to_s
^^ #{msg}
end
end
RUBY
......
......@@ -2,12 +2,9 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/policy_rule_boolean'
RSpec.describe RuboCop::Cop::Gitlab::PolicyRuleBoolean do
include CopHelper
subject(:cop) { described_class.new }
it 'registers offense for &&' do
......
......@@ -2,55 +2,38 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/predicate_memoization'
RSpec.describe RuboCop::Cop::Gitlab::PredicateMemoization do
include CopHelper
subject(:cop) { described_class.new }
shared_examples('registering offense') do |options|
let(:offending_lines) { options[:offending_lines] }
it 'registers an offense when a predicate method is memoizing via ivar' do
inspect_source(source)
aggregate_failures do
expect(cop.offenses.size).to eq(offending_lines.size)
expect(cop.offenses.map(&:line)).to eq(offending_lines)
end
end
end
shared_examples('not registering offense') do
it 'does not register offenses' do
inspect_source(source)
expect(cop.offenses).to be_empty
expect_no_offenses(source)
end
end
context 'when source is a predicate method memoizing via ivar' do
it_behaves_like 'registering offense', offending_lines: [3] do
context 'when source is a predicate method using ivar with assignment' do
it_behaves_like 'not registering offense' do
let(:source) do
<<~RUBY
class C
def really?
@really ||= true
@really = true
end
end
RUBY
end
end
end
it_behaves_like 'registering offense', offending_lines: [4] do
context 'when source is a predicate method using local with ||=' do
it_behaves_like 'not registering offense' do
let(:source) do
<<~RUBY
class C
def really?
value = true
@really ||= value
really ||= true
end
end
RUBY
......@@ -58,13 +41,13 @@ RSpec.describe RuboCop::Cop::Gitlab::PredicateMemoization do
end
end
context 'when source is a predicate method using ivar with assignment' do
context 'when source is a regular method memoizing via ivar' do
it_behaves_like 'not registering offense' do
let(:source) do
<<~RUBY
class C
def really?
@really = true
def really
@really ||= true
end
end
RUBY
......@@ -72,30 +55,37 @@ RSpec.describe RuboCop::Cop::Gitlab::PredicateMemoization do
end
end
context 'when source is a predicate method using local with ||=' do
it_behaves_like 'not registering offense' do
let(:source) do
<<~RUBY
context 'when source is a predicate method memoizing via ivar' do
let(:msg) { "Avoid using `@value ||= query` [...]" }
context 'when assigning to boolean' do
it 'registers an offense' do
node = "@really ||= true"
expect_offense(<<~CODE, node: node, msg: msg)
class C
def really?
really ||= true
%{node}
^{node} %{msg}
end
end
RUBY
CODE
end
end
end
context 'when source is a regular method memoizing via ivar' do
it_behaves_like 'not registering offense' do
let(:source) do
<<~RUBY
context 'when assigning to another variable that is a boolean' do
it 'registers an offense' do
node = "@really ||= value"
expect_offense(<<~CODE, node: node, msg: msg)
class C
def really
@really ||= true
def really?
value = true
%{node}
^{node} %{msg}
end
end
RUBY
CODE
end
end
end
......
......@@ -2,37 +2,31 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/rails_logger'
RSpec.describe RuboCop::Cop::Gitlab::RailsLogger do
include CopHelper
subject(:cop) { described_class.new }
described_class::LOG_METHODS.each do |method|
it "flags the use of Rails.logger.#{method} with a constant receiver" do
inspect_source("Rails.logger.#{method}('some error')")
node = "Rails.logger.#{method}('some error')"
expect(cop.offenses.size).to eq(1)
expect_offense(<<~CODE, node: node, msg: "Use a structured JSON logger instead of `Rails.logger`. [...]")
%{node}
^{node} %{msg}
CODE
end
end
it 'does not flag the use of Rails.logger with a constant that is not Rails' do
inspect_source("AppLogger.error('some error')")
expect(cop.offenses.size).to eq(0)
expect_no_offenses("AppLogger.error('some error')")
end
it 'does not flag the use of logger with a send receiver' do
inspect_source("file_logger.info('important info')")
expect(cop.offenses.size).to eq(0)
expect_no_offenses("file_logger.info('important info')")
end
it 'does not flag the use of Rails.logger.level' do
inspect_source("Rails.logger.level")
expect(cop.offenses.size).to eq(0)
expect_no_offenses("Rails.logger.level")
end
end
......@@ -2,12 +2,9 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/union'
RSpec.describe RuboCop::Cop::Gitlab::Union do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of Gitlab::SQL::Union.new' do
......
......@@ -2,29 +2,28 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/group_public_or_visible_to_user'
RSpec.describe RuboCop::Cop::GroupPublicOrVisibleToUser do
include CopHelper
let(:msg) do
"`Group.public_or_visible_to_user` should be used with extreme care. " \
"Please ensure that you are not using it on its own and that the amount of rows being filtered is reasonable."
end
subject(:cop) { described_class.new }
it 'flags the use of Group.public_or_visible_to_user with a constant receiver' do
inspect_source('Group.public_or_visible_to_user')
expect(cop.offenses.size).to eq(1)
expect_offense(<<~CODE)
Group.public_or_visible_to_user
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg}
CODE
end
it 'does not flat the use of public_or_visible_to_user with a constant that is not Group' do
inspect_source('Project.public_or_visible_to_user')
expect(cop.offenses.size).to eq(0)
it 'does not flag the use of public_or_visible_to_user with a constant that is not Group' do
expect_no_offenses('Project.public_or_visible_to_user')
end
it 'does not flag the use of Group.public_or_visible_to_user with a send receiver' do
inspect_source('foo.public_or_visible_to_user')
expect(cop.offenses.size).to eq(0)
expect_no_offenses('foo.public_or_visible_to_user')
end
end
......@@ -2,12 +2,9 @@
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/inject_enterprise_edition_module'
RSpec.describe RuboCop::Cop::InjectEnterpriseEditionModule do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of `prepend_if_ee EE` in the middle of a file' do
......@@ -185,18 +182,19 @@ RSpec.describe RuboCop::Cop::InjectEnterpriseEditionModule do
end
it 'autocorrects offenses by just disabling the Cop' do
source = <<~SOURCE
class Foo
prepend_if_ee 'EE::Foo'
include_if_ee 'Bar'
end
expect_offense(<<~SOURCE)
class Foo
prepend_if_ee 'EE::Foo'
^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
include_if_ee 'Bar'
end
SOURCE
expect(autocorrect_source(source)).to eq(<<~SOURCE)
class Foo
prepend_if_ee 'EE::Foo' # rubocop: disable Cop/InjectEnterpriseEditionModule
include_if_ee 'Bar'
end
expect_correction(<<~SOURCE)
class Foo
prepend_if_ee 'EE::Foo' # rubocop: disable Cop/InjectEnterpriseEditionModule
include_if_ee 'Bar'
end
SOURCE
end
......
......@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../../rubocop/cop/lint/last_keyword_argument'
RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument do
include CopHelper
subject(:cop) { described_class.new }
before do
......
......@@ -5,8 +5,6 @@ require 'rubocop'
require_relative '../../../rubocop/cop/put_project_routes_under_scope'
RSpec.describe RuboCop::Cop::PutProjectRoutesUnderScope do
include CopHelper
subject(:cop) { described_class.new }
%w[resource resources get post put patch delete].each do |route_method|
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/qa/ambiguous_page_object_name'
RSpec.describe RuboCop::Cop::QA::AmbiguousPageObjectName do
include CopHelper
let(:source_file) { 'qa/page.rb' }
subject(:cop) { described_class.new }
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/qa/element_with_pattern'
RSpec.describe RuboCop::Cop::QA::ElementWithPattern do
include CopHelper
let(:source_file) { 'qa/page.rb' }
subject(:cop) { described_class.new }
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/ruby_interpolation_in_translation'
# Disabling interpolation check as we deliberately want to have #{} in strings.
# rubocop:disable Lint/InterpolationCheck
RSpec.describe RuboCop::Cop::RubyInterpolationInTranslation do
subject(:cop) { described_class.new }
let(:msg) { "Don't use ruby interpolation \#{} inside translated strings, instead use %{}" }
it 'does not add an offence for a regular messages' do
inspect_source('_("Hello world")')
subject(:cop) { described_class.new }
expect(cop.offenses).to be_empty
it 'does not add an offense for a regular messages' do
expect_no_offenses('_("Hello world")')
end
it 'adds the correct offence when using interpolation in a string' do
inspect_source('_("Hello #{world}")')
offense = cop.offenses.first
expect(offense.location.source).to eq('#{world}')
expect(offense.message).to eq('Don\'t use ruby interpolation #{} inside translated strings, instead use %{}')
it 'adds the correct offense when using interpolation in a string' do
expect_offense(<<~CODE)
_("Hello \#{world}")
^^^^^ #{msg}
^^^^^^^^ #{msg}
CODE
end
it 'detects when using a ruby interpolation in the first argument of a pluralized string' do
inspect_source('n_("Hello #{world}", "Hello world")')
expect(cop.offenses).not_to be_empty
expect_offense(<<~CODE)
n_("Hello \#{world}", "Hello world")
^^^^^ #{msg}
^^^^^^^^ #{msg}
CODE
end
it 'detects when using a ruby interpolation in the second argument of a pluralized string' do
inspect_source('n_("Hello world", "Hello #{world}")')
expect(cop.offenses).not_to be_empty
expect_offense(<<~CODE)
n_("Hello world", "Hello \#{world}")
^^^^^ #{msg}
^^^^^^^^ #{msg}
CODE
end
it 'detects when using interpolation in a namespaced translation' do
inspect_source('s_("Hello|#{world}")')
expect(cop.offenses).not_to be_empty
end
it 'does not add an offence for messages defined over multiple lines' do
source = <<~SRC
_("Hello "\
"world ")
SRC
inspect_source(source)
expect(cop.offenses).to be_empty
end
it 'adds an offence for violations in a message defined over multiple lines' do
source = <<~SRC
_("Hello "\
"\#{world} ")
SRC
inspect_source(source)
expect(cop.offenses).not_to be_empty
expect_offense(<<~CODE)
s_("Hello|\#{world}")
^^^^^ #{msg}
^^^^^^^^ #{msg}
CODE
end
end
# rubocop:enable Lint/InterpolationCheck
......@@ -8,78 +8,76 @@ require 'rspec-parameterized'
require_relative '../../../rubocop/cop/static_translation_definition'
RSpec.describe RuboCop::Cop::StaticTranslationDefinition do
include CopHelper
using RSpec::Parameterized::TableSyntax
let(:msg) do
"The text you're translating will be already in the translated form when it's assigned to the constant. " \
"When a users changes the locale, these texts won't be translated again. " \
"Consider moving the translation logic to a method."
end
subject(:cop) { described_class.new }
shared_examples 'offense' do |code, highlight, line|
shared_examples 'offense' do |code|
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])
expect_offense(code)
end
end
shared_examples 'no offense' do |code|
it 'does not register an offense' do
inspect_source(code)
expect(cop.offenses).to be_empty
expect_no_offenses(code)
end
end
describe 'offenses' do
where(:code, :highlight, :line) do
where(:code) do
[
['A = _("a")', '_("a")', 1],
['B = s_("b")', 's_("b")', 1],
['C = n_("c")', 'n_("c")', 1],
[
<<~CODE,
class MyClass
def self.translations
@cache ||= { hello: _("hello") }
end
<<~CODE,
A = _("a")
^^^^^^ #{msg}
CODE
<<~CODE,
B = s_("b")
^^^^^^^ #{msg}
CODE
<<~CODE,
C = n_("c")
^^^^^^^ #{msg}
CODE
<<~CODE,
class MyClass
def self.translations
@cache ||= { hello: _("hello") }
^^^^^^^^^^ #{msg}
end
CODE
'_("hello")',
3
],
[
<<~CODE,
module MyModule
A = {
b: {
c: _("a")
}
end
CODE
<<~CODE,
module MyModule
A = {
b: {
c: _("a")
^^^^^^ #{msg}
}
end
CODE
'_("a")',
4
],
[
<<~CODE,
class MyClass
B = [
[
s_("a")
]
}
end
CODE
<<~CODE
class MyClass
B = [
[
s_("a")
^^^^^^^ #{msg}
]
end
CODE
's_("a")',
4
]
]
end
CODE
]
end
with_them do
include_examples 'offense', params[:code], params[:highlight], params[:line]
include_examples 'offense', params[:code]
end
end
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/usage_data/distinct_count_by_large_foreign_key'
RSpec.describe RuboCop::Cop::UsageData::DistinctCountByLargeForeignKey do
include CopHelper
let(:allowed_foreign_keys) { [:author_id, :user_id, :'merge_requests.target_project_id'] }
let(:msg) { 'Avoid doing `distinct_count` on foreign keys for large tables having above 100 million rows.' }
let(:config) do
RuboCop::Config.new('UsageData/DistinctCountByLargeForeignKey' => {
'AllowedForeignKeys' => allowed_foreign_keys
......@@ -21,36 +17,32 @@ RSpec.describe RuboCop::Cop::UsageData::DistinctCountByLargeForeignKey do
subject(:cop) { described_class.new(config) }
context 'when counting by disallowed key' do
it 'registers an offence' do
inspect_source('distinct_count(Issue, :creator_id)')
expect(cop.offenses.size).to eq(1)
it 'registers an offense' do
expect_offense(<<~CODE)
distinct_count(Issue, :creator_id)
^^^^^^^^^^^^^^ #{msg}
CODE
end
it 'does not register an offence when batch is false' do
inspect_source('distinct_count(Issue, :creator_id, batch: false)')
expect(cop.offenses).to be_empty
it 'does not register an offense when batch is false' do
expect_no_offenses('distinct_count(Issue, :creator_id, batch: false)')
end
it 'register an offence when batch is true' do
inspect_source('distinct_count(Issue, :creator_id, batch: true)')
expect(cop.offenses.size).to eq(1)
it 'registers an offense when batch is true' do
expect_offense(<<~CODE)
distinct_count(Issue, :creator_id, batch: true)
^^^^^^^^^^^^^^ #{msg}
CODE
end
end
context 'when calling by allowed key' do
it 'does not register an offence with symbol' do
inspect_source('distinct_count(Issue, :author_id)')
expect(cop.offenses).to be_empty
it 'does not register an offense with symbol' do
expect_no_offenses('distinct_count(Issue, :author_id)')
end
it 'does not register an offence with string' do
inspect_source("distinct_count(Issue, 'merge_requests.target_project_id')")
expect(cop.offenses).to be_empty
it 'does not register an offense with string' do
expect_no_offenses("distinct_count(Issue, 'merge_requests.target_project_id')")
end
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/usage_data/large_table'
RSpec.describe RuboCop::Cop::UsageData::LargeTable do
include CopHelper
let(:large_tables) { %i[Rails Time] }
let(:count_methods) { %i[count distinct_count] }
let(:allowed_methods) { %i[minimum maximum] }
let(:msg) { 'Use one of the count, distinct_count methods for counting on' }
let(:config) do
RuboCop::Config.new('UsageData/LargeTable' => {
......@@ -31,59 +28,54 @@ RSpec.describe RuboCop::Cop::UsageData::LargeTable do
context 'with large tables' do
context 'when calling Issue.count' do
it 'register an offence' do
inspect_source('Issue.count')
expect(cop.offenses.size).to eq(1)
it 'registers an offense' do
expect_offense(<<~CODE)
Issue.count
^^^^^^^^^^^ #{msg} Issue
CODE
end
end
context 'when calling Issue.active.count' do
it 'register an offence' do
inspect_source('Issue.active.count')
expect(cop.offenses.size).to eq(1)
it 'registers an offense' do
expect_offense(<<~CODE)
Issue.active.count
^^^^^^^^^^^^ #{msg} Issue
CODE
end
end
context 'when calling count(Issue)' do
it 'does not register an offence' do
inspect_source('count(Issue)')
expect(cop.offenses).to be_empty
it 'does not register an offense' do
expect_no_offenses('count(Issue)')
end
end
context 'when calling count(Ci::Build.active)' do
it 'does not register an offence' do
inspect_source('count(Ci::Build.active)')
expect(cop.offenses).to be_empty
it 'does not register an offense' do
expect_no_offenses('count(Ci::Build.active)')
end
end
context 'when calling Ci::Build.active.count' do
it 'register an offence' do
inspect_source('Ci::Build.active.count')
expect(cop.offenses.size).to eq(1)
it 'registers an offense' do
expect_offense(<<~CODE)
Ci::Build.active.count
^^^^^^^^^^^^^^^^ #{msg} Ci::Build
CODE
end
end
context 'when using allowed methods' do
it 'does not register an offence' do
inspect_source('Issue.minimum')
expect(cop.offenses).to be_empty
it 'does not register an offense' do
expect_no_offenses('Issue.minimum')
end
end
end
context 'with non related class' do
it 'does not register an offence' do
inspect_source('Rails.count')
expect(cop.offenses).to be_empty
it 'does not register an offense' do
expect_no_offenses('Rails.count')
end
end
end
......
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