Project

General

Profile

Actions

Feature #18742

closed

Introduce a way to tell if a method invokes the `super` keyword

Added by Dan0042 (Daniel DeLorme) about 2 years ago. Updated almost 2 years ago.

Status:
Rejected
Assignee:
-
Target version:
-
[ruby-core:108284]

Description

In order to implement a "no clobber" checker as in #18618, I would like to have a way to check if a method calls super or not.

So I'm thinking that something along the line of Method#calls_super? could return true/false if the method simply contains the super keyword. I'm not really interested in handling weird/artificial edge cases with eval and binding and whatnot.

class X
  def a
  end; p instance_method(:a).calls_super? #=> false

  def b
    super
  end; p instance_method(:b).calls_super? #=> true

  def c
    super if false
  end; p instance_method(:c).calls_super? #=> true

  def d
    eval 'super'
  end; p instance_method(:d).calls_super? #=> false (I doubt there's a reasonable way for this to return true)
end

With the above it would be possible to warn against a method that has a super_method but doesn't use the super keyword.

Actions #1

Updated by Dan0042 (Daniel DeLorme) about 2 years ago

  • Subject changed from Introduce a way to tell if a method invokes the `super` keryword to Introduce a way to tell if a method invokes the `super` keyword

Updated by byroot (Jean Boussier) about 2 years ago

You could walk the method Iseq like in this example script: https://github.com/ruby/ruby/pull/5809, and look for the invokesuper instruction.

That would be MRI specific, but would work today without any change.

Updated by Dan0042 (Daniel DeLorme) almost 2 years ago

Thank you for the suggestion, and I apologize for the late reply.

This works remarquably well.

class UnboundMethod
  def calls_super?
    iseqs = [RubyVM::InstructionSequence.of(self)]
    iseqs.any? do |iseq|
      iseq.each_child{ |c| iseqs << c }
      iseq.to_a.last.any?{ |v,| v == :invokesuper }
    end
  end
end

Interestingly I found that super if false is optimized away so example c didn't work; I had to use 0.times{super}

But I must say it feels a bit weird to use something so heavy just to get a bit of metadata about the method.

Updated by matz (Yukihiro Matsumoto) almost 2 years ago

  • Status changed from Open to Rejected

First, I am afraid that no_clobber checks using super would not work well. People would override methods without using super more often that you may expect. Some may copy code from the parent methods, some may just reimplement methods. So the biggest use-case is not valid from my POV.

Second, by the core method naming convention we do not use third-person singular present form (e.g 'include?' instead of 'includes?'). Some (especially native English speaker) may feel unnatural, but we set the rule, and we are not going to change it for the foreseeable future.

For no_clobber, I propose the following instead:

class A
  def foo
  end
  def bar
  end
end

class B<A
  override def foo
  end
  def bar
  end
  no_clobber # checks overriding methods here
end

So we close this issue for now. If you think of a new real world use-case, please revisit.

Matz.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0