feat(rules): add Clojure and ClojureScript review rules - #965
Open
chethanuk wants to merge 1 commit into
Open
Conversation
Contributor
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s). |
chethanuk
force-pushed
the
wf/clojure-language-support
branch
from
August 17, 2026 06:18
02bb357 to
2aa9ccb
Compare
Contributor
Author
|
@lizhengfeng101 Please review and merge this. Future main updates might cause more conflicts. Thanks :) |
chethanuk
force-pushed
the
wf/clojure-language-support
branch
from
August 17, 2026 07:37
2aa9ccb to
71931ce
Compare
wu21-web
approved these changes
Aug 17, 2026
Allowlist .clj/.cljs/.cljc/.edn and route them to a single clojure.md rule doc. ClojureScript is the same language on another host and .cljc compiles to both, so a split doc has no correct answer for .cljc under the first-match-wins router. EDN is included because deps.edn and friends carry real reviewable risk (unpinned git coords, aliases that run code, read-string vs clojure.edn/read-string); it gets its own section since it is data, not code. Exclude patterns are scoped by extension, never bare directories: IsExcludedPath applies every pattern to every path with no language dispatch. The compiled-output pattern covers .js/.cljs/.cljc/.edn under resources/public/js/compiled/, which is what the lein-cljsbuild and lein-figwheel :output-to/:output-dir convention emits: the Closure bundle, verbatim copies of the ClojureScript sources it compiled, and per-namespace *.cljs.cache.edn analysis caches. All four extensions are now allowlisted, so without it a diff that touches that tree reviews cljs/core.cljs as if someone had written it. .cpcache and .shadow-cljs stay because both hold .edn (launchpad writes .cpcache/launchpad.edn, shadow-cljs writes builds/*/bundle-info.edn) and both are tool-owned names nobody writes by hand. target/ is already dropped upstream by providerDirIgnoreDirs, and out/ is skipped because real Clojure gitignores root-anchor it, which this matcher cannot express. No pattern is added for .lein-*, .nrepl-port or .cpcache/*.cp: none of those extensions is allowlisted, so IsAllowedExt rejects them before IsExcludedPath is consulted.
chethanuk
force-pushed
the
wf/clojure-language-support
branch
from
August 17, 2026 12:27
71931ce to
3b4b7d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
.clj,.cljs,.cljcand.ednfiles were never reviewed. None of the four extensions is insupported_file_types.json, soIsAllowedExtdrops them at the filter gate before any rule can run, which is why a rule doc on its own would never have been reachable.This adds all four and registers one rule doc in
path_rule_map:**/*.{clj,cljs,cljc,edn}→clojure.md. Data only, no Go source changes.One doc for all four extensions. ClojureScript is the same language on a different host, and
.cljccompiles to both, so a Clojure/ClojureScript split has no correct answer for.cljc:path_rule_mapis first-match-wins, so whichever of the two patterns is listed first takes it and the other host's rules never load. Multi-extension docs already cover that case,haskell.mdfor.hs/.lhsandnim.mdfor.nim/.nims/.nimble. EDN is data rather than code, so it gets its own section: dependency coordinates pinned to a branch instead of an exact:git/sha, aliases whose:main-optsexecute code, andread-stringwhereclojure.edn/read-stringbelongs.Exclude patterns are scoped by extension rather than by bare directory, since
IsExcludedPathapplies every pattern to every path with no language dispatch. Test files are**/test/**/*.{clj,cljs,cljc}and**/*_test.{clj,cljs,cljc}.Compiled output needs
**/resources/public/js/compiled/**/*.{js,cljs,cljc,edn}. That directory is the lein-cljsbuild and lein-figwheel:output-to/:output-dirconvention, and the tree it writes holds the Closure bundle, verbatim copies of every ClojureScript source it compiled, and per-namespace*.cljs.cache.ednanalysis caches. All four of those extensions are allowlisted,.jsalready before this PR, so without the pattern a diff that touches that tree getscljs/core.cljsreviewed as if someone had written it.resources/public/js/vendor/analytics.jsis a negative row: hand-written JS one directory over stays reviewable.**/.cpcache/**and**/.shadow-cljs/**are the only bare directories. Both names are owned by a tool and never typed by hand, and both trees hold.ednfiles (launchpad writes.cpcache/launchpad.edn, shadow-cljs writesbuilds/*/*/bundle-info.edn), so both can actually fire now that.ednis allowlisted.Nothing is added for
target/.providerDirIgnoreDirsininternal/diff/git.goalready drops it at the diff level, before the allowlist is reached.out/is left alone because real Clojure.gitignores root-anchor it as/out/, which this matcher cannot express, and**/out/**would drop any directory namedoutrepo-wide.Limitation
Clojure diffs that used to be skipped are now reviewed. Anyone who does not want that opts out with user exclude globs, which are checked before the allowlist.
Type of Change
How Has This Been Tested?
make testpasses locallyRows appended to the existing table tests:
TestIsAllowedExt—.clj/.CLJ,.cljs/.CLJS,.cljc/.CLJCand.edn/.EDN.TestIsExcludedPath— both test-file patterns at the repo root and nested; the compiled bundle, a copiedcljs/core.cljs, a*.cljs.cache.ednand a subproject-prefixed output path;.cpcacheand.shadow-cljs; and three negatives,src/myapp/core.clj,src/myapp/test_helpers.cljand the hand-written JS above.TestResolve_DefaultRules—.clj,.cljsand.cljcpaths plusdeps.ednresolve by heading anchors unique toclojure.md.Checklist
go fmt,go vet)The rule table in
pages/src/content/docs/{en,ja,ru,zh}/review-rules.mdlists the new pattern.Related Issues
Part of #470.