Skip to content

fix(gen_srcs): skip unnecessary load symbols#87

Merged
miridius merged 1 commit into
mainfrom
dave/gen-srcs-skip-unused-load
May 11, 2026
Merged

fix(gen_srcs): skip unnecessary load symbols#87
miridius merged 1 commit into
mainfrom
dave/gen-srcs-skip-unused-load

Conversation

@miridius

@miridius miridius commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Depends on #85.

Problem

gen_srcs always emits load("@rules_clojure//:rules.bzl", "clojure_library", "clojure_test") in every BUILD file, even when:

  1. There are no clojure_test targets — the "clojure_test" symbol is loaded but never used.
  2. The directory has no .clj files at all — the entire load() statement is unnecessary.

Solution

Two changes to gen-dir, both derived from a single pass over the rules ns-rules emits — has-binary? and has-tests? come from the same reduce (= :clojure_binary :type, = :clojure_test :type). No re-scanning of paths.

1. Only include "clojure_test" when test files exist

Before (directory with only core.clj, after #85's docstring + load-before-package):

"""
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", "clojure_test")

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

clojure_library(name = "core", ...)

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 = "core", ...)

2. Skip load() for empty directories

Directories with no source files and no clj subdirs (has-content? false) get a BUILD with only the docstring header + package() — no load, no __clj_lib/__clj_files. Matches Gazelle's behaviour.

Before:

"""
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", "clojure_test")

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

After:

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

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

Also

  • test-path? regex anchored: #"_test.clj" was unanchored — matched _test.cljs, _test.cljbar, etc. Tightened to #"_test\.cljc?$" so the (or clj-path? cljc-path?) guard in has-tests? becomes redundant.
  • .clj-kondo/config.edn with exclude-destructured-keys-in-fn-args and exclude-destructured-as — this codebase uses map destructuring in fn args as documentation of expected keys, so unused-binding warnings on those are noise.
  • .gitignore adjusted so .clj-kondo/.cache/ is ignored but .clj-kondo/config.edn is tracked.

@miridius miridius force-pushed the dave/gen-srcs-skip-unused-load branch 4 times, most recently from 0af7ccf to 705fe48 Compare March 20, 2026 20:22
@miridius miridius force-pushed the dave/gen-srcs-skip-unused-load branch from 705fe48 to 8bb3b04 Compare May 11, 2026 13:03
@miridius miridius changed the base branch from main to dave/tool-518-fix-formatting-of-generated-build-files May 11, 2026 13:03
@miridius miridius marked this pull request as ready for review May 11, 2026 13:04
@miridius miridius force-pushed the dave/gen-srcs-skip-unused-load branch 2 times, most recently from 0f89d27 to 2137d8b Compare May 11, 2026 14:30
miridius added a commit that referenced this pull request May 11, 2026
Self-review findings on #87:

- **`has-binary?` + `has-tests?` from same reduce**: `has-binary?`
  already derives from emitted rule `:type`; `has-tests?` was scanning
  paths via a separate predicate. Drift risk. Fold into the same
  reduce: `[hb? ht? acc]`.
- **`test-path?` regex anchored**: `#"_test.clj"` was unanchored —
  matched `_test.cljs`, `_test.cljbar` etc. Tighten to
  `#"_test\.cljc?$"`. Side effect: the `(or clj-path? cljc-path?)`
  guard in the now-deleted `has-tests?` predicate becomes redundant.
- **Tests rewritten**:
  - Helper `load-symbols` parses the load() symbol set (drops the
    bzl-path) — replaces fragile `str/includes?` checks.
  - Helper `run-gen-dir!` sets up a temp dir, calls gen-dir, returns
    the BUILD content, cleans up.
  - New coverage: .cljc tests, .cljs-only (negative), clojure_binary
    via `:bazel/clojure_binary` metadata, subdirs-only with no
    own-files.
  - Empty-dir test extended: `__clj_lib` / `__clj_files` absent,
    package() still present.
@miridius miridius force-pushed the dave/tool-518-fix-formatting-of-generated-build-files branch from b1cf607 to 7945bd3 Compare May 11, 2026 17:33
@miridius miridius force-pushed the dave/gen-srcs-skip-unused-load branch 2 times, most recently from 731ce83 to cc5659a Compare May 11, 2026 17:40
@miridius miridius requested a review from a team May 11, 2026 21:24
Comment thread test/rules_clojure/gen_build_test.clj Outdated
Comment on lines +263 to +265
;; ---- gen-dir load-symbol pruning ----

;; ---- gen-dir load-symbol pruning ----

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
;; ---- gen-dir load-symbol pruning ----
;; ---- gen-dir load-symbol pruning ----
;; ---- gen-dir load-symbol pruning ----

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — duplicated heading removed in c0a9ebf.

@miridius miridius force-pushed the dave/gen-srcs-skip-unused-load branch from cc5659a to c0a9ebf Compare May 11, 2026 21:47
`gen_srcs` always emitted
`load("@rules_clojure//:rules.bzl", "clojure_library", "clojure_test")`
in every BUILD file, even when:

1. There were no `clojure_test` targets — the "clojure_test" symbol was
   loaded but never used.
2. The directory had no `.clj` files at all — the entire `load()`
   statement was unnecessary.

Both conditions are detected from a single `reduce` over the rules that
`ns-rules` emits — `has-binary?` and `has-tests?` come from the same
pass (`= :clojure_binary :type`, `= :clojure_test :type`). No
re-scanning of paths.

Empty directories (`has-content?` false) get a BUILD with the docstring
header + `package()` only — no load, no `__clj_lib` / `__clj_files`
filegroup. Matches Gazelle's behaviour.

Also:
- `test-path?` regex anchored: `#"_test.clj"` was unanchored — matched
  `_test.cljs`, `_test.cljbar`, etc. Tightened to `#"_test\.cljc?$"`.
- `.clj-kondo/config.edn` with
  `exclude-destructured-keys-in-fn-args` + `exclude-destructured-as` —
  this codebase uses map destructuring in fn args as documentation of
  expected keys, so unused-binding warnings are noise.
- `.gitignore` adjusted so `.clj-kondo/.cache/` is ignored but
  `.clj-kondo/config.edn` is tracked.

Test coverage (gen_build_test.clj):
- `load-symbols` helper parses the load() symbol set out of generated
  BUILD content; `run-gen-dir!` sets up a temp dir, calls gen-dir,
  returns content, cleans up.
- `gen-dir-load-symbols`: covers .clj test, .cljc test, .cljs-only
  (negative — `ns-rules` emits `clojure_test` only for clj/cljc), no
  tests.
- `gen-dir-includes-clojure-binary-when-ns-has-bazel-clojure-binary-meta`:
  a ns with `:bazel/clojure_binary` metadata adds "clojure_binary" to
  the load.
- `gen-dir-subdirs-only`: dir with no own files but a clj-only subdir
  still emits load + `__clj_lib` referencing the subdir.
- `gen-dir-skips-load-for-empty-dirs`: `__clj_lib` / `__clj_files`
  absent, `package()` still emitted.
@miridius miridius force-pushed the dave/gen-srcs-skip-unused-load branch from c0a9ebf to e6c6037 Compare May 11, 2026 21:50
@miridius miridius changed the base branch from dave/tool-518-fix-formatting-of-generated-build-files to main May 11, 2026 21:50
@miridius miridius merged commit 1d4cbba into main May 11, 2026
1 check was pending
@miridius miridius deleted the dave/gen-srcs-skip-unused-load branch May 13, 2026 19:33
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