diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index da9260054664bb33d54a84e5e091439c1bcd51af..7a4ebbdbadf0bf14d1b502fd0d9209a6ad89b9e4 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -159,25 +159,22 @@ When the number of RuboCop exceptions exceed the default [`exclude-limit` of 15] we may want to resolve exceptions over multiple commits. To minimize confusion, we should track our progress through the exception list. -When auto-generating the `.rubocop_todo.yml` exception list for a particular Cop, -and more than 15 files are affected, we should add the exception list to -a different file in the directory `.rubocop_todo/`. For example, the configuration for the cop -`Gitlab/NamespacedClass` is in `.rubocop_todo/gitlab/namespaced_class.yml`. - -This ensures that our list isn't mistakenly removed by another auto generation of -the `.rubocop_todo.yml`. This also allows us greater visibility into the exceptions -which are currently being resolved. - -One way to generate the initial list is to run the Rake task `rubocop:todo:generate`: +The preferred way to [generate the initial list or a list for specific RuboCop rules](../rake_tasks.md#generate-initial-rubocop-todo-list) +is to run the Rake task `rubocop:todo:generate`: ```shell +# Initial list bundle exec rake rubocop:todo:generate + +# List for specific RuboCop rules +bundle exec rake 'rubocop:todo:generate[Gitlab/NamespacedClass,Lint/Syntax]' ``` -You can then move the list from the freshly generated `.rubocop_todo.yml` for the Cop being actively -resolved and place it in the directory `.rubocop_todo/`. In this scenario, do not commit -auto-generated changes to the `.rubocop_todo.yml`, as an `exclude limit` that is higher than 15 -makes the `.rubocop_todo.yml` hard to parse. +This Rake task creates or updates the exception list in `.rubocop_todo/`. For +example, the configuration for the RuboCop rule `Gitlab/NamespacedClass` is +located in `.rubocop_todo/gitlab/namespaced_class.yml`. + +Make sure to commit any changes in `.rubocop_todo/` after running the Rake task. ### Reveal existing RuboCop exceptions diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index b98de46a72a9ebd958d5c95cb98364a65e5d572f..1e9367ecee4cc7d071a18a540ad2131987c76cde 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -196,6 +196,16 @@ One way to generate the initial list is to run the Rake task `rubocop:todo:gener bundle exec rake rubocop:todo:generate ``` +To generate TODO list for specific RuboCop rules, pass them comma-seperated as +argument to the Rake task: + +```shell +bundle exec rake 'rubocop:todo:generate[Gitlab/NamespacedClass,Lint/Syntax]' +bundle exec rake rubocop:todo:generate\[Gitlab/NamespacedClass,Lint/Syntax\] +``` + +Some shells require brackets to be escaped or quoted. + See [Resolving RuboCop exceptions](contributing/style_guides.md#resolving-rubocop-exceptions) on how to proceed from here. diff --git a/lib/tasks/rubocop.rake b/lib/tasks/rubocop.rake index 8c5edb5de8a3dd7d491587b85c13625c740c3fee..6eabdf51dcd5765a1070a5f65ea68d284c240ad7 100644 --- a/lib/tasks/rubocop.rake +++ b/lib/tasks/rubocop.rake @@ -1,4 +1,5 @@ # frozen_string_literal: true +# rubocop:disable Rails/RakeEnvironment unless Rails.env.production? require 'rubocop/rake_task' @@ -8,18 +9,59 @@ unless Rails.env.production? namespace :rubocop do namespace :todo do desc 'Generate RuboCop todos' - task :generate do # rubocop:disable Rails/RakeEnvironment + task :generate do |_task, args| require 'rubocop' + require 'active_support/inflector/inflections' + require_relative '../../rubocop/todo_dir' + require_relative '../../rubocop/formatter/todo_formatter' + + # Reveal all pending TODOs so RuboCop can pick them up and report + # during scan. + ENV['REVEAL_RUBOCOP_TODO'] = '1' + + # Save cop configuration like `RSpec/ContextWording` into + # `rspec/context_wording.yml` and not into + # `r_spec/context_wording.yml`. + ActiveSupport::Inflector.inflections(:en) do |inflect| + inflect.acronym 'RSpec' + inflect.acronym 'GraphQL' + end options = %w[ - --auto-gen-config - --auto-gen-only-exclude - --exclude-limit=100000 - --no-offense-counts + --parallel + --format RuboCop::Formatter::TodoFormatter ] + # Convert from Rake::TaskArguments into an Array to make `any?` work as + # expected. + cop_names = args.to_a + + todo_dir = RuboCop::TodoDir.new(RuboCop::TodoDir::DEFAULT_TODO_DIR) + + if cop_names.any? + # We are sorting the cop names to benefit from RuboCop cache which + # also takes passed parameters into account. + list = cop_names.sort.join(',') + options.concat ['--only', list] + + cop_names.each { |cop_name| todo_dir.inspect(cop_name) } + else + todo_dir.inspect_all + end + + puts <<~MSG + Generating RuboCop TODOs with: + rubocop #{options.join(' ')} + + This might take a while... + MSG + RuboCop::CLI.new.run(options) + + todo_dir.delete_inspected end end end end + +# rubocop:enable Rails/RakeEnvironment diff --git a/rubocop/formatter/todo_formatter.rb b/rubocop/formatter/todo_formatter.rb new file mode 100644 index 0000000000000000000000000000000000000000..7c0911f5eba1366e2b090a181af493fdf537ddbf --- /dev/null +++ b/rubocop/formatter/todo_formatter.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +require 'set' +require 'rubocop' +require 'yaml' + +require_relative '../todo_dir' + +module RuboCop + module Formatter + # This formatter dumps a YAML configuration file per cop rule + # into `.rubocop_todo/**/*.yml` which contains detected offenses. + # + # For example, this formatter stores offenses for `RSpec/VariableName` + # in `.rubocop_todo/rspec/variable_name.yml`. + class TodoFormatter < BaseFormatter + # Disable a cop which exceeds this limit. This way we ensure that we + # don't enable a cop by accident when moving it from + # .rubocop_todo.yml to .rubocop_todo/. + # We keep the cop disabled if it has been disabled previously explicitly + # via `Enabled: false` in .rubocop_todo.yml or .rubocop_todo/. + MAX_OFFENSE_COUNT = 15 + + Todo = Struct.new(:cop_name, :files, :offense_count) do + def initialize(cop_name) + super(cop_name, Set.new, 0) + + @cop_class = RuboCop::Cop::Registry.global.find_by_cop_name(cop_name) + end + + def record(file, offense_count) + files << file + self.offense_count += offense_count + end + + def autocorrectable? + @cop_class&.support_autocorrect? + end + end + + def initialize(output, options = {}) + directory = options.delete(:rubocop_todo_dir) || TodoDir::DEFAULT_TODO_DIR + @todos = Hash.new { |hash, cop_name| hash[cop_name] = Todo.new(cop_name) } + @todo_dir = TodoDir.new(directory) + @config_inspect_todo_dir = load_config_inspect_todo_dir(directory) + @config_old_todo_yml = load_config_old_todo_yml(directory) + check_multiple_configurations! + + super + end + + def file_finished(file, offenses) + return if offenses.empty? + + file = relative_path(file) + + offenses.map(&:cop_name).tally.each do |cop_name, offense_count| + @todos[cop_name].record(file, offense_count) + end + end + + def finished(_inspected_files) + @todos.values.sort_by(&:cop_name).each do |todo| + yaml = to_yaml(todo) + path = @todo_dir.write(todo.cop_name, yaml) + + output.puts "Written to #{relative_path(path)}\n" + end + end + + private + + def relative_path(path) + parent = File.expand_path('..', @todo_dir.directory) + path.delete_prefix("#{parent}/") + end + + def to_yaml(todo) + yaml = [] + yaml << '---' + yaml << '# Cop supports --auto-correct.' if todo.autocorrectable? + yaml << "#{todo.cop_name}:" + + if previously_disabled?(todo) && offense_count_exceeded?(todo) + yaml << " # Offense count: #{todo.offense_count}" + yaml << ' # Temporarily disabled due to too many offenses' + yaml << ' Enabled: false' + end + + yaml << ' Exclude:' + + files = todo.files.sort.map { |file| " - '#{file}'" } + yaml.concat files + yaml << '' + + yaml.join("\n") + end + + def offense_count_exceeded?(todo) + todo.offense_count > MAX_OFFENSE_COUNT + end + + def check_multiple_configurations! + cop_names = @config_inspect_todo_dir.keys & @config_old_todo_yml.keys + return if cop_names.empty? + + list = cop_names.sort.map { |cop_name| "- #{cop_name}" }.join("\n") + raise "Multiple configurations found for cops:\n#{list}\n" + end + + def previously_disabled?(todo) + cop_name = todo.cop_name + + config = @config_old_todo_yml[cop_name] || + @config_inspect_todo_dir[cop_name] || {} + return false if config.empty? + + config['Enabled'] == false + end + + def load_config_inspect_todo_dir(directory) + @todo_dir.list_inspect.each_with_object({}) do |path, combined| + config = YAML.load_file(path) + combined.update(config) if Hash === config + end + end + + # Load YAML configuration from `.rubocop_todo.yml`. + # We consider this file already old, obsolete, and to be removed soon. + def load_config_old_todo_yml(directory) + path = File.expand_path(File.join(directory, '../.rubocop_todo.yml')) + config = YAML.load_file(path) if File.exist?(path) + + config || {} + end + end + end +end diff --git a/rubocop/todo_dir.rb b/rubocop/todo_dir.rb new file mode 100644 index 0000000000000000000000000000000000000000..4aca4454a069ea51d10e6629bd4940fcecb92b67 --- /dev/null +++ b/rubocop/todo_dir.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'fileutils' +require 'active_support/inflector/inflections' + +module RuboCop + # Helper class to manage file access to RuboCop TODOs in .rubocop_todo directory. + class TodoDir + DEFAULT_TODO_DIR = File.expand_path('../.rubocop_todo', __dir__) + + # Suffix to indicate TODOs being inspected right now. + SUFFIX_INSPECT = '.inspect' + + attr_reader :directory + + def initialize(directory, inflector: ActiveSupport::Inflector) + @directory = directory + @inflector = inflector + end + + def read(cop_name, suffix = nil) + read_suffixed(cop_name) + end + + def write(cop_name, content) + path = path_for(cop_name) + + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, content) + + path + end + + def inspect(cop_name) + path = path_for(cop_name) + + if File.exist?(path) + FileUtils.mv(path, "#{path}#{SUFFIX_INSPECT}") + true + else + false + end + end + + def inspect_all + pattern = File.join(@directory, '**/*.yml') + + Dir.glob(pattern).count do |path| + FileUtils.mv(path, "#{path}#{SUFFIX_INSPECT}") + end + end + + def list_inspect + pattern = File.join(@directory, "**/*.yml.inspect") + + Dir.glob(pattern) + end + + def delete_inspected + pattern = File.join(@directory, '**/*.yml.inspect') + + Dir.glob(pattern).count do |path| + File.delete(path) + end + end + + private + + def read_suffixed(cop_name, suffix = nil) + path = path_for(cop_name, suffix) + + File.read(path) if File.exist?(path) + end + + def path_for(cop_name, suffix = nil) + todo_path = "#{@inflector.underscore(cop_name)}.yml#{suffix}" + + File.join(@directory, todo_path) + end + end +end diff --git a/spec/rubocop/formatter/todo_formatter_spec.rb b/spec/rubocop/formatter/todo_formatter_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e1b1de33bfe468de865f5093a02427a16a1b505e --- /dev/null +++ b/spec/rubocop/formatter/todo_formatter_spec.rb @@ -0,0 +1,284 @@ +# frozen_string_literal: true +# rubocop:disable RSpec/VerifiedDoubles + +require 'fast_spec_helper' +require 'stringio' +require 'fileutils' + +require_relative '../../../rubocop/formatter/todo_formatter' +require_relative '../../../rubocop/todo_dir' + +RSpec.describe RuboCop::Formatter::TodoFormatter do + let(:stdout) { StringIO.new } + let(:tmp_dir) { Dir.mktmpdir } + let(:real_tmp_dir) { File.join(tmp_dir, 'real') } + let(:symlink_tmp_dir) { File.join(tmp_dir, 'symlink') } + let(:rubocop_todo_dir) { "#{symlink_tmp_dir}/.rubocop_todo" } + let(:options) { { rubocop_todo_dir: rubocop_todo_dir } } + let(:todo_dir) { RuboCop::TodoDir.new(rubocop_todo_dir) } + + subject(:formatter) { described_class.new(stdout, options) } + + around do |example| + FileUtils.mkdir(real_tmp_dir) + FileUtils.symlink(real_tmp_dir, symlink_tmp_dir) + + Dir.chdir(symlink_tmp_dir) do + example.run + end + end + + after do + FileUtils.remove_entry(tmp_dir) + end + + context 'with offenses detected' do + let(:offense) { fake_offense('A/Offense') } + let(:offense_too_many) { fake_offense('B/TooManyOffenses') } + let(:offense_autocorrect) { fake_offense('B/AutoCorrect') } + + before do + stub_const("#{described_class}::MAX_OFFENSE_COUNT", 1) + + stub_rubocop_registry( + 'A/Offense' => { autocorrectable: false }, + 'B/AutoCorrect' => { autocorrectable: true } + ) + end + + def run_formatter + formatter.started(%w[a.rb b.rb c.rb d.rb]) + formatter.file_finished('c.rb', [offense_too_many]) + formatter.file_finished('a.rb', [offense_too_many, offense, offense_too_many]) + formatter.file_finished('b.rb', []) + formatter.file_finished('d.rb', [offense_autocorrect]) + formatter.finished(%w[a.rb b.rb c.rb d.rb]) + end + + it 'outputs its actions' do + run_formatter + + expect(stdout.string).to eq(<<~OUTPUT) + Written to .rubocop_todo/a/offense.yml + Written to .rubocop_todo/b/auto_correct.yml + Written to .rubocop_todo/b/too_many_offenses.yml + OUTPUT + end + + it 'creates YAML files', :aggregate_failures do + run_formatter + + expect(rubocop_todo_dir_listing).to contain_exactly( + 'a/offense.yml', 'b/auto_correct.yml', 'b/too_many_offenses.yml' + ) + + expect(todo_yml('A/Offense')).to eq(<<~YAML) + --- + A/Offense: + Exclude: + - 'a.rb' + YAML + + expect(todo_yml('B/AutoCorrect')).to eq(<<~YAML) + --- + # Cop supports --auto-correct. + B/AutoCorrect: + Exclude: + - 'd.rb' + YAML + + expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) + --- + B/TooManyOffenses: + Exclude: + - 'a.rb' + - 'c.rb' + YAML + end + + context 'when cop previously not explicitly disabled' do + before do + todo_dir.write('B/TooManyOffenses', <<~YAML) + --- + B/TooManyOffenses: + Exclude: + - 'x.rb' + YAML + end + + it 'does not disable cop' do + run_formatter + + expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) + --- + B/TooManyOffenses: + Exclude: + - 'a.rb' + - 'c.rb' + YAML + end + end + + context 'when cop previously explicitly disabled in rubocop_todo/' do + before do + todo_dir.write('B/TooManyOffenses', <<~YAML) + --- + B/TooManyOffenses: + Enabled: false + Exclude: + - 'x.rb' + YAML + + todo_dir.inspect_all + end + + it 'keeps cop disabled' do + run_formatter + + expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) + --- + B/TooManyOffenses: + # Offense count: 3 + # Temporarily disabled due to too many offenses + Enabled: false + Exclude: + - 'a.rb' + - 'c.rb' + YAML + end + end + + context 'when cop previously explicitly disabled in rubocop_todo.yml' do + before do + File.write('.rubocop_todo.yml', <<~YAML) + --- + B/TooManyOffenses: + Enabled: false + Exclude: + - 'x.rb' + YAML + end + + it 'keeps cop disabled' do + run_formatter + + expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) + --- + B/TooManyOffenses: + # Offense count: 3 + # Temporarily disabled due to too many offenses + Enabled: false + Exclude: + - 'a.rb' + - 'c.rb' + YAML + end + end + + context 'with cop configuration in both .rubocop_todo/ and .rubocop_todo.yml' do + before do + todo_dir.write('B/TooManyOffenses', <<~YAML) + --- + B/TooManyOffenses: + Exclude: + - 'a.rb' + YAML + + todo_dir.write('A/Offense', <<~YAML) + --- + A/Offense: + Exclude: + - 'a.rb' + YAML + + todo_dir.inspect_all + + File.write('.rubocop_todo.yml', <<~YAML) + --- + B/TooManyOffenses: + Exclude: + - 'x.rb' + A/Offense: + Exclude: + - 'y.rb' + YAML + end + + it 'raises an error' do + expect { run_formatter }.to raise_error(RuntimeError, <<~TXT) + Multiple configurations found for cops: + - A/Offense + - B/TooManyOffenses + TXT + end + end + end + + context 'without offenses detected' do + before do + formatter.started(%w[a.rb b.rb]) + formatter.file_finished('a.rb', []) + formatter.file_finished('b.rb', []) + formatter.finished(%w[a.rb b.rb]) + end + + it 'does not output anything' do + expect(stdout.string).to eq('') + end + + it 'does not write any YAML files' do + expect(rubocop_todo_dir_listing).to be_empty + end + end + + context 'without files to inspect' do + before do + formatter.started([]) + formatter.finished([]) + end + + it 'does not output anything' do + expect(stdout.string).to eq('') + end + + it 'does not write any YAML files' do + expect(rubocop_todo_dir_listing).to be_empty + end + end + + private + + def rubocop_todo_dir_listing + Dir.glob("#{rubocop_todo_dir}/**/*") + .select { |path| File.file?(path) } + .map { |path| path.delete_prefix("#{rubocop_todo_dir}/") } + end + + def todo_yml(cop_name) + todo_dir.read(cop_name) + end + + def fake_offense(cop_name) + double(:offense, cop_name: cop_name) + end + + def stub_rubocop_registry(**cops) + rubocop_registry = double(:rubocop_registry) + + allow(RuboCop::Cop::Registry).to receive(:global).and_return(rubocop_registry) + + allow(rubocop_registry).to receive(:find_by_cop_name) + .with(String).and_return(nil) + + cops.each do |cop_name, attributes| + allow(rubocop_registry).to receive(:find_by_cop_name) + .with(cop_name).and_return(fake_cop(**attributes)) + end + end + + def fake_cop(autocorrectable:) + double(:cop, support_autocorrect?: autocorrectable) + end +end + +# rubocop:enable RSpec/VerifiedDoubles diff --git a/spec/rubocop/todo_dir_spec.rb b/spec/rubocop/todo_dir_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ae59def885d60e53b2ca331c6320627f07f39ab4 --- /dev/null +++ b/spec/rubocop/todo_dir_spec.rb @@ -0,0 +1,218 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'fileutils' +require 'active_support/inflector/inflections' + +require_relative '../../rubocop/todo_dir' + +RSpec.describe RuboCop::TodoDir do + let(:todo_dir) { described_class.new(directory) } + let(:directory) { Dir.mktmpdir } + let(:cop_name) { 'RSpec/VariableInstance' } + let(:cop_name_underscore) { ActiveSupport::Inflector.underscore(cop_name) } + let(:yaml_path) { "#{File.join(directory, cop_name_underscore)}.yml" } + + around do |example| + Dir.chdir(directory) do + example.run + end + end + + after do + FileUtils.remove_entry(directory) + end + + describe '#initialize' do + context 'when passing inflector' do + let(:fake_inflector) { double(:inflector) } # rubocop:disable RSpec/VerifiedDoubles + let(:todo_dir) { described_class.new(directory, inflector: fake_inflector) } + + before do + allow(fake_inflector).to receive(:underscore) + .with(cop_name) + .and_return(cop_name_underscore) + end + + it 'calls .underscore' do + todo_dir.write(cop_name, 'a') + + expect(fake_inflector).to have_received(:underscore) + end + end + end + + describe '#directory' do + subject { todo_dir.directory } + + it { is_expected.to eq(directory) } + end + + describe '#read' do + let(:content) { 'a' } + + subject { todo_dir.read(cop_name) } + + context 'when file exists' do + before do + todo_dir.write(cop_name, content) + end + + it { is_expected.to eq(content) } + end + + context 'when file is missing' do + it { is_expected.to be_nil } + end + end + + describe '#write' do + let(:content) { 'a' } + + subject { todo_dir.write(cop_name, content) } + + it { is_expected.to eq(yaml_path) } + + it 'writes content to YAML file' do + subject + + expect(File.read(yaml_path)).to eq(content) + end + end + + describe '#inspect' do + subject { todo_dir.inspect(cop_name) } + + context 'with existing YAML file' do + before do + todo_dir.write(cop_name, 'a') + end + + it { is_expected.to eq(true) } + + it 'moves YAML file to .inspect' do + subject + + expect(File).not_to exist(yaml_path) + expect(File).to exist("#{yaml_path}.inspect") + end + end + + context 'with missing YAML file' do + it { is_expected.to eq(false) } + end + end + + describe '#inspect_all' do + subject { todo_dir.inspect_all } + + context 'with YAML files' do + before do + todo_dir.write(cop_name, 'a') + todo_dir.write('Other/Rule', 'a') + todo_dir.write('Very/Nested/Rule', 'a') + end + + it { is_expected.to eq(3) } + + it 'moves all YAML files to .inspect' do + subject + + expect(Dir.glob('**/*.yml')).to be_empty + expect(Dir.glob('**/*.yml.inspect').size).to eq(3) + end + end + + context 'with non-YAML files' do + before do + File.write('file', 'a') + File.write('file.txt', 'a') + File.write('file.yaml', 'a') # not .yml + end + + it { is_expected.to eq(0) } + + it 'does not move non-YAML files' do + subject + + expect(Dir.glob('**/*')) + .to contain_exactly('file', 'file.txt', 'file.yaml') + end + end + + context 'without files' do + it { is_expected.to eq(0) } + end + end + + describe '#list_inspect' do + let(:content) { 'a' } + + subject { todo_dir.list_inspect } + + context 'when file exists and is being inspected' do + before do + todo_dir.write(cop_name, content) + todo_dir.inspect_all + end + + it do + is_expected.to contain_exactly("#{yaml_path}.inspect") + end + end + + context 'when file exists but not being inspected' do + before do + todo_dir.write(cop_name, content) + end + + it { is_expected.to be_empty } + end + + context 'when file is missing' do + it { is_expected.to be_empty } + end + end + + describe '#delete_inspected' do + subject { todo_dir.delete_inspected } + + context 'with YAML files' do + before do + todo_dir.write(cop_name, 'a') + todo_dir.write('Other/Rule', 'a') + todo_dir.write('Very/Nested/Rule', 'a') + todo_dir.inspect_all + end + + it { is_expected.to eq(3) } + + it 'deletes all .inspected YAML files' do + subject + + expect(Dir.glob('**/*.yml.inspect')).to be_empty + end + end + + context 'with non-YAML files' do + before do + File.write('file.inspect', 'a') + File.write('file.txt.inspect', 'a') + File.write('file.yaml.inspect', 'a') # not .yml + end + + it { is_expected.to eq(0) } + + it 'does not delete non-YAML files' do + subject + + expect(Dir.glob('**/*')).to contain_exactly( + 'file.inspect', 'file.txt.inspect', 'file.yaml.inspect') + end + end + + context 'without files' do + it { is_expected.to eq(0) } + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 724b8be36ad1c039544e1ca6645139db6fd04679..a72c8d2c4e83d27f6303ae2a8850b9fd298ca930 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -458,11 +458,6 @@ end end - # Allows stdout to be redirected to reduce noise - config.before(:each, :silence_stdout) do - $stdout = StringIO.new - end - # Makes diffs show entire non-truncated values. config.before(:each, unlimited_max_formatted_output_length: true) do |_example| config.expect_with :rspec do |c| @@ -475,10 +470,6 @@ allow_any_instance_of(VersionCheck).to receive(:response).and_return({ "severity" => "success" }) end - config.after(:each, :silence_stdout) do - $stdout = STDOUT - end - config.disable_monkey_patching! end diff --git a/spec/support/silence_stdout.rb b/spec/support/silence_stdout.rb new file mode 100644 index 0000000000000000000000000000000000000000..b2bc65c5cda84c30659a700ff85d73f298486024 --- /dev/null +++ b/spec/support/silence_stdout.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +RSpec.configure do |config| + # Allows stdout to be redirected to reduce noise + config.before(:each, :silence_stdout) do + $stdout = StringIO.new + end + + config.after(:each, :silence_stdout) do + $stdout = STDOUT + end +end diff --git a/spec/tasks/rubocop_rake_spec.rb b/spec/tasks/rubocop_rake_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cf7e45aae2818b4920791520f6e9c4f75a5b43b3 --- /dev/null +++ b/spec/tasks/rubocop_rake_spec.rb @@ -0,0 +1,168 @@ +# frozen_string_literal: true +# rubocop:disable RSpec/VerifiedDoubles + +require 'fast_spec_helper' +require 'rake' +require 'fileutils' + +require_relative '../support/silence_stdout' +require_relative '../support/helpers/next_instance_of' +require_relative '../support/helpers/rake_helpers' +require_relative '../../rubocop/todo_dir' + +RSpec.describe 'rubocop rake tasks', :silence_stdout do + include RakeHelpers + + before do + stub_const('Rails', double(:rails_env)) + allow(Rails).to receive(:env).and_return(double(production?: false)) + + stub_const('ENV', ENV.to_hash.dup) + + Rake.application.rake_require 'tasks/rubocop' + end + + describe 'todo:generate', :aggregate_failures do + let(:tmp_dir) { Dir.mktmpdir } + let(:rubocop_todo_dir) { File.join(tmp_dir, '.rubocop_todo') } + let(:todo_dir) { RuboCop::TodoDir.new(rubocop_todo_dir) } + + around do |example| + Dir.chdir(tmp_dir) do + with_inflections do + example.run + end + end + end + + before do + allow(RuboCop::TodoDir).to receive(:new).and_return(todo_dir) + + # This Ruby file will trigger the following 3 offenses. + File.write('a.rb', <<~RUBY) + a+b + + RUBY + + # Mimic GitLab's .rubocop_todo.yml avoids relying on RuboCop's + # default.yml configuration. + File.write('.rubocop.yml', <<~YAML) + <% unless ENV['REVEAL_RUBOCOP_TODO'] == '1' %> + <% Dir.glob('.rubocop_todo/**/*.yml').each do |rubocop_todo_yaml| %> + - '<%= rubocop_todo_yaml %>' + <% end %> + - '.rubocop_todo.yml' + <% end %> + + AllCops: + NewCops: enable # Avoiding RuboCop warnings + + Layout/SpaceAroundOperators: + Enabled: true + + Layout/TrailingEmptyLines: + Enabled: true + + Lint/Syntax: + Enabled: true + + Style/FrozenStringLiteralComment: + Enabled: true + YAML + + # Required to verify that we are revealing all TODOs via + # ENV['REVEAL_RUBOCOP_TODO'] = '1'. + # This file can be removed from specs after we've moved all offenses from + # .rubocop_todo.yml to .rubocop_todo/**/*.yml. + File.write('.rubocop_todo.yml', <<~YAML) + # Too many offenses + Layout/SpaceAroundOperators: + Enabled: false + YAML + + # Previous offense now fixed. + todo_dir.write('Lint/Syntax', '') + end + + after do + FileUtils.remove_entry(tmp_dir) + end + + context 'without arguments' do + let(:run_task) { run_rake_task('rubocop:todo:generate') } + + it 'generates TODOs for all RuboCop rules' do + expect { run_task }.to output(<<~OUTPUT).to_stdout + Generating RuboCop TODOs with: + rubocop --parallel --format RuboCop::Formatter::TodoFormatter + + This might take a while... + Written to .rubocop_todo/layout/space_around_operators.yml + Written to .rubocop_todo/layout/trailing_empty_lines.yml + Written to .rubocop_todo/style/frozen_string_literal_comment.yml + OUTPUT + + expect(rubocop_todo_dir_listing).to contain_exactly( + 'layout/space_around_operators.yml', + 'layout/trailing_empty_lines.yml', + 'style/frozen_string_literal_comment.yml' + ) + end + + it 'sets acronyms for inflections' do + run_task + + expect(ActiveSupport::Inflector.inflections.acronyms).to include( + 'rspec' => 'RSpec', + 'graphql' => 'GraphQL' + ) + end + end + + context 'with cop names as arguments' do + let(:run_task) do + cop_names = %w[ + Style/FrozenStringLiteralComment Layout/TrailingEmptyLines + Lint/Syntax + ] + + run_rake_task('rubocop:todo:generate', cop_names) + end + + it 'generates TODOs for given RuboCop cops' do + expect { run_task }.to output(<<~OUTPUT).to_stdout + Generating RuboCop TODOs with: + rubocop --parallel --format RuboCop::Formatter::TodoFormatter --only Layout/TrailingEmptyLines,Lint/Syntax,Style/FrozenStringLiteralComment + + This might take a while... + Written to .rubocop_todo/layout/trailing_empty_lines.yml + Written to .rubocop_todo/style/frozen_string_literal_comment.yml + OUTPUT + + expect(rubocop_todo_dir_listing).to contain_exactly( + 'layout/trailing_empty_lines.yml', + 'style/frozen_string_literal_comment.yml' + ) + end + end + + private + + def rubocop_todo_dir_listing + Dir.glob("#{rubocop_todo_dir}/**/*") + .select { |path| File.file?(path) } + .map { |path| path.delete_prefix("#{rubocop_todo_dir}/") } + end + + def with_inflections + original = ActiveSupport::Inflector::Inflections.instance_variable_get(:@__instance__)[:en] + ActiveSupport::Inflector::Inflections.instance_variable_set(:@__instance__, en: original.dup) + + yield + ensure + ActiveSupport::Inflector::Inflections.instance_variable_set(:@__instance__, en: original) + end + end +end + +# rubocop:enable RSpec/VerifiedDoubles