From 20db6e1e09d6dae76eaa3b3556e63304a09704d7 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Mon, 24 Aug 2020 12:49:18 -0400 Subject: [PATCH 1/2] Add `:restore_registry` context and `stub_cop_class` helper class --- CHANGELOG.md | 1 + lib/rubocop/rspec/shared_contexts.rb | 12 +++++++++ spec/rubocop/config_loader_spec.rb | 6 +---- spec/rubocop/config_spec.rb | 10 ++------ spec/rubocop/cop/cop_spec.rb | 5 ++-- .../cop/mixin/enforce_superclass_spec.rb | 14 ++--------- spec/rubocop/cop/team_spec.rb | 25 +++++++++---------- .../disabled_config_formatter_spec.rb | 11 +++----- 8 files changed, 36 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 866a3629fc9..59c9bd2a8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#8472](https://github.com/rubocop-hq/rubocop/issues/8472): Add new `Lint/UselessMethodDefinition` cop. ([@fatkodima][]) * [#8531](https://github.com/rubocop-hq/rubocop/issues/8531): Add new `Lint/EmptyFile` cop. ([@fatkodima][]) * Add new `Lint/TrailingCommaInAttributeDeclaration` cop. ([@drenmi][]) +* [#8578](https://github.com/rubocop-hq/rubocop/pull/8578): Add `:restore_registry` context and `stub_cop_class` helper class. ([@marcandre][]) ### Bug fixes diff --git a/lib/rubocop/rspec/shared_contexts.rb b/lib/rubocop/rspec/shared_contexts.rb index e1d57aacaa0..e7367814c47 100644 --- a/lib/rubocop/rspec/shared_contexts.rb +++ b/lib/rubocop/rspec/shared_contexts.rb @@ -40,6 +40,18 @@ end end +RSpec.shared_context 'maintain registry', :restore_registry do + around(:each) do |example| + RuboCop::Cop::Registry.with_temporary_global { example.run } + end + + def stub_cop_class(name, inherit: RuboCop::Cop::Base, &block) + klass = Class.new(inherit, &block) + stub_const(name, klass) + klass + end +end + # This context assumes nothing and defines `cop`, among others. RSpec.shared_context 'config', :config do # rubocop:disable Metrics/BlockLength ### Meant to be overridden at will diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index ff430d29e10..ae4d93efc0e 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -621,11 +621,7 @@ def enabled?(cop) include_examples 'resolves enabled/disabled for all cops', true, false end - context 'when a third party require defines a new gem' do - around do |example| - RuboCop::Cop::Registry.with_temporary_global { example.run } - end - + context 'when a third party require defines a new gem', :restore_registry do context 'when the gem is not loaded' do before do create_file('.rubocop.yml', <<~YAML) diff --git a/spec/rubocop/config_spec.rb b/spec/rubocop/config_spec.rb index 0077304fe79..020ec9c5f41 100644 --- a/spec/rubocop/config_spec.rb +++ b/spec/rubocop/config_spec.rb @@ -845,7 +845,7 @@ def cop_enabled(cop_class) end end - describe '#for_department' do + describe '#for_department', :restore_registry do let(:hash) do { 'Foo' => { 'Bar' => 42, 'Baz' => true }, @@ -853,14 +853,8 @@ def cop_enabled(cop_class) } end - around do |test| - RuboCop::Cop::Registry.with_temporary_global do - test.run - end - end - before do - stub_const('RuboCop::Foo::Foo', Class.new(RuboCop::Cop::Base)) + stub_cop_class('RuboCop::Foo::Foo') end it "always returns the department's config" do diff --git a/spec/rubocop/cop/cop_spec.rb b/spec/rubocop/cop/cop_spec.rb index 9f240b5faf3..065c0860b06 100644 --- a/spec/rubocop/cop/cop_spec.rb +++ b/spec/rubocop/cop/cop_spec.rb @@ -166,12 +166,11 @@ end end - context 'when cop supports autocorrection' do + context 'when cop supports autocorrection', :restore_registry do let(:cop_class) do - stub_cop = Class.new(RuboCop::Cop::Cop) do + stub_cop_class('RuboCop::Cop::Test::StubCop', inherit: described_class) do def autocorrect(node); end end - stub_const('RuboCop::Cop::Test::StubCop', stub_cop) end context 'when offense was corrected' do diff --git a/spec/rubocop/cop/mixin/enforce_superclass_spec.rb b/spec/rubocop/cop/mixin/enforce_superclass_spec.rb index f2a9f84b20e..522f7e9ede8 100644 --- a/spec/rubocop/cop/mixin/enforce_superclass_spec.rb +++ b/spec/rubocop/cop/mixin/enforce_superclass_spec.rb @@ -1,15 +1,14 @@ # frozen_string_literal: true # rubocop:disable RSpec/FilePath -RSpec.describe RuboCop::Cop::EnforceSuperclass do +RSpec.describe RuboCop::Cop::EnforceSuperclass, :restore_registry do subject(:cop) { cop_class.new } let(:cop_class) { RuboCop::Cop::RSpec::ApplicationRecord } let(:msg) { 'Models should subclass `ApplicationRecord`' } before do - stub_const('RuboCop::Cop::RSpec::ApplicationRecord', - Class.new(RuboCop::Cop::Cop)) + stub_cop_class('RuboCop::Cop::RSpec::ApplicationRecord') stub_const("#{cop_class}::MSG", 'Models should subclass `ApplicationRecord`') stub_const("#{cop_class}::SUPERCLASS", 'ApplicationRecord') @@ -18,15 +17,6 @@ RuboCop::Cop::RSpec::ApplicationRecord.include(described_class) end - # `RuboCop::Cop::Cop` mutates its `registry` when inherited from. - # This can introduce nondeterministic failures in other parts of the - # specs if this mutation occurs before code that depends on this global cop - # store. The workaround is to replace the global cop store with a temporary - # store during these tests - around do |test| - RuboCop::Cop::Registry.with_temporary_global { test.run } - end - shared_examples 'no offense' do |code| it "registers no offenses for `#{code}`" do expect_no_offenses(code) diff --git a/spec/rubocop/cop/team_spec.rb b/spec/rubocop/cop/team_spec.rb index 6441dfafa94..ab973817761 100644 --- a/spec/rubocop/cop/team_spec.rb +++ b/spec/rubocop/cop/team_spec.rb @@ -211,13 +211,13 @@ def a end end - context 'when done twice' do + context 'when done twice', :restore_registry do let(:persisting_cop_class) do - klass = Class.new(RuboCop::Cop::Base) - klass.exclude_from_registry - klass.define_singleton_method(:support_multiple_source?) { true } - stub_const('Test::Persisting', klass) - klass + stub_cop_class('Test::Persisting') do + def self.support_multiple_source? + true + end + end end let(:cop_classes) { [persisting_cop_class, RuboCop::Cop::Base] } @@ -358,14 +358,13 @@ def a end end - context 'when cop with different checksum joins' do + context 'when cop with different checksum joins', :restore_registry do before do - stub_const('Test::CopWithExternalDeps', - Class.new(::RuboCop::Cop::Cop) do - def external_dependency_checksum - 'something other than nil' - end - end) + stub_cop_class('Test::CopWithExternalDeps') do + def external_dependency_checksum + 'something other than nil' + end + end end let(:new_cop_classes) do diff --git a/spec/rubocop/formatter/disabled_config_formatter_spec.rb b/spec/rubocop/formatter/disabled_config_formatter_spec.rb index 389381070b6..7d597fc120e 100644 --- a/spec/rubocop/formatter/disabled_config_formatter_spec.rb +++ b/spec/rubocop/formatter/disabled_config_formatter_spec.rb @@ -249,14 +249,11 @@ def io.path end end - context 'with auto-correct supported cop' do + context 'with auto-correct supported cop', :restore_registry do before do - stub_const('Test::Cop3', - Class.new(::RuboCop::Cop::Cop) do - def autocorrect - # Dummy method to respond to #support_autocorrect? - end - end) + stub_cop_class('Test::Cop3') do + extend RuboCop::Cop::AutoCorrector + end formatter.started(['test_auto_correct.rb']) formatter.file_started('test_auto_correct.rb', {}) From 9e5d8cfbdba7d3dddba4da9b53679b98f1b3fb07 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Mon, 24 Aug 2020 12:48:32 -0400 Subject: [PATCH 2/2] Add `Cop.documentation_url` --- CHANGELOG.md | 2 ++ lib/rubocop.rb | 1 + lib/rubocop/cop/base.rb | 16 +++++++++++++++ lib/rubocop/cop/documentation.rb | 22 +++++++++++++++++++++ lib/rubocop/cops_documentation_generator.rb | 5 +++-- spec/rubocop/cop/cop_spec.rb | 16 +++++++++++++++ 6 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 lib/rubocop/cop/documentation.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 59c9bd2a8e3..4d6af342fb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ * [#8531](https://github.com/rubocop-hq/rubocop/issues/8531): Add new `Lint/EmptyFile` cop. ([@fatkodima][]) * Add new `Lint/TrailingCommaInAttributeDeclaration` cop. ([@drenmi][]) * [#8578](https://github.com/rubocop-hq/rubocop/pull/8578): Add `:restore_registry` context and `stub_cop_class` helper class. ([@marcandre][]) +* [#8579](https://github.com/rubocop-hq/rubocop/pull/8579): Add `Cop.documentation_url`. ([@marcandre][]) + ### Bug fixes diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 787e1da8614..c458bd750e8 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -34,6 +34,7 @@ require_relative 'rubocop/cop/base' require_relative 'rubocop/cop/cop' require_relative 'rubocop/cop/commissioner' +require_relative 'rubocop/cop/documentation' require_relative 'rubocop/cop/corrector' require_relative 'rubocop/cop/force' require_relative 'rubocop/cop/severity' diff --git a/lib/rubocop/cop/base.rb b/lib/rubocop/cop/base.rb index 3fafc5b0f8b..c8525373377 100644 --- a/lib/rubocop/cop/base.rb +++ b/lib/rubocop/cop/base.rb @@ -56,6 +56,14 @@ def self.autocorrect_incompatible_with [] end + # Cops (other than builtin) are encouraged to implement this + # @return [String, nil] + # + # @api public + def self.documentation_url + Documentation.url_for(self) if builtin? + end + def initialize(config = nil, options = nil) @config = config || Config.new @options = options || { debug: false } @@ -297,6 +305,14 @@ def complete_investigation ### Actually private methods + def self.builtin? + return false unless (m = instance_methods(false).first) # any custom method will do + + path, _line = instance_method(m).source_location + path.start_with?(__dir__) + end + private_class_method :builtin? + def reset_investigation @currently_disabled_lines = @current_offenses = @processed_source = @current_corrector = nil end diff --git a/lib/rubocop/cop/documentation.rb b/lib/rubocop/cop/documentation.rb new file mode 100644 index 00000000000..651bcde37c9 --- /dev/null +++ b/lib/rubocop/cop/documentation.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Helpers for builtin documentation + module Documentation + module_function + + # @api private + def department_to_basename(department) + "cops_#{department.downcase}" + end + + # @api private + def url_for(cop_class) + base = department_to_basename(cop_class.department) + fragment = cop_class.cop_name.downcase.gsub(/[^a-z]/, '') + "https://docs.rubocop.org/rubocop/#{base}.html##{fragment}" + end + end + end +end diff --git a/lib/rubocop/cops_documentation_generator.rb b/lib/rubocop/cops_documentation_generator.rb index 50cd0043c2e..9dffe47ceac 100644 --- a/lib/rubocop/cops_documentation_generator.rb +++ b/lib/rubocop/cops_documentation_generator.rb @@ -2,6 +2,7 @@ # Class for generating documentation of all cops departments class CopsDocumentationGenerator # rubocop:disable Metrics/ClassLength + include ::RuboCop::Cop::Documentation # This class will only generate documentation for cops that belong to one of # the departments given in the `departments` array. E.g. if we only wanted # documentation for Lint cops: @@ -210,7 +211,7 @@ def print_cops_of_department(department) selected_cops.each do |cop| content << print_cop_with_doc(cop) end - file_name = "#{Dir.pwd}/docs/modules/ROOT/pages/cops_#{department.downcase}.adoc" + file_name = "#{Dir.pwd}/docs/modules/ROOT/pages/#{department_to_basename(department)}.adoc" File.open(file_name, 'w') do |file| puts "* generated #{file_name}" file.write("#{content.strip}\n") @@ -243,7 +244,7 @@ def cop_code(cop) def table_of_content_for_department(department) type_title = department[0].upcase + department[1..-1] - filename = "cops_#{department.downcase}.adoc" + filename = "#{department_to_basename(department)}.adoc" content = +"=== Department xref:#{filename}[#{type_title}]\n\n" cops_of_department(department).each do |cop| anchor = cop.cop_name.sub('/', '').downcase diff --git a/spec/rubocop/cop/cop_spec.rb b/spec/rubocop/cop/cop_spec.rb index 065c0860b06..b337ac3fadf 100644 --- a/spec/rubocop/cop/cop_spec.rb +++ b/spec/rubocop/cop/cop_spec.rb @@ -56,6 +56,22 @@ end end + describe '.documentation_url' do + subject(:url) { cop_class.documentation_url } + + describe 'for a builtin cop class' do + let(:cop_class) { RuboCop::Cop::Layout::BlockEndNewline } + + it { is_expected.to eq 'https://docs.rubocop.org/rubocop/cops_layout.html#layoutblockendnewline' } # rubocop:disable Layout/LineLength + end + + describe 'for a custom cop class', :restore_registry do + let(:cop_class) { stub_cop_class('Some::Cop') { def foo; end } } + + it { is_expected.to eq nil } + end + end + it 'keeps track of offenses' do cop.add_offense(nil, location: location, message: 'message')