Need a better way to handle required belongs_to associations.

[thomasdziedzic-census]

slack context: https://sorbet-ruby.slack.com/archives/CHN2L03NH/p1636647565028400

I'm trying to migrate our code base from using sorbet & sorbet-rails to using tapioca for all rbi generation.

Currently tapioca always generates T.nilable belongs_to associations where as previously in our code base, our rbis generated via sorbet-rails were not nilable. This has caused a lot of additional T.must's in our code base (1000s).

This created some friction so I forked the active_record_associations generator and added code to it to only generate nilable code if optional: true.

It seems like others have encountered this friction as well for instance https://github.com/Shopify/tapioca/issues/174

While digging into what sorbet-rails does, I found that it has a couple of bugs open for non-nilable belongs_to associations carrying nil values: https://github.com/chanzuckerberg/sorbet-rails/issues/253 & https://github.com/chanzuckerberg/sorbet-rails/issues/356

So it seems like tapioca might be too strict, and sorbet-rails might be too open :) Is there a middle road here?

[drop-george]

This is one of the biggest hurdles I'm seeing right now in moving from srb rbi to tapioca.

[rafaelfranca]

The types generated by tapioca is correct. All belongs_to associations can have nil value.

Given:

class MyModel < ApplicationRecord
  belongs_to :foo
end

This would be a valid runtime behavior in any Rails application

MyModel.new.foo # => nil

That is an invalid model, but for the purpose of type checking, at certain points of a lifetime of a program, all belongs_to associations can be nil, so the correct signature for those methods are really T.nilable. The fact that sorbet-rails already generated wrong types is a bummer.

[rafaelfranca]

I don't think there is a middle road here. We can't teach Sorbet about conditional runtime behavior (that is what required belongs_to associations are).

The opposite of this is to generate non-nilable types, but have programs failing at runtime because the type checker didn't caught the cases were the association retuned nil, which defeats the reason why someone would use a type checker.

Our choice is to require some friction, in order to achieve type safety.

[philsmy]

I appreciate that this issue is closed, but, I am unsure how we are supposed to proceed. All belongs_to seem to be T.nilable, even if we manually put a 'optional: false' attribute on the belongs_to. Are you saying that we have to wrap all instances of the associated object in a T.must? Or we create a method that returns the associated object without the T.nilable?

[rafaelfranca]

If you want the right type information, you would have to wrap all instances of the associated object in a T.must.

Even if the association is put as optional: false, Model.new.save is valid runtime code, and Sorbet should not tell otherwise.

[iMacTia]

@rafaelfranca I completely understand the Model.new scenario and I've been telling my team the same until now, however what I do not understand is the inconsistency in tapioca's behaviour.

The same logic could in fact be applied to nullable columns, but tapioca currently uses the null option to decide if an AR column should be T.nilable or not. Even better, this behaviour is configurable (albeit by defining a constant, which is not a great API IMO).

It's confusing that columns have this configurable behaviour and default to the more logical but less safe one, while associations have no configurability and default to the safer approach. Would you be open to:

1. Make associations types generation configurable, similarly to column types; and
2. Default to the more expected behaviour of making associations T.nilable based on their definition, like for columns