Commit 57e19bd6 authored by Peter Leitzen's avatar Peter Leitzen

Let Gitlab/RailsLogger check only log methods

The use of e.g. `Rails.logger.level` is fine for historical reasons.
parent ab2eb49f
...@@ -8,7 +8,7 @@ module RuboCop ...@@ -8,7 +8,7 @@ module RuboCop
class RailsLogger < ::RuboCop::Cop::Cop class RailsLogger < ::RuboCop::Cop::Cop
include CodeReuseHelpers 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 # @example
# #
...@@ -17,15 +17,26 @@ module RuboCop ...@@ -17,15 +17,26 @@ module RuboCop
# #
# # good # # good
# Gitlab::AppLogger.error("Project %{project_path} could not be saved" % { project_path: project.full_path }) # 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`. ' \ MSG = 'Use a structured JSON logger instead of `Rails.logger`. ' \
'https://docs.gitlab.com/ee/development/logging.html'.freeze 'https://docs.gitlab.com/ee/development/logging.html'.freeze
def_node_matcher :rails_logger?, <<~PATTERN # See supported log methods:
(send (const nil? :Rails) :logger ... ) # 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
def_node_matcher :rails_logger_log?, <<~PATTERN
(send
(send (const nil? :Rails) :logger)
{#{LOG_METHODS_PATTERN}} ...
)
PATTERN PATTERN
def on_send(node) def on_send(node)
return unless rails_logger?(node) return unless rails_logger_log?(node)
add_offense(node, location: :expression) add_offense(node, location: :expression)
end end
......
...@@ -10,22 +10,12 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do ...@@ -10,22 +10,12 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it 'flags the use of Rails.logger.error with a constant receiver' do described_class::LOG_METHODS.each do |method|
inspect_source("Rails.logger.error('some error')") 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) expect(cop.offenses.size).to eq(1)
end 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)
end end
it 'does not flag the use of Rails.logger with a constant that is not Rails' do 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 ...@@ -39,4 +29,10 @@ RSpec.describe RuboCop::Cop::Gitlab::RailsLogger, type: :rubocop do
expect(cop.offenses.size).to eq(0) expect(cop.offenses.size).to eq(0)
end 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 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