summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-08-27 17:30:20 +0200
committerYorick Peterse <yorickpeterse@gmail.com>2018-09-11 15:00:18 +0200
commit91c40973dc620430b173ea2df968587d2a708dff (patch)
treeeb871f8b742d8e165138d699be5e884a2c612b24
parentc56f2b96159afaf6f1e0831d0e7a756a40568cab (diff)
downloadgitlab-ce-91c40973dc620430b173ea2df968587d2a708dff.tar.gz
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.
-rw-r--r--rubocop/code_reuse_helpers.rb156
-rw-r--r--rubocop/cop/code_reuse/active_record.rb170
-rw-r--r--rubocop/cop/code_reuse/finder.rb39
-rw-r--r--rubocop/cop/code_reuse/presenter.rb41
-rw-r--r--rubocop/cop/code_reuse/serializer.rb41
-rw-r--r--rubocop/cop/code_reuse/service_class.rb39
-rw-r--r--rubocop/cop/code_reuse/worker.rb56
-rw-r--r--rubocop/rubocop.rb6
-rw-r--r--spec/rubocop/code_reuse_helpers_spec.rb249
-rw-r--r--spec/rubocop/cop/code_reuse/active_record_spec.rb138
-rw-r--r--spec/rubocop/cop/code_reuse/finder_spec.rb77
-rw-r--r--spec/rubocop/cop/code_reuse/presenter_spec.rb117
-rw-r--r--spec/rubocop/cop/code_reuse/serializer_spec.rb117
-rw-r--r--spec/rubocop/cop/code_reuse/service_class_spec.rb89
-rw-r--r--spec/rubocop/cop/code_reuse/worker_spec.rb104
15 files changed, 1439 insertions, 0 deletions
diff --git a/rubocop/code_reuse_helpers.rb b/rubocop/code_reuse_helpers.rb
new file mode 100644
index 00000000000..0929a55d901
--- /dev/null
+++ b/rubocop/code_reuse_helpers.rb
@@ -0,0 +1,156 @@
+# 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
diff --git a/rubocop/cop/code_reuse/active_record.rb b/rubocop/cop/code_reuse/active_record.rb
new file mode 100644
index 00000000000..d25e8548fd0
--- /dev/null
+++ b/rubocop/cop/code_reuse/active_record.rb
@@ -0,0 +1,170 @@
+# 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
diff --git a/rubocop/cop/code_reuse/finder.rb b/rubocop/cop/code_reuse/finder.rb
new file mode 100644
index 00000000000..1d70befe79b
--- /dev/null
+++ b/rubocop/cop/code_reuse/finder.rb
@@ -0,0 +1,39 @@
+# 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
diff --git a/rubocop/cop/code_reuse/presenter.rb b/rubocop/cop/code_reuse/presenter.rb
new file mode 100644
index 00000000000..5f8f2839ca6
--- /dev/null
+++ b/rubocop/cop/code_reuse/presenter.rb
@@ -0,0 +1,41 @@
+# 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
diff --git a/rubocop/cop/code_reuse/serializer.rb b/rubocop/cop/code_reuse/serializer.rb
new file mode 100644
index 00000000000..2212c50514e
--- /dev/null
+++ b/rubocop/cop/code_reuse/serializer.rb
@@ -0,0 +1,41 @@
+# 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
diff --git a/rubocop/cop/code_reuse/service_class.rb b/rubocop/cop/code_reuse/service_class.rb
new file mode 100644
index 00000000000..768b43fb684
--- /dev/null
+++ b/rubocop/cop/code_reuse/service_class.rb
@@ -0,0 +1,39 @@
+# 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
diff --git a/rubocop/cop/code_reuse/worker.rb b/rubocop/cop/code_reuse/worker.rb
new file mode 100644
index 00000000000..e38d2783d0f
--- /dev/null
+++ b/rubocop/cop/code_reuse/worker.rb
@@ -0,0 +1,56 @@
+# 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
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index d823fa4edb1..46bd7d3bec6 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -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'
diff --git a/spec/rubocop/code_reuse_helpers_spec.rb b/spec/rubocop/code_reuse_helpers_spec.rb
new file mode 100644
index 00000000000..2720141aad2
--- /dev/null
+++ b/spec/rubocop/code_reuse_helpers_spec.rb
@@ -0,0 +1,249 @@
+# 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
+
+ describe '#in_directory?' do
+ it 'returns true for a directory in the CE app/ directory' do
+ node = parse_source('10', Rails.root.join('app', 'models', 'foo.rb'))
+
+ expect(cop.in_directory?(node, 'models')).to eq(true)
+ end
+
+ it 'returns true for a directory in the EE app/ directory' do
+ node =
+ parse_source('10', Rails.root.join('ee', 'app', 'models', 'foo.rb'))
+
+ expect(cop.in_directory?(node, 'models')).to eq(true)
+ end
+
+ it 'returns false for a directory in the lib/ directory' do
+ node =
+ parse_source('10', Rails.root.join('lib', 'models', 'foo.rb'))
+
+ expect(cop.in_directory?(node, 'models')).to eq(false)
+ end
+ end
+
+ describe '#name_of_receiver' do
+ it 'returns the name of a send receiver' do
+ node = parse_source('Foo.bar')
+
+ expect(cop.name_of_receiver(node)).to eq('Foo')
+ end
+ end
+
+ describe '#each_class_method' do
+ it 'yields every class method to the supplied block' do
+ node = parse_source(<<~RUBY)
+ class Foo
+ class << self
+ def first
+ end
+ end
+
+ def self.second
+ end
+ end
+ RUBY
+
+ nodes = cop.each_class_method(node).to_a
+
+ expect(nodes.length).to eq(2)
+
+ expect(nodes[0].children[0]).to eq(:first)
+ expect(nodes[1].children[1]).to eq(:second)
+ end
+ end
+
+ describe '#each_send_node' do
+ it 'yields every send node to the supplied block' do
+ node = parse_source("foo\nbar")
+ nodes = cop.each_send_node(node).to_a
+
+ expect(nodes.length).to eq(2)
+ expect(nodes[0].children[1]).to eq(:foo)
+ expect(nodes[1].children[1]).to eq(:bar)
+ end
+ end
+
+ describe '#disallow_send_to' do
+ it 'disallows sending a message to a constant' do
+ def_node = parse_source(<<~RUBY)
+ def foo
+ FooFinder.new
+ end
+ RUBY
+
+ send_node = def_node.each_child_node(:send).first
+
+ expect(cop)
+ .to receive(:add_offense)
+ .with(send_node, location: :expression, message: 'oops')
+
+ cop.disallow_send_to(def_node, 'Finder', 'oops')
+ end
+ end
+end
diff --git a/spec/rubocop/cop/code_reuse/active_record_spec.rb b/spec/rubocop/cop/code_reuse/active_record_spec.rb
new file mode 100644
index 00000000000..a30fc52d26f
--- /dev/null
+++ b/spec/rubocop/cop/code_reuse/active_record_spec.rb
@@ -0,0 +1,138 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/code_reuse/active_record'
+
+describe RuboCop::Cop::CodeReuse::ActiveRecord 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
+ 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
+ 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
+ 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 whitelisting 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 whitelisting 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 whitelisting 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
diff --git a/spec/rubocop/cop/code_reuse/finder_spec.rb b/spec/rubocop/cop/code_reuse/finder_spec.rb
new file mode 100644
index 00000000000..b04e053a4c3
--- /dev/null
+++ b/spec/rubocop/cop/code_reuse/finder_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/code_reuse/finder'
+
+describe RuboCop::Cop::CodeReuse::Finder do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of a Finder inside another Finder' do
+ allow(cop)
+ .to receive(:in_finder?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooFinder
+ def execute
+ BarFinder.new.execute
+ ^^^^^^^^^^^^^ Finders can not be used inside a Finder.
+ end
+ end
+ SOURCE
+
+ expect(cop.offenses.size).to eq(1)
+ end
+
+ it 'flags the use of a Finder inside a model class method' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User
+ class << self
+ def second_method
+ BarFinder.new
+ ^^^^^^^^^^^^^ Finders can not be used inside model class methods.
+ end
+ end
+
+ def self.second_method
+ FooFinder.new
+ ^^^^^^^^^^^^^ Finders can not be used inside model class methods.
+ end
+ end
+ SOURCE
+ end
+
+ it 'does not flag the use of a Finder in a non Finder file' do
+ expect_no_offenses(<<~SOURCE)
+ class FooFinder
+ def execute
+ BarFinder.new.execute
+ end
+ end
+ SOURCE
+ end
+
+ it 'does not flag the use of a Finder in a regular class method' do
+ expect_no_offenses(<<~SOURCE)
+ class User
+ class << self
+ def second_method
+ BarFinder.new
+ end
+ end
+
+ def self.second_method
+ FooFinder.new
+ end
+ end
+ SOURCE
+ end
+end
diff --git a/spec/rubocop/cop/code_reuse/presenter_spec.rb b/spec/rubocop/cop/code_reuse/presenter_spec.rb
new file mode 100644
index 00000000000..4fe72619273
--- /dev/null
+++ b/spec/rubocop/cop/code_reuse/presenter_spec.rb
@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/code_reuse/presenter'
+
+describe RuboCop::Cop::CodeReuse::Presenter do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of a Presenter in a Service class' do
+ allow(cop)
+ .to receive(:in_service_class?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooService
+ def execute
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a Service class.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Presenter in a Finder' do
+ allow(cop)
+ .to receive(:in_finder?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooFinder
+ def execute
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a Finder.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Service class in a Presenter' do
+ allow(cop)
+ .to receive(:in_presenter?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooPresenter
+ def execute
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a Presenter.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Presenter in a Serializer' do
+ allow(cop)
+ .to receive(:in_serializer?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooSerializer
+ def execute
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a Serializer.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Presenter in a model instance method' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User < ActiveRecord::Base
+ def execute
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a model.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Presenter in a model class method' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User < ActiveRecord::Base
+ def self.execute
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a model.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Presenter in a worker' do
+ allow(cop)
+ .to receive(:in_worker?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooWorker
+ def perform
+ FooPresenter.new.execute
+ ^^^^^^^^^^^^^^^^ Presenters can not be used in a worker.
+ end
+ end
+ SOURCE
+ end
+end
diff --git a/spec/rubocop/cop/code_reuse/serializer_spec.rb b/spec/rubocop/cop/code_reuse/serializer_spec.rb
new file mode 100644
index 00000000000..4530b15eed7
--- /dev/null
+++ b/spec/rubocop/cop/code_reuse/serializer_spec.rb
@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/code_reuse/serializer'
+
+describe RuboCop::Cop::CodeReuse::Serializer do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of a Serializer in a Service class' do
+ allow(cop)
+ .to receive(:in_service_class?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooService
+ def execute
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a Service class.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Serializer in a Finder' do
+ allow(cop)
+ .to receive(:in_finder?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooFinder
+ def execute
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a Finder.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Serializer in a Presenter' do
+ allow(cop)
+ .to receive(:in_presenter?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooPresenter
+ def execute
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a Presenter.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Serializer in a Serializer' do
+ allow(cop)
+ .to receive(:in_serializer?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooSerializer
+ def execute
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a Serializer.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Serializer in a model instance method' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User < ActiveRecord::Base
+ def execute
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a model.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Serializer in a model class method' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User < ActiveRecord::Base
+ def self.execute
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a model.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Serializer in a worker' do
+ allow(cop)
+ .to receive(:in_worker?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooWorker
+ def perform
+ FooSerializer.new.execute
+ ^^^^^^^^^^^^^^^^^ Serializers can not be used in a worker.
+ end
+ end
+ SOURCE
+ end
+end
diff --git a/spec/rubocop/cop/code_reuse/service_class_spec.rb b/spec/rubocop/cop/code_reuse/service_class_spec.rb
new file mode 100644
index 00000000000..7b8d82f332e
--- /dev/null
+++ b/spec/rubocop/cop/code_reuse/service_class_spec.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/code_reuse/service_class'
+
+describe RuboCop::Cop::CodeReuse::ServiceClass do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of a Service class in a Finder' do
+ allow(cop)
+ .to receive(:in_finder?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooFinder
+ def execute
+ FooService.new.execute
+ ^^^^^^^^^^^^^^ Service classes can not be used in a Finder.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Service class in a Presenter' do
+ allow(cop)
+ .to receive(:in_presenter?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooPresenter
+ def execute
+ FooService.new.execute
+ ^^^^^^^^^^^^^^ Service classes can not be used in a Presenter.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Service class in a Serializer' do
+ allow(cop)
+ .to receive(:in_serializer?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooSerializer
+ def execute
+ FooService.new.execute
+ ^^^^^^^^^^^^^^ Service classes can not be used in a Serializer.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a Service class in a model' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User < ActiveRecord::Model
+ class << self
+ def first
+ FooService.new.execute
+ ^^^^^^^^^^^^^^ Service classes can not be used in a model.
+ end
+ end
+
+ def second
+ FooService.new.execute
+ ^^^^^^^^^^^^^^ Service classes can not be used in a model.
+ end
+ end
+ SOURCE
+ end
+
+ it 'does not flag the use of a Service class in a regular class' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ def execute
+ FooService.new.execute
+ end
+ end
+ SOURCE
+ end
+end
diff --git a/spec/rubocop/cop/code_reuse/worker_spec.rb b/spec/rubocop/cop/code_reuse/worker_spec.rb
new file mode 100644
index 00000000000..97acaeb7643
--- /dev/null
+++ b/spec/rubocop/cop/code_reuse/worker_spec.rb
@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/code_reuse/worker'
+
+describe RuboCop::Cop::CodeReuse::Worker do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of a worker in a controller' do
+ allow(cop)
+ .to receive(:in_controller?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooController
+ def index
+ FooWorker.perform_async
+ ^^^^^^^^^^^^^^^^^^^^^^^ Workers can not be used in a controller.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a worker in an API' do
+ allow(cop)
+ .to receive(:in_api?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class Foo < Grape::API
+ resource :projects do
+ get '/' do
+ FooWorker.perform_async
+ ^^^^^^^^^^^^^^^^^^^^^^^ Workers can not be used in a Grape API.
+ end
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a worker in a Finder' do
+ allow(cop)
+ .to receive(:in_finder?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooFinder
+ def execute
+ FooWorker.perform_async
+ ^^^^^^^^^^^^^^^^^^^^^^^ Workers can not be used in a Finder.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a worker in a Presenter' do
+ allow(cop)
+ .to receive(:in_presenter?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooPresenter
+ def execute
+ FooWorker.perform_async
+ ^^^^^^^^^^^^^^^^^^^^^^^ Workers can not be used in a Presenter.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a worker in a Serializer' do
+ allow(cop)
+ .to receive(:in_serializer?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class FooSerializer
+ def execute
+ FooWorker.perform_async
+ ^^^^^^^^^^^^^^^^^^^^^^^ Workers can not be used in a Serializer.
+ end
+ end
+ SOURCE
+ end
+
+ it 'flags the use of a worker in a model class method' do
+ allow(cop)
+ .to receive(:in_model?)
+ .and_return(true)
+
+ expect_offense(<<~SOURCE)
+ class User < ActiveRecord::Base
+ def self.execute
+ FooWorker.perform_async
+ ^^^^^^^^^^^^^^^^^^^^^^^ Workers can not be used in model class methods.
+ end
+ end
+ SOURCE
+ end
+end