From f306205a30ab6ba3670bce06bece46dba5031e3c Mon Sep 17 00:00:00 2001 From: Garett Arrowood Date: Sun, 31 Dec 2017 16:03:05 -0500 Subject: [PATCH] [Fix #5356] Add new Lint/UnneededCopEnableDirective cop --- config/enabled.yml | 5 + lib/rubocop.rb | 1 + lib/rubocop/comment_config.rb | 21 ++++ .../cop/lint/unneeded_cop_enable_directive.rb | 77 +++++++++++++ spec/rubocop/cli_spec.rb | 5 +- .../unneeded_cop_enable_directive_spec.rb | 109 ++++++++++++++++++ 6 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 lib/rubocop/cop/lint/unneeded_cop_enable_directive.rb create mode 100644 spec/rubocop/cop/lint/unneeded_cop_enable_directive_spec.rb diff --git a/config/enabled.yml b/config/enabled.yml index ee95f3fb19f3..70f90c8ce0b2 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -690,6 +690,11 @@ Lint/UnneededCopDisableDirective: It must be explicitly disabled. Enabled: true +Lint/UnneededCopEnableDirective: + Description: Checks for rubocop:enable comments that can be removed. + + Enabled: true + Lint/UnneededRequireStatement: Description: 'Checks for unnecessary `require` statement.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 7a43df95f892..6bc539fa6a44 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -296,6 +296,7 @@ require_relative 'rubocop/cop/lint/underscore_prefixed_variable_name' require_relative 'rubocop/cop/lint/unified_integer' require_relative 'rubocop/cop/lint/unneeded_cop_disable_directive' +require_relative 'rubocop/cop/lint/unneeded_cop_enable_directive' require_relative 'rubocop/cop/lint/unneeded_require_statement' require_relative 'rubocop/cop/lint/unneeded_splat_expansion' require_relative 'rubocop/cop/lint/unreachable_code' diff --git a/lib/rubocop/comment_config.rb b/lib/rubocop/comment_config.rb index 0ca3523be2c4..5eb148a43b54 100644 --- a/lib/rubocop/comment_config.rb +++ b/lib/rubocop/comment_config.rb @@ -34,8 +34,29 @@ def cop_disabled_line_ranges @cop_disabled_line_ranges ||= analyze end + def extra_enabled_comments + extra_enabled_comments_with_names([], {}) + end + private + def extra_enabled_comments_with_names(extras, names) + each_directive do |comment, cop_names, disabled| + next unless comment_only_line?(comment.loc.expression.line) + cop_names.each do |name| + names[name] ||= 0 + if disabled + names[name] += 1 + elsif names[name] > 0 + names[name] -= 1 + else + extras << [comment, name] + end + end + end + extras + end + def analyze analyses = Hash.new { |hash, key| hash[key] = CopAnalysis.new([], nil) } diff --git a/lib/rubocop/cop/lint/unneeded_cop_enable_directive.rb b/lib/rubocop/cop/lint/unneeded_cop_enable_directive.rb new file mode 100644 index 000000000000..9a0d44d2ec81 --- /dev/null +++ b/lib/rubocop/cop/lint/unneeded_cop_enable_directive.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +# The Lint/UnneededCopEnableDirective cop needs to be disabled so as +# to be able to provide a (bad) example of an unneeded enable. + +# rubocop:disable Lint/UnneededCopEnableDirective +module RuboCop + module Cop + module Lint + # This cop detects instances of rubocop:enable comments that can be + # removed. + # + # @example + # # bad + # foo = 1 + # # rubocop:enable Metrics/LineLength + # + # # good + # foo = 1 + class UnneededCopEnableDirective < Cop + include RangeHelp + + MSG = 'Unnecessary enabling of %s.'.freeze + + def investigate(processed_source) + return if processed_source.blank? + offenses = processed_source.comment_config.extra_enabled_comments + offenses.each do |comment, name| + add_offense( + [comment, name], + location: range_of_offense(comment, name), + message: format(MSG, cop: name) + ) + end + end + + def autocorrect(comment_and_name) + lambda do |corrector| + comment, name = *comment_and_name + range = range_of_offense(*comment_and_name) + index = cop_name_indention(comment, name) + make_corrections(corrector, comment, name, range, index) + end + end + + private + + def range_of_offense(comment, name) + comment_start = comment.loc.expression.begin_pos + offense_start = comment_start + cop_name_indention(comment, name) + range_between(offense_start, offense_start + name.size) + end + + def cop_name_indention(comment, name) + comment.text.index(name) + end + + # rubocop:disable Metrics/AbcSize + def make_corrections(corrector, comment, name, range, index) + if comment.text[index - 2] == ',' + corrector.remove( + range_between(range.begin_pos - 2, range.end_pos) + ) + elsif comment.text[index + name.size] == ',' + corrector.remove( + range_between(range.begin_pos, range.end_pos + 2) + ) + else + corrector.remove(comment.loc.expression) + end + end + # rubocop:enable Metrics/AbcSize + end + end + end +end +# rubocop:enable Lint/UnneededCopEnableDirective diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 19d84b55dd48..f637055d71a0 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -302,9 +302,10 @@ def and_with_args .to eq(['example.rb: Style/LineLength has the wrong ' \ 'namespace - should be Metrics', ''].join("\n")) - # 3 cops were disabled, then 2 were enabled again, so we - # should get 2 offenses reported. + # 2 real cops were disabled, and 1 that was incorrect + # 2 real cops was enabled, but only 1 had been disabled correctly expect($stdout.string).to eq(<<-RESULT.strip_indent) + #{abs('example.rb')}:6:21: W: Lint/UnneededCopEnableDirective: Unnecessary enabling of Metrics/LineLength. #{abs('example.rb')}:7:81: C: Metrics/LineLength: Line is too long. [95/80] #{abs('example.rb')}:9:5: C: Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols. RESULT diff --git a/spec/rubocop/cop/lint/unneeded_cop_enable_directive_spec.rb b/spec/rubocop/cop/lint/unneeded_cop_enable_directive_spec.rb new file mode 100644 index 000000000000..37b80ddd5b5b --- /dev/null +++ b/spec/rubocop/cop/lint/unneeded_cop_enable_directive_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::UnneededCopEnableDirective do + subject(:cop) { described_class.new } + + it 'registers offense for unnecessary enable' do + expect_offense(<<-RUBY.strip_indent) + foo + # rubocop:enable Metrics/LineLength + ^^^^^^^^^^^^^^^^^^ Unnecessary enabling of Metrics/LineLength. + RUBY + end + + it 'registers multiple offenses for same comment' do + expect_offense(<<-RUBY.strip_indent) + foo + # rubocop:enable Metrics/ModuleLength, Metrics/AbcSize + ^^^^^^^^^^^^^^^ Unnecessary enabling of Metrics/AbcSize. + ^^^^^^^^^^^^^^^^^^^^ Unnecessary enabling of Metrics/ModuleLength. + bar + RUBY + end + + it 'registers correct offense when combined with necessary enable' do + expect_offense(<<-RUBY.strip_indent) + # rubocop:disable Metrics/LineLength + fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = barrrrrrrrrrrrrrrrrrrrrrrrrr + # rubocop:enable Metrics/AbcSize, Metrics/LineLength + ^^^^^^^^^^^^^^^ Unnecessary enabling of Metrics/AbcSize. + bar + RUBY + end + + context 'autocorrection' do + context 'when entire comment unnecessarily enables' do + let(:source) do + <<-RUBY.strip_indent + foo + # rubocop:enable Metrics/LineLength + RUBY + end + + it 'removes unnecessary enables' do + corrected = autocorrect_source(source) + expect(corrected).to eq(<<-RUBY.strip_indent) + foo + + RUBY + end + end + + context 'when first cop unnecessarily enables' do + let(:source) do + <<-RUBY.strip_indent + # rubocop:disable Metrics/LineLength + foo + # rubocop:enable Metrics/AbcSize, Metrics/LineLength + RUBY + end + + it 'removes unnecessary enables' do + corrected = autocorrect_source(source) + expect(corrected).to eq(<<-RUBY.strip_indent) + # rubocop:disable Metrics/LineLength + foo + # rubocop:enable Metrics/LineLength + RUBY + end + end + + context 'when last cop unnecessarily enables' do + let(:source) do + <<-RUBY.strip_indent + # rubocop:disable Metrics/LineLength + foo + # rubocop:enable Metrics/LineLength, Metrics/AbcSize + RUBY + end + + it 'removes unnecessary enables' do + corrected = autocorrect_source(source) + expect(corrected).to eq(<<-RUBY.strip_indent) + # rubocop:disable Metrics/LineLength + foo + # rubocop:enable Metrics/LineLength + RUBY + end + end + + context 'when middle cop unnecessarily enables' do + let(:source) do + <<-RUBY.strip_indent + # rubocop:disable Metrics/LineLength, Lint/Debugger + foo + # rubocop:enable Metrics/LineLength, Metrics/AbcSize, Lint/Debugger + RUBY + end + + it 'removes unnecessary enables' do + corrected = autocorrect_source(source) + expect(corrected).to eq(<<-RUBY.strip_indent) + # rubocop:disable Metrics/LineLength, Lint/Debugger + foo + # rubocop:enable Metrics/LineLength, Lint/Debugger + RUBY + end + end + end +end