Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jcoyne opened this issue Dec 1, 2016 · 8 comments
Labels
enhancement stale Issues that haven't been active in a while

Comments

@jcoyne
Copy link

jcoyne commented Dec 1, 2016

The super is unreachable, although Style/MethodMissing demands it

        def method_missing(*_args)
          return []
          super
        end
Lint/UnreachableCode: Unreachable code detected.
          super
          ^^^^^
@bquorning
Copy link
Contributor

bquorning commented Dec 22, 2016

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

@Drenmi
Copy link
Collaborator

Drenmi commented Dec 22, 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:

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

WDYT?

@bquorning
Copy link
Contributor

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.

@chrisnicola
Copy link

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

@nattfodd
Copy link
Contributor

nattfodd commented Dec 20, 2017

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...

@stale
Copy link

stale bot commented May 8, 2019

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!

@stale stale bot added the stale Issues that haven't been active in a while label May 8, 2019
@stale
Copy link

stale bot commented Jun 7, 2019

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

@Victorcorcos
Copy link

Victorcorcos commented Aug 5, 2022

Great solution @bquorning

This is what I was looking for:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement stale Issues that haven't been active in a while
Projects
None yet
Development

No branches or pull requests

7 participants