search

chanzuckerberg / sorbet-rails

A set of tools to make the Sorbet typechecker work with Ruby on Rails seamlessly.
MIT License
638 stars 83 forks source link

Unsaved models can have nil non-nilable belongs-to relationships #253

Open lrworth opened 4 years ago

lrworth commented 4 years ago

Describe the bug: The belongs_to relation is required by default (see #144) but objects with this relation don't necessarily have an object there. I believe the presence of the field is checked on save, not on construction. This leads to code which typechecks but fails with undefined method f for nil:NilClass at runtime.

Steps to reproduce: I haven't had time to create a minimal reproduction, but:

  1. Create a non-optional belongs_to relation between two ActiveRecord models.
  2. Assert the generated type of this relation is not T.nilable.
  3. Construct a default instance of the model with the relation and call the method.

It will result in undefined method f for nil:NilClass.

Expected behavior: The generated type should be T.nilable because there are no static constraints preventing this (at least for unsaved models).

Versions:

hdoan741 commented 4 years ago

I see the issue. This can be tricky to address though. When we make the association T.nilable, sorbet might start complaining “method call on a nilable object”. And in a code base, code accessing an association is a lot more common than setting it up. While it’s correct, this not ideal change to make.

An alternative I came up with is to have a Unsaved* object, eg UnsavedWizard. It’ll be the return type of “new” (and some other?) In that class, everything will be nilable.

How does that sound to you?

On Thu, Jan 9, 2020 at 8:17 PM lrworth notifications@github.com wrote:

Describe the bug: The belongs_to relation is required by default (see #144 https://github.com/chanzuckerberg/sorbet-rails/pull/144) but objects with this relation don't necessarily have an object there. I believe the presence of the field is checked on save, not on construction. This leads to code which typechecks but fails with undefined method f' for nil:NilClass` at runtime.

Steps to reproduce: I haven't had time to create a minimal reproduction, but:

  1. Create a non-optional belongs_to relation between two ActiveRecord models.
  2. Assert the generated type of this relation is not T.nilable.
  3. Construct a default instance of the model with the relation and call the method.

It will result in undefined method f' for nil:NilClass`.

Expected behavior:

The generated type should be T.nilable because there are no static constraints preventing this (at least for unsaved models).

Versions:

  • Ruby: ruby 2.6.4p104 (2019-08-28) [x86_64-darwin17]
  • Rails: Rails 5.2.4.1
  • Sorbet: Sorbet typechecker 0.4.5144 git dbd5bee6b64af3f39b4291f64eec555f2fbb008f built on 2019-12-17 00:46:19 GMT debug_symbols=true
  • Sorbet-Rails: sorbet-runtime (0.4.5146)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/chanzuckerberg/sorbet-rails/issues/253?email_source=notifications&email_token=AAFH4AINT6BHCNNHQCICQW3Q47ZEBA5CNFSM4KFCEMRKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IFHTYHQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFH4AMEQCJTUVKW4GX2T23Q47ZEBANCNFSM4KFCEMRA .