pocke / rbs_rails

Apache License 2.0
285 stars 33 forks source link

Expand _ActiveRecord_Relation_ClassMethods to concrete module #254

Closed tk0miya closed 1 year ago

tk0miya commented 1 year ago

To avoid conflicts with generated .rbs files by rbs prototype rb, this expands _ActiveRecord_Relation_ClassMethods to the concrete module GeneratedFinderClassMethods. It helps to resolve the conflict in .rbs files.

pocke commented 1 year ago

conflicts with generated .rbs files by rbs prototype rb

I want to clarify the background problem.

I summarize the problem you described in ruby-jp slack in the following. Please tell me if I misunderstand something.

To generalize this problem, the conflict happens if the user defines the same name class method as methods defined in _ActiveRecord_Relation_ClassMethods into a model class.


This PR does the following things.

  1. Change the interface to the module.
  2. Generate a module per model class instead of using type parameters.
  3. Move the type definition for the interface/module from ruby/gem_rbs_collection to this repository.

The first change is acceptable. I also think the pseudo module is the best approach.

But I'm not sure the 2nd and 3rd changes are necessary. It will work even if the interface is replaced with a module with type parameters. For example:

# Generated RBS
class User < ApplicationRecord
  extend ActiveRecord_Relation_ClassMethods[User, ActiveRecord_Relation, Integer]
end

In this case, we have the following advantages.

What do you think? If you think the 2nd and 3rd changes are necessary, please let me know why.

tk0miya commented 1 year ago

Sorry for my late response. I did not notice your response...

I'm not sure defining a pseudo module to gem_rbs_collection is okay. But it's a simple way to resolve my problem if acceptable. +1 for choose the 1st way.

tk0miya commented 1 year ago

I posted https://github.com/ruby/gem_rbs_collection/pull/413 to do this on the gem_rbs_collection side. Thank you!