compilers/active_record_associations.rb always generates T.nilable(singular_model_name) on belongs_to and has_one

[YukiJikumaru]

Now I'm migrating to tapioca and caught a troublesome problem. compilers/active_record_associations.rb always generates T.nilable(singular_model_name) on belongs_to and has_one association.

>= Rails 5.0

_	optional	generated returns type	expected returns type
belongs_to	-	T.nilable(singular_model_name)	singular_model_name
belongs_to	true	T.nilable(singular_model_name)	T.nilable(singular_model_name)
belongs_to	false	T.nilable(singular_model_name)	singular_model_name

_	required	generated returns type	expected returns type
has_one	-	T.nilable(singular_model_name)	T.nilable(singular_model_name)
has_one	true	T.nilable(singular_model_name)	singular_model_name
has_one	false	T.nilable(singular_model_name)	T.nilable(singular_model_name)

https://github.com/Shopify/tapioca/blob/v0.10.1/lib/tapioca/dsl/compilers/active_record_associations.rb#L186

        def populate_single_assoc_getter_setter(klass, association_name, reflection)
          association_class = type_for(reflection)
          association_type = as_nilable_type(association_class) # ALWAYS NILABLE!

This behavior requires us to annotate massive T.must sigils.

---

Before migrating to tapioca, I used chanzuckerberg/sorbet-rails, this gem infers very good type.

https://github.com/chanzuckerberg/sorbet-rails/blob/v0.7.5/lib/sorbet-rails/model_plugins/active_record_assoc.rb#L34

  def populate_single_assoc_getter_setter(assoc_module_rbi, assoc_name, reflection)
    # TODO allow people to specify the possible values of polymorphic associations
    assoc_class = assoc_should_be_untyped?(reflection) ? "T.untyped" : "::#{reflection.klass.name}"
    assoc_type = (belongs_to_and_required?(reflection) || has_one_and_required?(reflection) || assoc_class == "T.untyped") ? assoc_class : "T.nilable(#{assoc_class})"

Then I propose to incorporate the functions of chanzuckerberg/sorbet-rails into compilers/active_record_associations.rb

[paracycle]

@YukiJikumaru The fact of the matter is, those conditions on associations, like optional or required, only change the behaviour of persisted records. However, ActiveRecord models can also be non-persisted, and they undergo a lot of operations (think custom validations) that have to consider the case where an association might not exist.

If you had a models defined like this:

class Post < ActiveRecord::Base
  belongs_to :blog, optional: false
  has_one :author, required: true
end

class Blog < ActiveRecord::Base
end

class Author < ActiveRecord::Base
end

then you can perfectly do this:

post = Post.new
post.author # => nil
post.blog # => nil

[YukiJikumaru]

@paracycle Thank you for your reply! I understand the source code's intent, why belongs_to and has_one association are nilable.

At the same time, another question came to my mind. Why all AR's attributes are not nilable?

ActiveRecord's instance is mutable, type soudness can be broken easily.

user = User.find(1)
user.id = nil
user.id # nil

Of course non-persisted record has no values, but tapioca generates non-nilable types.

post = Post.new

post.title # => String
post.content # => String

they undergo a lot of operations (think custom validations) that have to consider the case where an association might not exist.

This consideration should be applied to not only associations but also attributes .

The Rule "If there is a possibility of being NULL, make it nillable." applies to association but not to attributes. I would like to know why there is such a difference.

[rafaelfranca]

They apply to both. T.untyped is used for attribute https://github.com/Shopify/tapioca/blob/212c392dbd14a919edaffc754664177219139a18/lib/tapioca/dsl/helpers/active_record_column_type_helper.rb#L18. The only case they aren't is because you are using including StrongTypeGeneration in a model, which should make sure that at runtime, those attributes can't be nil.

StrongTypeGeneration isn't implemented anywhere, and the reference to it in this codebase is a left over from out application. But the same considerations should apply to attributes.

[YukiJikumaru]

@rafaelfranca Thank your for your advise!

For confirmation, I created a new plain Rails project and generated tpioca's dsl.

https://github.com/YukiJikumaru/tapioca_test/tree/v0.0.0

tapioca (0.10.2)
rails (6.1.7)

# https://github.com/YukiJikumaru/tapioca_test/blob/v0.0.0/db/schema.rb
ActiveRecord::Schema.define(version: 2022_10_01_120447) do

  create_table "posts", force: :cascade do |t|
    t.integer "user_id", null: false
    t.string "title", null: false
    t.text "content"
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["user_id"], name: "index_posts_on_user_id"
  end

  create_table "users", force: :cascade do |t|
    t.string "name", null: false
    t.string "nickname"
    t.integer "age"
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
  end

  add_foreign_key "posts", "users"
end

bundle exec tapioca dsl --only Tapioca::Dsl::Compilers::ActiveRecordColumns

Then T.untyped is not used for attributes.

# https://github.com/YukiJikumaru/tapioca_test/blob/v0.0.0/sorbet/rbi/dsl/user.rbi
    sig { returns(::String) }
    def name; end
    sig { returns(T.nilable(::String)) }
    def nickname; end
    sig { returns(T.nilable(::Integer)) }
    def age; end

I debuged tapioca-0.10.2/lib/tapioca/dsl/compilers/active_record_columns.rb and tabulated findings.

table_name	column_name	do_not_generate_strong_types?(@constant)	Object.const_defined?(:StrongTypeGeneration)	getter_type	setter_type
posts	id	false	false	T.nilable(::Integer)	::Integer
posts	user_id	false	false	::Integer	::Integer
posts	title	false	false	::String	::String
posts	content	false	false	T.nilable(::String)	T.nilable(::String)
posts	created_at	false	false	T.nilable(::ActiveSupport::TimeWithZone)	::ActiveSupport::TimeWithZone
posts	updated_at	false	false	T.nilable(::ActiveSupport::TimeWithZone)	::ActiveSupport::TimeWithZone

[rafaelfranca]

Right. Those signature are wrong though. Post.new.title will be nil for example, unless there is a default in the database. That is a bug in the RBI we generate.

[YukiJikumaru]

@rafaelfranca

ActiveRecord::Base's instances are mutable, so it's attributes and relations are all T.nilable. If the design is consistent, I think that's good code!

But Just my personal opinion, unsoud but easy escape hatch like chanzuckerberg/sorbet-rails is helpful for newcomer from big codebase to sorbet.

Thank you for your comments and explanations😀

[sk-]

@paracycle would it be possible to have two type definitions, PartialMyModel and PersistedMyModel? In that way you could return a persisted model when fetching them from the database with methods like MyModel.find_by.