Add more type signatures.

[reitermarkus]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?
• Have you successfully run brew man locally and committed any changes?

---

[MikeMcQuaid]

Let's talk either here or https://github.com/Homebrew/brew/pull/8952/files#r508398399 about the RBI vs. Ruby inline sig usage.

[reitermarkus]

Pretty much the only thing that needs to be in .rbi files now is include Kernel because re-including it in actual code breaks stuff.

[MikeMcQuaid]

Pretty much the only thing that needs to be in .rbi files now is include Kernel because re-including it in actual code breaks stuff.

My understanding on the conversation in #8781 was that we were still going to keep definitions in RBI files until we were further along with coverage here. I appreciate I approved that PR without that being the case but I think I misunderstood what was going on there given the size of the PR and I apologise for my mistake there.

To clarify: Ruby 3 has no intention of shipping: any syntax for type annotations in source files directly

Having brew typecheck require that types are added for new methods but not requiring them to be scrolled through feels like a much better intermediate step than one person adding type signatures inline to huge numbers of files.

[reitermarkus]

I think I misunderstood what was going on

I specifically added sorbet-runtime-stub in that PR so we can start adding annotations inline right now, without the runtime overhead.

[reitermarkus]

Having brew typecheck require that types are added for new methods

There's no way of knowing whether a method is “new”. At best we can do this on a per-file basis by enforcing # typed: true with RuboCop and excluding all existing files which are still # typed: false.

[MikeMcQuaid]

I specifically added sorbet-runtime-stub in that PR so we can start adding annotations inline right now, without the runtime overhead.

Yes, I appreciate that (and thanks for it). I'm concerned about the source code readability overhead for adding these type signatures inline. I think it harms readability to have them inline.

There's no way of knowing whether a method is “new”. At best we can do this on a per-file basis by enforcing # typed: true with RuboCop and excluding all existing files which are still # typed: false.

Yes, that's what I'm saying we should continue to do: exclude those files but have the definitions be in .rbi files rather than inline.

[reitermarkus]

I think it harms readability to have them inline.

I think it very much improves readability to have the signature right next to the function. You can very easily see how a given function is supposed to be used.

[reitermarkus]

have the definitions be in .rbi files rather than inline

The whole point of #8781 was to enable writing annotations inline because maintaining the exact same class hierarchy and method definitions in two different sets of files is a giant pain.

[MikeMcQuaid]

I think it very much improves readability to have the signature right next to the function. You can very easily see how a given function is supposed to be used.

I disagree. It requires understanding Sorbet type definitions to read our code which increases the barrier to entry for people who haven't worked with typed languages or type-annotations in languages. It's also far from commonplace in large Ruby projects at this point. We have a minority of maintainers who have worked with, added or changed these type annotations.

The whole point of #8781 was to enable writing annotations inline because maintaining the exact same class hierarchy and method definitions in two different sets of files is a giant pain.

And I voiced my objections there to writing annotations inline. I maintain those objections and should not have ✅ that PR. Given it was a +1714,-59 line change with many iterations: I'll forgive myself from the mistake of not fully understanding where it was at at the time it was merged.

It may be a pain but it's also a pain to have to scan through this information when reading files.

[MikeMcQuaid]

In case it's worth elaborating here on what I see as the underlying problem: we had a GSoC project to use Sorbet in Homebrew without a broader discussion of the pros and cons and whether it's desirable to (effectively) make Homebrew/brew use type annotations globally and transform our untyped codebase into a typed one.

From https://docs.brew.sh/Homebrew-Governance:

The TSC has the authority to decide on any technical matter, including technical disputes between Homebrew members. The TSC will also decide the long-term roadmap of Homebrew, including large features or major changes to the Homebrew software.

The Project Leader will represent Homebrew publicly, and manage all day-to-day technical decisions related to the operation of Homebrew.

It feels like these two aren't being adhered to with how this Sorbet rollout has been happening (and may be in slight conflict). I think this is something for @Homebrew/plc to discuss.

[reitermarkus]

It feels like these two aren't being adhered to

Can you elaborate on this? How are they not being adhered to?

[MikeMcQuaid]

From my perspective it does not feel like the TSC has been involved in this "major change(s) to the Homebrew software" and it does not feel on this or other PRs that I am "manag(ing) all day-to-day technical decisions related to the operation of Homebrew".

[MikeMcQuaid]

Have opened #8991 to be able to discuss the underlying issues directly.

[MikeMcQuaid]

Blocked on #8991 voting

[MikeMcQuaid]

TSC have voted: #8991 (comment)

• Homebrew should add type information for all Homebrew/brew Ruby files
• Homebrew should not enable type checking at brew runtime
• Homebrew/brew should not use separate RBI files for storing type information
• Missing types on new files should not cause a CI failure which requires them to be added

I'm not sure from this PR which of these are/are not currently the case but ✅ with the assumption that once/if they are all the case: this can be merged.

Relatedly (but not needing to block this PR) from #9099:

• Improve the documentation in https://docs.brew.sh/Typechecking
• Improve tooling/CI output (e.g. brew typecheck) to reference documentation (and vice-versa)
• Consider/investigate improving naming here with some of our own aliases to be Type::Signature

It would be good to prioritise this work (or debate it) before moving forward with many more types.

Thanks @reitermarkus!