Skip to content

Remove API placeholders in FormulaStruct and CaskStruct#21656

Merged
MikeMcQuaid merged 1 commit intomainfrom
remove-placeholders
Mar 3, 2026
Merged

Remove API placeholders in FormulaStruct and CaskStruct#21656
MikeMcQuaid merged 1 commit intomainfrom
remove-placeholders

Conversation

@Rylan12
Copy link
Copy Markdown
Member

@Rylan12 Rylan12 commented Mar 3, 2026

When generating the API, we replace HOMEBREW_PREFIX and Dir.home with placeholder values, but we don't actually replace the placeholders with the real values when loading from FormulaStruct (or CaskStruct). This PR adds the replacement logic to the APIHashable module so it's included by default anywhere the module is used.

Copilot AI review requested due to automatic review settings March 3, 2026 01:38
Copy link
Copy Markdown
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 aims to ensure API path placeholders (e.g., $HOMEBREW_PREFIX, /$HOME) are replaced with real local values when API data is loaded into FormulaStruct/CaskStruct, by centralizing placeholder-removal logic in APIHashable.

Changes:

  • Add APIHashable#deep_remove_placeholders to recursively replace API placeholders in hashes/arrays/strings.
  • Call placeholder removal during Homebrew::API::FormulaStruct.from_hash and Homebrew::API::CaskStruct.from_hash.
  • Add a temporary-looking filter in generate-formula-api.rb that limits generation to redis.

Reviewed changes

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

File Description
Library/Homebrew/dev-cmd/generate-formula-api.rb Adds a per-formula filter (currently hardcoded to only generate redis).
Library/Homebrew/api_hashable.rb Introduces recursive placeholder removal helper in APIHashable.
Library/Homebrew/api/formula_struct.rb Applies placeholder removal when constructing FormulaStruct from a hash.
Library/Homebrew/api/cask_struct.rb Applies placeholder removal when constructing CaskStruct from a hash.
Comments suppressed due to low confidence (2)

Library/Homebrew/api/formula_struct.rb:16

  • This change adds new behavior (placeholder replacement) to FormulaStruct.from_hash, but there are no tests covering placeholder removal when constructing/deserialize-ing a Homebrew::API::FormulaStruct. Adding a spec that passes a hash containing $HOMEBREW_PREFIX//$HOME/$HOMEBREW_CELLAR and asserts the resulting struct contains real paths would help prevent regressions.
      def self.from_hash(formula_hash)
        formula_hash = ::Formula.deep_remove_placeholders(formula_hash)
        formula_hash = formula_hash.transform_keys(&:to_sym)
                                   .slice(*decorator.all_props)
                                   .compact_blank
        new(**formula_hash)

Library/Homebrew/api/cask_struct.rb:15

  • This change adds new behavior (placeholder replacement) to CaskStruct.from_hash, but the existing api/cask_struct_spec.rb coverage for ::from_hash doesn’t assert placeholder removal. Add a test case that includes $HOMEBREW_PREFIX//$HOME/$HOMEBREW_CELLAR in raw_caveats or raw_artifacts and verifies the struct (or #caveats/#artifacts) resolves them correctly.
      def self.from_hash(cask_hash, ignore_types: false)
        return super(cask_hash) if ignore_types

        cask_hash = ::Cask::Cask.deep_remove_placeholders(cask_hash)
        cask_hash = cask_hash.transform_keys(&:to_sym)
                             .slice(*decorator.all_props)
                             .compact_blank
        new(**cask_hash)

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

@Rylan12 Rylan12 force-pushed the remove-placeholders branch from 53c8c72 to c2ac70b Compare March 3, 2026 01:47
Copy link
Copy Markdown
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!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Mar 3, 2026
Merged via the queue into main with commit 67cf811 Mar 3, 2026
38 checks passed
@MikeMcQuaid MikeMcQuaid deleted the remove-placeholders branch March 3, 2026 09:06
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