ruby / rbs

Type Signature for Ruby
Other
1.91k stars 201 forks source link

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

Open tmimura39 opened 1 year ago

tmimura39 commented 1 year ago

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 commented 1 year ago

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.