Commit a2d39b80 authored by Rémy Coutable's avatar Rémy Coutable 🏖 Committed by Robert Speicher

Use gitlab-styles

Signed-off-by: Rémy Coutable's avatarRémy Coutable <remy@rymai.me>
parent 299e04e3
Pipeline #15426262 passed with stages
in 40 minutes and 56 seconds
......@@ -16,6 +16,7 @@ engines:
enabled: true
rubocop:
enabled: true
channel: "gitlab-rubocop-0-52"
ratings:
paths:
- Gemfile.lock
......
......@@ -594,7 +594,7 @@ codequality:
script:
- cp .rubocop.yml .rubocop.yml.bak
- grep -v "rubocop-gitlab-security" .rubocop.yml.bak > .rubocop.yml
- docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > raw_codeclimate.json
- docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-v2 analyze -f json > raw_codeclimate.json
- cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,fingerprint,location})' > codeclimate.json
- mv .rubocop.yml.bak .rubocop.yml
artifacts:
......
This diff is collapsed.
This diff is collapsed.
......@@ -334,9 +334,11 @@ group :development, :test do
gem 'spring-commands-rspec', '~> 1.0.4'
gem 'spring-commands-spinach', '~> 1.1.0'
gem 'rubocop', '~> 0.49.1', require: false
gem 'rubocop-rspec', '~> 1.15.1', require: false
gem 'rubocop-gitlab-security', '~> 0.1.0', require: false
gem 'gitlab-styles', '~> 2.2.0', require: false
# Pin these dependencies, otherwise a new rule could break the CI pipelines
gem 'rubocop', '~> 0.52.0'
gem 'rubocop-rspec', '~> 1.20.1'
gem 'scss_lint', '~> 0.54.0', require: false
gem 'haml_lint', '~> 0.26.0', require: false
gem 'simplecov', '~> 0.14.0', require: false
......
......@@ -303,6 +303,10 @@ GEM
mime-types (>= 1.16)
posix-spawn (~> 0.3)
gitlab-markup (1.6.3)
gitlab-styles (2.2.0)
rubocop (~> 0.51)
rubocop-gitlab-security (~> 0.1.0)
rubocop-rspec (~> 1.19)
gitlab_omniauth-ldap (2.0.4)
net-ldap (~> 0.16)
omniauth (~> 1.3)
......@@ -777,21 +781,21 @@ GEM
pg
rails
sqlite3
rubocop (0.49.1)
rubocop (0.52.0)
parallel (~> 1.10)
parser (>= 2.3.3.1, < 3.0)
parser (>= 2.4.0.2, < 3.0)
powerpack (~> 0.1)
rainbow (>= 1.99.1, < 3.0)
rainbow (>= 2.2.2, < 4.0)
ruby-progressbar (~> 1.7)
unicode-display_width (~> 1.0, >= 1.0.1)
rubocop-gitlab-security (0.1.0)
rubocop (>= 0.47.1)
rubocop-rspec (1.15.1)
rubocop (>= 0.42.0)
rubocop-gitlab-security (0.1.1)
rubocop (>= 0.51)
rubocop-rspec (1.20.1)
rubocop (>= 0.51.0)
ruby-fogbugz (0.2.1)
crack (~> 0.4)
ruby-prof (0.16.2)
ruby-progressbar (1.8.1)
ruby-progressbar (1.9.0)
ruby-saml (1.4.1)
nokogiri (>= 1.5.10)
ruby_parser (3.9.0)
......@@ -1046,6 +1050,7 @@ DEPENDENCIES
github-linguist (~> 4.7.0)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-markup (~> 1.6.2)
gitlab-styles (~> 2.2.0)
gitlab_omniauth-ldap (~> 2.0.4)
gollum-lib (~> 4.2)
gollum-rugged_adapter (~> 0.4.4)
......@@ -1148,9 +1153,8 @@ DEPENDENCIES
rspec-retry (~> 0.4.5)
rspec-set (~> 0.1.3)
rspec_profiling (~> 0.0.5)
rubocop (~> 0.49.1)
rubocop-gitlab-security (~> 0.1.0)
rubocop-rspec (~> 1.15.1)
rubocop (~> 0.52.0)
rubocop-rspec (~> 1.20.1)
ruby-fogbugz (~> 0.2.1)
ruby-prof (~> 0.16.2)
ruby_parser (~> 3.8)
......
......@@ -18,7 +18,7 @@ Peek.into Peek::Views::Rblineprof
Peek.into Peek::Views::GC
Peek.into Peek::Views::Gitaly
# rubocop:disable Style/ClassAndModuleCamelCase
# rubocop:disable Naming/ClassAndModuleCamelCase
class PEEK_DB_CLIENT
class << self
attr_accessor :query_details
......
......@@ -93,9 +93,7 @@ module Gitlab
private
def entries
@entries
end
attr_reader :entries
end
end
end
......
......@@ -56,11 +56,12 @@ module Gitlab
# Do nothing if hooks already exist
unless real_local_hooks_path == File.realpath(global_hooks_path)
# Move the existing hooks somewhere safe
FileUtils.mv(
local_hooks_path,
"#{local_hooks_path}.old.#{Time.now.to_i}"
) if File.exist?(local_hooks_path)
if File.exist?(local_hooks_path)
# Move the existing hooks somewhere safe
FileUtils.mv(
local_hooks_path,
"#{local_hooks_path}.old.#{Time.now.to_i}")
end
# Create the hooks symlink
FileUtils.ln_sf(global_hooks_path, local_hooks_path)
......
require_relative '../model_helpers'
module RuboCop
module Cop
# Cop that prevents the use of `dependent: ...` in ActiveRecord models.
class ActiveRecordDependent < RuboCop::Cop::Cop
include ModelHelpers
MSG = 'Do not use `dependent: to remove associated data, ' \
'use foreign keys with cascading deletes instead'.freeze
METHOD_NAMES = [:has_many, :has_one, :belongs_to].freeze
def on_send(node)
return unless in_model?(node)
return unless METHOD_NAMES.include?(node.children[1])
node.children.last.each_node(:pair) do |pair|
key_name = pair.children[0].children[0]
add_offense(pair, :expression) if key_name == :dependent
end
end
end
end
end
require_relative '../model_helpers'
module RuboCop
module Cop
# Cop that prevents the use of `serialize` in ActiveRecord models.
class ActiveRecordSerialize < RuboCop::Cop::Cop
include ModelHelpers
MSG = 'Do not store serialized data in the database, use separate columns and/or tables instead'.freeze
def on_send(node)
return unless in_model?(node)
add_offense(node, :selector) if node.children[1] == :serialize
end
end
end
end
module RuboCop
module Cop
# This cop makes sure that custom error classes, when empty, are declared
# with Class.new.
#
# @example
# # bad
# class FooError < StandardError
# end
#
# # okish
# class FooError < StandardError; end
#
# # good
# FooError = Class.new(StandardError)
class CustomErrorClass < RuboCop::Cop::Cop
MSG = 'Use `Class.new(SuperClass)` to define an empty custom error class.'.freeze
def on_class(node)
_klass, parent, body = node.children
return if body
parent_klass = class_name_from_node(parent)
return unless parent_klass && parent_klass.to_s.end_with?('Error')
add_offense(node, :expression)
end
def autocorrect(node)
klass, parent, _body = node.children
replacement = "#{class_name_from_node(klass)} = Class.new(#{class_name_from_node(parent)})"
lambda do |corrector|
corrector.replace(node.source_range, replacement)
end
end
private
# The nested constant `Foo::Bar::Baz` looks like:
#
# s(:const,
# s(:const,
# s(:const, nil, :Foo), :Bar), :Baz)
#
# So recurse through that to get the name as written in the source.
#
def class_name_from_node(node, suffix = nil)
return unless node&.type == :const
name = node.children[1].to_s
name = "#{name}::#{suffix}" if suffix
if node.children[0]
class_name_from_node(node.children[0], name)
else
name
end
end
end
end
end
module RuboCop
module Cop
# This cop prevents usage of the `git` and `github` arguments to `gem` in a
# `Gemfile` in order to avoid additional points of failure beyond
# rubygems.org.
class GemFetcher < RuboCop::Cop::Cop
MSG = 'Do not use gems from git repositories, only use gems from RubyGems.'.freeze
GIT_KEYS = [:git, :github].freeze
def on_send(node)
return unless gemfile?(node)
func_name = node.children[1]
return unless func_name == :gem
node.children.last.each_node(:pair) do |pair|
key_name = pair.children[0].children[0].to_sym
if GIT_KEYS.include?(key_name)
add_offense(node, pair.source_range, MSG)
end
end
end
private
def gemfile?(node)
node
.location
.expression
.source_buffer
.name
.end_with?("Gemfile")
end
end
end
end
......@@ -30,12 +30,12 @@ module RuboCop
if only_ivar_or_assignment?(definition)
# We don't allow if any other ivar is used
definition.each_descendant(:ivar) do |offense|
add_offense(offense, :expression)
add_offense(offense, location: :expression)
end
# We allow initialize method and single ivar
elsif !initialize_method?(definition) && !single_ivar?(definition)
definition.each_descendant(:ivar, :ivasgn) do |offense|
add_offense(offense, :expression)
add_offense(offense, location: :expression)
end
end
end
......
require_relative '../model_helpers'
module RuboCop
module Cop
# Cop that prevents the use of `in_batches`
class InBatches < RuboCop::Cop::Cop
MSG = 'Do not use `in_batches`, use `each_batch` from the EachBatch module instead'.freeze
def on_send(node)
return unless node.children[1] == :in_batches
add_offense(node, :selector)
end
end
end
end
......@@ -9,14 +9,14 @@ module RuboCop
MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze
def_node_matcher :includes_sidekiq_worker?, <<~PATTERN
(send nil :include (const (const nil :Sidekiq) :Worker))
(send nil? :include (const (const nil? :Sidekiq) :Worker))
PATTERN
def on_send(node)
return if in_spec?(node)
return unless includes_sidekiq_worker?(node)
add_offense(node.arguments.first, :expression)
add_offense(node.arguments.first, location: :expression)
end
def autocorrect(node)
......
# frozen_string_literal: true
module RuboCop
module Cop
# Ensures a line break after guard clauses.
#
# @example
# # bad
# return unless condition
# do_stuff
#
# # good
# return unless condition
#
# do_stuff
#
# # bad
# raise if condition
# do_stuff
#
# # good
# raise if condition
#
# do_stuff
#
# Multiple guard clauses are allowed without
# line break.
#
# # good
# return unless condition_a
# return unless condition_b
#
# do_stuff
#
# Guard clauses in case statement are allowed without
# line break.
#
# # good
# case model
# when condition_a
# return true unless condition_b
# when
# ...
# end
#
# Guard clauses before end are allowed without
# line break.
#
# # good
# if condition_a
# do_something
# else
# do_something_else
# return unless condition
# end
#
# do_something_more
class LineBreakAfterGuardClauses < RuboCop::Cop::Cop
MSG = 'Add a line break after guard clauses'
def_node_matcher :guard_clause_node?, <<-PATTERN
[{(send nil? {:raise :fail :throw} ...) return break next} single_line?]
PATTERN
def on_if(node)
return unless node.single_line?
return unless guard_clause?(node)
return if next_line(node).blank? || clause_last_line?(next_line(node)) || guard_clause?(next_sibling(node))
add_offense(node, :expression, MSG)
end
def autocorrect(node)
lambda do |corrector|
corrector.insert_after(node.loc.expression, "\n")
end
end
private
def guard_clause?(node)
return false unless node.if_type?
guard_clause_node?(node.if_branch)
end
def next_sibling(node)
node.parent.children[node.sibling_index + 1]
end
def next_line(node)
processed_source[node.loc.line]
end
def clause_last_line?(line)
line =~ /^\s*(?:end|elsif|else|when|rescue|ensure)/
end
end
end
end
......@@ -29,7 +29,7 @@ module RuboCop
opts.each_node(:pair) do |pair|
if hash_key_type(pair) == :sym && hash_key_name(pair) == :default
add_offense(node, :selector)
add_offense(node, location: :selector)
end
end
end
......
......@@ -15,7 +15,7 @@ module RuboCop
name = node.children[1]
add_offense(node, :selector) if name == :add_foreign_key
add_offense(node, location: :selector) if name == :add_foreign_key
end
def method_name(node)
......
......@@ -21,7 +21,7 @@ module RuboCop
node.each_ancestor(:def) do |def_node|
next unless method_name(def_node) == :change
add_offense(def_node, :name)
add_offense(def_node, location: :name)
end
end
......
......@@ -27,7 +27,7 @@ module RuboCop
# data in these tables yet.
next if new_tables.include?(first_arg)
add_offense(send_node, :selector)
add_offense(send_node, location: :selector)
end
end
......
......@@ -13,7 +13,7 @@ module RuboCop
def on_send(node)
return unless in_migration?(node)
add_offense(node, :selector) if method_name(node) == :add_timestamps
add_offense(node, location: :selector) if method_name(node) == :add_timestamps
end
def method_name(node)
......
......@@ -17,7 +17,7 @@ module RuboCop
method_name = node.children[1]
if method_name == :datetime || method_name == :timestamp
add_offense(send_node, :selector, format(MSG, method_name))
add_offense(send_node, location: :selector, message: format(MSG, method_name))
end
end
end
......@@ -32,7 +32,7 @@ module RuboCop
last_argument = descendant.children.last
if last_argument == :datetime || last_argument == :timestamp
add_offense(node, :expression, format(MSG, last_argument))
add_offense(node, location: :expression, message: format(MSG, last_argument))
end
end
end
......
......@@ -29,7 +29,7 @@ module RuboCop
hash_key_name(pair) == :using
if hash_key_value(pair).to_s == 'hash'
add_offense(pair, :expression)
add_offense(pair, location: :expression)
end
end
end
......
......@@ -20,7 +20,7 @@ module RuboCop
send_method = send_node.children[1]
if send_method == :remove_column
add_offense(send_node, :selector)
add_offense(send_node, location: :selector)
end
end
end
......
......@@ -16,7 +16,7 @@ module RuboCop
return unless node.children[1] == :remove_concurrent_index
node.each_ancestor(:def) do |def_node|
add_offense(def_node, :name) if method_name(def_node) == :change
add_offense(def_node, location: :name) if method_name(def_node) == :change
end
end
......
......@@ -13,7 +13,7 @@ module RuboCop
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
add_offense(send_node, :selector) if method_name(send_node) == :remove_index
add_offense(send_node, location: :selector) if method_name(send_node) == :remove_index
end
end
......
......@@ -9,7 +9,7 @@ module RuboCop
include MigrationHelpers
def_node_matcher :add_column_with_default?, <<~PATTERN
(send nil :add_column_with_default $...)
(send nil? :add_column_with_default $...)
PATTERN
def_node_matcher :defines_change?, <<~PATTERN
......@@ -26,7 +26,7 @@ module RuboCop
node.each_ancestor(:def) do |def_node|
next unless defines_change?(def_node)
add_offense(def_node, :name)
add_offense(def_node, location: :name)
end
end
end
......
......@@ -28,7 +28,7 @@ module RuboCop
].freeze
def_node_matcher :add_column?, <<~PATTERN
(send nil :add_column $...)
(send nil? :add_column $...)
PATTERN
def on_send(node)
......@@ -54,7 +54,7 @@ module RuboCop
NULL_OFFENSE
end
add_offense(node, :expression, format(offense, table)) if offense
add_offense(node, location: :expression, message: format(offense, table)) if offense
end
def no_default?(opts)
......
......@@ -14,7 +14,7 @@ module RuboCop
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
add_offense(send_node, :selector) if method_name(send_node) == :timestamps
add_offense(send_node, location: :selector) if method_name(send_node) == :timestamps
end
end
......
......@@ -18,7 +18,7 @@ module RuboCop
spec_path = spec_filename(node)
unless File.exist?(File.expand_path(spec_path, rails_root))
add_offense(node, :expression, format(MSG, spec_path))
add_offense(node, location: :expression, message: format(MSG, spec_path))
end
end
......
......@@ -35,7 +35,7 @@ module RuboCop
].freeze
def_node_matcher :batch_update?, <<~PATTERN
(send nil ${:add_column_with_default :update_column_in_batches} $(sym ...) ...)
(send nil? ${:add_column_with_default :update_column_in_batches} $(sym ...) ...)
PATTERN
def on_send(node)
......@@ -49,7 +49,7 @@ module RuboCop
return unless LARGE_TABLES.include?(table)
add_offense(node, :expression, format(MSG, update_method, table))
add_offense(node, location: :expression, message: format(MSG, update_method, table))
end
end
end
......
require_relative '../model_helpers'
module RuboCop
module Cop
# Cop that prevents the use of polymorphic associations
class PolymorphicAssociations < RuboCop::Cop::Cop
include ModelHelpers
MSG = 'Do not use polymorphic associations, use separate tables instead'.freeze
def on_send(node)
return unless in_model?(node)
return unless node.children[1] == :belongs_to
node.children.last.each_node(:pair) do |pair|
key_name = pair.children[0].children[0]
add_offense(pair, :expression) if key_name == :polymorphic
end
end
end
end
end
......@@ -17,7 +17,7 @@ module RuboCop
return unless method_name(namespace_expr) == :namespace
return unless receiver(namespace_expr) == project_expr
add_offense(node, :selector)
add_offense(node, location: :selector)
end
def autocorrect(node)
......
module RuboCop
module Cop
# This cop prevents usage of 'redirect_to' in actions 'destroy' without specifying 'status'.
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/31840
class RedirectWithStatus < RuboCop::Cop::Cop
MSG = 'Do not use "redirect_to" without "status" in "destroy" action'.freeze
def on_def(node)
return unless in_controller?(node)
return unless destroy?(node) || destroy_all?(node)
node.each_descendant(:send) do |def_node|
next unless redirect_to?(def_node)
methods = []
def_node.children.last.each_node(:pair) do |pair|
methods << pair.children.first.children.first
end
add_offense(def_node, :selector) unless methods.include?(:status)
end
end
private
def in_controller?(node)
node.location.expression.source_buffer.name.end_with?('_controller.rb')
end
def destroy?(node)
node.children.first == :destroy
end
def destroy_all?(node)
node.children.first == :destroy_all
end
def redirect_to?(node)
node.children[1] == :redirect_to
end
end
end