summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-12-13 15:46:01 +0100
committerYorick Peterse <yorickpeterse@gmail.com>2018-12-13 16:09:21 +0100
commit7c580556319658daae5a77b252e83bfc54305e64 (patch)
tree9aa96540e79e73161fc08959c9b4dfd55295bc55
parent81ddb69255f36bccd79c714a2c12d542cf782f8d (diff)
downloadgitlab-ce-7c580556319658daae5a77b252e83bfc54305e64.tar.gz
Added Cop for injecting EE modules
This Cop enforces the rule that injecting EE modules (using prepend, include, or extend) is done by placing the injection on the last line of a file, instead of somewhere in the middle. By placing these lines at the very end, merge conflicts will not happen.
-rw-r--r--.rubocop.yml6
-rw-r--r--rubocop/cop/inject_enterprise_edition_module.rb47
-rw-r--r--rubocop/rubocop.rb1
-rw-r--r--spec/rubocop/cop/inject_enterprise_edition_module_spec.rb133
4 files changed, 187 insertions, 0 deletions
diff --git a/.rubocop.yml b/.rubocop.yml
index 741403af009..004449210e5 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -174,3 +174,9 @@ GitlabSecurity/PublicSend:
- 'ee/db/**/*'
- 'ee/lib/**/*.rake'
- 'ee/spec/**/*'
+
+Cop/InjectEnterpriseEditionModule:
+ Enabled: true
+ Exclude:
+ - 'spec/**/*'
+ - 'ee/spec/**/*'
diff --git a/rubocop/cop/inject_enterprise_edition_module.rb b/rubocop/cop/inject_enterprise_edition_module.rb
new file mode 100644
index 00000000000..c8b8aca51ab
--- /dev/null
+++ b/rubocop/cop/inject_enterprise_edition_module.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module RuboCop
+ module Cop
+ # Cop that blacklists the injecting of EE specific modules anywhere but on
+ # the last line of a file. Injecting a module in the middle of a file will
+ # cause merge conflicts, while placing it on the last line will not.
+ class InjectEnterpriseEditionModule < RuboCop::Cop::Cop
+ MSG = 'Injecting EE modules must be done on the last line of this file' \
+ ', outside of any class or module definitions'
+
+ METHODS = Set.new(%i[include extend prepend]).freeze
+
+ def_node_matcher :ee_const?, <<~PATTERN
+ (const (const _ :EE) _)
+ PATTERN
+
+ def on_send(node)
+ return unless METHODS.include?(node.children[1])
+ return unless ee_const?(node.children[2])
+
+ line = node.location.line
+ buffer = node.location.expression.source_buffer
+ last_line = buffer.last_line
+
+ # Parser treats the final newline (if present) as a separate line,
+ # meaning that a simple `line < last_line` would yield true even though
+ # the expression is the last line _of code_.
+ last_line -= 1 if buffer.source.end_with?("\n")
+
+ add_offense(node) if line < last_line
+ end
+
+ # Automatically correcting these offenses is not always possible, as
+ # sometimes code needs to be refactored to make this work. As such, we
+ # only allow developers to easily blacklist existing offenses.
+ def autocorrect(node)
+ lambda do |corrector|
+ corrector.insert_after(
+ node.source_range,
+ " # rubocop: disable #{cop_name}"
+ )
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 4489159f422..3e33419eb2e 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -41,3 +41,4 @@ require_relative 'cop/code_reuse/presenter'
require_relative 'cop/code_reuse/serializer'
require_relative 'cop/code_reuse/active_record'
require_relative 'cop/group_public_or_visible_to_user'
+require_relative 'cop/inject_enterprise_edition_module'
diff --git a/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb b/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb
new file mode 100644
index 00000000000..08ffc3c3a53
--- /dev/null
+++ b/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb
@@ -0,0 +1,133 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/inject_enterprise_edition_module'
+
+describe RuboCop::Cop::InjectEnterpriseEditionModule do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of `prepend EE` in the middle of a file' do
+ expect_offense(<<~SOURCE)
+ class Foo
+ prepend EE::Foo
+ ^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+ end
+ SOURCE
+ end
+
+ it 'flags the use of `prepend ::EE` in the middle of a file' do
+ expect_offense(<<~SOURCE)
+ class Foo
+ prepend ::EE::Foo
+ ^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+ end
+ SOURCE
+ end
+
+ it 'flags the use of `include EE` in the middle of a file' do
+ expect_offense(<<~SOURCE)
+ class Foo
+ include EE::Foo
+ ^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+ end
+ SOURCE
+ end
+
+ it 'flags the use of `include ::EE` in the middle of a file' do
+ expect_offense(<<~SOURCE)
+ class Foo
+ include ::EE::Foo
+ ^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+ end
+ SOURCE
+ end
+
+ it 'flags the use of `extend EE` in the middle of a file' do
+ expect_offense(<<~SOURCE)
+ class Foo
+ extend EE::Foo
+ ^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+ end
+ SOURCE
+ end
+
+ it 'flags the use of `extend ::EE` in the middle of a file' do
+ expect_offense(<<~SOURCE)
+ class Foo
+ extend ::EE::Foo
+ ^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+ end
+ SOURCE
+ end
+
+ it 'does not flag prepending of regular modules' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ prepend Foo
+ end
+ SOURCE
+ end
+
+ it 'does not flag including of regular modules' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ include Foo
+ end
+ SOURCE
+ end
+
+ it 'does not flag extending using regular modules' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ extend Foo
+ end
+ SOURCE
+ end
+
+ it 'does not flag the use of `prepend EE` on the last line' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ end
+
+ Foo.prepend(EE::Foo)
+ SOURCE
+ end
+
+ it 'does not flag the use of `include EE` on the last line' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ end
+
+ Foo.include(EE::Foo)
+ SOURCE
+ end
+
+ it 'does not flag the use of `extend EE` on the last line' do
+ expect_no_offenses(<<~SOURCE)
+ class Foo
+ end
+
+ Foo.extend(EE::Foo)
+ SOURCE
+ end
+
+ it 'autocorrects offenses by just disabling the Cop' do
+ source = <<~SOURCE
+ class Foo
+ prepend EE::Foo
+ include Bar
+ end
+ SOURCE
+
+ expect(autocorrect_source(source)).to eq(<<~SOURCE)
+ class Foo
+ prepend EE::Foo # rubocop: disable Cop/InjectEnterpriseEditionModule
+ include Bar
+ end
+ SOURCE
+ end
+end