Commit 91c40973 authored by Yorick Peterse's avatar Yorick Peterse

Added RuboCop cops to enforce code reuse rules

These Cops enforces the code reuse rules as defined in merge request
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21254.
parent c56f2b96
# frozen_string_literal: true
module RuboCop
module CodeReuseHelpers
# Returns true for a `(send const ...)` node.
def send_to_constant?(node)
node.type == :send && node.children&.first&.type == :const
end
# Returns `true` if the name of the receiving constant ends with a given
# `String`.
def send_receiver_name_ends_with?(node, suffix)
return false unless send_to_constant?(node)
receiver_name = name_of_receiver(node)
receiver_name != suffix &&
receiver_name.end_with?(suffix)
end
# Returns the file path (as a `String`) for an AST node.
def file_path_for_node(node)
node.location.expression.source_buffer.name
end
# Returns the name of a constant node.
#
# Given the AST node `(const nil :Foo)`, this method will return `:Foo`.
def name_of_constant(node)
node.children[1]
end
# Returns true if the given node resides in app/finders or ee/app/finders.
def in_finder?(node)
in_directory?(node, 'finders')
end
# Returns true if the given node resides in app/models or ee/app/models.
def in_model?(node)
in_directory?(node, 'models')
end
# Returns true if the given node resides in app/services or ee/app/services.
def in_service_class?(node)
in_directory?(node, 'services')
end
# Returns true if the given node resides in app/presenters or
# ee/app/presenters.
def in_presenter?(node)
in_directory?(node, 'presenters')
end
# Returns true if the given node resides in app/serializers or
# ee/app/serializers.
def in_serializer?(node)
in_directory?(node, 'serializers')
end
# Returns true if the given node resides in app/workers or ee/app/workers.
def in_worker?(node)
in_directory?(node, 'workers')
end
# Returns true if the given node resides in app/controllers or
# ee/app/controllers.
def in_controller?(node)
in_directory?(node, 'controllers')
end
# Returns true if the given node resides in lib/api or ee/lib/api.
def in_api?(node)
file_path_for_node(node).start_with?(
File.join(ce_lib_directory, 'api'),
File.join(ee_lib_directory, 'api')
)
end
# Returns `true` if the given AST node resides in the given directory,
# relative to app and/or ee/app.
def in_directory?(node, directory)
file_path_for_node(node).start_with?(
File.join(ce_app_directory, directory),
File.join(ee_app_directory, directory)
)
end
# Returns the receiver name of a send node.
#
# For the AST node `(send (const nil :Foo) ...)` this would return
# `'Foo'`.
def name_of_receiver(node)
name_of_constant(node.children.first).to_s
end
# Yields every defined class method in the given AST node.
def each_class_method(node)
return to_enum(__method__, node) unless block_given?
# class << self
# def foo
# end
# end
node.each_descendant(:sclass) do |sclass|
sclass.each_descendant(:def) do |def_node|
yield def_node
end
end
# def self.foo
# end
node.each_descendant(:defs) do |defs_node|
yield defs_node
end
end
# Yields every send node found in the given AST node.
def each_send_node(node, &block)
node.each_descendant(:send, &block)
end
# Registers a RuboCop offense for a `(send)` node with a receiver that ends
# with a given suffix.
#
# node - The AST node to check.
# suffix - The suffix of the receiver name, such as "Finder".
# message - The message to use for the offense.
def disallow_send_to(node, suffix, message)
each_send_node(node) do |send_node|
next unless send_receiver_name_ends_with?(send_node, suffix)
add_offense(send_node, location: :expression, message: message)
end
end
def ce_app_directory
File.join(rails_root, 'app')
end
def ee_app_directory
File.join(rails_root, 'ee', 'app')
end
def ce_lib_directory
File.join(rails_root, 'lib')
end
def ee_lib_directory
File.join(rails_root, 'ee', 'lib')
end
def rails_root
File.expand_path('..', __dir__)
end
end
end
# frozen_string_literal: true
require_relative '../../code_reuse_helpers'
module RuboCop
module Cop
module CodeReuse
# Cop that blacklists 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'
# Various methods from ActiveRecord::Querying that are blacklisted. 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 blacklisted method names. The values are
# booleans that indicate if the method should only be blacklisted if any
# arguments are provided.
NOT_ALLOWED = {
average: true,
calculate: true,
count_by_sql: true,
create_with: true,
distinct: false,
eager_load: true,
except: 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,
none: false,
offset: true,
order: true,
pluck: true,
preload: true,
readonly: false,
references: true,
reorder: true,
rewhere: true,
sum: false,
take: false,
take!: false,
unscope: false,
where: false,
with: true
}.freeze
# Directories that allow the use of the blacklisted methods. These
# directories are checked relative to both . and ee/
WHITELISTED_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_whitelisted_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 whitelisted
# directories.
def in_whitelisted_directory?(node)
path = file_path_for_node(node)
WHITELISTED_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 whitelist the surrounding scope.
#
# Despite this method's presence, you should not use it. This method
# exists to make it possible to whitelist 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 whitelisting 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 whitelisted_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}"
)
whitelisted_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 whitelisted_scopes
@whitelisted_scopes ||= Set.new
end
end
end
end
end
# frozen_string_literal: true
require_relative '../../code_reuse_helpers'
module RuboCop
module Cop
module CodeReuse
# Cop that enforces various code reuse rules for Finders.
class Finder < RuboCop::Cop::Cop
include CodeReuseHelpers
IN_FINDER = 'Finders can not be used inside a Finder.'
IN_MODEL_CLASS_METHOD =
'Finders can not be used inside model class methods.'
SUFFIX = 'Finder'
def on_class(node)
if in_finder?(node)
check_finder(node)
elsif in_model?(node)
check_model_class_methods(node)
end
end
def check_finder(node)
disallow_send_to(node, SUFFIX, IN_FINDER)
end
def check_model_class_methods(node)
each_class_method(node) do |def_node|
disallow_send_to(def_node, SUFFIX, IN_MODEL_CLASS_METHOD)
end
end
end
end
end
end
# frozen_string_literal: true
require_relative '../../code_reuse_helpers.rb'
module RuboCop
module Cop
module CodeReuse
# Cop that enforces various code reuse rules for Presenter classes.
class Presenter < RuboCop::Cop::Cop
include CodeReuseHelpers
IN_SERVICE = 'Presenters can not be used in a Service class.'
IN_FINDER = 'Presenters can not be used in a Finder.'
IN_PRESENTER = 'Presenters can not be used in a Presenter.'
IN_SERIALIZER = 'Presenters can not be used in a Serializer.'
IN_MODEL = 'Presenters can not be used in a model.'
IN_WORKER = 'Presenters can not be used in a worker.'
SUFFIX = 'Presenter'
def on_class(node)
message =
if in_service_class?(node)
IN_SERVICE
elsif in_finder?(node)
IN_FINDER
elsif in_presenter?(node)
IN_PRESENTER
elsif in_serializer?(node)
IN_SERIALIZER
elsif in_model?(node)
IN_MODEL
elsif in_worker?(node)
IN_WORKER
end
disallow_send_to(node, SUFFIX, message) if message
end
end
end
end
end
# frozen_string_literal: true
require_relative '../../code_reuse_helpers.rb'
module RuboCop
module Cop
module CodeReuse
# Cop that enforces various code reuse rules for Serializer classes.
class Serializer < RuboCop::Cop::Cop
include CodeReuseHelpers
IN_SERVICE = 'Serializers can not be used in a Service class.'
IN_FINDER = 'Serializers can not be used in a Finder.'
IN_PRESENTER = 'Serializers can not be used in a Presenter.'
IN_SERIALIZER = 'Serializers can not be used in a Serializer.'
IN_MODEL = 'Serializers can not be used in a model.'
IN_WORKER = 'Serializers can not be used in a worker.'
SUFFIX = 'Serializer'
def on_class(node)
message =
if in_service_class?(node)
IN_SERVICE
elsif in_finder?(node)
IN_FINDER
elsif in_presenter?(node)
IN_PRESENTER
elsif in_serializer?(node)
IN_SERIALIZER
elsif in_model?(node)
IN_MODEL
elsif in_worker?(node)
IN_WORKER
end
disallow_send_to(node, SUFFIX, message) if message
end
end
end
end
end
# frozen_string_literal: true
require_relative '../../code_reuse_helpers.rb'
module RuboCop
module Cop
module CodeReuse
# Cop that enforces various code reuse rules for Service classes.
class ServiceClass < RuboCop::Cop::Cop
include CodeReuseHelpers
IN_FINDER = 'Service classes can not be used in a Finder.'
IN_PRESENTER = 'Service classes can not be used in a Presenter.'
IN_SERIALIZER = 'Service classes can not be used in a Serializer.'
IN_MODEL = 'Service classes can not be used in a model.'
SUFFIX = 'Service'
def on_class(node)
check_all_send_nodes(node)
end
def check_all_send_nodes(node)
message =
if in_finder?(node)
IN_FINDER
elsif in_presenter?(node)
IN_PRESENTER
elsif in_serializer?(node)
IN_SERIALIZER
elsif in_model?(node)
IN_MODEL
end
disallow_send_to(node, SUFFIX, message) if message
end
end
end
end
end
# frozen_string_literal: true
require_relative '../../code_reuse_helpers.rb'
module RuboCop
module Cop
module CodeReuse
# Cop that enforces various code reuse rules for workers.
class Worker < RuboCop::Cop::Cop
include CodeReuseHelpers
IN_CONTROLLER = 'Workers can not be used in a controller.'
IN_API = 'Workers can not be used in a Grape API.'
IN_FINDER = 'Workers can not be used in a Finder.'
IN_PRESENTER = 'Workers can not be used in a Presenter.'
IN_SERIALIZER = 'Workers can not be used in a Serializer.'
IN_MODEL_CLASS_METHOD =
'Workers can not be used in model class methods.'
SUFFIX = 'Worker'
def on_class(node)
if in_model?(node)
check_model_class_methods(node)
else
check_all_send_nodes(node)
end
end
def check_all_send_nodes(node)
message =
if in_controller?(node)
IN_CONTROLLER
elsif in_api?(node)
IN_API
elsif in_finder?(node)
IN_FINDER
elsif in_presenter?(node)
IN_PRESENTER
elsif in_serializer?(node)
IN_SERIALIZER
end
disallow_send_to(node, SUFFIX, message) if message
end
def check_model_class_methods(node)
each_class_method(node) do |def_node|
disallow_send_to(def_node, SUFFIX, IN_MODEL_CLASS_METHOD)
end
end
end
end
end
end
......@@ -30,3 +30,9 @@ require_relative 'cop/rspec/factories_in_migration_specs'
require_relative 'cop/sidekiq_options_queue'
require_relative 'cop/destroy_all'
require_relative 'cop/ruby_interpolation_in_translation'
require_relative 'code_reuse_helpers'
require_relative 'cop/code_reuse/finder'
require_relative 'cop/code_reuse/service_class'
require_relative 'cop/code_reuse/presenter'
require_relative 'cop/code_reuse/serializer'
require_relative 'cop/code_reuse/active_record'
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require 'parser/current'
require_relative '../../rubocop/code_reuse_helpers'
describe RuboCop::CodeReuseHelpers do
def parse_source(source, path = 'foo.rb')
buffer = Parser::Source::Buffer.new(path)
buffer.source = source
builder = RuboCop::AST::Builder.new
parser = Parser::CurrentRuby.new(builder)
parser.parse(buffer)
end
let(:cop) do
Class.new do
include RuboCop::CodeReuseHelpers
end.new
end
describe '#send_to_constant?' do
it 'returns true when sending to a constant' do
node = parse_source('Foo.bar')
expect(cop.send_to_constant?(node)).to eq(true)
end
it 'returns false when sending to something other than a constant' do
node = parse_source('10')
expect(cop.send_to_constant?(node)).to eq(false)
end
end
describe '#send_receiver_name_ends_with?' do
it 'returns true when the receiver ends with a suffix' do
node = parse_source('FooFinder.new')
expect(cop.send_receiver_name_ends_with?(node, 'Finder')).to eq(true)
end
it 'returns false when the receiver is the same as a suffix' do
node = parse_source('Finder.new')
expect(cop.send_receiver_name_ends_with?(node, 'Finder')).to eq(false)
end
end
describe '#file_path_for_node' do
it 'returns the file path of a node' do
node = parse_source('10')
path = cop.file_path_for_node(node)
expect(path).to eq('foo.rb')
end
end
describe '#name_of_constant' do
it 'returns the name of a constant' do
node = parse_source('Foo')
expect(cop.name_of_constant(node)).to eq(:Foo)
end
end
describe '#in_finder?' do
it 'returns true for a node in the finders directory' do
node = parse_source('10', Rails.root.join('app', 'finders', 'foo.rb'))
expect(cop.in_finder?(node)).to eq(true)
end
it 'returns false for a node outside the finders directory' do
node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb'))
expect(cop.in_finder?(node)).to eq(false)
end
end
describe '#in_model?' do
it 'returns true for a node in the models directory' do
node = parse_source('10', Rails.root.join('app', 'models', 'foo.rb'))
expect(cop.in_model?(node)).to eq(true)
end
it 'returns false for a node outside the models directory' do
node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb'))
expect(cop.in_model?(node)).to eq(false)
end
end
describe '#in_service_class?' do
it 'returns true for a node in the services directory' do
node = parse_source('10', Rails.root.join('app', 'services', 'foo.rb'))
expect(cop.in_service_class?(node)).to eq(true)
end
it 'returns false for a node outside the services directory' do
node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb'))
expect(cop.in_service_class?(node)).to eq(false)
end
end
describe '#in_presenter?' do
it 'returns true for a node in the presenters directory' do
node = parse_source('10', Rails.root.join('app', 'presenters', 'foo.rb'))
expect(cop.in_presenter?(node)).to eq(true)
end
it 'returns false for a node outside the presenters directory' do
node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb'))
expect(cop.in_presenter?(node)).to eq(false)
end
end
describe '#in_serializer?' do
it 'returns true for a node in the serializers directory' do
node = parse_source('10', Rails.root.join('app', 'serializers', 'foo.rb'))
expect(cop.in_serializer?(node)).to eq(true)
end
it 'returns false for a node outside the serializers directory' do
node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb'))
expect(cop.in_serializer?(node)).to eq(false)
end
end
describe '#in_worker?' do
it 'returns true for a node in the workers directory' do
node = parse_source('10', Rails.root.join('app', 'workers', 'foo.rb'))
expect(cop.in_worker?(node)).to eq(true)
end
it 'returns false for a node outside the workers directory' do
node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb'))
expect(cop.in_worker?(node)).to eq(false)
end
end
describe '#in_api?' do
it 'returns true for a node in the API directory' do
node = parse_source('10', Rails.root.join('lib', 'api', 'foo.rb'))
expect(cop.in_api?(node)).to eq(true)
end
it 'returns false for a node outside the API directory' do
node = parse_source('10', Rails.root.join('lib', 'foo', 'foo.rb'))
expect(cop.in_api?(node)).to eq(false)
end
end