8000 Faster runs via RuboCop::Runner by jdelStrother · Pull Request #573 · sds/haml-lint · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Faster runs via RuboCop::Runner #573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 24, 2025
Merged

Faster runs via RuboCop::Runner #573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
643d477
Move rubocop_cli back to per-instance
jdelStrother Jun 21, 2025
03f7892
Switch to using RuboCop::Runner rather than RuboCop::CLI
jdelStrother Jun 7, 2025
0986bde
Remove OffenseCollector
jdelStrother Jun 7, 2025
6f9c84e
Fix rubocop options for rubocop < v1.38
jdelStrother Jun 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 72 additions & 66 deletions lib/haml_lint/linter/rubocop.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,31 @@ module HamlLint
#
# The work is spread across the classes in the HamlLint::RubyExtraction module.
class Linter::RuboCop < Linter
# Processes a ruby file and reports RuboCop offenses
class Runner < ::RuboCop::Runner
attr_reader :offenses

def run(path, code, config:)
@offenses = []
@options[:stdin] = code
@config_store.instance_variable_set(:@options_config, config)
super([path])
end

def corrected_code
@options[:stdin]
end

# Executed when a file has been scanned by RuboCop, adding the reported
# offenses to our collection.
#
# @param _file [String]
# @param offenses [Array<RuboCop::Cop::Offense>]
def file_finished(_file, offenses)
@offenses = offenses
end
end

include LinterRegistry

supports_autocorrect(true)
Expand Down Expand Up @@ -98,9 +123,9 @@ def self.cops_names_not_supporting_autocorrect

def rubocop_config_for(path)
user_config_path = ENV['HAML_LINT_RUBOCOP_CONF'] || config['config_file']
user_config_path ||= self.class.rubocop_config_store.user_rubocop_config_path_for(path)
user_config_path ||= rubocop_config_store.user_rubocop_config_path_for(path)
user_config_path = File.absolute_path(user_config_path)
self.class.rubocop_config_store.config_object_pointing_to(user_config_path)
rubocop_config_store.config_object_pointing_to(user_config_path)
end

# Extracted here so that tests can stub this to always return true
Expand Down Expand Up @@ -168,15 +193,13 @@ def new_haml_validity_checks(new_haml_string)
false
end

# A single CLI instance is shared between files to avoid RuboCop
# A single RuboCop runner is shared between files to avoid RuboCop
# having to repeatedly reload .rubocop.yml.
def self.rubocop_cli # rubocop:disable Lint/IneffectiveAccessModifier
# The ivar is stored on the class singleton rather than the Linter instance
# because it can't be Marshal.dump'd (as used by Parallel.map)
@rubocop_cli ||= ::RuboCop::CLI.new
def rubocop_runner
@rubocop_runner ||= Runner.new(rubocop_options, ::RuboCop::ConfigStore.new)
end

def self.rubocop_config_store # rubocop:disable Lint/IneffectiveAccessModifier
def rubocop_config_store
@rubocop_config_store ||= RubocopConfigStore.new
end

Expand All @@ -190,7 +213,7 @@ def self.rubocop_config_store # rubocop:disable Lint/IneffectiveAccessModifier
def process_ruby_source(ruby_code, source_map)
filename = document.file || 'ruby_script.rb'

offenses, corrected_ruby = run_rubocop(self.class.rubocop_cli, ruby_code, filename)
offenses, corrected_ruby = run_rubocop(rubocop_runner, ruby_code, filename)

extract_lints_from_offenses(offenses, source_map)
corrected_ruby
Expand All @@ -199,55 +222,35 @@ def process_ruby_source(ruby_code, source_map)
# Runs RuboCop, returning the offenses and corrected code. Raises when RuboCop
# fails to run correctly.
#
# @param rubocop_cli [RuboCop::CLI] There to simplify tests by using a stub
# @param rubocop_runner [HamlLint::Linter::RuboCop::Runner] There to simplify tests by using a stub
# @param ruby_code [String] The ruby code to run through RuboCop
# @param path [String] the path to tell RuboCop we are running
# @return [Array<RuboCop::Cop::Offense>, String]
def run_rubocop(rubocop_cli, ruby_code, path) # rubocop:disable Metrics
rubocop_status = nil
stdout_str, stderr_str = HamlLint::Utils.with_captured_streams(ruby_code) do
rubocop_cli.config_store.instance_variable_set(:@options_config, rubocop_config_for(path))
rubocop_status = rubocop_cli.run(rubocop_flags + ['--stdin', path])
end
def run_rubocop(rubocop_runner, ruby_code, path) # rubocop:disable Metrics
rubocop_runner.run(path, ruby_code, config: rubocop_config_for(path))

