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

[prototype runtime] Use the original Module#name to avoid generating incorrect module name #526

Conversation

pocke
Copy link
Member

@pocke pocke commented Dec 17, 2020

Problem

rbs prototype runtime command generates RBS with incorrect module names if Module#name is re-defined.
For example:

# test.rb

module X
  module M
    def self.name() 'FakeNameM' end
    def self.to_s() 'FakeToS' end
    X = 1
  end

  class C
    def self.name() 'FakeNameC' end
    include M
  end

  class C2 < C
  end
end
$ exe/rbs prototype runtime -R test.rb X::*
class FakeNameC                 # ← It should be C, but not
  include FakeNameM             # ← It should be M, but not

  def self.name: () -> untyped
end

class FakeNameC < FakeNameC     # ← It should be X::C2 < C, but not
end

module X::M
  def self.name: () -> untyped

  def self.to_s: () -> untyped
end

FakeToS::X: Integer             # ← It should be X::M::X, but not

Solution

This pull request fixes this problem by using the original Module#name to get a constant name.
It will display the following result.

$ exe/rbs prototype runtime -R test.rb X::*
class X::C
  include M

  def self.name: () -> untyped
end

class X::C2 < X::C
end

module X::M
  def self.name: () -> untyped

  def self.to_s: () -> untyped
end

X::M::X: Integer

By the way, prototype runtime still uses other Module's methods that can be re-defined. For example, it uses Module#instance_method to get a method object of a module.
At first, I thought I should replace them with the original methods, but I didn't.
Because I guess it is not problematic in most cases. On the contrary, redefined methods can be more appropriate than the original methods.
For example, delegator's instance_method is re-defined to work with delegated methods. https://github.com/ruby/ruby/blob/53e352fd718cd2cae6d77298e6e92736dddcfeeb/lib/delegate.rb#L436
So I think we don't need to use the original method for them.

@pocke pocke force-pushed the _prototype_runtime__Use_the_original_Module_name_to_avoid_generating_incorrect_module_name branch from 085a76d to 2b3ad6e Compare December 17, 2020 15:32
Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@soutaro soutaro added this to the Ruby 3.0 milestone Dec 18, 2020
@pocke pocke merged commit 086a581 into ruby:master Dec 18, 2020
@pocke pocke deleted the _prototype_runtime__Use_the_original_Module_name_to_avoid_generating_incorrect_module_name branch December 18, 2020 09:58
pocke added a commit to pocke/rbs that referenced this pull request Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants