Skip to content

sorbet: Fix the formula test helper block typing - #23397

Merged
issyl0 merged 1 commit into
mainfrom
srb-stop-lying
Aug 2, 2026
Merged

sorbet: Fix the formula test helper block typing#23397
issyl0 merged 1 commit into
mainfrom
srb-stop-lying

Conversation

@issyl0

@issyl0 issyl0 commented Aug 2, 2026

Copy link
Copy Markdown
Member

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

  • This formula helper returning Class.new(::Formula, &block) is not of instance type Formula (set in 540457b), but T.class_of(Formula).

  • But the accurate T.proc.bind(T.class_of(::Formula)) can't be used instead because Sorbet errors with: "Malformed bind: Can only bind to simple class names". See sorbet: Type blocks at source more precisely than T.untyped #23318 (comment).

  • Uses of this test helper that need to be typechecked still need T.bind(self, T.class_of(Formula)) inline until Sorbet can bind to T.class_of of a class with a fixed type_template (maybe never).

- This `formula` helper returning `Class.new(::Formula, &block)` is not
  of _instance_ type `Formula` (set in 540457b), but
  `T.class_of(Formula)`.

- But the accurate `T.proc.bind(T.class_of(::Formula))` can't be used
  instead because Sorbet errors with: "Malformed `bind`: Can only bind
  to simple class names". See
  #23318 (comment).

- Uses of this test helper that need to be typechecked still need
  `T.bind(self, T.class_of(Formula))` inline until Sorbet can bind to
  `T.class_of` of a class with a fixed `type_template` (maybe never).
Copilot AI review requested due to automatic review settings August 2, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts Sorbet typing in the RSpec formula test helper to avoid incorrectly typing the helper’s Class.new(::Formula, &block) block as being bound to a Formula instance, which doesn’t match how Ruby evaluates that block (it’s evaluated in the class body context).

Changes:

  • Update the formula helper block type from T.proc.bind(::Formula).void to an unbound T.proc.void to avoid incorrect instance binding.
Show a summary per file
File Description
Library/Homebrew/test/support/helper/formula.rb Fixes the Sorbet signature for the formula helper’s block to avoid an incorrect bind(::Formula) typing.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

@issyl0
issyl0 enabled auto-merge August 2, 2026 14:59

@p-linnane p-linnane left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@issyl0
issyl0 added this pull request to the merge queue Aug 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 2, 2026
@issyl0
issyl0 added this pull request to the merge queue Aug 2, 2026
Merged via the queue into main with commit 7b0f22a Aug 2, 2026
47 checks passed
@issyl0
issyl0 deleted the srb-stop-lying branch August 2, 2026 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants