Fix formatting of generated BUILD.bazel files

[miridius]

Problem

Generated BUILD.bazel files use non-standard formatting: all kwargs on one line, no attribute sorting, no trailing commas, #autogenerated comment header. This causes IDEs configured with buildifier to reformat the entire file on open, creating noisy diffs (banksy#15022 comment).

Solution

Rewrite the emit-bazel* multimethod system in gen_build.clj to produce output identical to buildifier --type=build. All formatting rules were verified empirically by running buildifier on sample files and by regenerating the entire banksy-3 tree (1373 BUILD files) and confirming gazelle from #84 produces byte-identical output.

• Attribute sorting: by buildifier's NamePriority table (name first, srcs early, deps last, unknowns alphabetically)
• List formatting: multi-element lists go multi-line (8-space indent, trailing commas); single-element stays inline
• Label sorting: phase-based ordering matching buildifier's byStringExpr (: < // < @, then lexicographic within phase). Only for known sortable attrs (deps, srcs, resources, etc.) — ordering-sensitive attrs like aot are preserved
• Function calls: multi-line with 4-space indent for >1 kwarg; inline for single kwarg (package(...)) or positional-only (load(...))
• load() args sorted: positional symbol args after the bzl path go alphabetical (buildifier rule)
• Dict literals: {"k": "v"} (no space before colon)
• Label shortening: //path/to/pkg:pkg → //path/to/pkg when the target name equals the trailing path segment
• Standalone vectors: inline with ", " separator (multi-line still happens for kwarg-context list values)
• Headers: triple-quote docstring replaces #autogenerated comment; load before package; final \n at end of file

A buildifier round-trip test pipes generated content through buildifier --type=build (provided as a bazel_dep via buildifier_prebuilt, resolved at test time through runfiles-env) and asserts zero diff.

Before

#autogenerated, do not edit
package(default_visibility = ["//visibility:public"])
load("@rules_clojure//:rules.bzl", "clojure_library", "clojure_test")

clojure_library(name = "build",
    deps = ["//resources:data_readers","@deps//:ns_io_github_clojure_tools_build_clojure_tools_build_api","@deps//:org_clojure_clojure"],
    resources = ["build.clj"],
    resource_strip_prefix = "dev",
    srcs = ["build.clj"],
    aot = ["build"],
    runtime_deps = ["@deps//:com_fzakaria_slf4j_timbre"],
    jvm_flags = ["-Dclojure.main.report=stderr"])

After

"""
Do not edit this file manually!
It is automatically generated by the `//:gen_srcs` target from `rules_clojure`.
"""

load("@rules_clojure//:rules.bzl", "clojure_library")

package(default_visibility = ["//visibility:public"])

clojure_library(
    name = "build",
    srcs = ["build.clj"],
    aot = ["build"],
    jvm_flags = ["-Dclojure.main.report=stderr"],
    resource_strip_prefix = "dev",
    resources = ["build.clj"],
    runtime_deps = ["@deps//:com_fzakaria_slf4j_timbre"],
    deps = [
        "//resources:data_readers",
        "@deps//:ns_io_github_clojure_tools_build_clojure_tools_build_api",
        "@deps//:org_clojure_clojure",
    ],
)

CI / build

• MODULE.bazel gains buildifier_prebuilt as a dev_dependency; the gen-build-test target declares the buildifier binary in data and exports its runfiles path via the BUILDIFIER env var. The round-trip test resolves it through test-utils/runfiles-env — no which/PATH lookup.
• .circleci/config.yml: no install step needed (deps come from bazel).

Reviewers: note

The first re-run of gen_srcs after this lands will produce a one-time diff on every generated BUILD.bazel (header swap + reformat + label shortening + load symbol sort). The round-trip test fails explicitly if buildifier is absent rather than silently skipping.

[english]

nice one!

[miridius]

@english — heads-up that the branch has changed substantively since your approval on e190648. The current HEAD is 7945bd3 (single squashed commit). Additions:

1. buildifier_prebuilt is now a bazel dev_dependency. The round-trip test you reviewed relied on (shell/sh "which" "buildifier") to find buildifier on PATH, which didn't work in CI. gen-build-test now declares the binary as a data dep and the test resolves it through test-utils/runfiles-env (env var BUILDIFIER). Pins v8.5.1.2.
2. Dict literal spacing: {"k" : "v"} → {"k": "v"} (no space before colon).
3. load() arg sort: positional symbol args after the bzl path go alphabetical (buildifier rule).
4. Label shortening: //path/to/pkg:pkg collapses to //path/to/pkg when the target name equals the trailing path segment.
5. Trailing newline at end of every generated BUILD.bazel.

(1) fixes a CI failure, (2)–(5) were caught dogfooding against #84's Gazelle plugin output in banksy.

Do you want to re-review or are you happy to keep the existing approval?

posted by Claude