Skip to content

Allow callers to explicitly provide types for set and add commands#1448

Open
vladmos wants to merge 1 commit intobazelbuild:mainfrom
vladmos:custom_types
Open

Allow callers to explicitly provide types for set and add commands#1448
vladmos wants to merge 1 commit intobazelbuild:mainfrom
vladmos:custom_types

Conversation

@vladmos
Copy link
Member

@vladmos vladmos commented Feb 4, 2026

Buildozer tries to guess the format of an attribute's value before adding it to the file, sometimes it does it incorrectly. This PR provides a way to explicitly set a type of the attribute. Currently it's only supported for the set, set_if_absent and add commands, but can be extended further for other commands as well (likely will also be required for dict_set and dict_add).

The syntax is backward-compatible with the old command usages:

set foo:string bar  # sets foo = "bar"
set name:ident FOO  # sets name = FOO

Attribute names can't contain colons, hence no previosly existing calls should be affected by the change. It should also be straightforward for the callers because resembles the syntax for type hints in Python (and Starlark).

@vladmos vladmos requested a review from a team as a code owner February 4, 2026 16:01
@vladmos vladmos requested review from oreflow and removed request for a team February 4, 2026 16:01
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @vladmos, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances Buildozer's attribute manipulation commands by providing a mechanism for users to explicitly define the type of an attribute's value. This addresses issues where Buildozer's automatic type inference might lead to incorrect formatting or interpretation of values, particularly for set, set_if_absent, and add operations. The change introduces a clear and intuitive syntax for type hints, improving the reliability and predictability of Buildozer's output.

Highlights

  • Explicit Type Specification: Introduced a new syntax (e.g., attr:type value) for set, set_if_absent, and add commands, allowing callers to explicitly specify the type of an attribute value. This prevents Buildozer from incorrectly guessing the attribute's type.
  • Supported Types: The new type hints support string, label, int, raw, ident, list, raw_list, list_of_strings, and list_of_labels.
  • Backward Compatibility: The new syntax is backward-compatible, as attribute names cannot contain colons, ensuring existing calls are unaffected.
  • Internal Type Handling: Added an AttrType enum and a parseAttr function to handle the new type specification internally, modifying cmdAdd, cmdSet, and cmdSetIfAbsent to utilize this explicit type information when constructing attribute expressions.
  • New Test Cases: Extensive new test cases have been added to buildozer_test.sh to cover various scenarios of explicit type usage for add, set, and set_if_absent commands, including handling unknown types.
Changelog
  • buildozer/buildozer_test.sh
    • Added test_add_override_ident to verify adding an identifier type.
    • Added test_add_override_ident2 to verify adding multiple raw list items.
    • Added test_add_override_unknown_type to ensure proper error handling for unrecognized types.
    • Added test_set_if_absent_override_string to test setting a string type if absent.
    • Added test_set_override_string to test setting a string type.
    • Added test_set_override_ident to test setting an identifier type.
    • Added test_set_override_list_of_strings to test setting a list of strings type.
    • Added test_set_override_list_of_idents to test setting a list of identifiers type.
  • edit/buildozer.go
    • Introduced AttrType enum for explicit attribute type representation.
    • Implemented parseAttr function to extract attribute name and its explicit type from command arguments.
    • Modified cmdAdd to use parseAttr and pass the explicit type to AddValueToListAttribute.
    • Modified cmdReplace to pass notProvidedTypeAttr to getAttrValueExpr.
    • Modified cmdSet and cmdSetIfAbsent to use parseAttr and pass the explicit type to getAttrValueExpr.
    • Updated getAttrValueExpr to prioritize explicit AttrType over inferred types (IsIntList, IsList, IsString) for value expression creation, supporting rawAttr, rawListAttr, stringListAttr, and stringAttr.
Activity
  • No specific activity (comments, reviews, etc.) has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request introduces explicit type hints for set and add commands, allowing callers to specify how attribute values should be interpreted. This is a valuable enhancement for controlling attribute formatting, especially when the automatic type inference might be incorrect. The new AttrType enum and parseAttr function provide a clean way to handle these explicit types. New test cases have been added to buildozer/buildozer_test.sh to cover the new functionality, which is good. However, there is a minor issue in the getAttrValueExpr function where a redundant and unreachable code block exists for rawListAttr type handling.

Comment on lines +655 to +661
case attrType == rawListAttr:
// list of arbitrary objects (idents, ints, already quoted strings, etc.)
var list []build.Expr
for _, arg := range args {
list = append(list, &build.Ident{Name: arg})
}
return &build.ListExpr{List: list}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The case attrType == rawListAttr: at line 655 is unreachable. The previous case statement at line 641 already handles attrType == rawListAttr || (attrType == notProvidedTypeAttr && IsIntList(attr)). Since the switch statement executes the first matching case, the block at lines 655-661 will never be reached. This block should be removed to avoid dead code and potential confusion, or the logic should be refactored to ensure correct and distinct handling if different behaviors are intended for rawListAttr under different conditions.

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.

1 participant