dev-cmd/typecheck: Support typechecking in taps

[issyl0]

• 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 typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

• Theoretically, anyway?
• Part of Use Sorbet typed: strict in all (non-package) files in Homebrew organisation #17297.

$ brew typecheck homebrew/bundle
No sorbet/ directory found. Maybe you want to run 'srb init'?

A type checker for Ruby

Usage:
  srb                                 Same as "srb t"
  srb (init | initialize)             Initializes the `sorbet` directory
  srb rbi [options]                   Manage the `sorbet` directory
  srb (t | tc | typecheck) [options]  Typechecks the code

Options:
  -h, --help     View help for this subcommand.
  --version      Show version.

For full help:
  https://sorbet.org
Check https://docs.brew.sh/Typechecking for more information on how to resolve these errors.

[MikeMcQuaid]

$ brew typecheck homebrew/bundle
No sorbet/ directory found. Maybe you want to run 'srb init'?

A type checker for Ruby

Usage:
  srb                                 Same as "srb t"
  srb (init | initialize)             Initializes the `sorbet` directory
  srb rbi [options]                   Manage the `sorbet` directory
  srb (t | tc | typecheck) [options]  Typechecks the code

Options:
  -h, --help     View help for this subcommand.
  --version      Show version.

For full help:
  https://sorbet.org
Check https://docs.brew.sh/Typechecking for more information on how to resolve these errors.

Maybe we should run sorbet init for them or at least direct them to a brew type check --init invocation or something that will do it? Should ideally avoid having the default state like this reference a srb command that the user won't know how to run.

[Bo98]

Maybe we should run sorbet init for them or at least direct them to a brew type check --init invocation or something that will do it? Should ideally avoid having the default state like this reference a srb command that the user won't know how to run.

Any init should be explicit IMO.

For this PR, we could do something like:

raise UsageError, "Typechecking is not supported for the #{tap} tap" unless (tap.path/"sorbet").directory?

or similar

[issyl0]

Additionally I was thinking about syncing sorbet/config between taps like we do with other things?

[Bo98]

Does it support symlinks? Ideally we reuse the brew sorbet directory rather than messing around with syncing.

Though maybe we need to do syncing for the rbi directory. If so: perhaps we gitignore it and make brew typecheck copy it. Merging PRs everywhere on dependency updates would be a nightmare.

Homebrew taps are somewhat unique in that they are all brew plugins rather than standalone projects.

[Bo98]

Something else to try: don't cd but instead just pass --dir tap.path to srb tc. That might take brew's sorbet directory but typecheck against the tap files.

[MikeMcQuaid]

For this PR, we could do something like:

raise UsageError, "Typechecking is not supported for the #{tap} tap" unless (tap.path/"sorbet").directory?

or similar

This makes sense to me. I still think it makes sense to tell a tap author how to get around this, though, but that doesn't need to block this PR.

Additionally I was thinking about syncing sorbet/config between taps like we do with other things?

This makes sense to me.

Though maybe we need to do syncing for the rbi directory.

I don't think so. I agree with trying to use Homebrew's rbis everywhere. In reality pretty much no taps (homebrew-formula-analytics maybe aside) actually use gems at runtime.

[issyl0]

Something else to try: don't cd but instead just pass --dir tap.path to srb tc. That might take brew's sorbet directory but typecheck against the tap files.

I tried this and it gives results. Not sure if they're correct results, but results all the same.

[issyl0]

On closer inspection with brew typecheck homebrew/services I am more convinced these results are nonsense, it should be the other way around but instead Sorbet is taking the tap as the source for the methods?

./cask/artifact/symlinked.rb:38: Too many arguments provided for method Formatter.error. Expected: 1, got: 2 https://srb.help/7004
    38 |          Formatter.error(string, label: "Broken Link")
                                          ^^^^^^^^^^^^^^^^^^^^
    /opt/homebrew/Library/Taps/homebrew/homebrew-services/spec/spec_helper.rb:44: error defined here
    44 |  def error(string)
          ^^^^^^^^^^^^^^^^^
  Autocorrect: Use -a to autocorrect
    ./cask/artifact/symlinked.rb:38: Delete
    38 |          Formatter.error(string, label: "Broken Link")

[MikeMcQuaid]

I am more convinced these results are nonsense, it should be the other way around but instead Sorbet is taking the tap as the source for the methods?

Yes, this looks wrong. It shouldn't be checking cask/artifact/symlinked.rb here.

[issyl0]

I reset to the previous way in the follow-up commit. It's now more like what I'd expect, apart from Sorbet doesn't know about things like Keg and Plist, but at least it's now scoped only to a tap.

[Bo98]

I don't think so. I agree with trying to use Homebrew's rbis everywhere. In reality pretty much no taps (homebrew-formula-analytics maybe aside) actually use gems at runtime.

rbi directory does include stuff like parlour.rbi that are Homebrew definitions.

