diff --git a/CHANGELOG.md b/CHANGELOG.md index 523c69ecbec..a3205b44afe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ ### Changes * [#8487](https://github.com/rubocop-hq/rubocop/pull/8487): Detect `<` and `>` as comparison operators in `Style/ConditionalAssignment` cop. ([@biinari][]) +* [#8044](https://github.com/rubocop-hq/rubocop/pull/8044): **(Breaking)** Change logic for cop department name computation. Cops inside deep namespaces (5 or more levels deep) now belong to departments with names that are calculated by joining module names starting from the third one with slashes as separators. For example, cop `Rubocop::Cop::Foo::Bar::Baz` now belongs to `Foo/Bar` department (previously it was `Bar`). Until third-party gems are updated, this might require you to update your `.rubocop.yml` project file with the new cop names. ([@dsavochkin][]) ## 0.89.0 (2020-08-05) diff --git a/config/default.yml b/config/default.yml index d8e2d72c4d7..442a99233c7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -2304,6 +2304,18 @@ Naming/VariableNumber: - normalcase - non_integer +# TODO: this should be removed after rubocop-rspec is updated to reflect changes from +# https://github.com/rubocop-hq/rubocop/pull/8490 +RSpec/Rails/HttpStatus: + Description: Enforces use of symbolic or numeric value to describe HTTP status. + Enabled: true + EnforcedStyle: symbolic + SupportedStyles: + - numeric + - symbolic + VersionAdded: '1.23' + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/HttpStatus + #################### Security ############################## Security/Eval: diff --git a/lib/rubocop.rb b/lib/rubocop.rb index e96e16eb34f..ef6d6695976 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -594,6 +594,7 @@ require_relative 'rubocop/config_loader_resolver' require_relative 'rubocop/config_loader' require_relative 'rubocop/config_obsoletion' +require_relative 'rubocop/config_obsoletion_external' require_relative 'rubocop/config_store' require_relative 'rubocop/config_validator' require_relative 'rubocop/target_finder' diff --git a/lib/rubocop/config.rb b/lib/rubocop/config.rb index f8dc40c47ed..380fcd0825b 100644 --- a/lib/rubocop/config.rb +++ b/lib/rubocop/config.rb @@ -22,6 +22,7 @@ class Config attr_reader :loaded_path def initialize(hash = {}, loaded_path = nil) + ConfigObsoletionExternal.new(hash).fix @loaded_path = loaded_path @for_cop = Hash.new do |h, cop| qualified_cop_name = Cop::Cop.qualified_cop_name(cop, loaded_path) diff --git a/lib/rubocop/config_obsoletion_external.rb b/lib/rubocop/config_obsoletion_external.rb new file mode 100644 index 00000000000..1cf5121e094 --- /dev/null +++ b/lib/rubocop/config_obsoletion_external.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module RuboCop + # This class handles deprecated config entries defined in external gems. It correctly assigns + # them a new (deep) department name. + class ConfigObsoletionExternal + DEPRECATED_EXTERNAL_COPS = { + 'rubocop-rspec' => { + 'FactoryBot' => 'RSpec/FactoryBot', + 'Capybara' => 'RSpec/Capybara', + 'Capybara/CurrentPathExpectation' => 'RSpec/Capybara/CurrentPathExpectation', + 'Capybara/FeatureMethods' => 'RSpec/Capybara/FeatureMethods', + 'Capybara/VisibilityMatcher' => 'RSpec/Capybara/VisibilityMatcher', + 'FactoryBot/AttributeDefinedStatically' => 'RSpec/FactoryBot/AttributeDefinedStatically', + 'FactoryBot/CreateList' => 'RSpec/FactoryBot/CreateList', + 'FactoryBot/FactoryClassName' => 'RSpec/FactoryBot/FactoryClassName' + }, + 'rubocop-i18n' => { + 'GetText' => 'I18n/GetText', + 'RailsI18n' => 'I18n/RailsI18n', + 'GetText/DecorateFunctionMessage' => 'I18n/GetText/DecorateFunctionMessage', + 'GetText/DecorateString' => 'I18n/GetText/DecorateString', + 'GetText/DecorateStringFormattingUsingInterpolation' => + 'I18n/GetText/DecorateStringFormattingUsingInterpolation', + 'GetText/DecorateStringFormattingUsingPercent' => + 'I18n/GetText/DecorateStringFormattingUsingPercent', + 'RailsI18n/DecorateString' => 'I18n/RailsI18n/DecorateString', + 'RailsI18n/DecorateStringFormattingUsingInterpolation' => + 'I18n/RailsI18n/DecorateStringFormattingUsingInterpolation' + } + }.freeze + + def initialize(hash) + @hash = hash + end + + def fix + DEPRECATED_EXTERNAL_COPS.each_value do |gem_entries| + gem_entries.each do |old_name, new_name| + next unless @hash.key?(old_name) + + @hash[new_name] = @hash.delete(old_name) + end + end + end + end +end diff --git a/lib/rubocop/cop/badge.rb b/lib/rubocop/cop/badge.rb index be1710879c4..8ec58d93a4f 100644 --- a/lib/rubocop/cop/badge.rb +++ b/lib/rubocop/cop/badge.rb @@ -10,37 +10,23 @@ module Cop # allow for badge references in source files that omit the department for # RuboCop to infer. class Badge - # Error raised when a badge parse fails. - class InvalidBadge < Error - MSG = 'Invalid badge %p. ' \ - 'Expected `Department/CopName` or `CopName`.' - - def initialize(token) - super(format(MSG, badge: token)) - end - end - attr_reader :department, :cop_name def self.for(class_name) - new(*class_name.split('::').last(2)) + parts = class_name.split('::') + parts_without_first_two = parts[2..-1] + name_deep_enough = parts_without_first_two && parts_without_first_two.length >= 2 + new(name_deep_enough ? parts_without_first_two : parts.last(2)) end def self.parse(identifier) - parts = identifier.split('/', 2) - - raise InvalidBadge, identifier if parts.size > 2 - - if parts.one? - new(nil, *parts) - else - new(*parts) - end + new(identifier.split('/')) end - def initialize(department, cop_name) - @department = department.to_sym if department - @cop_name = cop_name + def initialize(class_name_parts) + department_parts = class_name_parts[0...-1] + @department = (department_parts.join('/').to_sym unless department_parts.empty?) + @cop_name = class_name_parts.last end def ==(other) @@ -66,7 +52,7 @@ def qualified? end def with_department(department) - self.class.new(department, cop_name) + self.class.new([department.to_s.split('/'), cop_name].flatten) end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index b519e685191..08682e3c8d9 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -15,8 +15,11 @@ let(:configuration_keys) { config.keys } + # TODO: %w[RSpec/Rails/HttpStatus] must be removed after rubocop-rspec is updated to reflect + # https://github.com/rubocop-hq/rubocop/pull/8490 it 'has configuration for all cops' do - expect(configuration_keys).to match_array(%w[AllCops] + cop_names) + expect(configuration_keys) + .to match_array(%w[AllCops] + cop_names + %w[RSpec/Rails/HttpStatus]) end it 'has a nicely formatted description for all cops' do diff --git a/spec/rubocop/cop/badge_spec.rb b/spec/rubocop/cop/badge_spec.rb index c2338ef6e76..9ee31f2179d 100644 --- a/spec/rubocop/cop/badge_spec.rb +++ b/spec/rubocop/cop/badge_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Badge do - subject(:badge) { described_class.new('Test', 'ModuleMustBeAClassCop') } + subject(:badge) { described_class.new(%w[Test ModuleMustBeAClassCop]) } it 'exposes department name' do expect(badge.department).to be(:Test) @@ -11,34 +11,59 @@ expect(badge.cop_name).to eql('ModuleMustBeAClassCop') end - describe '.parse' do - it 'parses Department/CopName syntax' do - expect(described_class.parse('Foo/Bar')) - .to eq(described_class.new('Foo', 'Bar')) + describe '.new' do + shared_examples 'assignment of department and name' do |class_name_parts, department, name| + it 'assigns department' do + expect(described_class.new(class_name_parts).department).to eq(department) + end + + it 'assigns name' do + expect(described_class.new(class_name_parts).cop_name).to eq(name) + end end - it 'parses unqualified badge references' do - expect(described_class.parse('Bar')) - .to eql(described_class.new(nil, 'Bar')) + include_examples 'assignment of department and name', %w[Foo], nil, 'Foo' + include_examples 'assignment of department and name', %w[Foo Bar], :Foo, 'Bar' + include_examples 'assignment of department and name', %w[Foo Bar Baz], :'Foo/Bar', 'Baz' + include_examples 'assignment of department and name', %w[Foo Bar Baz Qux], :'Foo/Bar/Baz', 'Qux' + end + + describe '.parse' do + shared_examples 'cop identifier parsing' do |identifier, class_name_parts| + it 'parses identifier' do + expect(described_class.parse(identifier)).to eq(described_class.new(class_name_parts)) + end end + + include_examples 'cop identifier parsing', 'Bar', %w[Bar] + include_examples 'cop identifier parsing', 'Foo/Bar', %w[Foo Bar] + include_examples 'cop identifier parsing', 'Foo/Bar/Baz', %w[Foo Bar Baz] + include_examples 'cop identifier parsing', 'Foo/Bar/Baz/Qux', %w[Foo Bar Baz Qux] end describe '.for' do - it 'parses cop class name' do - expect(described_class.for('RuboCop::Cop::Foo::Bar')) - .to eq(described_class.new('Foo', 'Bar')) + shared_examples 'cop class name parsing' do |class_name, class_name_parts| + it 'parses cop class name' do + expect(described_class.for(class_name)).to eq(described_class.new(class_name_parts)) + end end + + include_examples 'cop class name parsing', 'Foo', %w[Foo] + include_examples 'cop class name parsing', 'Foo::Bar', %w[Foo Bar] + include_examples 'cop class name parsing', 'RuboCop::Cop::Foo', %w[Cop Foo] + include_examples 'cop class name parsing', 'RuboCop::Cop::Foo::Bar', %w[Foo Bar] + include_examples 'cop class name parsing', 'RuboCop::Cop::Foo::Bar::Baz', %w[Foo Bar Baz] end it 'compares by value' do - badge1 = described_class.new('Foo', 'Bar') - badge2 = described_class.new('Foo', 'Bar') + badge1 = described_class.new(%w[Foo Bar]) + badge2 = described_class.new(%w[Foo Bar]) expect(Set.new([badge1, badge2]).one?).to be(true) end it 'can be converted to a string with the Department/CopName format' do - expect(described_class.new('Foo', 'Bar').to_s).to eql('Foo/Bar') + expect(described_class.new(%w[Foo Bar]).to_s).to eql('Foo/Bar') end describe '#qualified?' do @@ -49,5 +74,9 @@ it 'says `Department/CopName` is qualified' do expect(described_class.parse('Department/Bar').qualified?).to be(true) end + + it 'says `Deep/Department/CopName` is qualified' do + expect(described_class.parse('Deep/Department/Bar').qualified?).to be(true) + end end end diff --git a/spec/rubocop/cop/registry_spec.rb b/spec/rubocop/cop/registry_spec.rb index fe73c800bc3..bb5bbb60b94 100644 --- a/spec/rubocop/cop/registry_spec.rb +++ b/spec/rubocop/cop/registry_spec.rb @@ -172,6 +172,24 @@ it 'exposes a list of cops' do expect(registry.cops).to eql(cops) end + + context 'with cops having the same inner-most module' do + let(:cops) do + [ + RuboCop::Cop::Foo::Bar, + RuboCop::Cop::Baz::Foo::Bar + ] + end + + before do + stub_const('RuboCop::Cop::Baz::Foo::Bar', Class.new(RuboCop::Cop::Base)) + stub_const('RuboCop::Cop::Foo::Bar', Class.new(RuboCop::Cop::Base)) + end + + it 'exposes both cops' do + expect(registry.cops).to eql(cops) + end + end end it 'exposes the number of stored cops' do