if ENV['HAML_LINT_INTERNAL_DEBUG'] == 'true'
if OffenseCollector.offenses.empty?
if rubocop_runner.offenses.empty?
puts "------ No lints found by RuboCop in #{@document.file}"
else
puts "------ Raw lints found by RuboCop in #{@document.file}"
OffenseCollector.offenses.each do |offense|
rubocop_runner.offenses.each do |offense|
puts offense
end
puts '------'
end
end

unless [::RuboCop::CLI::STATUS_SUCCESS, ::RuboCop::CLI::STATUS_OFFENSES].include?(rubocop_status)
if stderr_str.start_with?('Infinite loop')
msg = "RuboCop exited unsuccessfully with status #{rubocop_status}." \
' This appears to be due to an autocorrection infinite loop.'
if ENV['HAML_LINT_DEBUG'] == 'true'
msg += " DEBUG: RuboCop's output:\n"
msg += stderr_str.strip
else
msg += " First line of RuboCop's output (Use --debug mode to see more):\n"
msg += stderr_str.each_line.first.strip
end

raise HamlLint::Exceptions::InfiniteLoopError, msg
end

raise HamlLint::Exceptions::ConfigurationError,
"RuboCop exited unsuccessfully with status #{rubocop_status}." \
' Here is its output to check the stack trace or see if there was' \
" a misconfiguration:\n#{stderr_str}"
end

if @autocorrect
corrected_ruby = stdout_str.partition("#{'=' * 20}\n").last
corrected_ruby = rubocop_runner.corrected_code
end

[OffenseCollector.offenses, corrected_ruby]
[rubocop_runner.offenses, corrected_ruby]
rescue ::RuboCop::Error => e
raise HamlLint::Exceptions::ConfigurationError,
"RuboCop raised #{e}." \
' Here is its output to check the stack trace or see if there was' \
" a misconfiguration:\n#{e.message}\n#{e.backtrace}"
end

# Aggregates RuboCop offenses and converts them to {HamlLint::Lint}s
Expand Down Expand Up @@ -294,14 +297,27 @@ def record_lint(line, message, severity, corrected:)
corrected: corrected)
end

# Returns flags that will be passed to RuboCop CLI.
# rubocop:disable Style/MutableConstant
DEFAULT_FLAGS = %w[--format RuboCop::Formatter::BaseFormatter]
begin
::RuboCop::Options.new.parse(['--raise-cop-error'])
DEFAULT_FLAGS << '--raise-cop-error'
rescue OptionParser::InvalidOption
# older versions of RuboCop don't support this flag
end
DEFAULT_FLAGS.freeze
# rubocop:enable Style/MutableConstant

# Returns options that will be passed to the RuboCop runner.
#
# @return [Array<String>]
def rubocop_flags
flags = %w[--format HamlLint::OffenseCollector]
# @return [Hash]
def rubocop_options
# using BaseFormatter suppresses any default output
flags = DEFAULT_FLAGS
flags += ignored_cops_flags
flags += rubocop_autocorrect_flags
flags
options, _args = ::RuboCop::Options.new.parse(flags)
options
end

def rubocop_autocorrect_flags
Expand Down Expand Up @@ -342,29 +358,19 @@ def ignored_cops_flags
return [] if ignored_cops.empty?
['--except', ignored_cops.uniq.join(',')]
end
end

# Collects offenses detected by RuboCop.
class OffenseCollector < ::RuboCop::Formatter::BaseFormatter
class << self
# List of offenses reported by RuboCop.
attr_accessor :offenses
end

# Executed when RuboCop begins linting.
#
# @param _target_files [Array<String>]
def started(_target_files)
self.class.offenses = []
# Exclude ivars that don't marshal properly
def marshal_dump
excluded_ivars = %i[@rubocop_runner @rubocop_config_store @user_config_path_to_config_object]
(instance_variables - excluded_ivars).to_h do |ivar|
[ivar, instance_variable_get(ivar)]
end
end

# Executed when a file has been scanned by RuboCop, adding the reported
# offenses to our collection.
#
# @param _file [String]
# @param offenses [Array<RuboCop::Cop::Offense>]
def file_finished(_file, offenses)
self.class.offenses += offenses
def marshal_load(ivars)
ivars.each do |k, v|
instance_variable_set(k, v)
end
end
end

Expand Down
73 changes: 27 additions & 46 deletions spec/haml_lint/linter/rubocop_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
end

