diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2018-12-13 15:46:01 +0100 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-12-13 16:09:21 +0100 |
commit | 7c580556319658daae5a77b252e83bfc54305e64 (patch) | |
tree | 9aa96540e79e73161fc08959c9b4dfd55295bc55 | |
parent | 81ddb69255f36bccd79c714a2c12d542cf782f8d (diff) | |
download | gitlab-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.yml | 6 | ||||
-rw-r--r-- | rubocop/cop/inject_enterprise_edition_module.rb | 47 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 1 | ||||
-rw-r--r-- | spec/rubocop/cop/inject_enterprise_edition_module_spec.rb | 133 |
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 |