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

In certain cases rbs prototype rb defines the wrong type definition for Object Class #1345

Open
tmimura39 opened this issue May 29, 2023 · 1 comment
Milestone

Comments

@tmimura39
Copy link

The following outputs results that rbs prototype rb does not expect.

$ cat on_load.rb
class MyClass; end
# Include (lazy) MyClass in ActiveRecord::Base
# ActiveSupport.on_load uses class_eval internally.
# https://github.com/rails/rails/blob/66099147482ea431febf20936cec903f197d24be/activesupport/lib/active_support/lazy_load_hooks.rb
ActiveSupport.on_load(:active_record) { include MyClass }

$ rbs prototype rb on_load.rb -o sig --force
Processing `on_load.rb`...
  Generating RBS for `on_load.rb`...
    - Writing RBS to existing file `sig/on_load.rbs`...

$ cat sig/on_load.rbs
class MyClass
end

# Unspecified `include` without a receiver generates a type definition that treats the `Object` as included.
# https://github.com/ruby/rbs/blob/0b5eb4197c1dc2c09be0c43e99d8f17ac74004ad/lib/rbs/prototype/rb.rb#L47-L62
class Object
  include MyClass
end

This is true not only for include but also for extend and prepend.
This is a problem because if rbs prototype accidentally defines an Object type, there is no way around it even if you manually override the type.

The rbs prototype will not know the inner workings of ActiveSupport.on_load and will have difficulty resolving the root cause.

For example, what if the behavior of the rbs prototype command is to ignore includes in method blocks (and extend, prepend...) in a method block (and extend, prepend...) should be ignored.
(Careful decision making is required as this would be an incompatible change.)

Workaround

This problem can be avoided by specifying an include receiver.
However, you need to manually define the type of include to ActiveRecord::Base.

$ cat on_load.rb
class MyClass; end
ActiveSupport.on_load(:active_record) { ActiveRecord::Base.include(MyClass) } # ActiveRecord::Base or self

$ rbs prototype rb on_load.rb -o sig --force
Processing `on_load.rb`...
  Generating RBS for `on_load.rb`...
    - Writing RBS to existing file `sig/on_load.rbs`...

$ cat sig/on_load.rbs
class MyClass
end
@soutaro
Copy link
Member

soutaro commented May 29, 2023

It would make more sense to ignore mixins from non-module context. You may have to add mixins in RBS for the case of hooks like included, but it's easier to add extra RBS files than deleting unnecessary mixins from existing module definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants