Rubocop: Style/MethodMissing fallback on super doesn't make sense when you are responding to everything

Created on 1 Dec 2016 · 7Comments · Source: rubocop-hq/rubocop

The super is unreachable, although Style/MethodMissing demands it

        def method_missing(*_args)
          return []
          super
        end

Lint/UnreachableCode: Unreachable code detected.
          super
          ^^^^^

enhancement stale

Source

jcoyne

👍7

Most helpful comment

This still doesn't make sense with certain helper object patters like a black-hole NullObject:

class NullObject
  def nil?; true; end

  def !; true; end

  def ==(other)
    other.instance_of?(self.class) || other.nil?
  end

  def method_missing(name, *args, &block)
    self
  end

  def respond_to_missing?(m, include_private)
    true
  end
end

chrisnicola on 9 Jun 2017

👍12

All 7 comments

I have the same problem. In a presenter library, we use method missing to send private method calls to the view context:

private

# Delegates private method calls to the view context
def method_missing(method, *args, &block)
  context.public_send(method, *args, &block)
end

I get the message “When using method_missing, fall back on super”, but calling super makes no sense here.

cc/ @Drenmi

bquorning on 22 Dec 2016

There are actually two intimately connected parts to this:

Only catch methods with a well-defined prefix, such as find_by_* -- make your code as assertive as possible.

Call super at the end of your statement.

The first one is pretty error prone to check for though, because of the different ways this can be done. I can see how this is totally confusing when super is unreachable or superfluous though.

It is probably possible to detect when super doesn't make sense, but I'm more inclined to improve the violation message, because it:

Retains the advice that you shouldn't method_missing "all the things."
Keeps the cop simple.

WDYT?

Drenmi on 22 Dec 2016

👍1

So my example from above should be changed to:

def method_missing(method, *args, &block)
  if context.respond_to?(method)
    context.public_send(method, *args, &block)
  else
    super
  end
end

Yeah, that makes sense. Changing the violation message may be the way to go.

bquorning on 22 Dec 2016

👍2

This still doesn't make sense with certain helper object patters like a black-hole NullObject:

class NullObject
  def nil?; true; end

  def !; true; end

  def ==(other)
    other.instance_of?(self.class) || other.nil?
  end

  def method_missing(name, *args, &block)
    self
  end

  def respond_to_missing?(m, include_private)
    true
  end
end

chrisnicola on 9 Jun 2017

👍12

Any updates on this? I have a class, which delegates method calls to another classes... I believe super does not make sense here:

class API
  def method_missing(method_name, *arguments, &block)
    client.send(method_name, *aguments, &block)
  end

  private

  def client
    if Config.fake_client?
      FakeHTTPClient
    else
      RealHTTPClient
    end
  end
end

Maybe we should drop check for super call at all? It depends a lot on logic used for method_missing...

nattfodd on 20 Dec 2017

👍5

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!