Complete support for Starlark type annotations

[tetromino]

This change introduces parsing, AST, formatting, and linting support for experimental Bazel Starlark type annotations
(bazelbuild/bazel#27370) and Buck2 type syntax:

• Type alias statements (type OptionalDict[T, U] = dict[T, U] | None).
• Generic def statements (def foo[T, U](x: T) -> U).
• Var statements (x: list[int])
• type_* AST nodes to cleanly distinguish type syntax from standard expressions. In particular, this allows us to improve formatting for complex type applications, and to allow the ellipsis (...) only in type expressions.

Not supported yet: cast and isinstance expressions - these will be added in a follow-up PR; supporting them is a bit tricky due to the limitations of yacc's LALR(1) grammar.

Disclosure: Gemini-assisted.

TAG=agy
CONV=ae2ff586-d57d-4fd4-a22a-883aec5d0802

Buildtools PR checklist

• The code in this PR is covered by unit/integration tests.
• I have tested these changes and provide testing instructions below.
• I have either responded to, or resolved all Gemini comments on the PR.
• I have read Google Eng Practices on Small Changes, this PR either follows these guidelines or the description provides reasoning for why they can not be followed.

@brandjon @susinmotion FYI

[tetromino]

Note that this PR is required to unblock dogfooding of Starlark types in Bazel.

[oreflow]

Cleaning up past reviewers due to lack of capacity, and buildtools were now moved to be bazel-contrib community supported.

[meisterT]

cc @fmeum

[tetromino]

cc @alexeagle also

[fmeum]

[🤖 Claude] This review was performed by Claude (Claude Code), at fmeum's request; fmeum triaged the findings but Claude wrote every comment. Each finding was reproduced locally at head 80211fc against base 5d08cfd — none are speculative. Inline comments cover the findings that anchor to diff lines; the ones below touch code this PR doesn't modify but whose behavior it changes.

build/parse.y:1630 — Tuples in isinstance are silently rewritten to lists

toTypeExpr rewrites every non-empty TupleExpr into a TypeListExpr, so a tuple literal passed to cast/isinstance is silently reformatted into a list, changing program semantics:

if isinstance(x, (int, str)):

formats to

if isinstance(x, [int, str]):

For a user-defined isinstance helper doing membership/type checks, a tuple argument becomes a list.

warn/warn_control_flow.go:729 — Bare x: int statement treated as a use, not a declaration

A bare var statement x: int (a TypedIdent used as a statement) is treated as a use rather than a declaration: findUninitializedVariables reports the annotation itself as reading an uninitialized variable, and the unused-variable collector marks the name as used.

def f():
    x: int
    x = 1
    return x

→ false Variable "x" may not have been initialized. on the annotation line; conversely, x: int marking x as used suppresses legitimate unused-variable findings.

warn/warn_control_flow.go:563 — Type alias names are never treated as definitions by the variable checks

The redefined-variable and unused-variable checks never treat TypeAliasStmt as defining its Name (their statement scans only match AssignExpr):

• type A = int followed by type A = str and A = 5 produces no redefined-variable finding.
• In

def f():
    a = 1
    type a = int
    return 2

no warning is emitted at all: WalkOnce exposes the alias Name as an ordinary Ident, which counts as a use of a and hides the genuinely unused a = 1.

bzlenv/bzlenv.go:181 — bzlenv never declares type parameters or bare annotated variables

declareLocalVariables handles only AssignExpr/IfStmt/ForStmt, so a bare x: int statement inside a function declares nothing — while the same statement at top level is declared via declareGlobals, an asymmetric resolution. And DefStmt/TypeAliasStmt TypeParams are never declared in any scope, so def foo[T](x: T) -> T leaves T unresolvable for every bzlenv-based consumer (warn_bazel_api, type inference, naming checks), which misclassify it as an undeclared global.

[fmeum]

[🤖 Claude] Comments before a closing type bracket detach and corrupt the file

in.order() for TypeListExpr/CastExpr/IsInstanceExpr registers no closing-bracket End anchor (unlike CallExpr, which does in.order(&v.End)), so a comment before the closing bracket re-attaches to the next pre-order node. In an annotated assignment this splits the statement and produces unparseable output while buildifier exits 0:

x: dict[
    str,
    # keep me
] = {}

is rewritten to

x: dict[
    str,
] =
# keep me
{}

and the next parse fails at 4:1. Similarly, in

def f(x: list[
    int,
    # c
]): pass

the comment silently migrates into the function body, and comments before the ) of a multiline cast/isinstance move out of the expression. Base handles the equivalent inputs correctly.

