From 5a7b8f5ade59361945d545ff6a0732b6788e58ee Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Wed, 19 May 2021 09:11:07 +0300 Subject: [PATCH] Unify DirectiveComment specs --- lib/rubocop/directive_comment.rb | 11 +- spec/rubocop/directive_comment_spec.rb | 356 +++++++++++++------------ 2 files changed, 187 insertions(+), 180 deletions(-) diff --git a/lib/rubocop/directive_comment.rb b/lib/rubocop/directive_comment.rb index 0cfa28843f9..0231be5b1ee 100644 --- a/lib/rubocop/directive_comment.rb +++ b/lib/rubocop/directive_comment.rb @@ -25,10 +25,11 @@ def self.before_comment(line) line.split(DIRECTIVE_COMMENT_REGEXP).first end - attr_reader :comment, :mode, :cops + attr_reader :comment, :cop_registry, :mode, :cops - def initialize(comment) + def initialize(comment, cop_registry = Cop::Registry.global) @comment = comment + @cop_registry = cop_registry @mode, @cops = match_captures end @@ -123,15 +124,15 @@ def parsed_cop_names end def department?(name) - Cop::Registry.global.department?(name) + cop_registry.department?(name) end def all_cop_names - exclude_redundant_directive_cop(Cop::Registry.global.names) + exclude_redundant_directive_cop(cop_registry.names) end def cop_names_for_department(department) - names = Cop::Registry.global.names_for_department(department) + names = cop_registry.names_for_department(department) has_redundant_directive_cop = department == REDUNDANT_DIRECTIVE_COP_DEPARTMENT has_redundant_directive_cop ? exclude_redundant_directive_cop(names) : names end diff --git a/spec/rubocop/directive_comment_spec.rb b/spec/rubocop/directive_comment_spec.rb index 2d7eac4feb4..dc60a94cb32 100644 --- a/spec/rubocop/directive_comment_spec.rb +++ b/spec/rubocop/directive_comment_spec.rb @@ -1,309 +1,330 @@ # frozen_string_literal: true RSpec.describe RuboCop::DirectiveComment do - subject(:directive_comment) { described_class.new(comment) } - + let(:directive_comment) { described_class.new(comment, cop_registry) } let(:comment) { instance_double(Parser::Source::Comment, text: text) } - let(:comment_cop_names) { 'all' } - let(:text) { "#rubocop:enable #{comment_cop_names}" } + let(:cop_registry) do + instance_double(RuboCop::Cop::Registry, names: all_cop_names, department?: department?) + end + let(:text) { '#rubocop:enable all' } + let(:all_cop_names) { %w[] } + let(:department?) { false } describe '.before_comment' do subject { described_class.before_comment(text) } - [ - ['when line has code', 'def foo # rubocop:disable all', 'def foo '], - ['when line has NO code', '# rubocop:disable all', ''] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when line has code' do + let(:text) { 'def foo # rubocop:disable all' } - it { is_expected.to eq example[2] } - end + it { is_expected.to eq('def foo ') } + end + + context 'when line has NO code' do + let(:text) { '# rubocop:disable all' } + + it { is_expected.to eq('') } end end describe '#match?' do - subject(:match) { directive_comment.match?(cop_names) } + subject { directive_comment.match?(cop_names) } - let(:comment_cop_names) { 'Metrics/AbcSize, Metrics/PerceivedComplexity, Style/Not' } + let(:text) { '#rubocop:enable Metrics/AbcSize, Metrics/PerceivedComplexity, Style/Not' } - context 'no comment_cop_names' do + context 'when there are no cop names' do let(:cop_names) { [] } - it 'returns false' do - expect(match).to eq(false) - end + it { is_expected.to eq(false) } end - context 'same cop names as in the comment' do + context 'when cop names are same as in the comment' do let(:cop_names) { %w[Metrics/AbcSize Metrics/PerceivedComplexity Style/Not] } - it 'returns true' do - expect(match).to eq(true) - end + it { is_expected.to eq(true) } end - context 'same cop names as in the comment in a different order' do + context 'when cop names are same but in a different order' do let(:cop_names) { %w[Style/Not Metrics/AbcSize Metrics/PerceivedComplexity] } - it 'returns true' do - expect(match).to eq(true) - end + it { is_expected.to eq(true) } end - context 'subset of names' do + context 'when cop names are subset of names' do let(:cop_names) { %w[Metrics/AbcSize Style/Not] } - it 'returns false' do - expect(match).to eq(false) - end + it { is_expected.to eq(false) } end - context 'superset of names' do + context 'when cop names are superset of names' do let(:cop_names) { %w[Lint/Void Metrics/AbcSize Metrics/PerceivedComplexity Style/Not] } - it 'returns false' do - expect(match).to eq(false) - end + it { is_expected.to eq(false) } end - context 'duplicate names' do + context 'when cop names are same but have duplicated names' do let(:cop_names) { %w[Metrics/AbcSize Metrics/AbcSize Metrics/PerceivedComplexity Style/Not] } - it 'returns true' do - expect(match).to eq(true) - end + it { is_expected.to eq(true) } end - context 'all' do - let(:comment_cop_names) { 'all' } + context 'when disabled all cops' do + let(:text) { '#rubocop:enable all' } let(:cop_names) { %w[all] } - it 'returns true' do - expect(match).to eq(true) - end + it { is_expected.to eq(true) } end end describe '#match_captures' do subject { directive_comment.match_captures } - [ - ['when disable', '# rubocop:disable all', ['disable', 'all', nil, nil]], - ['when enable', '# rubocop:enable Foo/Bar', ['enable', 'Foo/Bar', nil, 'Foo/']], - ['when todo', '# rubocop:todo all', ['todo', 'all', nil, nil]], - ['when typo', '# rudocop:todo Dig/ThisMine', nil] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when disable' do + let(:text) { '# rubocop:disable all' } + + it { is_expected.to eq(['disable', 'all', nil, nil]) } + end - it { is_expected.to eq example[2] } - end + context 'when enable' do + let(:text) { '# rubocop:enable Foo/Bar' } + + it { is_expected.to eq(['enable', 'Foo/Bar', nil, 'Foo/']) } + end + + context 'when todo' do + let(:text) { '# rubocop:todo all' } + + it { is_expected.to eq(['todo', 'all', nil, nil]) } + end + + context 'when typo' do + let(:text) { '# rudocop:todo Dig/ThisMine' } + + it { is_expected.to eq(nil) } end end describe '#single_line?' do subject { directive_comment.single_line? } - [ - ['when relates to single line', 'def foo # rubocop:disable all', true], - ['when does NOT relate to single line', '# rubocop:disable all', false] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when relates to single line' do + let(:text) { 'def foo # rubocop:disable all' } - it { is_expected.to eq example[2] } - end + it { is_expected.to eq(true) } + end + + context 'when does NOT relate to single line' do + let(:text) { '# rubocop:disable all' } + + it { is_expected.to eq(false) } end end describe '#disabled?' do subject { directive_comment.disabled? } - [ - ['when disable', '# rubocop:disable all', true], - ['when enable', '# rubocop:enable Foo/Bar', false], - ['when todo', '# rubocop:todo all', true] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when disable' do + let(:text) { '# rubocop:disable all' } - it { is_expected.to eq example[2] } - end + it { is_expected.to eq(true) } + end + + context 'when enable' do + let(:text) { '# rubocop:enable Foo/Bar' } + + it { is_expected.to eq(false) } + end + + context 'when todo' do + let(:text) { '# rubocop:todo all' } + + it { is_expected.to eq(true) } end end describe '#enabled?' do subject { directive_comment.enabled? } - [ - ['when disable', '# rubocop:disable all', false], - ['when enable', '# rubocop:enable Foo/Bar', true], - ['when todo', '# rubocop:todo all', false] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when disable' do + let(:text) { '# rubocop:disable all' } + + it { is_expected.to eq(false) } + end - it { is_expected.to eq example[2] } - end + context 'when enable' do + let(:text) { '# rubocop:enable Foo/Bar' } + + it { is_expected.to eq(true) } + end + + context 'when todo' do + let(:text) { '# rubocop:todo all' } + + it { is_expected.to eq(false) } end end describe '#all_cops?' do subject { directive_comment.all_cops? } - [ - ['when mentioned all', '# rubocop:disable all', true], - ['when mentioned specific cops', '# rubocop:enable Foo/Bar', false] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when mentioned all' do + let(:text) { '# rubocop:disable all' } - it { is_expected.to eq example[2] } - end + it { is_expected.to eq(true) } end - end - describe '#cop_names' do - subject { directive_comment.cop_names } + context 'when mentioned specific cops' do + let(:text) { '# rubocop:enable Foo/Bar' } - let(:all_cop_names) { %w[] } - let(:department?) { false } - let(:global) do - instance_double(RuboCop::Cop::Registry, names: all_cop_names, department?: department?) + it { is_expected.to eq(false) } end + end - before { allow(RuboCop::Cop::Registry).to receive(:global).and_return(global) } + describe '#cop_names' do + subject { directive_comment.cop_names } context 'when only cop specified' do - let(:comment_cop_names) { 'Foo/Bar' } + let(:text) { '#rubocop:enable Foo/Bar' } - it { is_expected.to eq %w[Foo/Bar] } + it { is_expected.to eq(%w[Foo/Bar]) } end context 'when all cops mentioned' do - let(:comment_cop_names) { 'all' } + let(:text) { '#rubocop:enable all' } let(:all_cop_names) { %w[all_names Lint/RedundantCopDisableDirective] } - it { is_expected.to eq %w[all_names] } + it { is_expected.to eq(%w[all_names]) } end context 'when only department specified' do - let(:comment_cop_names) { 'Foo' } + let(:text) { '#rubocop:enable Foo' } let(:department?) { true } before do - allow(global).to receive(:names_for_department).with('Foo').and_return(%w[Foo/Bar Foo/Baz]) + allow(cop_registry).to receive(:names_for_department) + .with('Foo').and_return(%w[Foo/Bar Foo/Baz]) end - it { is_expected.to eq %w[Foo/Bar Foo/Baz] } + it { is_expected.to eq(%w[Foo/Bar Foo/Baz]) } end context 'when couple departments specified' do - let(:comment_cop_names) { 'Foo, Baz' } + let(:text) { '#rubocop:enable Foo, Baz' } let(:department?) { true } before do - allow(global).to receive(:names_for_department).with('Foo').and_return(%w[Foo/Bar Foo/Baz]) - allow(global).to receive(:names_for_department).with('Baz').and_return(%w[Baz/Bar]) + allow(cop_registry).to receive(:names_for_department).with('Baz').and_return(%w[Baz/Bar]) + allow(cop_registry).to receive(:names_for_department) + .with('Foo').and_return(%w[Foo/Bar Foo/Baz]) end - it { is_expected.to eq %w[Foo/Bar Foo/Baz Baz/Bar] } + it { is_expected.to eq(%w[Foo/Bar Foo/Baz Baz/Bar]) } end context 'when department and cops specified' do - let(:comment_cop_names) { 'Foo, Baz/Cop' } + let(:text) { '#rubocop:enable Foo, Baz/Cop' } before do - allow(global).to receive(:department?).with('Foo').and_return(true) - allow(global).to receive(:names_for_department).with('Foo').and_return(%w[Foo/Bar Foo/Baz]) + allow(cop_registry).to receive(:department?).with('Foo').and_return(true) + allow(cop_registry).to receive(:names_for_department) + .with('Foo').and_return(%w[Foo/Bar Foo/Baz]) end - it { is_expected.to eq %w[Foo/Bar Foo/Baz Baz/Cop] } + it { is_expected.to eq(%w[Foo/Bar Foo/Baz Baz/Cop]) } end context 'when redundant directive cop department specified' do - let(:comment_cop_names) { 'Lint' } + let(:text) { '#rubocop:enable Lint' } let(:department?) { true } before do - allow(global).to receive(:names_for_department) + allow(cop_registry).to receive(:names_for_department) .with('Lint').and_return(%w[Lint/One Lint/Two Lint/RedundantCopDisableDirective]) end - it { is_expected.to eq %w[Lint/One Lint/Two] } + it { is_expected.to eq(%w[Lint/One Lint/Two]) } end end describe '#department_names' do subject { directive_comment.department_names } - let(:department?) { false } - let(:global) { instance_double(RuboCop::Cop::Registry, department?: department?) } - - before { allow(RuboCop::Cop::Registry).to receive(:global).and_return(global) } - context 'when only cop specified' do - let(:comment_cop_names) { 'Foo/Bar' } + let(:text) { '#rubocop:enable Foo/Bar' } - it { is_expected.to eq [] } + it { is_expected.to eq([]) } end context 'when all cops mentioned' do - let(:comment_cop_names) { 'all' } + let(:text) { '#rubocop:enable all' } - it { is_expected.to eq [] } + it { is_expected.to eq([]) } end context 'when only department specified' do - let(:comment_cop_names) { 'Foo' } + let(:text) { '#rubocop:enable Foo' } let(:department?) { true } - it { is_expected.to eq %w[Foo] } + it { is_expected.to eq(%w[Foo]) } end context 'when couple departments specified' do - let(:comment_cop_names) { 'Foo, Baz' } + let(:text) { '#rubocop:enable Foo, Baz' } let(:department?) { true } - it { is_expected.to eq %w[Foo Baz] } + it { is_expected.to eq(%w[Foo Baz]) } end context 'when department and cops specified' do - let(:comment_cop_names) { 'Foo, Baz/Cop' } + let(:text) { '#rubocop:enable Foo, Baz/Cop' } before do - allow(global).to receive(:department?).with('Foo').and_return(true) + allow(cop_registry).to receive(:department?).with('Foo').and_return(true) end - it { is_expected.to eq %w[Foo] } + it { is_expected.to eq(%w[Foo]) } end end describe '#line_number' do - let(:comment) { instance_double(Parser::Source::Comment, text: text, loc: loc) } - let(:loc) { instance_double(Parser::Source::Map, expression: expression) } - let(:expression) { instance_double(Parser::Source::Range, line: 1) } + let(:loc) do + instance_double( + Parser::Source::Map, + expression: instance_double(Parser::Source::Range, line: 1) + ) + end + + before { allow(comment).to receive(:loc).and_return(loc) } it 'returns line number for directive' do - expect(directive_comment.line_number).to be 1 + expect(directive_comment.line_number).to eq(1) end end describe '#enabled_all?' do subject { directive_comment.enabled_all? } - [ - ['when enabled all cops', 'def foo # rubocop:enable all', true], - ['when enabled specific cops', '# rubocop:enable Foo/Bar', false], - ['when disabled all cops', '# rubocop:disable all', false], - ['when disabled specific cops', '# rubocop:disable Foo/Bar', false] - ].each do |example| - context example[0] do - let(:text) { example[1] } + context 'when enabled all cops' do + let(:text) { 'def foo # rubocop:enable all' } - it { is_expected.to eq example[2] } - end + it { is_expected.to eq(true) } + end + + context 'when enabled specific cops' do + let(:text) { '# rubocop:enable Foo/Bar' } + + it { is_expected.to eq(false) } + end + + context 'when disabled all cops' do + let(:text) { '# rubocop:disable all' } + + it { is_expected.to eq(false) } + end + + context 'when disabled specific cops' do + let(:text) { '# rubocop:disable Foo/Bar' } + + it { is_expected.to eq(false) } end end @@ -313,25 +334,25 @@ context 'when enabled all cops' do let(:text) { 'def foo # rubocop:enable all' } - it { is_expected.to eq false } + it { is_expected.to eq(false) } end context 'when enabled specific cops' do let(:text) { '# rubocop:enable Foo/Bar' } - it { is_expected.to eq false } + it { is_expected.to eq(false) } end context 'when disabled all cops' do let(:text) { '# rubocop:disable all' } - it { is_expected.to eq true } + it { is_expected.to eq(true) } end context 'when disabled specific cops' do let(:text) { '# rubocop:disable Foo/Bar' } - it { is_expected.to eq false } + it { is_expected.to eq(false) } end end @@ -341,90 +362,75 @@ context 'when few cops used' do let(:text) { '# rubocop:enable Foo/Bar, Foo/Baz' } - it { is_expected.to eq 2 } + it { is_expected.to eq(2) } end context 'when few department used' do let(:text) { '# rubocop:enable Foo, Bar, Baz' } - it { is_expected.to eq 3 } + it { is_expected.to eq(3) } end context 'when cops and departments used' do let(:text) { '# rubocop:enable Foo/Bar, Foo/Baz, Bar, Baz' } - it { is_expected.to eq 4 } + it { is_expected.to eq(4) } end end describe '#in_directive_department?' do - subject { directive_comment.in_directive_department?(cop) } - - let(:global) { instance_double(RuboCop::Cop::Registry, department?: department?) } - - before { allow(RuboCop::Cop::Registry).to receive(:global).and_return(global) } + subject { directive_comment.in_directive_department?('Foo/Bar') } context 'when cop department disabled' do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Foo' } let(:department?) { true } - it { is_expected.to be true } + it { is_expected.to eq(true) } end context 'when another department disabled' do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Bar' } let(:department?) { true } - it { is_expected.to be false } + it { is_expected.to eq(false) } end context 'when cop disabled' do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Foo/Bar' } - let(:department?) { false } - it { is_expected.to be false } + it { is_expected.to eq(false) } end end describe '#overridden_by_department?' do - subject { directive_comment.overridden_by_department?(cop) } - - let(:global) { instance_double(RuboCop::Cop::Registry, department?: false) } + subject { directive_comment.overridden_by_department?('Foo/Bar') } before do - allow(RuboCop::Cop::Registry).to receive(:global).and_return(global) - allow(global).to receive(:department?).with('Foo').and_return(true) + allow(cop_registry).to receive(:department?).with('Foo').and_return(true) end context "when cop is overridden by it's department" do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Foo, Foo/Bar' } - it { is_expected.to be true } + it { is_expected.to eq(true) } end context "when cop is not overridden by it's department" do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Bar, Foo/Bar' } - it { is_expected.to be false } + it { is_expected.to eq(false) } end context 'when there are no departments' do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Foo/Bar' } - it { is_expected.to be false } + it { is_expected.to eq(false) } end context 'when there are no cops' do - let(:cop) { 'Foo/Bar' } let(:text) { '# rubocop:enable Foo' } - it { is_expected.to be false } + it { is_expected.to eq(false) } end end end