context 'general testing' do
let!(:rubocop_cli) { spy('rubocop_cli') }
let!(:rubocop_runner) { spy('rubocop_runner') }

# Need this block before including linter context so that stubbing occurs
# before linter is run
before do
rubocop_cli.stub(:run).and_return(::RuboCop::CLI::STATUS_SUCCESS)
HamlLint::Linter::RuboCop.stub(:rubocop_cli).and_return(rubocop_cli)
HamlLint::OffenseCollector.stub(:offenses)
.and_return([offence].compact)
subject.stub(:rubocop_runner).and_return(rubocop_runner)
rubocop_runner.stub(:run)
rubocop_runner.stub(:offenses)
.and_return([offence].compact)
end

include_context 'linter'
Expand All @@ -29,18 +29,14 @@
%span to be
HAML

it 'does not specify the --config flag by default' do
expect(rubocop_cli).to have_received(:run).with(array_excluding('--config'))
end

context 'when RuboCop does not report offences' do
it { should_not report_lint }
end

context 'when RuboCop reports offences' do
let(:line) { 4 }
let(:message) { 'Lint message' }
let(:cop_name) { 'Lint/SomeCopName' }
let(:cop_name) { 'Style/StringLiterals' }
let(:severity) { double('Severity', name: :warning) }

let(:offence) do
Expand All @@ -53,7 +49,7 @@
end

context 'and the offence is from an ignored cop' do
let(:config) { super().merge('ignored_cops' => ['Lint/SomeCopName']) }
let(:config) { super().merge('ignored_cops' => ['Style/StringLiterals']) }
it { should_not report_lint }
end
end
Expand Down Expand Up @@ -336,58 +332,43 @@ def run_with_config(config)
end

describe '#run_rubocop' do
subject { described_class.new(config).send(:run_rubocop, rubocop_cli, 'foo', 'some_file.rb') }
subject { described_class.new(config).send(:run_rubocop, rubocop_runner, 'foo', 'some_file.rb') }

let(:config) { {} }
let(:rubocop_cli) { spy('rubocop_cli') }
let(:rubocop_runner) { spy('rubocop_runner') }

before do
::RuboCop::CLI.stub(:new).and_return(rubocop_cli)
rubocop_cli.stub(:run).and_return(rubocop_cli_status)
HamlLint::OffenseCollector.stub(:offenses).and_return([])
end

context 'when RuboCop exits with a success status' do
let(:rubocop_cli_status) { ::RuboCop::CLI::STATUS_SUCCESS }

it { should eq [[], nil] }
HamlLint::Linter::RuboCop::Runner.stub(:new).and_return(rubocop_runner)
rubocop_runner.stub(:offenses).and_return([])
end

context 'when RuboCop exits with an offense status' do
let(:rubocop_cli_status) { ::RuboCop::CLI::STATUS_OFFENSES }
context 'when RuboCop returns true' do
let(:rubocop_runner_status) { true }

it { should eq [[], nil] }
end

context 'when RuboCop exits with an error status' do
let(:rubocop_cli_status) { ::RuboCop::CLI::STATUS_ERROR }

it {
expect { subject }.to raise_error(HamlLint::Exceptions::ConfigurationError,
/RuboCop exited unsuccessfully with status 2/)
}
end

context 'when RuboCop exits with an unexpected status' do
let(:rubocop_cli_status) { 123 }
context 'when RuboCop reports offenses' do
let(:offense) { spy('offense') }
before do
rubocop_runner.stub(:run) do
rubocop_runner.offenses << offense
end
end

it {
expect { subject }.to raise_error(HamlLint::Exceptions::ConfigurationError,
/RuboCop exited unsuccessfully with status 123/)
}
it { should eq [[offense], nil] }
end

context 'when RuboCop has an infinite loop' do
context 'when RuboCop throws an error' do
before do
HamlLint::Utils.stub(:with_captured_streams).and_return(['',
'Infinite loop detected in foo.rb and caused by ...'])
rubocop_runner.stub(:run).and_raise(
RuboCop::ErrorWithAnalyzedFileLocation.new(cause: RuntimeError.new, node: spy('node'), cop: spy('cop'))
)
end

let(:rubocop_cli_status) { ::RuboCop::CLI::STATUS_ERROR }

it {
expect { subject }.to raise_error(HamlLint::Exceptions::InfiniteLoopError,
/Infinite loop/)
expect { subject }.to raise_error(HamlLint::Exceptions::ConfigurationError,
/RuboCop raised RuboCop::ErrorWithAnalyzedFileLocation/)
}
end
end
Expand Down
0