I am more convinced these results are nonsense, it should be the other way around but instead Sorbet is taking the tap as the source for the methods?

Yes, this looks wrong. It shouldn't be checking cask/artifact/symlinked.rb here.

It actually looks somewhat correct to me: we don't have a "Homebrew/brew RBI" so it needs to check through Homebrew/brew to find definitions (AFAIK you can't configure Tapioca to generate RBIs from non-gems).

In this case, the spec_helper.rb definition in homebrew-services is wrong.

Unfortunately the error presents itself in a way that's perhaps confusing since it seems to run through homebrew-services first before Homebrew/brew so it marks Homebrew/brew as wrong rather than homebrew-services (ideally it would do Homebrew/brew first - maybe it supports multiple --dir to specify order?).

[issyl0]

maybe it supports multiple --dir to specify order?

I reset to the --dir approach (this is getting super confusing), and apparently not?

diff --git a/Library/Homebrew/dev-cmd/typecheck.rb b/Library/Homebrew/dev-cmd/typecheck.rb
index d6c5e33500..490e41a520 100644
--- a/Library/Homebrew/dev-cmd/typecheck.rb
+++ b/Library/Homebrew/dev-cmd/typecheck.rb
@@ -101,7 +101,7 @@ module Homebrew
             cd("sorbet") do
               srb_exec += ["--file", "../#{args.file}"] if args.file
               srb_exec += ["--dir", "../#{args.dir}"] if args.dir
-              srb_exec += ["--dir", tap_dir.to_s] if tap_dir
+              srb_exec += ["--dir", HOMEBREW_LIBRARY_PATH.to_s, "--dir", tap_dir.to_s] if tap_dir
             end
           end
           success = system(*srb_exec)

$ brew tc homebrew/services
/opt/homebrew/Library/Homebrew/download_strategy.rbi:10: Duplicate type member Elem https://srb.help/4011
    10 |  ContentDisposition = Struct.new :type, :filename, :creation_date,
    11 |                                  :modification_date, :read_date, :size, :parameters
    ./download_strategy.rbi:10: Also defined here
    10 |  ContentDisposition = Struct.new :type, :filename, :creation_date,
    11 |                                  :modification_date, :read_date, :size, :parameters
Errors: 81

[Bo98]

Might be picking up the --dir . in sorbet/config. Try remove that

[issyl0]

Might be picking up the --dir . in sorbet/config. Try remove that

OK, having removed that sorbet/config line it's back to how it was when I thought the output looked like nonsense. 😩 So nothing has changed with the multiple --dir specified. So we gained nothing.

[MikeMcQuaid]

I have no idea how/if this is possible but: I almost wonder if somehow the tap approach needs to be:

• by default brew typecheck/srb in Homebrew/brew skips all taps
• somehow running brew typecheck homebrew/bundle unignores only the homebrew/bundle tap and type checks both all of homebrew/bundle but also all of Homebrew/brew to find the types it needs
• brew typecheck refuses to work on an arbitrary path that's not a tap.

[issyl0]

by default brew typecheck/srb in Homebrew/brew skips all taps

This is the current (master) behaviour.

brew typecheck refuses to work on an arbitrary path that's not a tap.

Does this mean that we can't allow Sorbet's --file and --dir options anymore? (I've never used them with brew typecheck, FWIW, no idea what their output is and if that also looks like nonsense for similar reasons.)

[MikeMcQuaid]

Does this mean that we can't allow Sorbet's --file and --dir options anymore? (I've never used them with brew typecheck, FWIW, no idea what their output is and if that also looks like nonsense for similar reasons.)

I don't know, maybe ignore that, I more mean that we'll need to ensure we have this "you're in a subdirectory of Homebrew/brew" approach so typing works as expected within the Homebrew ecosystem.

[issyl0]

I thought I had got further, copying the RBI files in the sorbet/ directory into the taps too, but nope, backed out the latest commit as it was still bad (just in a different way?). I tried to rejoin the Sorbet community Slack to ask their opinions on how to do this, but I left it years ago and now I can't rejoin? 🤷🏻

[Bo98]

Looks like Sorbet internally sorts files by file size so the output we got from --dir might be the best we'll get (with autofix disabled). It's perhaps a bit confusing but errors are genuine. The confusing output of redefinition errors are probably not too common beyond rspec files.

Given Tapioca only supports gems, the only other way I could think about would be a temporary directory that converts Homebrew to a gemfile, adds it to a fake gemfile along with copying Tapioca and its dependencies which then runs tapioca gem on that to a temporary RBI output which is then used when typechecking. That is incredibly hacky and I don't recommend it.

Basically: the tooling for this doesn't seem to exist yet so might have to settle for something "close enough".

[issyl0]

Yeah the exporting a fake gem feels way too hacky. I did have a look at Parlour generation for that but we probably don't care that much.

[MikeMcQuaid]

Looks good thanks @issyl0! Will hold off merging until after next tag 🔜 today.