Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mismatched typing for TypeCoerce[<T>].new.from (with Tapioca) #77

Open
mattxwang opened this issue Sep 16, 2022 · 0 comments
Open

mismatched typing for TypeCoerce[<T>].new.from (with Tapioca) #77

mattxwang opened this issue Sep 16, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@mattxwang
Copy link
Contributor

mattxwang commented Sep 16, 2022

Deep Dive Summary

Describe the bug:

Right at the end of my internship, we successfully moved infra-tasks to fully use the new Tapioca for RBI gen. Awesome! However, I did run into one hiccup (that I can no longer see :/ ) in using Tapioca to generate RBIs - in particular, that the RBI it generated is incompatible with sorbet-coerce!

After doing some thinking, I think that Tapioca is right, and our provided/canonical RBI is wrong (or at the very least, does not fully describe sorbet-coerce's behaviour). Here's the (relevant portion of) the RBI that's in bundled_rbi / in sorbet-typed / committed in traject, infra-tasks, etc.:

module TypeCoerce
  extend T::Sig
  extend T::Generic

  Elem = type_member

  sig { params(args: T.untyped, raise_coercion_error: T.nilable(T::Boolean), coerce_empty_to_nil: T::Boolean).returns(Elem) }
  def from(args, raise_coercion_error: nil, coerce_empty_to_nil: false); end
end

In other words, we are marking TypeCoerce as a generic, and so something like TypeCoerce[<T>].new.from(...) should behave in this order:

  • TypeCoerce[<T>] is the generic type with the type_member
  • TypeCoerce[<T>].new creates an instance
  • TypeCoerce[<T>].new.from(...) uses the sig we just defined, and so it must be of type <T>

Compare this with what the code actually does. Looking at sorbet-coerce.rb:

module TypeCoerce
  def self.[](type)
    TypeCoerce::Converter.new(type)
  end
end

I don't have the context on why this was implemented this way (my guess - Sorbet's generics API wasn't finalized yet).

However, when Tapioca does its runtime introspection, it says:

  • TypeCoerce[<T>] is not a generic type: it's overloaded (?) and returns an instance of TypeCoerce::Converter
  • uh oh, TypeCoerce::Converter is not typed (and has no sigs). Let's call it T.untyped then!
  • TypeCoerce[<T>].new is calling .new on T.untyped. no bueno (or, T.untyped)
  • TypeCoerce[<T>].new.from(...) is calling .from on T.untyped. double no bueno (or, T.untyped)!
  • we never actually hit our manually-written sig that we wrote! :(

This creates a large T.untyped chain, and means that the result of the coercion is T.untyped - surely not what we want.

In my opinion, this is incorrect typing: while it's true at a very black-boxed level, any amount of runtime introspection causes tension with sorbet.

I'm not 100% sure this is a correct diagnosis of the behaviour - and it's harder for me to confirm now that I can't see the codebase. But, I'm pretty confident that there's something wrong with our bundled types, and hopefully this is at least a good start!

(and, note that this bug doesn't affect the runtime (like the other sorbet-coerce bug I ran into) - since at runtime, our expression still typechecks)

potential solutions

I think there are two ways forward:

  1. Fix how sorbet-coerce works with tapioca at an "interface" level, either by overriding Tapioca's RBI or manually introducing a shim. This is what I did in infra-tasks, but it feels unsustainable/hacky.
    • an extension of this could be to type Converter!
  2. Actually fix this incorrect typing, by making Converter use sorbet generics under-the-hood (instead of passing the type as a field)

things included in the issue template

Steps to reproduce:

Create a new project that consumes sorbet-coerce (and uses it somewhere). Then,

  • install tapioca
  • run bundle exec tapioca init (implicitly - runs tapioca gem)
  • run bundle exec srb tc - the static typecheck fails

Note that, at runtime, the code works! Though, it seems like this is somewhat undefined behaviour (re: Jake Zimmerman's previous comment in the Sorbet slack about Sorbet not supporting programming at the type-level).

Expected behavior:

Static typecheck should pass.

Actual behaviour:

Static typecheck fails.

Versions:

  • Ruby: 3.1
  • Sorbet: ~ 0.5.10400 (not super important)
  • Sorbet-Coerce: 0.7.0
  • Tapioca: 0.10.0
@mattxwang mattxwang added the bug Something isn't working label Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant