Commit 07257f8b authored by Doug Stull's avatar Doug Stull Committed by Rémy Coutable

Update rubocop with gitlab-styles

- keep up to date and add new cops.
parent 914c4a91
......@@ -7,9 +7,11 @@ require:
- rubocop-rspec
inherit_from:
- .rubocop_manual_todo.yml
- .rubocop_todo.yml
- ./rubocop/rubocop-migrations.yml
- ./rubocop/rubocop-usage-data.yml
- ./rubocop/rubocop-code_reuse.yml
inherit_mode:
merge:
......
This diff is collapsed.
This diff is collapsed.
......@@ -370,11 +370,7 @@ group :development, :test do
gem 'spring', '~> 2.0.0'
gem 'spring-commands-rspec', '~> 1.0.4'
gem 'gitlab-styles', '~> 4.3.0', require: false
# Pin these dependencies, otherwise a new rule could break the CI pipelines
gem 'rubocop', '~> 0.82.0'
gem 'rubocop-performance', '~> 1.5.2'
gem 'rubocop-rspec', '~> 1.37.0'
gem 'gitlab-styles', '~> 5.0.0', require: false
gem 'scss_lint', '~> 0.56.0', require: false
gem 'haml_lint', '~> 0.36.0', require: false
......
......@@ -449,12 +449,12 @@ GEM
gitlab-puma (>= 2.7, < 5)
gitlab-sidekiq-fetcher (0.5.2)
sidekiq (~> 5)
gitlab-styles (4.3.0)
rubocop (~> 0.82.0)
gitlab-styles (5.0.0)
rubocop (~> 0.89.1)
rubocop-gitlab-security (~> 0.1.0)
rubocop-performance (~> 1.5.2)
rubocop-rails (~> 2.5)
rubocop-rspec (~> 1.36)
rubocop-performance (~> 1.8.1)
rubocop-rails (~> 2.8)
rubocop-rspec (~> 1.44)
gitlab_chronic_duration (0.10.6.2)
numerizer (~> 0.2)
gitlab_omniauth-ldap (2.1.1)
......@@ -602,7 +602,6 @@ GEM
jaeger-client (1.1.0)
opentracing (~> 0.3)
thrift
jaro_winkler (1.5.4)
jira-ruby (2.0.0)
activesupport
atlassian-jwt
......@@ -834,7 +833,7 @@ GEM
rubypants (~> 0.2)
orm_adapter (0.5.0)
os (1.0.0)
parallel (1.19.1)
parallel (1.19.2)
parser (2.7.2.0)
ast (~> 2.4.1)
parslet (1.8.2)
......@@ -958,7 +957,7 @@ GEM
redis-store (>= 1.2, < 2)
redis-store (1.8.1)
redis (>= 4, < 5)
regexp_parser (1.5.1)
regexp_parser (1.8.2)
regexp_property_values (0.3.5)
representable (3.0.4)
declarative (< 0.1.0)
......@@ -1019,24 +1018,29 @@ GEM
pg
rails
sqlite3
rubocop (0.82.0)
jaro_winkler (~> 1.5.1)
rubocop (0.89.1)
parallel (~> 1.10)
parser (>= 2.7.0.1)
parser (>= 2.7.1.1)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.7)
rexml
rubocop-ast (>= 0.3.0, < 1.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 2.0)
rubocop-ast (0.8.0)
parser (>= 2.7.1.5)
rubocop-gitlab-security (0.1.1)
rubocop (>= 0.51)
rubocop-performance (1.5.2)
rubocop (>= 0.71.0)
rubocop-rails (2.5.2)
activesupport
rubocop-performance (1.8.1)
rubocop (>= 0.87.0)
rubocop-ast (>= 0.4.0)
rubocop-rails (2.8.1)
activesupport (>= 4.2.0)
rack (>= 1.1)
rubocop (>= 0.72.0)
rubocop-rspec (1.37.0)
rubocop (>= 0.68.1)
rubocop (>= 0.87.0)
rubocop-rspec (1.44.1)
rubocop (~> 0.87)
rubocop-ast (>= 0.7.1)
ruby-enum (0.7.2)
i18n
ruby-fogbugz (0.2.1)
......@@ -1249,7 +1253,7 @@ GEM
xpath (3.2.0)
nokogiri (~> 1.8)
yajl-ruby (1.4.1)
zeitwerk (2.4.0)
zeitwerk (2.4.1)
PLATFORMS
ruby
......@@ -1353,7 +1357,7 @@ DEPENDENCIES
gitlab-puma (~> 4.3.3.gitlab.2)
gitlab-puma_worker_killer (~> 0.1.1.gitlab.1)
gitlab-sidekiq-fetcher (= 0.5.2)
gitlab-styles (~> 4.3.0)
gitlab-styles (~> 5.0.0)
gitlab_chronic_duration (~> 0.10.6.2)
gitlab_omniauth-ldap (~> 2.1.1)
gon (~> 6.2)
......@@ -1473,9 +1477,6 @@ DEPENDENCIES
rspec-retry (~> 0.6.1)
rspec_junit_formatter
rspec_profiling (~> 0.0.6)
rubocop (~> 0.82.0)
rubocop-performance (~> 1.5.2)
rubocop-rspec (~> 1.37.0)
ruby-fogbugz (~> 0.2.1)
ruby-prof (~> 1.3.0)
ruby-progressbar
......
......@@ -3,8 +3,5 @@
---
inherit_from: ../../../../../lib/gitlab/background_migration/.rubocop.yml
CodeReuse/ActiveRecord:
Enabled: false
Style/Documentation:
Enabled: false
......@@ -190,7 +190,7 @@ module Gitlab
%r{\A(ee/)?vendor/} => :backend,
%r{\A(Gemfile|Gemfile.lock|Rakefile)\z} => :backend,
%r{\A[A-Z_]+_VERSION\z} => :backend,
%r{\A\.rubocop(_todo)?\.yml\z} => :backend,
%r{\A\.rubocop((_manual)?_todo)?\.yml\z} => :backend,
%r{\Afile_hooks/} => :backend,
%r{\A(ee/)?qa/} => :qa,
......
# frozen_string_literal: true
require_relative '../../code_reuse_helpers'
module RuboCop
module Cop
module CodeReuse
# Cop that denies the use of ActiveRecord methods outside of models.
class ActiveRecord < RuboCop::Cop::Cop
include CodeReuseHelpers
MSG = 'This method can only be used inside an ActiveRecord model: ' \
'https://gitlab.com/gitlab-org/gitlab-foss/issues/49653'
# Various methods from ActiveRecord::Querying that are denied. We
# exclude some generic ones such as `any?` and `first`, as these may
# lead to too many false positives, since `Array` also supports these
# methods.
#
# The keys of this Hash are the denied method names. The values are
# booleans that indicate if the method should only be denied if any
# arguments are provided.
NOT_ALLOWED = {
average: true,
calculate: true,
count_by_sql: true,
create_with: true,
distinct: false,
eager_load: true,
exists?: true,
find_by: true,
find_by!: true,
find_by_sql: true,
find_each: true,
find_in_batches: true,
find_or_create_by: true,
find_or_create_by!: true,
find_or_initialize_by: true,
first!: false,
first_or_create: true,
first_or_create!: true,
first_or_initialize: true,
from: true,
group: true,
having: true,
ids: false,
includes: true,
joins: true,
limit: true,
lock: false,
many?: false,
offset: true,
order: true,
pluck: true,
preload: true,
readonly: false,
references: true,
reorder: true,
rewhere: true,
take: false,
take!: false,
unscope: false,
where: false,
with: true
}.freeze
# Directories that allow the use of the denied methods. These
# directories are checked relative to both . and ee/
ALLOWED_DIRECTORIES = %w[
app/models
config
danger
db
lib/backup
lib/banzai
lib/gitlab/background_migration
lib/gitlab/cycle_analytics
lib/gitlab/database
lib/gitlab/import_export
lib/gitlab/project_authorizations
lib/gitlab/sql
lib/system_check
lib/tasks
qa
rubocop
spec
].freeze
def on_send(node)
return if in_allowed_directory?(node)
receiver = node.children[0]
send_name = node.children[1]
first_arg = node.children[2]
if receiver && NOT_ALLOWED.key?(send_name)
# If the rule requires an argument to be given, but none are
# provided, we won't register an offense. This prevents us from
# adding offenses for `project.group`, while still covering
# `Project.group(:name)`.
return if NOT_ALLOWED[send_name] && !first_arg
add_offense(node, location: :selector)
end
end
# Returns true if the node resides in one of the allowed
# directories.
def in_allowed_directory?(node)
path = file_path_for_node(node)
ALLOWED_DIRECTORIES.any? do |directory|
path.start_with?(
File.join(rails_root, directory),
File.join(rails_root, 'ee', directory)
)
end
end
# We can not auto correct code like this, as it requires manual
# refactoring. Instead, we'll just allow the surrounding scope.
#
# Despite this method's presence, you should not use it. This method
# exists to make it possible to allow large chunks of offenses we
# can't fix in the short term. If you are writing new code, follow the
# code reuse guidelines, instead of allowing any new offenses.
def autocorrect(node)
scope = surrounding_scope_of(node)
indent = indentation_of(scope)
lambda do |corrector|
# This prevents us from inserting the same enable/disable comment
# for a method or block that has multiple offenses.
next if allowed_scopes.include?(scope)
corrector.insert_before(
scope.source_range,
"# rubocop: disable #{cop_name}\n#{indent}"
)
corrector.insert_after(
scope.source_range,
"\n#{indent}# rubocop: enable #{cop_name}"
)
allowed_scopes << scope
end
end
def indentation_of(node)
' ' * node.loc.expression.source_line[/\A */].length
end
def surrounding_scope_of(node)
%i[def defs block begin].each do |type|
if (found = node.each_ancestor(type).first)
return found
end
end
end
def allowed_scopes
@allowed_scopes ||= Set.new
end
end
end
end
end
......@@ -5,7 +5,7 @@ module RuboCop
def in_qa_file?(node)
path = node.location.expression.source_buffer.name
path.start_with?(File.join(RuboCop::PathUtil.pwd, 'qa'))
path.start_with?(File.join(Dir.pwd, 'qa'))
end
end
end
# Denies the use of ActiveRecord methods outside of configured
# excluded directories
# Directories that allow the use of the denied methods.
# To start we provide a default configuration that matches
# a standard Rails app and enable.
# The default configuration can be overridden by
# providing your own Exclusion list as follows:
# CodeReuse/ActiveRecord:
# Enabled: true
# Exclude:
# - app/models/**/*.rb
# - config/**/*.rb
# - db/**/*.rb
# - lib/tasks/**/*.rb
# - spec/**/*.rb
# - lib/gitlab/**/*.rb
CodeReuse/ActiveRecord:
Exclude:
- app/models/**/*.rb
- config/**/*.rb
- db/**/*.rb
- lib/tasks/**/*.rake
- spec/**/*.rb
- danger/**/*.rb
- lib/backup/**/*.rb
- lib/banzai/**/*.rb
- lib/gitlab/background_migration/**/*.rb
- lib/gitlab/cycle_analytics/**/*.rb
- lib/gitlab/database/**/*.rb
- lib/gitlab/database_importers/common_metrics/importer.rb
- lib/gitlab/import_export/**/*.rb
- lib/gitlab/project_authorizations.rb
- lib/gitlab/sql/**/*.rb
- lib/system_check/**/*.rb
- qa/**/*.rb
- rubocop/**/*.rb
- ee/app/models/**/*.rb
- ee/spec/**/*.rb
- ee/db/fixtures/**/*.rb
- ee/lib/tasks/**/*.rake
- ee/lib/ee/gitlab/background_migration/**/*.rb
......@@ -236,13 +236,16 @@ RSpec.describe Gitlab::Danger::Helper do
'.gitlab/ci/frontend.gitlab-ci.yml' | %i[frontend engineering_productivity]
'app/models/foo' | [:backend]
'bin/foo' | [:backend]
'config/foo' | [:backend]
'lib/foo' | [:backend]
'rubocop/foo' | [:backend]
'spec/foo' | [:backend]
'spec/foo/bar' | [:backend]
'app/models/foo' | [:backend]
'bin/foo' | [:backend]
'config/foo' | [:backend]
'lib/foo' | [:backend]
'rubocop/foo' | [:backend]
'.rubocop.yml' | [:backend]
'.rubocop_todo.yml' | [:backend]
'.rubocop_manual_todo.yml' | [:backend]
'spec/foo' | [:backend]
'spec/foo/bar' | [:backend]
'ee/app/foo' | [:backend]
'ee/bin/foo' | [:backend]
......
......@@ -108,7 +108,7 @@ RSpec.describe Key, :mailer do
expect(build(:key, key: 'ssh-rsa an-invalid-key==')).not_to be_valid
end
where(:factory, :chars, :expected_sections) do
where(:factory, :characters, :expected_sections) do
[
[:key, ["\n", "\r\n"], 3],
[:key, [' ', ' '], 3],
......@@ -122,7 +122,7 @@ RSpec.describe Key, :mailer do
let!(:original_fingerprint_sha256) { key.fingerprint_sha256 }
it 'accepts a key with blank space characters after stripping them' do
modified_key = key.key.insert(100, chars.first).insert(40, chars.last)
modified_key = key.key.insert(100, characters.first).insert(40, characters.last)
_, content = modified_key.split
key.update!(key: modified_key)
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require_relative '../../../../rubocop/cop/code_reuse/active_record'
RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of "where" without any arguments' do
expect_offense(<<~SOURCE)
def foo
User.where
^^^^^ This method can only be used inside an ActiveRecord model: https://gitlab.com/gitlab-org/gitlab-foss/issues/49653
end
SOURCE
end
it 'flags the use of "where" with arguments' do
expect_offense(<<~SOURCE)
def foo
User.where(id: 10)
^^^^^ This method can only be used inside an ActiveRecord model: https://gitlab.com/gitlab-org/gitlab-foss/issues/49653
end
SOURCE
end
it 'does not flag the use of "group" without any arguments' do
expect_no_offenses(<<~SOURCE)
def foo
project.group
end
SOURCE
end
it 'flags the use of "group" with arguments' do
expect_offense(<<~SOURCE)
def foo
project.group(:name)
^^^^^ This method can only be used inside an ActiveRecord model: https://gitlab.com/gitlab-org/gitlab-foss/issues/49653
end
SOURCE
end
it 'does not flag the use of ActiveRecord models in a model' do
path = rails_root_join('app', 'models', 'foo.rb').to_s
expect_no_offenses(<<~SOURCE, path)
def foo
project.group(:name)
end
SOURCE
end
it 'does not flag the use of ActiveRecord models in a spec' do
path = rails_root_join('spec', 'foo_spec.rb').to_s
expect_no_offenses(<<~SOURCE, path)
def foo
project.group(:name)
end
SOURCE
end
it 'does not flag the use of ActiveRecord models in a background migration' do
path = rails_root_join('lib', 'gitlab', 'background_migration', 'foo.rb').to_s
expect_no_offenses(<<~SOURCE, path)
def foo
project.group(:name)
end
SOURCE
end
it 'does not flag the use of ActiveRecord models in lib/gitlab/database' do
path = rails_root_join('lib', 'gitlab', 'database', 'foo.rb').to_s
expect_no_offenses(<<~SOURCE, path)
def foo
project.group(:name)
end
SOURCE
end
it 'autocorrects offenses in instance methods by allowing them' do
corrected = autocorrect_source(<<~SOURCE)
def foo
User.where
end
SOURCE
expect(corrected).to eq(<<~SOURCE)
# rubocop: disable CodeReuse/ActiveRecord
def foo
User.where
end
# rubocop: enable CodeReuse/ActiveRecord
SOURCE
end
it 'autocorrects offenses in class methods by allowing them' do
corrected = autocorrect_source(<<~SOURCE)
def self.foo
User.where
end
SOURCE
expect(corrected).to eq(<<~SOURCE)
# rubocop: disable CodeReuse/ActiveRecord
def self.foo
User.where
end
# rubocop: enable CodeReuse/ActiveRecord
SOURCE
end
it 'autocorrects offenses in blocks by allowing them' do
corrected = autocorrect_source(<<~SOURCE)
get '/' do
User.where
end
SOURCE
expect(corrected).to eq(<<~SOURCE)
# rubocop: disable CodeReuse/ActiveRecord
get '/' do
User.where
end
# rubocop: enable CodeReuse/ActiveRecord
SOURCE
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require_relative '../../../../rubocop/cop/rspec/be_success_matcher'
RSpec.describe RuboCop::Cop::RSpec::BeSuccessMatcher, type: :rubocop do
......
# frozen_string_literal: true
require 'tempfile'
# This module provides methods that make it easier to test Cops.
module CopHelper
extend RSpec::SharedContext
let(:ruby_version) { 2.4 }
let(:rails_version) { false }
def inspect_source_file(source)
Tempfile.open('tmp') { |f| inspect_source(source, f) }
end
def inspect_source(source, file = nil)
RuboCop::Formatter::DisabledConfigFormatter.config_to_allow_offenses = {}
RuboCop::Formatter::DisabledConfigFormatter.detected_styles = {}
processed_source = parse_source(source, file)
raise 'Error parsing example code' unless processed_source.valid_syntax?
_investigate(cop, processed_source)
end
def parse_source(source, file = nil)
if file&.respond_to?(:write)
file.write(source)
file.rewind
file = file.path
end
RuboCop::ProcessedSource.new(source, ruby_version, file)
end
def autocorrect_source_file(source)
Tempfile.open('tmp') { |f| autocorrect_source(source, f) }
end
def autocorrect_source(source, file = nil)
RuboCop::Formatter::DisabledConfigFormatter.config_to_allow_offenses = {}
RuboCop::Formatter::DisabledConfigFormatter.detected_styles = {}
cop.instance_variable_get(:@options)[:auto_correct] = true
processed_source = parse_source(source, file)
_investigate(cop, processed_source)
@last_corrector.rewrite
end
def _investigate(cop, processed_source)
team = RuboCop::Cop::Team.new([cop], nil, raise_error: true)
report = team.investigate(processed_source)
@last_corrector = report.correctors.first || RuboCop::Cop::Corrector.new(processed_source)
report.offenses
end
end
module RuboCop
module Cop
# Monkey-patch Cop for tests to provide easy access to messages and
# highlights.
# this file is an exact copy of source except for this line
# where we change to the new Base class defined in rubocop and skirt around our superclass mismatch for class Cop
# when running a rubocop spec.
class Base
def messages
offenses.sort.map(&:message)
end
def highlights
offenses.sort.map { |o| o.location.source }
end
end
end
end
......@@ -5,7 +5,8 @@ require_relative "helpers/stub_metrics"
require_relative "helpers/stub_object_storage"
require_relative "helpers/stub_env"
require_relative "helpers/fast_rails_root"
require 'rubocop/rspec/support'
require_relative 'rubocop_patch'
RSpec.configure do |config|
config.mock_with :rspec
......
# frozen_string_literal: true
# There is an issue between rubocop versions 0.86 and 0.87 (verified by testing locally)
# where the monkey patching in cop_helper is referencing class Cop and should really be referencing class Base instead
# the gem's version of the cop_helper causes this issue when testing a rubocop cop locally and in CI
# The only difference in this file as compared to gem source file is that we include our own cop_helper instead
# which is a direct copy with a fix for the monkey patching part.
# Doing this, resolves the issue.
# Ideally we should move away from using the cop_helper at all as is the direction of rubocop as seen
# here - https://github.com/rubocop-hq/rubocop/issues/8003
# more info https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46477
require_relative 'helpers/cop_helper'
require 'rubocop/rspec/host_environment_simulation_helper'
require 'rubocop/rspec/shared_contexts'
require 'rubocop/rspec/expect_offense'
RSpec.configure do |config|
config.include CopHelper
config.include HostEnvironmentSimulatorHelper
end
......@@ -4,6 +4,6 @@
source 'https://rubygems.org'
gem 'overcommit'
gem 'gitlab-styles', '~> 4.3.0', require: false
gem 'gitlab-styles', '~> 5.0.0', require: false
gem 'scss_lint', '~> 0.56.0', require: false
gem 'haml_lint', '~> 0.34.0', require: false
GEM
remote: https://rubygems.org/
specs:
activesupport (6.0.3)
activesupport (6.0.3.4)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
tzinfo (~> 1.1)
zeitwerk (~> 2.2, >= 2.2.2)
ast (2.4.0)
ast (2.4.1)
childprocess (3.0.0)
concurrent-ruby (1.1.6)
concurrent-ruby (1.1.7)
ffi (1.12.2)
gitlab-styles (4.3.0)
rubocop (~> 0.82.0)
gitlab-styles (5.0.0)
rubocop (~> 0.89.1)
rubocop-gitlab-security (~> 0.1.0)
rubocop-performance (~> 1.5.2)
rubocop-rails (~> 2.5)
rubocop-rspec (~> 1.36)
rubocop-performance (~> 1.8.1)
rubocop-rails (~> 2.8)
rubocop-rspec (~> 1.44)
haml (5.1.2)
temple (>= 0.8.0)
tilt
......@@ -25,42 +25,47 @@ GEM
rainbow
rubocop (>= 0.50.0)
sysexits (~> 1.1)
i18n (1.8.2)
i18n (1.8.5)
concurrent-ruby (~> 1.0)
iniparse (1.5.0)
jaro_winkler (1.5.4)
minitest (5.11.3)
minitest (5.14.2)
overcommit (0.53.0)
childprocess (>= 0.6.3, < 4)
iniparse (~> 1.4)
parallel (1.19.1)
parser (2.7.1.2)
ast (~> 2.4.0)
rack (2.0.9)
parallel (1.19.2)
parser (2.7.2.0)
ast (~> 2.4.1)
rack (2.2.3)
rainbow (3.0.0)
rake (12.3.3)
rb-fsevent (0.10.2)
rb-inotify (0.9.10)
ffi (>= 0.5.0, < 2)
regexp_parser (1.8.2)
rexml (3.2.4)
rubocop (0.82.0)
jaro_winkler (~> 1.5.1)
rubocop (0.89.1)
parallel (~> 1.10)
parser (>= 2.7.0.1)
parser (>= 2.7.1.1)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.7)
rexml
rubocop-ast (>= 0.3.0, < 1.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 2.0)
rubocop-ast (0.8.0)
parser (>= 2.7.1.5)
rubocop-gitlab-security (0.1.1)
rubocop (>= 0.51)
rubocop-performance (1.5.2)
rubocop (>= 0.71.0)
rubocop-rails (2.5.2)
activesupport
rubocop-performance (1.8.1)
rubocop (>= 0.87.0)
rubocop-ast (>= 0.4.0)
rubocop-rails (2.8.1)
activesupport (>= 4.2.0)
rack (>= 1.1)
rubocop (>= 0.72.0)
rubocop-rspec (1.37.0)
rubocop (>= 0.68.1)
rubocop (>= 0.87.0)
rubocop-rspec (1.44.1)
rubocop (~> 0.87)
rubocop-ast (>= 0.7.1)
ruby-progressbar (1.10.1)
sass (3.5.5)
sass-listen (~> 4.0.0)
......@@ -77,13 +82,13 @@ GEM
tzinfo (1.2.7)
thread_safe (~> 0.1)
unicode-display_width (1.7.0)
zeitwerk (2.3.0)
zeitwerk (2.4.1)
PLATFORMS
ruby
DEPENDENCIES
gitlab-styles (~> 4.3.0)
gitlab-styles (~> 5.0.0)
haml_lint (~> 0.34.0)
overcommit
scss_lint (~> 0.56.0)
......
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