Commit 5046b606 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch '336751_add_rubocop_rule' into 'master'

Add rubocop rule for ignore_column(s) usages

See merge request gitlab-org/gitlab!67146
parents c0eb26a2 a4409dd4
......@@ -2,18 +2,60 @@
module RuboCop
module Cop
# Cop that blacklists the usage of Group.public_or_visible_to_user
# Cop that blacklists the usage of `ActiveRecord::Base.ignored_columns=` directly
class IgnoredColumns < RuboCop::Cop::Cop
MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.'
USE_CONCERN_MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.'
WRONG_MODEL_MSG = 'If the model exists in CE and EE, the column has to be ignored ' \
'in the CE model. If the model only exists in EE, then it has to be added there.'
def_node_matcher :ignored_columns?, <<~PATTERN
(send (self) :ignored_columns)
PATTERN
def_node_matcher :ignore_columns?, <<~PATTERN
(send nil? :ignore_columns ...)
PATTERN
def_node_matcher :ignore_column?, <<~PATTERN
(send nil? :ignore_column ...)
PATTERN
def on_send(node)
return unless ignored_columns?(node)
if ignored_columns?(node)
add_offense(node, location: :expression, message: USE_CONCERN_MSG)
end
if using_ignore?(node) && used_in_wrong_model?
add_offense(node, location: :expression, message: WRONG_MODEL_MSG)
end
end
private
def using_ignore?(node)
ignore_columns?(node) || ignore_column?(node)
end
def used_in_wrong_model?
file_path = processed_source.file_path
ee_model?(file_path) && ce_model_exists?(file_path)
end
def ee_model?(path)
path.include?(ee_directory)
end
def ee_directory
File.join(rails_root, 'ee')
end
def rails_root
File.expand_path('../..', __dir__)
end
add_offense(node, location: :expression)
def ce_model_exists?(path)
File.exist?(path.gsub(%r{/ee/}, '/'))
end
end
end
......
......@@ -14,4 +14,82 @@ RSpec.describe RuboCop::Cop::IgnoredColumns do
end
RUBY
end
context 'when only CE model exist' do
let(:file_path) { full_path('app/models/bar.rb') }
it 'does not flag ignore_columns usage in CE model' do
expect_no_offenses(<<~RUBY, file_path)
class Bar < ApplicationRecord
ignore_columns :foo, remove_with: '14.3', remove_after: '2021-09-22'
end
RUBY
end
it 'flags ignore_column usage in EE model' do
expect_no_offenses(<<~RUBY, file_path)
class Baz < ApplicationRecord
ignore_column :bar, remove_with: '14.3', remove_after: '2021-09-22'
end
RUBY
end
end
context 'when only EE model exist' do
let(:file_path) { full_path('ee/app/models/ee/bar.rb') }
before do
allow(File).to receive(:exist?).with(full_path('app/models/bar.rb')).and_return(false)
end
it 'flags ignore_columns usage in EE model' do
expect_no_offenses(<<~RUBY, file_path)
class Bar < ApplicationRecord
ignore_columns :foo, remove_with: '14.3', remove_after: '2021-09-22'
end
RUBY
end
it 'flags ignore_column usage in EE model' do
expect_no_offenses(<<~RUBY, file_path)
class Bar < ApplicationRecord
ignore_column :foo, remove_with: '14.3', remove_after: '2021-09-22'
end
RUBY
end
end
context 'when CE and EE model exist' do
let(:file_path) { full_path('ee/app/models/ee/bar.rb') }
before do
allow(File).to receive(:exist?).with(full_path('app/models/bar.rb')).and_return(true)
end
it 'flags ignore_columns usage in EE model' do
expect_offense(<<~RUBY, file_path)
class Bar < ApplicationRecord
ignore_columns :foo, remove_with: '14.3', remove_after: '2021-09-22'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If the model exists in CE and EE, [...]
end
RUBY
end
it 'flags ignore_column usage in EE model' do
expect_offense(<<~RUBY, file_path)
class Bar < ApplicationRecord
ignore_column :foo, remove_with: '14.3', remove_after: '2021-09-22'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If the model exists in CE and EE, [...]
end
RUBY
end
end
private
def full_path(path)
rails_root = '../../../'
File.expand_path(File.join(rails_root, path), __dir__)
end
end
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