Commit 277c8def authored by Sean McGivern's avatar Sean McGivern

Merge branch 'pl-rubocop-rails-logger-level' into 'master'

Let Gitlab/RailsLogger check only log methods

See merge request gitlab-org/gitlab!41987
parents 7f675ac2 72b6bc82
......@@ -543,3 +543,8 @@ Migration/CreateTableWithForeignKeys:
# Disable this cop for all the existing migrations
Exclude:
- !ruby/regexp /\Adb\/(?:post_)?migrate\/(?:201[0-9]\d+|20200[0-8][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9])_.+\.rb\z/
Gitlab/RailsLogger:
Exclude:
- 'spec/**/*.rb'
- 'ee/spec/**/*.rb'
......@@ -186,7 +186,7 @@ module Gitlab
end
def log_level
debug_logging? ? :debug : Rails.logger.level # rubocop:disable Gitlab/RailsLogger
debug_logging? ? :debug : Rails.logger.level
end
def debug_logging?
......
......@@ -14,7 +14,7 @@ module Gitlab
end
def self.build
super.tap { |logger| logger.level = Rails.logger.level } # rubocop:disable Gitlab/RailsLogger
super.tap { |logger| logger.level = Rails.logger.level }
end
end
end
......
......@@ -8,7 +8,7 @@ module RuboCop
class RailsLogger < ::RuboCop::Cop::Cop
include CodeReuseHelpers
# This cop checks for the Rails.logger in the codebase
# This cop checks for the Rails.logger log methods in the codebase
#
# @example
#
......@@ -17,34 +17,29 @@ module RuboCop
#
# # good
# Gitlab::AppLogger.error("Project %{project_path} could not be saved" % { project_path: project.full_path })
#
# # OK
# Rails.logger.level
MSG = 'Use a structured JSON logger instead of `Rails.logger`. ' \
'https://docs.gitlab.com/ee/development/logging.html'.freeze
def_node_matcher :rails_logger?, <<~PATTERN
(send (const nil? :Rails) :logger ... )
PATTERN
# See supported log methods:
# https://ruby-doc.org/stdlib-2.6.6/libdoc/logger/rdoc/Logger.html
LOG_METHODS = %i[debug error fatal info warn].freeze
LOG_METHODS_PATTERN = LOG_METHODS.map(&:inspect).join(' ').freeze
WHITELISTED_DIRECTORIES = %w[
spec
].freeze
def_node_matcher :rails_logger_log?, <<~PATTERN
(send
(send (const nil? :Rails) :logger)
{#{LOG_METHODS_PATTERN}} ...
)
PATTERN
def on_send(node)
return if in_whitelisted_directory?(node)
return unless rails_logger?(node)
return unless rails_logger_log?(node)
add_offense(node, location: :expression)
end
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
end
end
end
......
......@@ -10,22 +10,12 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do
subject(:cop) { described_class.new }
it 'flags the use of Rails.logger.error with a constant receiver' do
inspect_source("Rails.logger.error('some error')")
described_class::LOG_METHODS.each do |method|
it "flags the use of Rails.logger.#{method} with a constant receiver" do
inspect_source("Rails.logger.#{method}('some error')")
expect(cop.offenses.size).to eq(1)
end
it 'flags the use of Rails.logger.info with a constant receiver' do
inspect_source("Rails.logger.info('some info')")
expect(cop.offenses.size).to eq(1)
end
it 'flags the use of Rails.logger.warn with a constant receiver' do
inspect_source("Rails.logger.warn('some warning')")
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.size).to eq(1)
end
end
it 'does not flag the use of Rails.logger with a constant that is not Rails' do
......@@ -39,4 +29,10 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do
expect(cop.offenses.size).to eq(0)
end
it 'does not flag the use of Rails.logger.level' do
inspect_source("Rails.logger.level")
expect(cop.offenses.size).to eq(0)
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