[tetromino]

I think the conclusion is that TypeList and TypeDict were a bad idea: they don't add any value in the current state of the code, and are a route for bugs to enter due to differences in behavior from normal ListExpr/DictExpr. So I am removing them and using normal ListExpr/DictExpr instead.

And I'm updating TypeApplication to print like a CallExpr.

Fixed by a179a58

[fmeum]

[🤖 Claude] WalkOnce drops Edit() results on TypeAliasStmt; unchecked assertions panic

Two issues in the new traversal code:

1. The TypeAliasStmt case passes the callback pointers to local copies of Name and TypeParams without writing the results back (unlike the sibling DefStmt case), so Edit() replacements are silently dropped: renaming every Ident on type Foo[T] = dict[T, Foo] renames only the right-hand side and leaves the declaration untouched.
2. The new write-backs elsewhere use unchecked type assertions: an Edit on def foo(T: int) that substitutes a StringExpr for the parameter Ident panics with interface conversion: build.Expr is *build.StringExpr, not *build.Ident at v.Ident = ident.(*Ident) — a crash class that didn't exist before, because TypedIdent children weren't walked pre-PR.

[tetromino]

For the type params of TypeAliasStmt and DefStmt, I think to make them editable, we have to pass them as a single list node (otherwise, the editing function cannot add/remove type params). This means, naturally, storing them as a ListExpr. (Which incidentally helps with fixing the formatting of comments in type params!)

As for the identifier of TypeAliasStatement or TypedIdent, an editor function that changes the node to a non-identifier breaks Starlark syntax, and we absolutely should panic here, although of course with a nicer message. (Better would be to report a nice structured error, but alas WalkOnce is a stable API as far as I can tell, and has no error reporting provision...)

Fixed by d8d781a and a01af04

[fmeum]

[🤖 Claude] Comments inside type parameter lists are displaced

DefStmt/TypeAliasStmt print type parameters via a bare printf loop instead of p.seq, so comments attached inside [T, U] have no anchor and are displaced:

• def foo[T, # comment (with U](a): on the next line) → the comment moves inside the parens, above a.
• A multiline type parameter list with a leading # a comment line → the comment lands at column 0 inside the brackets and the parameter is misindented to 8 spaces.
• type Foo[T, # comment → the comment moves to the end of the statement.

All mangled forms persist on reformatting.

[tetromino]

Fixed as a side effect of d8d781a

[fmeum]

[🤖 Claude] Misparsed alias statements report the wrong error position

The manual validation in the type_alias_stmt action (yylex.Error("syntax error near " + $1.(*Ident).Name)) fires only after the whole rule is reduced, so a misparse reports a later position and a different token than before: foo bar = baz now reports 2:1: syntax error near foo (past the end of the line, naming the first token) where base reports 1:8: syntax error near bar. Editor and CI diagnostics point at the wrong place for every such error.

[tetromino]

Nice catch! Fixed by b13f47f

[fmeum]

[🤖 Claude] Multi-line type aliases format with a double space before the backslash

TypeAliasStmt printing emits " = " and breakline() then adds another space before the backslash continuation, so every multi-line alias formats to = \ with a double space — and since type_expr accepts neither a newline after = nor parentheses, every alias too long for one line hits it:

type T = \
    int | float

formats (idempotently, verified byte-for-byte) to

type T =  \
    int | float

[tetromino]

Fixed by b13f47f

[tetromino]

build/parse.y:1630 — Tuples in isinstance are silently rewritten to lists

Claude has a point. However, fixing it properly is a fair bit of additional code. For ease of review, I will remove support for cast / isinstance for now, and re-add them in a follow-up PR.

[tetromino]

warn/warn_control_flow.go:729 — Bare x: int statement treated as a use, not a declaration

Fixed by 12fce3f

warn/warn_control_flow.go:563 — Type alias names are never treated as definitions by the variable checks

We only want to allow type aliases at top level; I added a check to the parser to enforce this.

As for redefinition of type aliases, I'm a bit on the fence whether that should be a fatal error or a lint warning. (Redefinition of type aliases is incompatible with the static type checking algorithm!) In Bazel at the moment, it's fatal only if type checking is enabled but non-fatal if when parsing type annotations, so I suppose we could make it a warning for now in Buildifier too - but a more strongly-worded one that for redefinition of globals.

Fixed by c36c8d9 and c36c8d9

bzlenv/bzlenv.go:181 — bzlenv never declares type parameters or bare annotated variables

Good call, fixed by b4c3ac8

[tetromino]

@fmeum - ready for re-review!