search

pocke / rbs_rails

Apache License 2.0
285 stars 34 forks source link

Generator breaks on using composite primary key. #234

Closed kouwasi closed 2 years ago

kouwasi commented 2 years ago 
class MyModel < ApplicationRecord
  self.primary_key = :my_id, :other_id
end

When try to generate that AR model's rbs, The generator can't generate rbs correctly.

Ruby: 3.1.2 Rails: 6.0.6

rails aborted!
NoMethodError: undefined method `type' for nil:NilClass

        sql_type_to_class(col.type)
                             ^^^^^
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/active_record.rb:58:in `pk_type'
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/active_record.rb:31:in `klass_decl'
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/active_record.rb:25:in `generate'
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/active_record.rb:11:in `class_to_rbs'
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/rake_task.rb:45:in `block (2 levels) in def_generate_rbs_for_models'
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/rake_task.rb:38:in `each'
/usr/local/bundle/gems/rbs_rails-0.11.0/lib/rbs_rails/rake_task.rb:38:in `block in def_generate_rbs_for_models'
kouwasi commented 2 years ago

Aww, I found the solution. I solved with delete self.primary_key declaration.

On a model having composite pk declaration, primary_key method returns string of array. (eg. "[:my_id, :other_id]") So, that why generator can't work correctly.

This is correct way IMO.

pocke commented 2 years ago

Thanks for reporting this issue.

If my understanding is correct, Rails doesn't support composite PK officially, but the gem does.

I'd like to focus on Rails' official feature for RBS Rails. RBS Rails is designed to generate types for "rails" gem, but not for other extension gems.

I think RBS Rails should be extendable for extensions such as composite_primary_key, but currently, it does not have a public API to extend it. Sorry for the inconvenience.

kouwasi commented 2 years ago

Rails doesn't support composite PK officially

That's true.

In my case, I'm using many to many join table with composite pk. But it is not correct way on ActiveRecord(and RDB?), I guess. Actually, rails shown the warning Active Record does not support composite primary key.

Sorry for the inconvenience.

Don't worry about it! I think you are right to follows official Rails apis.