Lint/RedundantCopEnableDirective bad autocorrection

[marcandre]

I removed one line of code that changed the complexity of a method enough that the disabling was no longer necessary.

Autocorrection left me with an empty # rubocop enable:

$ bundle exec rubocop -a lib/rubocop/cop/lint/duplicate_methods.rb
Inspecting 1 file
W

Offenses:

lib/rubocop/cop/lint/duplicate_methods.rb:103:9: W: [Corrected] Lint/RedundantCopDisableDirective: Unnecessary disabling of Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity.
        # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/rubocop/cop/lint/duplicate_methods.rb:114:26: W: [Corrected] Lint/RedundantCopEnableDirective: Unnecessary enabling of Metrics/CyclomaticComplexity.
        # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/rubocop/cop/lint/duplicate_methods.rb:114:56: W: [Corrected] Lint/RedundantCopEnableDirective: Unnecessary enabling of Metrics/PerceivedComplexity.
        # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
                                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 3 offenses detected, 3 offenses corrected

$ git diff
diff --git a/lib/rubocop/cop/lint/duplicate_methods.rb b/lib/rubocop/cop/lint/duplicate_methods.rb
index 31c99cb15..079afd9de 100644
--- a/lib/rubocop/cop/lint/duplicate_methods.rb
+++ b/lib/rubocop/cop/lint/duplicate_methods.rb
@@ -99,8 +99,6 @@ module RuboCop
         PATTERN
 
         def_node_matcher :sym_name, '(sym $_name)'
-
-        # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
         def on_send(node)
           return unless METHOD_DEF_METHODS.include?(node.method_name)
 
@@ -113,7 +111,7 @@ module RuboCop
             on_attr(node, *attr)
           end
         end
-        # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+        # rubocop:enable
 
         private

Actual original code

# frozen_string_literal: true

module RuboCop
  module Cop
    module Lint
      # This cop checks for duplicated instance (or singleton) method
      # definitions.
      #
      # @example
      #
      #   # bad
      #
      #   def foo
      #     1
      #   end
      #
      #   def foo
      #     2
      #   end
      #
      # @example
      #
      #   # bad
      #
      #   def foo
      #     1
      #   end
      #
      #   alias foo bar
      #
      # @example
      #
      #   # good
      #
      #   def foo
      #     1
      #   end
      #
      #   def bar
      #     2
      #   end
      #
      # @example
      #
      #   # good
      #
      #   def foo
      #     1
      #   end
      #
      #   alias bar foo
      class DuplicateMethods < Cop
        MSG = 'Method `%<method>s` is defined at both %<defined>s and ' \
              '%<current>s.'

        METHOD_DEF_METHODS = %i[alias_method attr_reader attr_writer
                                attr_accessor attr].to_set.freeze

        def initialize(config = nil, options = nil)
          super
          @definitions = {}
        end

        def on_def(node)
          # if a method definition is inside an if, it is very likely
          # that a different definition is used depending on platform, etc.
          return if node.ancestors.any?(&:if_type?)
          return if possible_dsl?(node)

          found_instance_method(node, node.method_name)
        end

        def on_defs(node)
          return if node.ancestors.any?(&:if_type?)
          return if possible_dsl?(node)

          if node.receiver.const_type?
            _, const_name = *node.receiver
            check_const_receiver(node, node.method_name, const_name)
          elsif node.receiver.self_type?
            check_self_receiver(node, node.method_name)
          end
        end

        def_node_matcher :method_alias?, <<~PATTERN
          (alias (sym $_name) sym)
        PATTERN

        def on_alias(node)
          return unless (name = method_alias?(node))
          return if node.ancestors.any?(&:if_type?)
          return if possible_dsl?(node)

          found_instance_method(node, name)
        end

        def_node_matcher :alias_method?, <<~PATTERN
          (send nil? :alias_method (sym $_name) _)
        PATTERN

        def_node_matcher :sym_name, '(sym $_name)'

        # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
        def on_send(node)
          return unless METHOD_DEF_METHODS.include?(node.method_name)

          if (name = alias_method?(node))
            return if node.ancestors.any?(&:if_type?)
            return if possible_dsl?(node)

            found_instance_method(node, name)
          elsif (attr = node.attribute_accessor?)
            on_attr(node, *attr)
          end
        end
        # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

        private

        def check_const_receiver(node, name, const_name)
          qualified = lookup_constant(node, const_name)
          return unless qualified

          found_method(node, "#{qualified}.#{name}")
        end

        def check_self_receiver(node, name)
          enclosing = node.parent_module_name
          return unless enclosing

          found_method(node, "#{enclosing}.#{name}")
        end

        def message_for_dup(node, method_name)
          format(MSG, method: method_name, defined: source_location(@definitions[method_name]),
                      current: source_location(node))
        end

        def found_instance_method(node, name)
          return unless (scope = node.parent_module_name)

          if scope =~ /\A#<Class:(.*)>\Z/
            found_method(node, "#{Regexp.last_match(1)}.#{name}")
          else
            found_method(node, "#{scope}##{name}")
          end
        end

        def found_method(node, method_name)
          if @definitions.key?(method_name)
            loc = case node.type
                  when :def, :defs
                    node.loc.keyword.join(node.loc.name)
                  else
                    node.loc.expression
                  end
            message = message_for_dup(node, method_name)

            add_offense(node, location: loc, message: message)
          else
            @definitions[method_name] = node
          end
        end

        def on_attr(node, attr_name, args)
          case attr_name
          when :attr
            writable = args.size == 2 && args.last.true_type?
            found_attr(node, [args.first], readable: true, writable: writable)
          when :attr_reader
            found_attr(node, args, readable: true)
          when :attr_writer
            found_attr(node, args, writable: true)
          when :attr_accessor
            found_attr(node, args, readable: true, writable: true)
          end
        end

        def found_attr(node, args, readable: false, writable: false)
          args.each do |arg|
            name = sym_name(arg)
            next unless name

            found_instance_method(node, name) if readable
            found_instance_method(node, "#{name}=") if writable
          end
        end

        def lookup_constant(node, const_name)
          # this method is quite imperfect and can be fooled
          # to do much better, we would need to do global analysis of the whole
          # codebase
          node.each_ancestor(:class, :module, :casgn) do |ancestor|
            namespace, mod_name = *ancestor.defined_module
            loop do
              if mod_name == const_name
                return qualified_name(ancestor.parent_module_name,
                                      namespace,
                                      mod_name)
              end

              break if namespace.nil?

              namespace, mod_name = *namespace
            end
          end
        end

        def qualified_name(enclosing, namespace, mod_name)
          if enclosing != 'Object'
            if namespace
              "#{enclosing}::#{namespace.const_name}::#{mod_name}"
            else
              "#{enclosing}::#{mod_name}"
            end
          elsif namespace
            "#{namespace.const_name}::#{mod_name}"
          else
            mod_name
          end
        end

        def possible_dsl?(node)
          # DSL methods may evaluate a block in the context of a newly created
          # class or module
          # Assume that if a method definition is inside any block call which
          # we can't identify, it could be a DSL
          node.each_ancestor(:block).any? do |ancestor|
            ancestor.method_name != :class_eval && !ancestor.class_constructor?
          end
        end

        def source_location(node)
          range = node.location.expression
          path = smart_path(range.source_buffer.name)
          "#{path}:#{range.line}"
        end
      end
    end
  end
end

[marcandre]

Again today on lib/rubocop/cop/style/hash_syntax.rb @ 7685308

[marcandre]

Also: Lint/RedundantCopDisableDirective should not remove the line preceding the # rubocop:disable, as shown in the example.