Commit 9bcc657e authored by Igor Drozdov's avatar Igor Drozdov

Merge branch '220098-remove-codereuse-activerecord-cop-for-sum-calls' into 'master'

Remove CodeReuse/ActiveRecord cop for sum calls

Closes #220098

See merge request gitlab-org/gitlab!43603
parents 19a477fa 40495d35
- sum_added_lines = diff_files.sum(&:added_lines) # rubocop: disable CodeReuse/ActiveRecord - sum_added_lines = diff_files.sum(&:added_lines)
- sum_removed_lines = diff_files.sum(&:removed_lines) # rubocop: disable CodeReuse/ActiveRecord - sum_removed_lines = diff_files.sum(&:removed_lines)
.commit-stat-summary.dropdown .commit-stat-summary.dropdown
Showing Showing
%button.diff-stats-summary-toggler.js-diff-stats-dropdown{ type: "button", data: { toggle: "dropdown", display: "static" } }< %button.diff-stats-summary-toggler.js-diff-stats-dropdown{ type: "button", data: { toggle: "dropdown", display: "static" } }<
......
...@@ -37,7 +37,7 @@ module EE ...@@ -37,7 +37,7 @@ module EE
def lfs_push_size def lfs_push_size
strong_memoize(:lfs_push_size) do strong_memoize(:lfs_push_size) do
objects.sum { |o| o[:size] } # rubocop: disable CodeReuse/ActiveRecord objects.sum { |o| o[:size] }
end end
end end
end end
......
...@@ -31,7 +31,7 @@ module Elastic ...@@ -31,7 +31,7 @@ module Elastic
end end
def queue_size def queue_size
Elastic::IndexingControl::WORKERS.sum do |worker_class| # rubocop:disable CodeReuse/ActiveRecord Elastic::IndexingControl::WORKERS.sum do |worker_class|
new(worker_class).queue_size new(worker_class).queue_size
end end
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
- return unless milestone.weight_available? - return unless milestone.weight_available?
- total_weight = milestone.issues_visible_to_user(current_user).sum(:weight) # rubocop: disable CodeReuse/ActiveRecord - total_weight = milestone.issues_visible_to_user(current_user).sum(:weight)
.block.weight .block.weight
.sidebar-collapsed-icon.has-tooltip{ title: milestone_weight_tooltip_text(total_weight), data: { container: 'body', placement: 'left' } } .sidebar-collapsed-icon.has-tooltip{ title: milestone_weight_tooltip_text(total_weight), data: { container: 'body', placement: 'left' } }
= sprite_icon('weight') = sprite_icon('weight')
......
...@@ -45,7 +45,7 @@ module EE ...@@ -45,7 +45,7 @@ module EE
end end
def seeds_size def seeds_size
@command.stage_seeds.sum(&:size) # rubocop: disable CodeReuse/ActiveRecord @command.stage_seeds.sum(&:size)
end end
end end
end end
......
...@@ -30,7 +30,7 @@ module Gitlab ...@@ -30,7 +30,7 @@ module Gitlab
end end
def all def all
counts.values.sum # rubocop:disable CodeReuse/ActiveRecord counts.values.sum
end end
private private
......
...@@ -46,13 +46,11 @@ module Gitlab ...@@ -46,13 +46,11 @@ module Gitlab
end end
end end
# rubocop:disable CodeReuse/ActiveRecord
def number_of_generated_jobs def number_of_generated_jobs
value.sum do |config| value.sum do |config|
config.values.reduce(1) { |acc, values| acc * values.size } config.values.reduce(1) { |acc, values| acc * values.size }
end end
end end
# rubocop:enable CodeReuse/ActiveRecord
end end
end end
end end
......
...@@ -118,7 +118,7 @@ module Gitlab ...@@ -118,7 +118,7 @@ module Gitlab
next unless line_too_long?(line) next unless line_too_long?(line)
url_size = line.scan(%r((https?://\S+))).sum { |(url)| url.length } # rubocop:disable CodeReuse/ActiveRecord url_size = line.scan(%r((https?://\S+))).sum { |(url)| url.length }
# If the line includes a URL, we'll allow it to exceed MAX_LINE_LENGTH characters, but # If the line includes a URL, we'll allow it to exceed MAX_LINE_LENGTH characters, but
# only if the line _without_ the URL does not exceed this limit. # only if the line _without_ the URL does not exceed this limit.
......
...@@ -504,7 +504,7 @@ module Gitlab ...@@ -504,7 +504,7 @@ module Gitlab
changes_size = 0 changes_size = 0
changes_list.each do |change| changes_list.each do |change|
changes_size += repository.new_blobs(change[:newrev]).sum(&:size) # rubocop: disable CodeReuse/ActiveRecord changes_size += repository.new_blobs(change[:newrev]).sum(&:size)
check_size_against_limit(changes_size) check_size_against_limit(changes_size)
end end
......
...@@ -22,7 +22,7 @@ module Gitlab ...@@ -22,7 +22,7 @@ module Gitlab
def check def check
return unless http_servers return unless http_servers
http_servers.sum(&:worker_processes) # rubocop: disable CodeReuse/ActiveRecord http_servers.sum(&:worker_processes)
end end
# Traversal of ObjectSpace is expensive, on fully loaded application # Traversal of ObjectSpace is expensive, on fully loaded application
......
...@@ -37,7 +37,7 @@ module Gitlab ...@@ -37,7 +37,7 @@ module Gitlab
%i[get_request_count query_time read_bytes write_bytes].each do |method| %i[get_request_count query_time read_bytes write_bytes].each do |method|
define_method method do define_method method do
STORAGES.sum(&method) # rubocop:disable CodeReuse/ActiveRecord STORAGES.sum(&method)
end end
end end
end end
......
...@@ -54,7 +54,7 @@ module Gitlab ...@@ -54,7 +54,7 @@ module Gitlab
end end
def unicorn_workers_count def unicorn_workers_count
http_servers.sum(&:worker_processes) # rubocop: disable CodeReuse/ActiveRecord http_servers.sum(&:worker_processes)
end end
# Traversal of ObjectSpace is expensive, on fully loaded application # Traversal of ObjectSpace is expensive, on fully loaded application
......
...@@ -231,7 +231,7 @@ module Gitlab ...@@ -231,7 +231,7 @@ module Gitlab
def rss_increase_by_jobs def rss_increase_by_jobs
Gitlab::SidekiqDaemon::Monitor.instance.jobs_mutex.synchronize do Gitlab::SidekiqDaemon::Monitor.instance.jobs_mutex.synchronize do
Gitlab::SidekiqDaemon::Monitor.instance.jobs.sum do |job| # rubocop:disable CodeReuse/ActiveRecord Gitlab::SidekiqDaemon::Monitor.instance.jobs.sum do |job|
rss_increase_by_job(job) rss_increase_by_job(job)
end end
end end
......
...@@ -22,7 +22,7 @@ module Gitlab ...@@ -22,7 +22,7 @@ module Gitlab
end end
def sum(relation, column, *rest) def sum(relation, column, *rest)
relation.select(relation.all.table[column].sum).to_sql # rubocop:disable CodeReuse/ActiveRecord relation.select(relation.all.table[column].sum).to_sql
end end
private private
......
...@@ -23,7 +23,7 @@ module Peek ...@@ -23,7 +23,7 @@ module Peek
private private
def duration def duration
detail_store.map { |entry| entry[:duration] }.sum * 1000 # rubocop:disable CodeReuse/ActiveRecord detail_store.map { |entry| entry[:duration] }.sum * 1000
end end
def calls def calls
......
...@@ -5,20 +5,20 @@ require_relative '../../code_reuse_helpers' ...@@ -5,20 +5,20 @@ require_relative '../../code_reuse_helpers'
module RuboCop module RuboCop
module Cop module Cop
module CodeReuse module CodeReuse
# Cop that blacklists the use of ActiveRecord methods outside of models. # Cop that denies the use of ActiveRecord methods outside of models.
class ActiveRecord < RuboCop::Cop::Cop class ActiveRecord < RuboCop::Cop::Cop
include CodeReuseHelpers include CodeReuseHelpers
MSG = 'This method can only be used inside an ActiveRecord model: ' \ MSG = 'This method can only be used inside an ActiveRecord model: ' \
'https://gitlab.com/gitlab-org/gitlab-foss/issues/49653' 'https://gitlab.com/gitlab-org/gitlab-foss/issues/49653'
# Various methods from ActiveRecord::Querying that are blacklisted. We # Various methods from ActiveRecord::Querying that are denied. We
# exclude some generic ones such as `any?` and `first`, as these may # exclude some generic ones such as `any?` and `first`, as these may
# lead to too many false positives, since `Array` also supports these # lead to too many false positives, since `Array` also supports these
# methods. # methods.
# #
# The keys of this Hash are the blacklisted method names. The values are # The keys of this Hash are the denied method names. The values are
# booleans that indicate if the method should only be blacklisted if any # booleans that indicate if the method should only be denied if any
# arguments are provided. # arguments are provided.
NOT_ALLOWED = { NOT_ALLOWED = {
average: true, average: true,
...@@ -57,7 +57,6 @@ module RuboCop ...@@ -57,7 +57,6 @@ module RuboCop
references: true, references: true,
reorder: true, reorder: true,
rewhere: true, rewhere: true,
sum: false,
take: false, take: false,
take!: false, take!: false,
unscope: false, unscope: false,
...@@ -65,9 +64,9 @@ module RuboCop ...@@ -65,9 +64,9 @@ module RuboCop
with: true with: true
}.freeze }.freeze
# Directories that allow the use of the blacklisted methods. These # Directories that allow the use of the denied methods. These
# directories are checked relative to both . and ee/ # directories are checked relative to both . and ee/
WHITELISTED_DIRECTORIES = %w[ ALLOWED_DIRECTORIES = %w[
app/models app/models
config config
danger danger
...@@ -88,7 +87,7 @@ module RuboCop ...@@ -88,7 +87,7 @@ module RuboCop
].freeze ].freeze
def on_send(node) def on_send(node)
return if in_whitelisted_directory?(node) return if in_allowed_directory?(node)
receiver = node.children[0] receiver = node.children[0]
send_name = node.children[1] send_name = node.children[1]
...@@ -105,12 +104,12 @@ module RuboCop ...@@ -105,12 +104,12 @@ module RuboCop
end end
end end
# Returns true if the node resides in one of the whitelisted # Returns true if the node resides in one of the allowed
# directories. # directories.
def in_whitelisted_directory?(node) def in_allowed_directory?(node)
path = file_path_for_node(node) path = file_path_for_node(node)
WHITELISTED_DIRECTORIES.any? do |directory| ALLOWED_DIRECTORIES.any? do |directory|
path.start_with?( path.start_with?(
File.join(rails_root, directory), File.join(rails_root, directory),
File.join(rails_root, 'ee', directory) File.join(rails_root, 'ee', directory)
...@@ -119,12 +118,12 @@ module RuboCop ...@@ -119,12 +118,12 @@ module RuboCop
end end
# We can not auto correct code like this, as it requires manual # We can not auto correct code like this, as it requires manual
# refactoring. Instead, we'll just whitelist the surrounding scope. # refactoring. Instead, we'll just allow the surrounding scope.
# #
# Despite this method's presence, you should not use it. This method # Despite this method's presence, you should not use it. This method
# exists to make it possible to whitelist large chunks of offenses we # 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 # can't fix in the short term. If you are writing new code, follow the
# code reuse guidelines, instead of whitelisting any new offenses. # code reuse guidelines, instead of allowing any new offenses.
def autocorrect(node) def autocorrect(node)
scope = surrounding_scope_of(node) scope = surrounding_scope_of(node)
indent = indentation_of(scope) indent = indentation_of(scope)
...@@ -132,7 +131,7 @@ module RuboCop ...@@ -132,7 +131,7 @@ module RuboCop
lambda do |corrector| lambda do |corrector|
# This prevents us from inserting the same enable/disable comment # This prevents us from inserting the same enable/disable comment
# for a method or block that has multiple offenses. # for a method or block that has multiple offenses.
next if whitelisted_scopes.include?(scope) next if allowed_scopes.include?(scope)
corrector.insert_before( corrector.insert_before(
scope.source_range, scope.source_range,
...@@ -144,7 +143,7 @@ module RuboCop ...@@ -144,7 +143,7 @@ module RuboCop
"\n#{indent}# rubocop: enable #{cop_name}" "\n#{indent}# rubocop: enable #{cop_name}"
) )
whitelisted_scopes << scope allowed_scopes << scope
end end
end end
...@@ -160,8 +159,8 @@ module RuboCop ...@@ -160,8 +159,8 @@ module RuboCop
end end
end end
def whitelisted_scopes def allowed_scopes
@whitelisted_scopes ||= Set.new @allowed_scopes ||= Set.new
end end
end end
end end
......
...@@ -84,7 +84,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do ...@@ -84,7 +84,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do
SOURCE SOURCE
end end
it 'autocorrects offenses in instance methods by whitelisting them' do it 'autocorrects offenses in instance methods by allowing them' do
corrected = autocorrect_source(<<~SOURCE) corrected = autocorrect_source(<<~SOURCE)
def foo def foo
User.where User.where
...@@ -100,7 +100,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do ...@@ -100,7 +100,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do
SOURCE SOURCE
end end
it 'autocorrects offenses in class methods by whitelisting them' do it 'autocorrects offenses in class methods by allowing them' do
corrected = autocorrect_source(<<~SOURCE) corrected = autocorrect_source(<<~SOURCE)
def self.foo def self.foo
User.where User.where
...@@ -116,7 +116,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do ...@@ -116,7 +116,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do
SOURCE SOURCE
end end
it 'autocorrects offenses in blocks by whitelisting them' do it 'autocorrects offenses in blocks by allowing them' do
corrected = autocorrect_source(<<~SOURCE) corrected = autocorrect_source(<<~SOURCE)
get '/' do get '/' do
User.where User.where
......
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