Skip to content

Conversation

@cho-m
Copy link
Member

@cho-m cho-m commented Dec 26, 2025

  • 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 lgtm (style, typechecking and tests) with your changes locally?

Narrowing some types to be more accurate. This includes replace T.untyped so static type checking coverage increases.

Copilot AI review requested due to automatic review settings December 26, 2025 14:17
Copy link
Contributor

Copilot AI left a comment

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 improves type safety in the UnpackStrategy module and related code by replacing T.untyped with more specific types and narrowing return type signatures. This increases static type checking coverage and makes the code more maintainable.

Key changes:

  • Changed strategies return type from T.nilable(T::Array[UnpackStrategyType]) to T::Array[UnpackStrategyType] since it's never nil
  • Replaced T.untyped return types with more specific types (UnpackStrategy, void)
  • Replaced map.compact pattern with filter_map and explicit type guard for better type safety

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Library/Homebrew/unpack_strategy.rb Narrowed return types for strategies, detect, extract_nestedly, and each_directory methods; removed unnecessary safe navigation operators on strategies since it's now non-nilable
Library/Homebrew/cask/audit.rb Replaced map with filter_map and added explicit Formula type check for better type safety when filtering dependencies

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

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks, nice work!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Dec 27, 2025
Merged via the queue into main with commit 1184277 Dec 27, 2025
62 of 63 checks passed
@MikeMcQuaid MikeMcQuaid deleted the unpack_strategy-update-types branch December 27, 2025 09:52
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