Kimirock/metabase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Give :discouraged-var and :discouraged-namespace their own per-symbol ratchet budgets (#83352)

Browse Source 
* Remove stale :discouraged-namespace ignores

These six ignores suppressed a namespace-level policy that no longer
exists in config.edn -- clojure.pprint, clojure.java.io, clj-yaml.core,
metabase.util.jvm, and metabase.config.core aren't configured as
discouraged namespaces anymore (clojure.pprint is only discouraged at
the var level now). Verified clean with a scoped kondo run requesting
:discouraged-namespace at :warning over just these files: zero findings.

* Give :discouraged-var and :discouraged-namespace their own per-symbol ratchet budgets

One shared budget per linter let a new ignore on a previously-clean var
land silently as long as the linter's aggregate count had room, and made
"introduce a policy for just this one var" impossible. Each configured
symbol now gets its own field: :discouraged-var-counts and
:discouraged-namespace-counts, siblings of :ignore-counts, keyed to
mirror the symbol's own namespace and name rather than the linter
that discourages it (metabase.entity-retrieval.core/search-unfiltered
-> :entity-retrieval.core/search-unfiltered; an enterprise namespace of
the same shape gets an `ee.` prefix so it can't collide with its OSS
counterpart).

Attribution resolves which symbol an inline ignore covers by running a
scoped `clojure -M:kondo` analysis pass over the files that carry one of
these two ignores (JSON output, since a generated var can have a name
that isn't valid EDN) and matching each ignore to the nearest covered
usage of a known symbol -- this is the one thing in the ratchet tooling
that needs a live kondo run, so `./bin/mage kondo-ratchets` is no longer
pure Babashka. `:comment-exempt` is untouched: occurrences still carry
the bare linter name in :linters, only the two new count fields are
keyed per symbol.

`--seed :discouraged-var`/`:discouraged-namespace` (no specific symbol)
seeds every symbol currently configured under it in one shot, unused
ones at zero, the same ergonomics as landing a new linter. A stale flat
:ignore-counts entry for either linter is self-healing: `fix!` drops it
every run and `check` fails if one reappears, since it would otherwise
double-count every occurrence the new field already covers.

Ported onto the prod/test ratchet split: both ratchet files carry the
per-symbol fields, and each file's occurrences are attributed separately.

* Attribute discouraged-var ignores from kondo findings, not nearest usage

Matching each ignore to the nearest later usage of a configured symbol
counted usages kondo never flagged (scoped :config-in-ns entries, :off
overrides) and let a stale ignore claim an unrelated usage. Attribution
now lints copies of the files with the discouraged ignores disabled,
joins each finding to its var or namespace usage at the same row and
column, and charges it to the last ignore before it naming the linter.
An ignore counts once per file and distinct symbol it covers. The copies
rename the ignore key to one of the same length rather than blanking the
ignore, so every form reads as before and every row and column stays put.

A failed kondo run now throws instead of zeroing every budget. Findings
that resolve to no configured symbol fail the check and stop the shrink
from writing; ignores that cover no finding only warn. Without an
:attribute, a discouraged ignore throws rather than reading as zero.
Each ratchet file's occurrences are attributed separately.

`--seed :discouraged-var/clojure.core/println` seeds one symbol, a bare
`--seed :discouraged-var` seeds every symbol at its actual count, and an
empty suffix is rejected. Bulk seeding at zero is gone: the next shrink
dropped those zeros anyway. The shrink workflow runs prepare-backend,
since the shrink now needs the Clojure CLI.

Budgets are re-seeded from the new attribution. Several were undercounted
or charged to the wrong symbol before.

This squashes the unpushed review-fix commits on top of the per-symbol
commit, since #83383 rewrote the same functions each of them changed.

* Remove a stale :discouraged-namespace ignore on clj-yaml.core

clj-yaml.core is no longer a discouraged namespace, so the ignore on its
require in lib-be.metadata.jvm suppresses nothing.

* Don't charge findings kondo reports with the ignores in place

Attribution charged every discouraged finding on the disabled-ignore
copies to the last ignore before it, including findings no ignore
suppresses. Unsuppressed usages in dev/ inflated the eval, println and prn
budgets, and a new usage of a symbol an earlier ignore already covers
went uncounted.

Lint the original files in the same kondo run and drop any finding they
also report, so only findings an ignore suppresses are attributed. An
ignore nested inside another for the same linter still takes the
findings under it, leaving the outer one reported as covering nothing.

* Parse kondo's JSON output with babashka.json

mage already depends on babashka.json, so attribution doesn't need
cheshire or the :discouraged-namespace ignore that came with it. That
returns the cheshire.core budget to 4.

* Attribute prod and test ignores in one kondo run

check and fix! attributed the prod and test occurrences in separate
calls, and each call started its own JVM and lint pass. :attribute now
takes a seq of occurrence groups and returns one attribution per group,
so attribute-occurrences! lints every file once and splits the result.
kondo-ratchets drops from about 39s to 33s.

* Drop budgets for symbols no longer configured instead of refusing

Removing a symbol from config.edn left its per-symbol budget naming an
unknown key, and both check and the shrink threw on it. The shrink, the
tool meant to repair that drift, couldn't. Two PRs could each pass alone
and break the shrink on master: one seeding a symbol, the other removing
it from the config. check now warns about such a budget, and the shrink
drops it at zero like any unused budget, since attribution never charges
an unconfigured symbol. --seed still rejects an unconfigured symbol.

A bare --seed :discouraged-var no longer warns once per configured symbol
with no ignores, and check's ok line counts the per-symbol budgets
alongside :ignore-counts.

* Share the ratchet setup and config walks between their callers

check and fix! repeated the same setup: validate the linters, read the
module and test ratchet files, scan, split prod from test, and attribute.
They now share read-policies and scan-and-attribute. fix! still resolves
and validates --seed before the scan, so a bad seed fails without waiting
on kondo.

The repository-linter and discouraged-symbol lookups each walked
config.edn with an atom to collect nested :linters maps. Both now read
them from one linters-maps. The budget-line formatter is shared, and the
empty and mergeable field sets derive from ratchet-field-order. Dropped
the unused one-argument discouraged-symbols.

* Correct the ratchet task's runtime claim and plain up its wording

The kondo-ratchets :doc said the check takes seconds; with the kondo
attribution pass it takes about half a minute. The CLAUDE.md paragraph
and the ratchet-file headers no longer open sentences with code.

The unattributed-ignore warning now names a nested ignore for the same
linter as a cause: the inner ignore takes the finding, so the outer one
reads as covering nothing even though removing it would surface a
warning.

* Test that a failed kondo run throws rather than reading as no findings

If a failed or garbled kondo run read as an empty result, attribution
would count zero ignores and the shrink would drop every per-symbol
budget. The exit and parse checks move into parse-kondo-output, which
tests now cover for a nonzero exit and for output that isn't a map.

The attribution tests build matching findings and var usages with one
var-hits helper, and the check test's attribution stub reads as what it
returns.

* Refuse colliding discouraged symbols before an unseeded shrink

Dropping the unknown-key validation also dropped the shrink's only call
to discouraged-count-keys, which throws when two configured symbols share
a ratchet key. An unseeded shrink could then merge both symbols' ignores
into one budget. read-policies now reads the configured keys up front, so
check and fix! both refuse a collision before scanning.

* Reject kondo output without a findings list

A run that exits cleanly but prints a map with no :findings, such as {},
parsed as a result with no findings. Attribution would then count zero
ignores and the shrink would drop every per-symbol budget.
parse-kondo-output now requires :findings to be a list. A missing
analysis bucket already fails loudly, since every finding comes back
unresolved.

* Note where nested ignores leave a budget short

Attribution charges a finding to the last ignore before it, so an outer
ignore around a nested one for the same linter counts toward no budget.
State that next to the rule, where a reader of the code will look for it.

* Flag the nested-ignore undercount as a known gap

Lead the comment with the gotcha and its consequence, a budget one short
that an unattributed warning hides behind, so a reader doesn't mistake
the last-ignore rule for a complete one.

* Key aliased discouraged vars by the namespace the file requires

Kondo resolves a re-exported var to the namespace it learned the var came
from, and what it has learned depends on its cache and on which files
share the run. lib.core re-exports ->legacy-MBQL from lib.convert, so
after an ordinary lint of lib/core.cljc the same ten ignores moved from
:lib.core/->legacy-MBQL to :lib.convert/->legacy-MBQL and the check
failed.

A var used through an alias now takes its namespace from the file's own
require for that alias, per language so a .cljc file's clj and cljs
requires stay apart, and the attribution lint runs with --cache false.

* Split the attribution test by behaviour

One deftest each for counting, which findings are charged, and how a
finding resolves to a symbol, so a failure names the behaviour that broke.

* Say a stale flat discouraged budget is never read, not double-counted

The check removes these linters from the occurrences before comparing
:ignore-counts, so a leftover flat entry is ignored rather than counted a
second time.

* Mark the nested-ignore gap as a way past the ratchet, with a TODO

A new ignore wrapped around an existing one for the same linter needs no
budget, so say that plainly and record the planned fix: lint copies with
one ignore disabled each and charge a finding to the copy that reveals it.
This commit is contained in:
Chris Truter
2026-10-02 18:42:32 -04:00
committed by GitHub
parent 18b6929ad7
commit ade2ab95fa
19 changed files with 1466 additions and 374 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
.clj-kondo/ratchets-test.edn
View File

@@ -1,35 +1,56 @@
;; Budgets for kondo suppressions in test code: inline `:clj-kondo/ignore` forms per linter
;; (:ignore-counts). Production code has its own budgets in .clj-kondo/ratchets.edn, which also holds
;; every config-level waiver (:config-counts) -- config.edn's scoping doesn't cleanly split.
;; Each :ignore-counts value is a non-negative integer budget, or :unlimited for no ceiling.
;; The :discouraged-var-counts and :discouraged-namespace-counts fields budget those linters' inline
;; ignores per symbol, keyed by the symbol with `metabase.` dropped and `metabase-enterprise.` shortened
;; to `ee.`.
;; Each :ignore-counts value is a non-negative integer budget, or :unlimited for no ceiling; every other
;; budget is a non-negative integer.
;; Checks fail when a count exceeds its numeric budget; unused budget is allowed.
;; Any ignore outside :comment-exempt needs an explanatory comment directly above or trailing on its line.
;; `./bin/mage kondo-ratchets-shrink` lowers budgets unless `--seed` explicitly adds or raises one.
;; `./bin/mage kondo-ratchets-shrink` lowers budgets unless `--seed` explicitly adds or raises one;
;; a seed like `:discouraged-var/clojure.core/println` seeds one symbol, and `:discouraged-var` all of them.
;; The workflow runs it on master, so feature branches do not need to record reductions.
;; Raising or adding a budget (`--seed` for inline ignores, a manual edit otherwise) or widening the
;; exemptions must be explained in the PR.
;; :all is the vector-less ignore form, which suppresses every linter on the next form.
{:ignore-counts {:case-symbol-test 1
:clojure-lsp/unused-public-var 1
:deprecated-namespace 66
:deprecated-var 32
:discouraged-namespace 8
:discouraged-var 59
:equals-true 1
:metabase/disallow-hardcoded-driver-names-in-tests 34
:metabase/discourage-dynamic-vars 41
:metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests 13
:metabase/namespace-name 1
:metabase/prefer-with-dynamic-fn-redefs :unlimited
:metabase/test-helpers-use-non-thread-safe-functions 18
:metabase/validate-deftest 9
:missing-protocol-method 3
:type-mismatch 6
:uninitialized-var 1
:unresolved-symbol 2
:unused-binding 6
:unused-private-var 2}
:comment-exempt #{:deprecated-namespace
:discouraged-namespace
:discouraged-var
:metabase/discourage-dynamic-vars}}
{:ignore-counts {:case-symbol-test 1
:clojure-lsp/unused-public-var 1
:deprecated-namespace 66
:deprecated-var 32
:equals-true 1
:metabase/disallow-hardcoded-driver-names-in-tests 34
:metabase/discourage-dynamic-vars 41
:metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests 13
:metabase/namespace-name 1
:metabase/prefer-with-dynamic-fn-redefs :unlimited
:metabase/test-helpers-use-non-thread-safe-functions 18
:metabase/validate-deftest 9
:missing-protocol-method 3
:type-mismatch 6
:uninitialized-var 1
:unresolved-symbol 2
:unused-binding 6
:unused-private-var 2}
:discouraged-namespace-counts {:clojure.tools.logging 5
:toucan2.core 1}
:discouraged-var-counts {:clojure.core/pr 1
:clojure.core/print 1
:clojure.core/printf 2
:clojure.core/println 9
:clojure.core/prn 1
:clojure.core/time 1
:clojure.java.jdbc/get-connection 1
:clojure.java.jdbc/with-db-connection 1
:clojure.pprint/print-table 1
:clojure.string/lower-case 1
:compojure.core/routes 2
:ee.entity-retrieval.core/search-unfiltered 6
:entity-retrieval.core/search-unfiltered 1
:entity-retrieval.mirror/search-unfiltered 1
:lib.equality/find-column-indexes-for-refs 2
:test/with-temp 28}
:comment-exempt #{:deprecated-namespace
:discouraged-namespace
:discouraged-var
:metabase/discourage-dynamic-vars}}

177
.clj-kondo/ratchets.edn
View File

@@ -1,79 +1,118 @@
;; Budgets for kondo suppressions: inline `:clj-kondo/ignore` forms per linter (:ignore-counts), and
;; config-level waivers in .clj-kondo/config.edn (:config-counts -- :off switches and :exclude entries).
;; Each :ignore-counts value is a non-negative integer budget, or :unlimited for no ceiling.
;; The :discouraged-var-counts and :discouraged-namespace-counts fields budget those linters' inline
;; ignores per symbol, keyed by the symbol with `metabase.` dropped and `metabase-enterprise.` shortened
;; to `ee.`.
;; Each :ignore-counts value is a non-negative integer budget, or :unlimited for no ceiling; every other
;; budget is a non-negative integer.
;; Checks fail when a count exceeds its numeric budget; unused budget is allowed.
;; Any ignore outside :comment-exempt needs an explanatory comment directly above or trailing on its line.
;; `./bin/mage kondo-ratchets-shrink` lowers budgets unless `--seed` explicitly adds or raises one.
;; `./bin/mage kondo-ratchets-shrink` lowers budgets unless `--seed` explicitly adds or raises one;
;; a seed like `:discouraged-var/clojure.core/println` seeds one symbol, and `:discouraged-var` all of them.
;; The workflow runs it on master, so feature branches do not need to record reductions.
;; Raising or adding a budget (`--seed` for inline ignores, a manual edit otherwise) or widening the
;; exemptions must be explained in the PR.
;; :all is the vector-less ignore form, which suppresses every linter on the next form.
;;
;; Test code's :ignore-counts has its own budget in .clj-kondo/ratchets-test.edn.
;; Test code's inline ignores have their own budgets in .clj-kondo/ratchets-test.edn.
;; :config-counts stays here either way -- config.edn's scoping doesn't cleanly split.
{:ignore-counts {:aliased-namespace-symbol 3
:clojure-lsp/unused-public-var 7
:consistent-alias 2
:def-fn 1
:deprecated-namespace 29
:deprecated-var 53
:discouraged-java-method 4
:discouraged-namespace 60
:discouraged-var 73
:duplicate-require 1
:main-without-gen-class 1
:metabase/discourage-dynamic-vars 324
:metabase/discourage-millis-duration 1
:metabase/modules 23
:metabase/namespace-name 3
:metabase/validate-defendpoint-has-response-schema 427
:metabase/validate-defendpoint-query-params-use-kebab-case 48
:metabase/validate-defendpoint-route-uses-kebab-case 44
:missing-docstring 4
:non-arg-vec-return-type-hint 1
:reduce-without-init 3
:redundant-do 1
:syntax 2
:type-mismatch 2
:unquote-not-syntax-quoted 1
:unresolved-namespace 3
:unresolved-symbol 9
:unsorted-required-namespaces :unlimited
:unused-alias 2
:unused-binding 3
:unused-excluded-var 2
:unused-namespace 2
:unused-private-var 5
:unused-value 1}
:config-counts {:constant-condition 1
:deprecated-namespace 7
:deprecated-var 16
:discouraged-namespace 14
:discouraged-var 14
:inline-def 1
:main-without-gen-class 1
:metabase/defsetting-must-specify-export 1
:metabase/defsetting-namespace 1
:metabase/discourage-dynamic-vars 1
:metabase/prefer-with-dynamic-fn-redefs 4
:metabase/t2-query-namespace 2
:metabase/table-or-field-query 1
:metabase/unsafe-app-db-query 1
:missing-docstring 1
:private-call 1
:redundant-ignore 1
:redundant-primitive-coercion 1
:refer-all 1
:type-mismatch 1
:unexpected-recur 1
:unresolved-symbol 15
:unresolved-var 1
:unused-referred-var 4}
:comment-exempt #{:deprecated-namespace
:discouraged-namespace
:discouraged-var
:metabase/discourage-dynamic-vars
:metabase/modules
:metabase/validate-defendpoint-has-response-schema
:metabase/validate-defendpoint-query-params-use-kebab-case
:metabase/validate-defendpoint-route-uses-kebab-case}}
{:ignore-counts {:aliased-namespace-symbol 3
:clojure-lsp/unused-public-var 7
:consistent-alias 2
:def-fn 1
:deprecated-namespace 29
:deprecated-var 53
:discouraged-java-method 4
:duplicate-require 1
:main-without-gen-class 1
:metabase/discourage-dynamic-vars 324
:metabase/discourage-millis-duration 1
:metabase/modules 23
:metabase/namespace-name 3
:metabase/validate-defendpoint-has-response-schema 427
:metabase/validate-defendpoint-query-params-use-kebab-case 48
:metabase/validate-defendpoint-route-uses-kebab-case 44
:missing-docstring 4
:non-arg-vec-return-type-hint 1
:reduce-without-init 3
:redundant-do 1
:syntax 2
:type-mismatch 2
:unquote-not-syntax-quoted 1
:unresolved-namespace 3
:unresolved-symbol 9
:unsorted-required-namespaces :unlimited
:unused-alias 2
:unused-binding 3
:unused-excluded-var 2
:unused-namespace 2
:unused-private-var 5
:unused-value 1}
:discouraged-namespace-counts {:cheshire.core 4
:clojure.tools.logging 3
:grouper.core 1
:legacy-mbql.normalize 14
:legacy-mbql.schema 14
:legacy-mbql.util 2
:query-processor.store 8
:toucan2.core 3
:util.malli.schema 2}
:discouraged-var-counts {:clojure.core/eval 6
:clojure.core/print 1
:clojure.core/println 9
:clojure.core/prn 3
:clojure.core/tap> 5
:clojure.core/update-keys 1
:clojure.java.jdbc/get-connection 2
:clojure.pprint/pprint 2
:clojure.string/lower-case 2
:clojure.string/upper-case 1
:clojurewerkz.quartzite.jobs/defjob 1
:compojure.core/context 2
:compojure.core/defroutes 4
:compojure.core/routes 2
:ee.transform-testing.expectations.empty/->Empty 1
:ee.transform-testing.expectations.equals/->EqualsRows 1
:ee.transform-testing.expectations.equals/->EqualsSql 1
:entity-retrieval.core/search-unfiltered 1
:entity-retrieval.mirror/search-unfiltered 1
:lib.core/->legacy-MBQL 10
:lib.equality/find-column-indexes-for-refs 1
:malli.core/explain 1
:malli.core/explainer 1
:malli.core/validate 1
:malli.core/validator 1
:me.flowthing.pp/pprint 2
:next.jdbc/active-tx? 1}
:config-counts {:constant-condition 1
:deprecated-namespace 7
:deprecated-var 16
:discouraged-namespace 14
:discouraged-var 14
:inline-def 1
:main-without-gen-class 1
:metabase/defsetting-must-specify-export 1
:metabase/defsetting-namespace 1
:metabase/discourage-dynamic-vars 1
:metabase/prefer-with-dynamic-fn-redefs 4
:metabase/t2-query-namespace 2
:metabase/table-or-field-query 1
:metabase/unsafe-app-db-query 1
:missing-docstring 1
:private-call 1
:redundant-ignore 1
:redundant-primitive-coercion 1
:refer-all 1
:type-mismatch 1
:unexpected-recur 1
:unresolved-symbol 15
:unresolved-var 1
:unused-referred-var 4}
:comment-exempt #{:deprecated-namespace
:discouraged-namespace
:discouraged-var
:metabase/discourage-dynamic-vars
:metabase/modules
:metabase/validate-defendpoint-has-response-schema
:metabase/validate-defendpoint-query-params-use-kebab-case
:metabase/validate-defendpoint-route-uses-kebab-case}}

4
.github/workflows/backend.yml vendored
View File

@@ -58,8 +58,8 @@ jobs:
- name: Run migration checks
if: ${{ inputs.force-run || inputs.run-project-migration-checks }}
run: ./bin/mage project-tests migrations
# Babashka, not a JVM test run, so it is cheap enough to run alongside the backend checks.
# It still needs the JDK prepare-backend installs, to resolve clj-kondo on a cold cache.
# Babashka plus a scoped clj-kondo run, not a JVM test run, so it is cheap enough to run alongside the
# backend checks. It needs the JDK and Clojure CLI prepare-backend installs.
- name: Run ratchet checks
if: ${{ inputs.force-run || inputs.run-project-backend-checks || inputs.run-project-ratchet-checks }}
run: ./bin/mage kondo-ratchets

9
.github/workflows/kondo-ratchets-shrink.yml vendored
View File

@@ -53,11 +53,10 @@ jobs:
# get the force-push rejected. Nothing here needs it: the repo is public, so the fetch of the
# automation branch is anonymous, and every gh call carries its own token.
persist-credentials: false
- name: Set up Java for cold dependency resolution
uses: actions/setup-java@v5.2.0
with:
distribution: temurin
java-version: 25
# The shrink runs clj-kondo through the Clojure CLI to attribute :discouraged-var/:discouraged-namespace
# ignores, so it needs the CLI as well as the JDK.
- name: Prepare back-end environment
uses: ./.github/actions/prepare-backend
- name: Shrink ratchets
run: |
cp "$KONDO_RATCHETS_FILE" "$RUNNER_TEMP/kondo-ratchets-before.edn"

6
CLAUDE.md
View File

@@ -125,6 +125,12 @@ cluster, name the new half: the failure proposes a name and anchor to add. When
another way; the failure shows the requires joining them. When an anchor leaves every cycle, anchor its name on
another member if the cluster survives, or remove the line.
Ignores for `:discouraged-var` and `:discouraged-namespace` are budgeted per symbol, not per linter, in both
files. Use `--seed :discouraged-var/clojure.core/println` to seed one symbol's budget, or a bare
`--seed :discouraged-var` (or `:discouraged-namespace`) to seed every symbol configured under that linter.
To learn which symbol an ignore covers, `./bin/mage kondo-ratchets` runs `clojure -M:kondo` over the files
carrying one, so it needs the Clojure CLI as well as Babashka.
## Tool Preferences
If `clojure-mcp` tools are available, prefer them over shell-based alternatives for Clojure development.

16
bb.edn
View File

@@ -311,18 +311,20 @@
(project-tests/run!)))}
kondo-ratchets
{:doc "Validate the kondo and module ratchet files against the source tree (Babashka; seconds, not a test run)."
{:doc "Validate the kondo and module ratchet files against the source tree (about half a minute, most of it a kondo run over the files with a discouraged-var or discouraged-namespace ignore)."
:examples [["./bin/mage kondo-ratchets" "Fail on an invalid ratchets file, an over-budget suppression, or a missing justification comment"]]
:requires [[dev.kondo-ratchet]]
:task (task! (dev.kondo-ratchet/check))}
:requires [[dev.kondo-ratchet] [mage.kondo-ratchet]]
:task (task! (dev.kondo-ratchet/check {:attribute mage.kondo-ratchet/attribute-occurrences!}))}
kondo-ratchets-shrink
{:doc "Lower kondo and module budgets to match the source tree. This is the only Mage task that writes the ratchet files."
:examples [["./bin/mage kondo-ratchets-shrink" "Lower budgets to current counts without raising over-budget entries"]
["./bin/mage kondo-ratchets-shrink --seed :some-linter" "Set :some-linter's budget to its current count when introducing the linter"]]
:options [["-s" "--seed LINTER" "Set LINTER's budget to the actual count (may add or raise it)"]]
:requires [[dev.kondo-ratchet]]
:task (task! (dev.kondo-ratchet/fix! (:options parsed)))}
["./bin/mage kondo-ratchets-shrink --seed :some-linter" "Set :some-linter's budget to its current count when introducing the linter"]
["./bin/mage kondo-ratchets-shrink --seed :discouraged-var/clojure.core/println" "Set one discouraged symbol's budget to its current count"]
["./bin/mage kondo-ratchets-shrink --seed :discouraged-var" "Set every discouraged-var symbol's budget to its current count"]]
:options [["-s" "--seed LINTER" "Set LINTER's budget to the actual count (may add or raise it); :discouraged-var/SYMBOL seeds one discouraged symbol, a bare :discouraged-var/:discouraged-namespace all of them"]]
:requires [[dev.kondo-ratchet] [mage.kondo-ratchet]]
:task (task! (dev.kondo-ratchet/fix! (assoc (:options parsed) :attribute mage.kondo-ratchet/attribute-occurrences!)))}
kondo-redundant-ignores
{:doc "Report inline kondo ignores that are no longer necessary (slow: full kondo run)."

657
dev/src/dev/kondo_ratchet.clj
View File

@@ -8,8 +8,7 @@
(:require
[clojure.edn :as edn]
[clojure.java.io :as io]
[clojure.string :as str]
[clojure.walk :as walk]))
[clojure.string :as str]))
(set! *warn-on-reflection* true)
@@ -34,6 +33,9 @@
(def ^:private module-config-file
".clj-kondo/config/modules/config.edn")
(def ^:private kondo-config-file
".clj-kondo/config.edn")
(defn- read-ratchets-form
"The one EDN map at `path`.
An empty file, a non-map, or a second form is an error rather than an empty policy set, so a damaged
@@ -53,8 +55,23 @@
{:file path})))
form)))
(def discouragement-linters
"Linters configured per symbol, whose ignores are budgeted per symbol in a [[discouraged-count-field]]."
#{:discouraged-namespace :discouraged-var})
(defn discouraged-count-field
"The ratchets.edn field holding `linter`'s per-symbol budgets: `:discouraged-var` -> `:discouraged-var-counts`."
[linter]
(keyword (str (name linter) "-counts")))
(def ^:private discouraged-count-fields
(into #{} (map discouraged-count-field) discouragement-linters))
(def ^:private ratchet-field-order
(into [:ignore-counts] cat [(sort discouraged-count-fields) [:config-counts :comment-exempt]]))
(def ^:private empty-policies
{:ignore-counts {}, :config-counts {}, :comment-exempt #{}})
(into {} (map (fn [field] [field (if (= field :comment-exempt) #{} {})])) ratchet-field-order))
(defn- validate-budgets!
"Throw unless `budgets`, the value of `field`, maps names to non-negative integers."
@@ -72,12 +89,14 @@
"Validate and return `ratchets`:
- `:ignore-counts` maps linters to non-negative integers or `:unlimited`
- `:discouraged-var-counts` and `:discouraged-namespace-counts` map per-symbol keys
([[discouraged-count-key]]) to non-negative integers
- `:config-counts` maps linters to non-negative integers
- `:comment-exempt` is a set of linters
All names must be keywords. Throws on malformed input."
[ratchets]
(let [{:keys [ignore-counts config-counts comment-exempt]} (merge empty-policies ratchets)]
(let [{:keys [ignore-counts config-counts comment-exempt], :as merged} (merge empty-policies ratchets)]
(when-not (map? ignore-counts)
(throw (ex-info ":ignore-counts must be a map of linter policies"
{:ignore-counts ignore-counts})))
@@ -87,13 +106,18 @@
(throw (ex-info (format "%s has invalid policy %s; expected a non-negative integer or :unlimited"
linter (pr-str policy))
{:linter linter, :policy policy}))))
(doseq [field discouraged-count-fields]
(validate-budgets! field (get merged field)))
(validate-budgets! :config-counts config-counts)
(when-not (set? comment-exempt)
(throw (ex-info ":comment-exempt must be a set of linters"
{:comment-exempt comment-exempt})))
;; the merge keys on (str linter) and renders the same way, so a non-keyword name would be rewritten
;; as a keyword, collide with one, or produce a file that no longer reads back
(doseq [policy-name (concat (keys ignore-counts) (keys config-counts) comment-exempt)]
(doseq [policy-name (concat (keys ignore-counts)
(mapcat (comp keys #(get merged %)) discouraged-count-fields)
(keys config-counts)
comment-exempt)]
(when-not (keyword? policy-name)
(throw (ex-info (format "%s is not a linter name; policies and exemptions are keyed by keyword"
(pr-str policy-name))
@@ -223,17 +247,15 @@
(map #(io/file dir %))
(filter #(.isFile ^java.io.File %)))))
(defn- linters-map-keys
"Keys of every `:linters` map nested anywhere in `config`, so scoped linters under `:config-in-ns`,
`:config-in-call`, and library configs count too."
(defn- linters-maps
"Every `:linters` map nested anywhere in `config`, so scoped linters under `:config-in-ns`, `:config-in-call`,
and library configs count too."
[config]
(let [found (atom #{})]
(walk/prewalk (fn [x]
(when (and (map? x) (map? (:linters x)))
(swap! found into (keys (:linters x))))
x)
config)
@found))
(for [x (tree-seq coll? seq config)
:when (map? x)
:let [linters (:linters x)]
:when (map? linters)]
linters))
(defn repository-linters
"Names of every linter configured in a tracked `.edn` file under `dir` (default: `.clj-kondo`), which is
@@ -244,14 +266,71 @@
(into #{}
(comp (filter (fn [^java.io.File f]
(str/ends-with? (.getName f) ".edn")))
(mapcat (fn [^java.io.File f]
(linters-map-keys (edn/read-string (slurp f))))))
(map (comp edn/read-string slurp))
(mapcat linters-maps)
(mapcat keys))
(tracked-files dir))))
(def external-linters
"Diagnostics from tools other than kondo that ignores still name, plus `:all` for the vector-less form."
#{:all :clojure-lsp/unused-public-var})
(defn kondo-config
"The parsed `.clj-kondo/config.edn`."
[]
(edn/read-string (slurp kondo-config-file)))
(defn discouraged-symbols
"Every symbol configured under `linter` in `config`, including its `:config-in-ns` and `:config-in-call` scopes."
[linter config]
;; A scoped entry still names a discouraged symbol, so every scope counts.
(into #{}
(comp (map linter)
(filter map?)
(mapcat keys))
(linters-maps config)))
(def ^:private enterprise-prefix
"metabase-enterprise.")
(def ^:private oss-prefix
"metabase.")
(defn- drop-metabase-prefix
"`s` without its `metabase.` prefix, or with `metabase-enterprise.` shortened to `ee.`."
[s]
;; Shortened, not dropped, so an OSS namespace and its enterprise mirror keep different names.
(cond
(str/starts-with? s enterprise-prefix) (str "ee." (subs s (count enterprise-prefix)))
(str/starts-with? s oss-prefix) (subs s (count oss-prefix))
:else s))
(defn discouraged-count-key
"The ratchet key for `sym`, a symbol configured under one of the [[discouragement-linters]]: the symbol as a
keyword, with [[drop-metabase-prefix]] applied to its namespace.
For example, `metabase.search.core/search` -> `:search.core/search`,
`metabase-enterprise.search.core/search` -> `:ee.search.core/search`, and
`clojure.tools.logging` -> `:clojure.tools.logging`."
[sym]
(if-let [ns-part (namespace sym)]
(keyword (drop-metabase-prefix ns-part) (name sym))
(keyword (drop-metabase-prefix (name sym)))))
(defn discouraged-count-keys
"The [[discouraged-count-key]] of every symbol configured under `linter` ([[discouraged-symbols]]).
Throws when two symbols share a key."
([linter]
(discouraged-count-keys linter (kondo-config)))
([linter config]
(let [by-key (group-by discouraged-count-key (discouraged-symbols linter config))]
;; two symbols on one key would share a budget without anyone noticing
(doseq [[k syms] by-key
:when (< 1 (count syms))]
(throw (ex-info (format "%s symbols %s share the ratchet key %s"
linter (str/join ", " (sort-by str syms)) k)
{:linter linter, :key k, :symbols (vec syms)})))
(set (keys by-key)))))
(defn known-linters
"Every linter name a policy may use: kondo's built-ins, the repository's own, and [[external-linters]]."
[]
@@ -283,6 +362,40 @@
(known-linter-hint))
{:unknown (vec unknown)})))))
(defn configured-count-keys
"[[discouraged-count-keys]] for each of the [[discouragement-linters]]."
[]
(let [config (kondo-config)]
(into {} (for [linter discouragement-linters]
[linter (discouraged-count-keys linter config)]))))
(defn discouraged-in-ignore-counts
"The [[discouragement-linters]] with a stale flat budget in `ignore-counts`, sorted."
[ignore-counts]
;; Their ignores are budgeted per symbol, so a flat budget here is never read.
(into (sorted-set-by #(compare (str %1) (str %2)))
(filter discouragement-linters)
(keys ignore-counts)))
(defn ignore-counts-occurrences
"`occurrences` without the [[discouragement-linters]] in their `:linters`, which are budgeted per symbol
rather than in `:ignore-counts`."
[occurrences]
(map #(update % :linters (partial remove discouragement-linters)) occurrences))
(defn unconfigured-budget-warnings
"Warnings for budgets in `ratchets`, read from `ratchets-file`, whose symbol is no longer configured under
its linter. `configured` is [[configured-count-keys]]."
[ratchets-file ratchets configured]
;; Attribution never charges an unconfigured symbol, so fix! drops these at zero like any unused budget.
(for [linter (sort discouragement-linters)
:let [budgeted (keys (get ratchets (discouraged-count-field linter)))
unknown (sort-by str (remove (configured linter) budgeted))]
:when (seq unknown)]
(format (str "WARNING: %s budgets %s symbols no longer configured in %s: %s"
" -- `./bin/mage kondo-ratchets-shrink` drops them")
ratchets-file linter kondo-config-file (str/join ", " unknown))))
(defn- validate-test-config-counts!
"Throw unless `test-ratchets` has an empty :config-counts -- config-level suppressions are tracked only
in `prod-file` (see [[config-suppressions]]), and [[fix!]] silently drops anything here otherwise.
@@ -575,9 +688,6 @@
(remove still-needed)
exempt)))
(def ^:private kondo-config-file
".clj-kondo/config.edn")
(defn- excluded-items
"How many items an `:exclude` value waives: sequential entries count each, and a map's values count
their elements when sequential (`{compojure.core [GET POST]}` is 2) or 1 otherwise (a scoping map like
@@ -613,7 +723,7 @@
group an entry came from instead of summing by linter name, and top-level `:linters`/`:config-in-comment`
entries would still have no test/prod home -- real effort for a softer guarantee than :ignore-counts."
([]
(config-suppressions (edn/read-string (slurp kondo-config-file))))
(config-suppressions (kondo-config)))
([config]
(let [counts (fn [linters-map]
(into {}
@@ -647,10 +757,15 @@
[linter entry])))
(def ^:private shared-header
(str ";; Each :ignore-counts value is a non-negative integer budget, or :unlimited for no ceiling.\n"
(str ";; The :discouraged-var-counts and :discouraged-namespace-counts fields budget those linters' inline\n"
";; ignores per symbol, keyed by the symbol with `metabase.` dropped and `metabase-enterprise.` shortened\n"
";; to `ee.`.\n"
";; Each :ignore-counts value is a non-negative integer budget, or :unlimited for no ceiling; every other\n"
";; budget is a non-negative integer.\n"
";; Checks fail when a count exceeds its numeric budget; unused budget is allowed.\n"
";; Any ignore outside :comment-exempt needs an explanatory comment directly above or trailing on its line.\n"
";; `./bin/mage kondo-ratchets-shrink` lowers budgets unless `--seed` explicitly adds or raises one.\n"
";; `./bin/mage kondo-ratchets-shrink` lowers budgets unless `--seed` explicitly adds or raises one;\n"
";; a seed like `:discouraged-var/clojure.core/println` seeds one symbol, and `:discouraged-var` all of them.\n"
";; The workflow runs it on master, so feature branches do not need to record reductions.\n"
";; Raising or adding a budget (`--seed` for inline ignores, a manual edit otherwise) or widening the\n"
";; exemptions must be explained in the PR.\n"
@@ -661,7 +776,7 @@
";; config-level waivers in .clj-kondo/config.edn (:config-counts -- :off switches and :exclude entries).\n"
shared-header
";;\n"
";; Test code's :ignore-counts has its own budget in .clj-kondo/ratchets-test.edn.\n"
";; Test code's inline ignores have their own budgets in .clj-kondo/ratchets-test.edn.\n"
";; :config-counts stays here either way -- config.edn's scoping doesn't cleanly split.\n"))
(def ^:private test-header
@@ -687,20 +802,24 @@
"}"))))
(defn- render-policies
[header {:keys [ignore-counts config-counts comment-exempt]} config-counts?]
(let [counts-indent (apply str (repeat (count "{:ignore-counts {") \space))
exempt-indent (apply str (repeat (count " :comment-exempt #{") \space))]
[header ratchets config-counts?]
(let [fields (cond->> ratchet-field-order (not config-counts?) (remove #{:config-counts}))
width (apply max (map (comp count str) fields))
exempt (:comment-exempt ratchets)]
(str header
"{:ignore-counts " (render-counts ignore-counts counts-indent)
(when config-counts?
(str "\n :config-counts " (render-counts config-counts counts-indent)))
"\n :comment-exempt "
(if (empty? comment-exempt)
"#{}"
(str "#{"
(str/join (str "\n" exempt-indent)
(sort-by str comment-exempt))
"}"))
"{"
(str/join "\n "
(for [field fields
:let [label (format (str "%-" width "s ") field)
;; past the line's leading `{` or space, the label, and the value's own bracket
indent (apply str (repeat (+ 2 (count label)) \space))]]
(str label
(cond
(not= field :comment-exempt) (render-counts (get ratchets field) indent)
(empty? exempt) "#{}"
:else (str "#{"
(str/join (str "\n " indent) (sort-by str exempt))
"}")))))
"}\n")))
(defn render
@@ -769,7 +888,10 @@
(into (set base) (concat ours theirs))))
(def ^:private merge-fields
#{:comment-exempt :config-counts :ignore-counts})
(set ratchet-field-order))
(def ^:private merge-count-fields
(disj merge-fields :comment-exempt))
(defn- validate-merge-shape
"`ratchets` when it contains only the policy fields and `:disabled`, and each policy field passes
@@ -787,21 +909,20 @@
(defn merge-ratchets
"Three-way merge `base`, `ours`, and `theirs`, validating every stage first.
A disabled target stays disabled; a disabled incoming stage leaves `ours` unchanged. Otherwise merge
count maps with [[merge-counts]] and exemptions with [[merge-exemptions]]."
every count field ([[merge-count-fields]]) with [[merge-counts]] and exemptions with [[merge-exemptions]]."
[base ours theirs]
(let [[base ours theirs] (map validate-merge-shape [base ours theirs])]
(cond
(disabled? ours) {:disabled true}
(disabled? theirs) ours
:else {:ignore-counts (merge-counts (:ignore-counts base {})
(:ignore-counts ours {})
(:ignore-counts theirs {}))
:config-counts (merge-counts (:config-counts base {})
(:config-counts ours {})
(:config-counts theirs {}))
:comment-exempt (merge-exemptions (:comment-exempt base #{})
(:comment-exempt ours #{})
(:comment-exempt theirs #{}))})))
:else (into {:comment-exempt (merge-exemptions (:comment-exempt base #{})
(:comment-exempt ours #{})
(:comment-exempt theirs #{}))}
(map (fn [field]
[field (merge-counts (get base field {})
(get ours field {})
(get theirs field {}))]))
merge-count-fields))))
(defn merge-module-ratchets
"Three-way merge module budgets, validating every stage first."
@@ -854,44 +975,105 @@
(str "WARNING: :comment-exempt is no longer needed for these linters: " (str/join ", " linters)
" -- delete the stale entries by hand"))))
(defn unattributed-lines
"Warning lines for `unattributed` ignores (from `:attribute` -- see [[check]]/[[fix!]]): an ignore naming a
[[discouragement-linters]] linter with no such finding under it: stale, or in code kondo never lints
(a custom reader conditional like `:cljs-dev`)."
[unattributed]
(for [{:keys [file line linters]} unattributed]
(format (str "WARNING: %s:%d ignores %s but kondo reports no such finding under it -- probably stale, under a"
" nested ignore for the same linter, or in a reader branch kondo skips")
file line (str/join ", " linters))))
(defn unresolved-lines
"Lines for `unresolved` findings (from `:attribute`): a [[discouragement-linters]] finding an ignore
suppresses that can't be resolved to a configured symbol, so no per-symbol budget covers it."
[unresolved]
(for [{:keys [file line linters]} unresolved]
(format " %s:%d: %s finding not resolved to a configured symbol" file line (str/join ", " linters))))
(def ^:private unresolved-header
(str "ignored discouraged-var/namespace findings with no per-symbol budget -- they can't be budgeted,"
" so remove the ignore or the usage:"))
(defn- seed-lines
"[[fix!]]'s report lines for `seeded` keys in one count field: seeded at its actual count, or a warning
when there's nothing to seed."
[recorded actual seeded]
(for [name seeded
:let [n (get actual name 0)]]
(cond
(pos? n) (format "seeded %s at %d" name n)
(contains? recorded name) (format "WARNING: %s has no inline ignores -- dropping its policy" name)
:else (format "WARNING: %s has no inline ignores -- nothing to seed" name))))
(defn- budget-lines
"[[fix!]]'s report lines for bounded (non-`:unlimited`), non-seeded entries in one count field: dropped
at zero, lowered to actual, or over budget (suggesting `--seed` on the name `seed-hint` returns for it)."
[recorded actual seeded seed-hint]
(for [[name budget] (sort-by (comp str first) (apply dissoc recorded seeded))
:let [n (get actual name 0)]
;; a hand-written 0 with no ignores is dropped too, so it needs a line
:when (and (integer? budget) (or (not= n budget) (zero? budget)))]
(cond
(zero? n) (format "dropped %s (no ignores left)" name)
(< n budget) (format "lowered %s %d -> %d" name budget n)
:else (format "WARNING: %s is over budget (%d recorded, %d actual) -- remove ignores, or accept them all with `--seed %s`"
name budget n (seed-hint name)))))
(defn- missing-budget-lines
"[[fix!]]'s report lines for entries with ignores but no budget at all, suggesting `--seed` on the name
`seed-hint` returns for it."
[recorded actual seeded seed-hint]
(for [[name n] (sort-by (comp str first) (apply dissoc actual (concat seeded (keys recorded))))]
(format "WARNING: %s has %d ignores but no budget entry -- seed one with `./bin/mage kondo-ratchets-shrink --seed %s`"
name n (seed-hint name))))
(defn discouraged-seed-name
"The `--seed` argument for one per-symbol budget: `linter` and the ratchet key, e.g.
`:discouraged-var/clojure.core/println`."
[linter k]
(str linter "/" (subs (str k) 1)))
(defn- ignore-change-report
"Change-report lines for one file's :ignore-counts, scoped to `occurrences`: seeding, lowered/dropped/
over-budget counts, unused `:unlimited` policies, and ignores with no budget entry. Its :comment-exempt
is reported separately, via [[stale-exemptions-warning]], so callers can place it relative to other
sections (e.g. after config/module counts, as [[change-report]] does).
"Change-report lines for one ratchets file's inline-ignore budgets, scoped to `occurrences`: seeding,
lowered/dropped/over-budget counts, unused `:unlimited` policies, and ignores with no budget entry, for
`:ignore-counts` and each [[discouraged-count-field]], then stale discouraged-var and discouraged-namespace
ignores. `seeds` and `attribution` are as [[fix!]] gets them from [[resolve-seed]] and `:attribute`.
Its :comment-exempt is reported separately, via [[stale-exemptions-warning]].
Shared by [[change-report]] (the prod ratchets file) and [[fix!]]'s pass over the test ratchets file."
[ignore-counts occurrences seeded]
(let [actual (actual-counts occurrences)]
[{:keys [ignore-counts], :as ratchets} occurrences {:keys [unattributed], :as attribution} seeds]
(let [ignore-counts (apply dissoc ignore-counts discouragement-linters)
actual (actual-counts (ignore-counts-occurrences occurrences))
seeded (get seeds :ignore-counts [])]
(concat
(for [linter seeded
:let [n (get actual linter 0)]]
(cond
(pos? n) (format "seeded %s at %d" linter n)
(contains? ignore-counts linter) (format "WARNING: %s has no inline ignores -- dropping its policy"
linter)
:else (format "WARNING: %s has no inline ignores -- nothing to seed"
linter)))
(for [[linter budget] (sort-by (comp str first) (apply dissoc ignore-counts seeded))
:let [n (get actual linter 0)]
;; a hand-written 0 with no ignores is dropped too, so it needs a line
:when (and (integer? budget) (or (not= n budget) (zero? budget)))]
(cond
(zero? n) (format "dropped %s (no ignores left)" linter)
(< n budget) (format "lowered %s %d -> %d" linter budget n)
:else (format "WARNING: %s is over budget (%d recorded, %d actual) -- remove ignores, or accept them all with `--seed %s`"
linter budget n linter)))
(seed-lines ignore-counts actual seeded)
(budget-lines ignore-counts actual seeded identity)
(some-> (unexercised-unlimited-warning (apply dissoc ignore-counts seeded) actual) vector)
(for [[linter n] (sort-by (comp str first) (apply dissoc actual (concat seeded (keys ignore-counts))))]
(format "WARNING: %s has %d ignores but no budget entry -- seed one with `./bin/mage kondo-ratchets-shrink --seed %s`"
linter n linter)))))
(missing-budget-lines ignore-counts actual seeded identity)
(mapcat (fn [linter]
(let [field (discouraged-count-field linter)
recorded (get ratchets field)
actual (get-in attribution [:actual linter] {})
seeded (get seeds field [])
seed-hint (partial discouraged-seed-name linter)
;; a bulk seed names every configured symbol, so only those it changes are worth a line
reported (cond->> seeded
(contains? (:bulk seeds) field)
(filter #(or (contains? actual %) (contains? recorded %))))]
(concat (seed-lines recorded actual reported)
(budget-lines recorded actual seeded seed-hint)
(missing-budget-lines recorded actual seeded seed-hint))))
(sort discouragement-linters))
(unattributed-lines unattributed))))
(defn change-report
"The lines [[fix!]] prints for budget changes, budget violations, unused `:unlimited` policies, and
stale comment exemptions."
[{:keys [ignore-counts config-counts comment-exempt]} module-ratchets
occurrences config-actual module-actual seeded]
"The lines [[fix!]] prints for the prod ratchets file: [[ignore-change-report]], then config and module
budget changes and stale comment exemptions."
[{:keys [config-counts comment-exempt], :as ratchets} module-ratchets occurrences attribution
config-actual module-actual seeds]
(concat
(ignore-change-report ignore-counts occurrences seeded)
(ignore-change-report ratchets occurrences attribution seeds)
(for [[linter {:keys [recorded actual]}] (count-drift config-counts config-actual)]
(cond
(zero? actual) (format "dropped config %s (no suppressions left)" linter)
@@ -907,16 +1089,30 @@
metric recorded actual)))
(some-> (stale-exemptions-warning comment-exempt occurrences) vector)))
(def ^:private policy-count-fields
"The count fields both the prod and the test ratchets file carry."
(into [:ignore-counts] (sort discouraged-count-fields)))
(defn- test-field
"The name [[shrink-summary]] gives a test ratchets file's `field`."
[field]
(keyword (str "test-" (name field))))
(def ^:private count-fields
[:ignore-counts :config-counts :module-counts :test-ignore-counts])
(concat policy-count-fields [:config-counts :module-counts] (map test-field policy-count-fields)))
(defn- display-name
[field linter]
(case field
:config-counts (str ":config/" (subs (str linter) 1))
:module-counts (str ":module/" (subs (str linter) 1))
:test-ignore-counts (str ":test/" (subs (str linter) 1))
(str linter)))
(let [k (subs (str linter) 1)]
(case field
:discouraged-var-counts (str ":discouraged-var/" k)
:discouraged-namespace-counts (str ":discouraged-namespace/" k)
:config-counts (str ":config/" k)
:module-counts (str ":module/" k)
:test-ignore-counts (str ":test/" k)
:test-discouraged-var-counts (str ":test/discouraged-var/" k)
:test-discouraged-namespace-counts (str ":test/discouraged-namespace/" k)
(str linter))))
(defn- shrink-changes
[before after]
@@ -985,11 +1181,13 @@
"The budgets in a prod, module, and test ratchet file triple, combined into the shape
[[shrink-summary]] compares."
[ratchets-path module-ratchets-path test-ratchets-path]
(let [read-ratchets* #(binding [*ratchets-file* %] (read-ratchets))]
(assoc (read-ratchets* ratchets-path)
:module-counts (binding [*module-ratchets-file* module-ratchets-path]
(read-module-ratchets))
:test-ignore-counts (:ignore-counts (read-ratchets* test-ratchets-path)))))
(let [read-ratchets* #(binding [*ratchets-file* %] (read-ratchets))
test-ratchets (read-ratchets* test-ratchets-path)]
(into (assoc (read-ratchets* ratchets-path)
:module-counts (binding [*module-ratchets-file* module-ratchets-path]
(read-module-ratchets)))
(for [field policy-count-fields]
[(test-field field) (get test-ratchets field)]))))
(defn shrink-pr-body-from-files
"[[shrink-pr-body]] for ratchet files on disk, before and after shrinking."
@@ -998,49 +1196,134 @@
(read-ratchet-snapshots after modules-after test-after)
workflow-url))
(defn- resolve-seed
"The budgets `seed` (a linter keyword or its `:foo` string form) sets, as a map of count field to keys:
- `:discouraged-var/<symbol>` (likewise `:discouraged-namespace/<symbol>`) seeds that symbol's budget, named
by the configured symbol (`clojure.core/println`) or its ratchet key (`search.core/search`)
- a bare `:discouraged-var` seeds every symbol configured under it, and names its field under `:bulk`
- any other linter seeds its own `:ignore-counts` budget
Returns `{}` when `seed` is nil. Throws on a symbol not configured under its linter."
[seed]
(if-not seed
{}
(let [seed (str/replace-first seed #"^:" "")
[linter-name sym] (str/split seed #"/" 2)
linter (keyword linter-name)]
(if-not (contains? discouragement-linters linter)
{:ignore-counts [(keyword seed)]}
(let [known (discouraged-count-keys linter)
k (some-> sym symbol discouraged-count-key)]
;; an empty suffix (`:discouraged-var/`) is a typo, not a request to seed every symbol
(when (and sym (not (contains? known k)))
(throw (ex-info (format "%s is not a symbol configured under %s in %s"
(pr-str sym) linter kondo-config-file)
{:linter linter, :symbol sym})))
(if k
{(discouraged-count-field linter) [k]}
{(discouraged-count-field linter) (vec (sort-by str known))
:bulk #{(discouraged-count-field linter)}}))))))
(defn- no-attribution
"The default `:attribute` for [[check]] and [[fix!]], for a tree with no [[discouragement-linters]] ignores.
Throws when there are some."
[groups]
;; Without a kondo run every per-symbol count would read as zero, and fix! would drop those budgets.
(when (some #(some discouragement-linters (:linters %)) (apply concat groups))
(throw (ex-info (str "attributing :discouraged-var/:discouraged-namespace ignores needs a kondo run;"
" pass `:attribute mage.kondo-ratchet/attribute-occurrences!`")
{})))
(vec (repeat (count groups) {:actual {}, :unattributed [], :unresolved []})))
(defn- lowered-policies
"The inline-ignore budgets of `ratchets`, one ratchets file's policies, lowered to the counts in
`occurrences` and `attribution`, with `seeds` applied (see [[fix!]])."
[{:keys [ignore-counts comment-exempt], :as ratchets} occurrences attribution seeds]
(into {:ignore-counts (lowered-counts (apply dissoc ignore-counts discouragement-linters)
(actual-counts occurrences)
(get seeds :ignore-counts []))
:comment-exempt comment-exempt}
(for [linter discouragement-linters
:let [field (discouraged-count-field linter)]]
[field (lowered-counts (get ratchets field)
(get-in attribution [:actual linter] {})
(get seeds field []))])))
(defn- read-policies
"Validate `ratchets`, the prod policies, against the known linters, and read the module and test ratchet files.
Returns `{:known _, :configured _, :module-ratchets _, :test-ratchets _, :test-disabled? _}`, with `:configured`
from [[configured-count-keys]].
Throws when two configured symbols share a ratchet key."
[ratchets]
(let [known (known-linters)]
(validate-linters! ratchets known)
(let [test-ratchets (read-validated-test-ratchets known *ratchets-file*)]
{:known known
:configured (configured-count-keys)
:module-ratchets (read-module-ratchets)
:test-ratchets test-ratchets
:test-disabled? (disabled? test-ratchets)})))
(defn- scan-and-attribute
"The tree's ignores, split into prod and test, and what `attribute` makes of each.
Test ignores go unattributed when `test-disabled?`.
Returns `{:prod-occ _, :test-occ _, :attribution _, :test-attribution _}`."
[attribute test-disabled?]
(let [{test-occ true, prod-occ false} (group-by test-occurrence? (scan))
[attribution test-attribution] (attribute [prod-occ (if test-disabled? [] test-occ)])]
{:prod-occ prod-occ
:test-occ test-occ
:attribution attribution
:test-attribution test-attribution}))
(defn fix!
"Lower budgets and normalize the prod, test, and module ratchet files.
Refuses to touch a file containing an unknown linter or to seed one, rather than silently dropping it.
`--seed LINTER` (`{:seed \"...\"}` here) sets that budget to the actual count and bounds an unlimited
one, independently in whichever file(s) actually have occurrences for it.
one, independently in whichever file(s) actually have occurrences for it; see [[resolve-seed]] for
seeding [[discouragement-linters]] budgets.
`:attribute` (default [[no-attribution]]) takes a seq of occurrence seqs from [[scan]] and returns, for each
in order, the per-symbol counts of the [[discouragement-linters]] ignores among them,
`{:actual {linter {key count}}, :unattributed _, :unresolved _}`.
Throws before writing when any `:unresolved` finding has no symbol to budget it under.
Prints a [[change-report]] per file, or `unchanged` on a no-op.
Does nothing, including seeding, when [[*ratchets-file*]] sets `:disabled` to `true`; the test ratchets
file honors its own `:disabled` independently, skipping only its own pass."
([]
(fix! nil))
([{:keys [seed]}]
(let [{:keys [ignore-counts config-counts comment-exempt] :as ratchets} (read-ratchets)]
([{:keys [seed attribute], :or {attribute no-attribution}}]
;; Attribution is passed in because it needs a kondo run through mage, which isn't on the JVM classpath.
(let [{:keys [config-counts] :as ratchets} (read-ratchets)]
(if (disabled? ratchets)
(println (str *ratchets-file* " is disabled -- nothing to do"))
(let [module-ratchets (read-module-ratchets)
known (known-linters)
_ (validate-linters! ratchets known)
test-ratchets (read-validated-test-ratchets known *ratchets-file*)
test-disabled? (disabled? test-ratchets)
seeded (if seed [(keyword (str/replace-first seed #"^:" ""))] [])
_ (validate-seed! seeded known)
occurrences (scan)
{test-occ true, prod-occ false} (group-by test-occurrence? occurrences)
actual (actual-counts prod-occ)
test-actual (actual-counts test-occ)
config-actual (config-suppressions)
module-actual (module-escape-hatches)
outputs (cond-> [[*ratchets-file*
(render {:ignore-counts (lowered-counts ignore-counts actual seeded)
:config-counts (lowered-counts config-counts config-actual [])
:comment-exempt comment-exempt})]
[*module-ratchets-file*
(render-module-ratchets
(lowered-counts module-ratchets module-actual []))]]
(not test-disabled?)
(conj [*test-ratchets-file*
(render-test {:ignore-counts (lowered-counts (:ignore-counts test-ratchets)
test-actual seeded)
:comment-exempt (:comment-exempt test-ratchets)})]))
changed (filterv (fn [[path text]] (not= (slurp path) text)) outputs)]
(run! println (change-report ratchets module-ratchets prod-occ config-actual module-actual seeded))
(let [{:keys [known module-ratchets test-ratchets test-disabled?]} (read-policies ratchets)
;; Seeds are checked before the scan, so a bad --seed fails without waiting on kondo.
seeds (resolve-seed seed)
_ (validate-seed! (:ignore-counts seeds) known)
{:keys [prod-occ test-occ attribution test-attribution]} (scan-and-attribute attribute test-disabled?)
unresolved (concat (:unresolved attribution) (:unresolved test-attribution))
_ (when (seq unresolved)
(throw (ex-info (str/join "\n" (cons unresolved-header (unresolved-lines unresolved)))
{:unresolved unresolved})))
config-actual (config-suppressions)
module-actual (module-escape-hatches)
outputs (cond-> [[*ratchets-file*
(render (assoc (lowered-policies ratchets prod-occ attribution seeds)
:config-counts (lowered-counts config-counts config-actual [])))]
[*module-ratchets-file*
(render-module-ratchets
(lowered-counts module-ratchets module-actual []))]]
(not test-disabled?)
(conj [*test-ratchets-file*
(render-test (lowered-policies test-ratchets test-occ test-attribution seeds))]))
changed (filterv (fn [[path text]] (not= (slurp path) text)) outputs)]
(doseq [[path policies] (cond-> [[*ratchets-file* ratchets]]
(not test-disabled?) (conj [*test-ratchets-file* test-ratchets]))
linter (discouraged-in-ignore-counts (:ignore-counts policies))]
(println (format "dropped stale :ignore-counts entry for %s in %s (tracked per-symbol in %s now)"
linter path (discouraged-count-field linter))))
(run! println (change-report ratchets module-ratchets prod-occ attribution config-actual module-actual seeds))
(when-not test-disabled?
(run! println (concat (ignore-change-report (:ignore-counts test-ratchets) test-occ seeded)
(run! println (concat (ignore-change-report test-ratchets test-occ test-attribution seeds)
(some-> (stale-exemptions-warning (:comment-exempt test-ratchets) test-occ)
vector))))
(if (empty? changed)
@@ -1049,24 +1332,42 @@
(spit path text)
(println (str "wrote " path)))))))))
(defn- budget-line
[[k {:keys [recorded actual]}]]
(format " %s: %d recorded, %d actual" k recorded actual))
(defn- ignore-check-lines
"Budget, justification, and formatting diagnostics for one ratchets file's :ignore-counts and
:comment-exempt, scoped to `occurrences`. `ratchets-file` names the file in the not-normalized message, and
`render-fn` gives its canonical text.
"Budget, justification, and formatting diagnostics for one ratchets file's inline-ignore budgets and
:comment-exempt, scoped to `occurrences`, plus the unresolved discouraged-var and discouraged-namespace
findings among them. `attribution` is the `:attribute` result for `occurrences` (see [[fix!]]).
`ratchets-file` names the file in the not-normalized message, and `render-fn` gives its canonical text.
Shared by [[check-report]] (the prod ratchets file) and [[check]]'s pass over the test ratchets file."
[ratchets-file render-fn {:keys [ignore-counts comment-exempt] :or {comment-exempt #{}} :as ratchets}
occurrences ratchets-text]
(let [over (over-budget ignore-counts occurrences)
uncommented (unjustified comment-exempt occurrences)
budget-line (fn [[k {:keys [recorded actual]}]]
(format " %s: %d recorded, %d actual" k recorded actual))]
occurrences {:keys [unresolved], :as attribution} ratchets-text]
(let [over (over-budget ignore-counts (ignore-counts-occurrences occurrences))
stale-flat (discouraged-in-ignore-counts ignore-counts)
uncommented (unjustified comment-exempt occurrences)]
(concat
(when (seq stale-flat)
[(str ":ignore-counts still has a flat budget for " (str/join ", " stale-flat)
" -- each now has its own field (" (str/join ", " (map discouraged-count-field stale-flat))
"); run `./bin/mage kondo-ratchets-shrink` to drop the stale entry")])
(when (seq over)
(cons (str "over budget -- remove an ignore, or seed the budget with"
" `./bin/mage kondo-ratchets-shrink --seed <linter>` and explain the increase in the PR:")
(mapcat (fn [[_ {:keys [examples]} :as entry]]
(cons (budget-line entry) (map #(str " " %) examples)))
over)))
(mapcat (fn [linter]
(when-let [over (seq (counts-over-budget (get ratchets (discouraged-count-field linter))
(get-in attribution [:actual linter] {})))]
(cons (str (name linter) " symbols over budget -- remove an ignore, or seed the symbol's budget"
" with `./bin/mage kondo-ratchets-shrink --seed <name below>` and explain the"
" increase in the PR:")
(map (fn [[k v]] (budget-line [(discouraged-seed-name linter k) v])) over))))
(sort discouragement-linters))
(when (seq unresolved)
(cons unresolved-header (unresolved-lines unresolved)))
(when (seq uncommented)
(cons (str "ignores without required comments -- add a `;;` comment above the form or at the end"
" of the same line, explaining why the suppression is necessary:")
@@ -1078,16 +1379,16 @@
" to fix the formatting")]))))
(defn check-report
"Diagnostics for budget violations, unjustified ignores, and noncanonical ratchet text.
Lower counts are valid; post-merge automation records them."
"Diagnostics for budget violations, unjustified ignores, unresolved discouraged-var and
discouraged-namespace findings, and noncanonical ratchet text.
Lower counts are valid; post-merge automation records them.
`attribution` is the `:attribute` result for `occurrences` (see [[fix!]])."
[{:keys [config-counts] :as ratchets}
module-ratchets occurrences config-actual module-actual ratchets-text module-ratchets-text]
module-ratchets occurrences attribution config-actual module-actual ratchets-text module-ratchets-text]
(let [config-over (counts-over-budget config-counts config-actual)
module-over (counts-over-budget module-ratchets module-actual)
budget-line (fn [[k {:keys [recorded actual]}]]
(format " %s: %d recorded, %d actual" k recorded actual))]
module-over (counts-over-budget module-ratchets module-actual)]
(concat
(ignore-check-lines *ratchets-file* render ratchets occurrences ratchets-text)
(ignore-check-lines *ratchets-file* render ratchets occurrences attribution ratchets-text)
(when (seq config-over)
(cons (str "config suppressions over budget -- remove the entry from " kondo-config-file
", or raise the budget manually and explain the increase in the PR:")
@@ -1100,6 +1401,11 @@
[(str *module-ratchets-file* " is not normalized -- run `./bin/mage kondo-ratchets-shrink`"
" to fix the formatting")]))))
(defn- policy-count
"How many inline-ignore budgets `ratchets` holds, flat and per symbol."
[ratchets]
(transduce (map #(count (get ratchets %))) + policy-count-fields))
(defn- exit!
"Fail the babashka task with `message`, without mage's default stack trace."
[message]
@@ -1113,44 +1419,49 @@
(defn check
"Validate the prod, test, and module ratchet files and current counts. Fail on missing or malformed
configuration, unknown linters, budget violations, or unjustified ignores. Report stale unlimited
policies and exemptions without failing. `{:disabled true}` in [[*ratchets-file*]] disables checking
entirely; the test ratchets file honors its own `:disabled` independently, skipping only its own checks."
[]
;; Turn validation errors into concise Mage output instead of a Babashka stack trace.
(try
(let [ratchets (read-ratchets)]
(if (disabled? ratchets)
(println (str *ratchets-file* " is disabled -- nothing to check"))
(let [known (known-linters)]
(validate-linters! ratchets known)
(let [module-ratchets (read-module-ratchets)
test-ratchets (read-validated-test-ratchets known *ratchets-file*)
test-disabled? (disabled? test-ratchets)
occurrences (scan)
{test-occ true, prod-occ false} (group-by test-occurrence? occurrences)
lines (concat
(check-report ratchets module-ratchets prod-occ
configuration, unknown linters, budget violations, unjustified ignores, or discouraged-var and
discouraged-namespace findings with no symbol to budget them under. Report stale unlimited policies,
exemptions, discouraged-var and discouraged-namespace ignores, and budgets for symbols no longer configured,
without failing.
`{:disabled true}` in [[*ratchets-file*]] disables checking entirely; the test ratchets file honors its
own `:disabled` independently, skipping only its own checks.
`:attribute` is as for [[fix!]]."
([]
(check nil))
([{:keys [attribute], :or {attribute no-attribution}}]
;; Turn validation errors into concise Mage output instead of a Babashka stack trace.
(try
(let [ratchets (read-ratchets)]
(if (disabled? ratchets)
(println (str *ratchets-file* " is disabled -- nothing to check"))
(let [{:keys [configured module-ratchets test-ratchets test-disabled?]} (read-policies ratchets)
{:keys [prod-occ test-occ attribution test-attribution]} (scan-and-attribute attribute test-disabled?)
lines (concat
(check-report ratchets module-ratchets prod-occ attribution
(config-suppressions) (module-escape-hatches)
(slurp *ratchets-file*) (slurp *module-ratchets-file*))
(when-not test-disabled?
(ignore-check-lines *test-ratchets-file* render-test test-ratchets test-occ
(slurp *test-ratchets-file*))))]
(some-> (unexercised-unlimited-warning (:ignore-counts ratchets) (actual-counts prod-occ)) println)
(some-> (stale-exemptions-warning (:comment-exempt ratchets) prod-occ) println)
(when-not test-disabled?
(some-> (unexercised-unlimited-warning (:ignore-counts test-ratchets) (actual-counts test-occ)) println)
(some-> (stale-exemptions-warning (:comment-exempt test-ratchets) test-occ) println))
(if (empty? lines)
(do
(println (format "ok -- %d ignore forms within %d policies"
(count prod-occ) (count (:ignore-counts ratchets))))
(when-not test-disabled?
(println (format "ok -- %d test ignore forms within %d test policies"
(count test-occ) (count (:ignore-counts test-ratchets))))))
(do (run! println lines)
(exit! "ratchet files drifted from the source tree")))))))
(catch clojure.lang.ExceptionInfo e
(if (:babashka/exit (ex-data e))
(throw e)
(fail! (ex-message e))))))
test-attribution (slurp *test-ratchets-file*))))]
(some-> (unexercised-unlimited-warning (:ignore-counts ratchets) (actual-counts prod-occ)) println)
(some-> (stale-exemptions-warning (:comment-exempt ratchets) prod-occ) println)
(run! println (unattributed-lines (:unattributed attribution)))
(run! println (unconfigured-budget-warnings *ratchets-file* ratchets configured))
(when-not test-disabled?
(some-> (unexercised-unlimited-warning (:ignore-counts test-ratchets) (actual-counts test-occ)) println)
(some-> (stale-exemptions-warning (:comment-exempt test-ratchets) test-occ) println)
(run! println (unattributed-lines (:unattributed test-attribution)))
(run! println (unconfigured-budget-warnings *test-ratchets-file* test-ratchets configured)))
(if (empty? lines)
(do
(println (format "ok -- %d ignore forms within %d policies"
(count prod-occ) (policy-count ratchets)))
(when-not test-disabled?
(println (format "ok -- %d test ignore forms within %d test policies"
(count test-occ) (policy-count test-ratchets)))))
(do (run! println lines)
(exit! "ratchet files drifted from the source tree"))))))
(catch clojure.lang.ExceptionInfo e
(if (:babashka/exit (ex-data e))
(throw e)
(fail! (ex-message e)))))))

86
dev/test/metabase/core/kondo_ratchet_check_test.clj
View File

@@ -18,17 +18,19 @@
(defn- report-lines
"Return [[kondo-ratchet/check-report]] output for `ratchets`. Supply the defaults that
[[kondo-ratchet/read-ratchets]] adds to a partial file."
[[kondo-ratchet/read-ratchets]] adds to a partial file. `ratchets` may also carry the module ratchets as
`:module-counts` and the `:attribute` result as `:attribution`."
([ratchets occurrences text]
(report-lines ratchets occurrences {} {} text))
(report-lines ratchets occurrences {} text))
([ratchets occurrences config-actual text]
(report-lines ratchets occurrences config-actual {} text))
([ratchets occurrences config-actual module-actual text]
(let [module-ratchets (:module-counts ratchets {})]
(let [module-ratchets (:module-counts ratchets {})
attribution (merge {:actual {}, :unresolved []} (:attribution ratchets))]
(vec (kondo-ratchet/check-report (-> {:config-counts {}, :comment-exempt #{}}
(merge ratchets)
(dissoc :module-counts))
module-ratchets occurrences config-actual module-actual text
(dissoc :module-counts :attribution))
module-ratchets occurrences attribution config-actual module-actual text
(kondo-ratchet/render-module-ratchets module-ratchets))))))
(deftest ^:parallel clean-test
@@ -36,6 +38,14 @@
(is (= []
(report-lines ratchets (occurrences {:a 2, :b 1}) (kondo-ratchet/render ratchets))))))
(deftest ^:parallel stale-flat-discouraged-entry-test
(let [ratchets {:ignore-counts {:discouraged-var 3, :a 1}}]
(is (= [(str ":ignore-counts still has a flat budget for :discouraged-var -- each now has its own field "
"(:discouraged-var-counts); run `./bin/mage kondo-ratchets-shrink` to drop the stale entry")]
(report-lines ratchets (occurrences {:discouraged-var 3, :a 1}) (kondo-ratchet/render ratchets)))
"a flat entry for a linter with its own per-symbol field fails the check even when its count
happens to match, since nothing reads it and it would only mislead")))
(deftest ^:parallel over-budget-test
(let [ratchets {:ignore-counts {:a 1}}]
(is (= ["over budget -- remove an ignore, or seed the budget with `./bin/mage kondo-ratchets-shrink --seed <linter>` and explain the increase in the PR:"
@@ -76,6 +86,27 @@
(report-lines ratchets [] {:a 2, :b 1, :new 1} (kondo-ratchet/render ratchets)))
"a lowered config count passes; growth and new entries are reported")))
(deftest ^:parallel discouraged-counts-over-budget-test
(let [ratchets {:ignore-counts {}
:discouraged-var-counts {:a/x 1}
:discouraged-namespace-counts {:some.ns 1}
:attribution {:actual {:discouraged-var {:a/x 2, :a/new 1}
:discouraged-namespace {:some.ns 3}}}}]
(is (= ["discouraged-namespace symbols over budget -- remove an ignore, or seed the symbol's budget with `./bin/mage kondo-ratchets-shrink --seed <name below>` and explain the increase in the PR:"
" :discouraged-namespace/some.ns: 1 recorded, 3 actual"
"discouraged-var symbols over budget -- remove an ignore, or seed the symbol's budget with `./bin/mage kondo-ratchets-shrink --seed <name below>` and explain the increase in the PR:"
" :discouraged-var/a/new: 0 recorded, 1 actual"
" :discouraged-var/a/x: 1 recorded, 2 actual"]
(report-lines ratchets [] (kondo-ratchet/render ratchets)))
"each field reports its over-budget symbols by their own seed name")))
(deftest ^:parallel unresolved-discouraged-finding-test
(let [ratchets {:ignore-counts {}
:attribution {:unresolved [{:file "f.clj", :line 3, :linters [:discouraged-var]}]}}]
(is (= ["ignored discouraged-var/namespace findings with no per-symbol budget -- they can't be budgeted, so remove the ignore or the usage:"
" f.clj:3: :discouraged-var finding not resolved to a configured symbol"]
(report-lines ratchets [] (kondo-ratchet/render ratchets))))))
(deftest ^:parallel module-over-budget-test
(let [ratchets {:ignore-counts {}, :module-counts {:api-any 1, :friend-edges 3}}]
(is (= ["module escape hatches over budget -- remove one from .clj-kondo/config/modules/config.edn, or raise the budget manually and explain the increase in the PR:"
@@ -90,9 +121,10 @@
(defn- check-with!
"Output lines of [[kondo-ratchet/check]] against `ratchets` written to a temp file, with `occurrences`
standing in for the tree scan; `:thrown?` says whether it failed. `:test-counts` stands in for the test
ratchets file, defaulting to an empty (clean) budget; `{:disabled true}` there is honored."
[ratchets occurrences]
standing in for the tree scan and `opts` passed to it; `:thrown?` says whether it failed. `:test-counts`
stands in for the test ratchets file, defaulting to an empty (clean) budget; `{:disabled true}` there is
honored."
[ratchets occurrences & [opts]]
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-check-test"
(make-array java.nio.file.attribute.FileAttribute 0)))
@@ -119,7 +151,7 @@
{:lines (str/split-lines
(with-out-str
(try
(kondo-ratchet/check)
(kondo-ratchet/check opts)
(catch clojure.lang.ExceptionInfo _
(reset! thrown? true)))))
:thrown? @thrown?}))))
@@ -186,6 +218,42 @@
"a disabled test-ratchets file opts the test tree out of enforcement, even with an unbudgeted
test-only linter")))
(deftest check-discouraged-attribution-test
(let [ratchets {:ignore-counts {}}
occurrences [{:file "f.clj", :line 1, :linters [:discouraged-var], :justified? true}]
attribution {:actual {}, :unattributed [], :unresolved []}
;; check attributes the prod and test occurrences in one call; only the prod ones carry anything here
with-attribution (fn [m] {:attribute (constantly [(merge attribution m) attribution])})
finding-at (fn [line] [{:file "f.clj", :line line, :linters [:discouraged-var]}])]
(testing "without an :attribute, a discouraged-var ignore fails instead of counting as zero"
(is (=? {:lines [#"attributing :discouraged-var/:discouraged-namespace ignores needs a kondo run.*"]
:thrown? true}
(check-with! ratchets occurrences))))
(testing "an ignore covering no finding warns without failing"
(is (= {:lines ["WARNING: f.clj:1 ignores :discouraged-var but kondo reports no such finding under it -- probably stale, under a nested ignore for the same linter, or in a reader branch kondo skips"
"ok -- 1 ignore forms within 0 policies"
"ok -- 0 test ignore forms within 0 test policies"]
:thrown? false}
(check-with! ratchets occurrences
(with-attribution {:unattributed (finding-at 1)})))))
(testing "a finding with no configured symbol fails"
(is (=? {:lines ["ignored discouraged-var/namespace findings with no per-symbol budget -- they can't be budgeted, so remove the ignore or the usage:"
" f.clj:2: :discouraged-var finding not resolved to a configured symbol"]
:thrown? true}
(check-with! ratchets occurrences
(with-attribution {:unresolved (finding-at 2)})))))))
(deftest check-unconfigured-budget-test
(let [ratchets {:ignore-counts {:a 1}, :discouraged-var-counts {:a/gone 2, :a/x 1}}]
(mt/with-dynamic-fn-redefs [kondo-ratchet/discouraged-count-keys (constantly #{:a/x})]
(is (=? {:lines [#"WARNING: .*ratchets\.edn budgets :discouraged-var symbols no longer configured in \.clj-kondo/config\.edn: :a/gone -- `\./bin/mage kondo-ratchets-shrink` drops them"
"ok -- 1 ignore forms within 3 policies"
"ok -- 0 test ignore forms within 0 test policies"]
:thrown? false}
(check-with! ratchets (occurrences {:a 1})))
"a budget for a symbol removed from the config warns without failing, and the policy count
includes the per-symbol budgets"))))
(deftest ^:parallel stale-test
(let [ratchets {:ignore-counts {:a 5, :gone 2}}]
(is (= []

409
dev/test/metabase/core/kondo_ratchet_test.clj
View File

@@ -185,36 +185,53 @@
:ns-prefix "metabase.legacy-b"}}))))
(deftest ^:parallel render-test
(testing "renders stable text with sorted entries"
(testing "renders stable text with sorted entries, one field per line, each own field aligned under its
own opening bracket regardless of another field's name length"
(let [ratchets {:ignore-counts {:all 1
:discouraged-var 3
:metabase/modules 2
:unused-alias :unlimited}
:discouraged-var-counts
{:entity-retrieval.core/search-unfiltered 2
:ee.entity-retrieval.core/search-unfiltered 6}
:discouraged-namespace-counts {:clojure.tools.logging 8}
:config-counts {:inline-def 1
:unresolved-symbol 18}
:comment-exempt #{:metabase/modules :discouraged-var}}
text (kondo-ratchet/render ratchets)]
(is (str/ends-with? text (str "{:ignore-counts {:all 1\n"
" :discouraged-var 3\n"
" :metabase/modules 2\n"
" :unused-alias :unlimited}\n"
" :config-counts {:inline-def 1\n"
" :unresolved-symbol 18}\n"
" :comment-exempt #{:discouraged-var\n"
" :metabase/modules}}\n")))
(is (str/ends-with? text
(str "{:ignore-counts {:all 1\n"
" :metabase/modules 2\n"
" :unused-alias :unlimited}\n"
" :discouraged-namespace-counts {:clojure.tools.logging 8}\n"
" :discouraged-var-counts {:ee.entity-retrieval.core/search-unfiltered 6\n"
" :entity-retrieval.core/search-unfiltered 2}\n"
" :config-counts {:inline-def 1\n"
" :unresolved-symbol 18}\n"
" :comment-exempt #{:discouraged-var\n"
" :metabase/modules}}\n")))
(is (= ratchets (edn/read-string text)))
(is (= text (kondo-ratchet/render (edn/read-string text))))))
(testing "empty ratchets"
(is (str/ends-with? (kondo-ratchet/render {:ignore-counts {}, :config-counts {}, :comment-exempt #{}})
"{:ignore-counts {}\n :config-counts {}\n :comment-exempt #{}}\n"))))
(is (str/ends-with? (kondo-ratchet/render {:ignore-counts {}, :discouraged-var-counts {}
:discouraged-namespace-counts {}, :config-counts {}
:comment-exempt #{}})
(str "{:ignore-counts {}\n"
" :discouraged-namespace-counts {}\n"
" :discouraged-var-counts {}\n"
" :config-counts {}\n"
" :comment-exempt #{}}\n")))))
(deftest ^:parallel render-test-test
(let [ratchets {:ignore-counts {:discouraged-var 3}
:comment-exempt #{:discouraged-var}}
(let [ratchets {:ignore-counts {:unused-binding 3}
:discouraged-namespace-counts {}
:discouraged-var-counts {:clojure.core/println 2}
:comment-exempt #{:discouraged-var}}
text (kondo-ratchet/render-test (assoc ratchets :config-counts {}))]
(is (str/starts-with? text ";; Budgets for kondo suppressions in test code"))
(is (str/ends-with? text (str "{:ignore-counts {:discouraged-var 3}\n"
" :comment-exempt #{:discouraged-var}}\n"))
(is (str/ends-with? text (str "{:ignore-counts {:unused-binding 3}\n"
" :discouraged-namespace-counts {}\n"
" :discouraged-var-counts {:clojure.core/println 2}\n"
" :comment-exempt #{:discouraged-var}}\n"))
"no :config-counts -- only the prod file tracks them")
(is (= ratchets (edn/read-string text)))
(is (= text (kondo-ratchet/render-test (edn/read-string text)))))
@@ -233,9 +250,11 @@
(let [file (doto (java.io.File/createTempFile "kondo-ratchets" ".edn")
(spit (pr-str {:ignore-counts {:bounded 4, :free :unlimited}})))]
(binding [kondo-ratchet/*ratchets-file* (.getPath file)]
(is (= {:ignore-counts {:bounded 4, :free :unlimited}
:config-counts {}
:comment-exempt #{}}
(is (= {:ignore-counts {:bounded 4, :free :unlimited}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{}}
(kondo-ratchet/read-ratchets))))))
(deftest read-ratchets-validates-policies-test
@@ -394,9 +413,11 @@
(str/split-lines (with-out-str (kondo-ratchet/fix! {:seed "free"}))))
"the prod occurrences seed the prod budget; the seeded linter has none under `test/`, so the
test report says so and the test file is left untouched"))
(is (= {:ignore-counts {:free 2}
:config-counts {}
:comment-exempt #{}}
(is (= {:ignore-counts {:free 2}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{}}
(kondo-ratchet/read-ratchets))))))
(deftest ^:synchronized seed-unknown-linter-test
@@ -446,9 +467,11 @@
(str "wrote " (.getPath budgets))]
(run!))
"bounded zeros go; decision policies stay and are reported when no longer needed")
(is (= {:ignore-counts {:free :unlimited, :empty :unlimited}
:config-counts {}
:comment-exempt #{:empty}}
(is (= {:ignore-counts {:free :unlimited, :empty :unlimited}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{:empty}}
(kondo-ratchet/read-ratchets)))
(is (= ["WARNING: :unlimited policies with no ignores left: :empty -- delete an entry by hand once its linter no longer needs one"
"WARNING: :comment-exempt is no longer needed for these linters: :empty -- delete the stale entries by hand"
@@ -456,6 +479,167 @@
(run!))
"a second run changes nothing and still reports")))))
(def ^:private no-findings
{:actual {}, :unattributed [], :unresolved []})
(deftest ^:synchronized fix-refuses-without-attribution-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-test"
(make-array java.nio.file.attribute.FileAttribute 0)))
budgets (doto (io/file dir "ratchets.edn")
(spit (kondo-ratchet/render {:ignore-counts {}, :config-counts {}, :comment-exempt #{}})))
modules (doto (io/file dir "module-ratchets.edn")
(spit (kondo-ratchet/render-module-ratchets {})))
test-budgets (empty-test-ratchets-file! dir)
occurrences [{:file "f.clj", :line 1, :linters [:discouraged-var]}]
before (slurp budgets)]
(binding [kondo-ratchet/*ratchets-file* (.getPath budgets)
kondo-ratchet/*module-ratchets-file* (.getPath modules)
kondo-ratchet/*test-ratchets-file* (.getPath test-budgets)]
(mt/with-dynamic-fn-redefs [kondo-ratchet/known-linters (constantly #{:discouraged-var})
kondo-ratchet/scan (constantly occurrences)
kondo-ratchet/config-suppressions (constantly {})
kondo-ratchet/module-escape-hatches (constantly {})]
(testing "a discouraged-var ignore with no :attribute throws rather than reading as zero"
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"needs a kondo run"
(kondo-ratchet/fix!))))
(testing "an unresolved finding throws before writing"
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"f.clj:1: :discouraged-var finding not resolved"
(kondo-ratchet/fix! {:attribute (constantly
[(assoc no-findings :unresolved occurrences)
no-findings])}))))
(is (= before (slurp budgets)))))))
(deftest ^:synchronized fix-splits-discouraged-counts-by-file-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-test"
(make-array java.nio.file.attribute.FileAttribute 0)))
budgets (doto (io/file dir "ratchets.edn")
(spit (kondo-ratchet/render {:ignore-counts {}, :config-counts {}, :comment-exempt #{}})))
modules (doto (io/file dir "module-ratchets.edn")
(spit (kondo-ratchet/render-module-ratchets {})))
test-budgets (empty-test-ratchets-file! dir)
occurrences [{:file "src/f.clj", :line 1, :linters [:discouraged-var]}
{:file "test/g.clj", :line 1, :linters [:discouraged-var]}]
;; stands in for kondo: one finding per prod occurrence, two per test occurrence
attribute (fn [groups]
(mapv (fn [occs]
(assoc no-findings :actual {:discouraged-var {:a/x (reduce + (for [{:keys [file]} occs]
(if (str/starts-with? file "test/")
2
1)))}}))
groups))]
(binding [kondo-ratchet/*ratchets-file* (.getPath budgets)
kondo-ratchet/*module-ratchets-file* (.getPath modules)
kondo-ratchet/*test-ratchets-file* (.getPath test-budgets)]
(mt/with-dynamic-fn-redefs [kondo-ratchet/known-linters (constantly #{:discouraged-var})
kondo-ratchet/discouraged-count-keys (constantly #{:a/x})
kondo-ratchet/scan (constantly occurrences)
kondo-ratchet/config-suppressions (constantly {})
kondo-ratchet/module-escape-hatches (constantly {})]
(is (= ["seeded :a/x at 1"
"seeded :a/x at 2"
(str "wrote " (.getPath budgets))
(str "wrote " (.getPath test-budgets))]
(str/split-lines (with-out-str (kondo-ratchet/fix! {:seed ":discouraged-var/a/x"
:attribute attribute}))))
"each file's symbol budget comes from attributing that file's own occurrences")
(is (=? {:discouraged-var-counts {:a/x 1}} (kondo-ratchet/read-ratchets)))
(is (=? {:discouraged-var-counts {:a/x 2}}
(binding [kondo-ratchet/*ratchets-file* (.getPath test-budgets)]
(kondo-ratchet/read-ratchets))))))))
(deftest ^:synchronized fix-drops-unconfigured-and-bulk-seeds-quietly-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-test"
(make-array java.nio.file.attribute.FileAttribute 0)))
budgets (doto (io/file dir "ratchets.edn")
(spit (kondo-ratchet/render {:ignore-counts {}
:discouraged-var-counts {:a/gone 2, :a/y 1}
:config-counts {}
:comment-exempt #{}})))
modules (doto (io/file dir "module-ratchets.edn")
(spit (kondo-ratchet/render-module-ratchets {})))
test-budgets (empty-test-ratchets-file! dir)
occurrences [{:file "src/f.clj", :line 1, :linters [:discouraged-var]}]
attribute (constantly [(assoc no-findings :actual {:discouraged-var {:a/x 1}}) no-findings])]
(binding [kondo-ratchet/*ratchets-file* (.getPath budgets)
kondo-ratchet/*module-ratchets-file* (.getPath modules)
kondo-ratchet/*test-ratchets-file* (.getPath test-budgets)]
(mt/with-dynamic-fn-redefs [kondo-ratchet/known-linters (constantly #{:discouraged-var})
kondo-ratchet/discouraged-count-keys (constantly #{:a/x :a/y :a/z})
kondo-ratchet/scan (constantly occurrences)
kondo-ratchet/config-suppressions (constantly {})
kondo-ratchet/module-escape-hatches (constantly {})]
(is (= ["seeded :a/x at 1"
"WARNING: :a/y has no inline ignores -- dropping its policy"
"dropped :a/gone (no ignores left)"
(str "wrote " (.getPath budgets))]
(str/split-lines (with-out-str (kondo-ratchet/fix! {:seed ":discouraged-var"
:attribute attribute}))))
"a budget for a symbol no longer configured is dropped rather than refused, and a bulk seed
reports only the symbols it changes: unused :a/z gets no line, here or for the test file")
(is (=? {:discouraged-var-counts {:a/x 1}} (kondo-ratchet/read-ratchets)))))))
(deftest ^:synchronized fix-refuses-colliding-symbols-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-test"
(make-array java.nio.file.attribute.FileAttribute 0)))
budgets (doto (io/file dir "ratchets.edn")
(spit (kondo-ratchet/render {:ignore-counts {}, :config-counts {}, :comment-exempt #{}})))
modules (doto (io/file dir "module-ratchets.edn")
(spit (kondo-ratchet/render-module-ratchets {})))
test-budgets (empty-test-ratchets-file! dir)
before (slurp budgets)]
(binding [kondo-ratchet/*ratchets-file* (.getPath budgets)
kondo-ratchet/*module-ratchets-file* (.getPath modules)
kondo-ratchet/*test-ratchets-file* (.getPath test-budgets)]
(mt/with-dynamic-fn-redefs [kondo-ratchet/known-linters (constantly #{})
kondo-ratchet/kondo-config (constantly '{:linters {:discouraged-var {a/x {}
metabase.a/x {}}}})
kondo-ratchet/scan (constantly [])
kondo-ratchet/config-suppressions (constantly {})
kondo-ratchet/module-escape-hatches (constantly {})]
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"share the ratchet key :a/x"
(kondo-ratchet/fix! {:attribute (constantly [no-findings no-findings])}))
"an unseeded shrink refuses two symbols on one key rather than merging their budgets")
(is (= before (slurp budgets)))))))
(deftest ^:synchronized fix-drops-stale-flat-discouraged-entry-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-test"
(make-array java.nio.file.attribute.FileAttribute 0)))
ratchets {:ignore-counts {:discouraged-var 3, :a 1}
:discouraged-var-counts {}
:config-counts {}
:comment-exempt #{}}
budgets (doto (io/file dir "ratchets.edn") (spit (kondo-ratchet/render ratchets)))
test-budgets (empty-test-ratchets-file! dir)
modules (doto (io/file dir "module-ratchets.edn")
(spit (kondo-ratchet/render-module-ratchets {})))
occurrences [{:file "f.clj", :line 1, :linters [:discouraged-var]}
{:file "f.clj", :line 2, :linters [:discouraged-var]}
{:file "f.clj", :line 3, :linters [:discouraged-var]}
{:file "f.clj", :line 4, :linters [:a]}]]
(binding [kondo-ratchet/*ratchets-file* (.getPath budgets)
kondo-ratchet/*module-ratchets-file* (.getPath modules)
kondo-ratchet/*test-ratchets-file* (.getPath test-budgets)]
(mt/with-dynamic-fn-redefs [kondo-ratchet/known-linters (constantly #{:discouraged-var :a})
kondo-ratchet/scan (constantly occurrences)
kondo-ratchet/config-suppressions (constantly {})
kondo-ratchet/module-escape-hatches (constantly {})]
(is (= [(str "dropped stale :ignore-counts entry for :discouraged-var in " (.getPath budgets)
" (tracked per-symbol in :discouraged-var-counts now)")
(str "wrote " (.getPath budgets))]
(str/split-lines (with-out-str (kondo-ratchet/fix! {:attribute (constantly [no-findings no-findings])}))))
"the stale flat entry is dropped without needing --seed")
(is (= {:ignore-counts {:a 1}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{}}
(kondo-ratchet/read-ratchets)))))))
(deftest ^:synchronized fix-lowers-module-counts-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
"kondo-ratchet-test"
@@ -508,11 +692,11 @@
(str/split-lines (with-out-str (kondo-ratchet/fix!))))
"the two src occurrences lower the prod budget to 2; the one test occurrence lowers the test
budget to 1, independently -- neither count masks the other"))
(is (= {:ignore-counts {:a 2}, :config-counts {}, :comment-exempt #{}}
(kondo-ratchet/read-ratchets)))
(is (= {:ignore-counts {:a 1}, :config-counts {}, :comment-exempt #{}}
(binding [kondo-ratchet/*ratchets-file* (.getPath test-budgets)]
(kondo-ratchet/read-ratchets)))))))
(is (=? {:ignore-counts {:a 2}, :config-counts {}, :comment-exempt #{}}
(kondo-ratchet/read-ratchets)))
(is (=? {:ignore-counts {:a 1}, :config-counts {}, :comment-exempt #{}}
(binding [kondo-ratchet/*ratchets-file* (.getPath test-budgets)]
(kondo-ratchet/read-ratchets)))))))
(deftest ^:synchronized fix-validates-test-ratchets-against-the-right-file-test
(let [dir (.toFile (java.nio.file.Files/createTempDirectory
@@ -598,6 +782,80 @@
(is (not-any? builtin [:metabase/modules :unresolved-require])
"repository linters and made-up names are not built-ins")))
(deftest ^:parallel discouraged-symbols-test
(let [config '{:linters {:discouraged-var {a/x {:message "no"}, a/y {:message "no"}}
:discouraged-namespace {some.ns {:message "no"}}}
:config-in-ns {scope-a {:linters {:discouraged-var {a/x {:level :off}, b/z {:message "no"}}}}}
:config-in-call
{scope-b {:linters {:discouraged-namespace {other.ns {:message "no"}}}}}}]
(is (= #{'a/x 'a/y 'b/z}
(kondo-ratchet/discouraged-symbols :discouraged-var config))
"scopes are unioned by symbol -- a symbol appearing at the top level and again under a scope
(a/x) counts once, and a symbol that exists only under a scope (b/z) still counts")
(is (= #{'some.ns 'other.ns}
(kondo-ratchet/discouraged-symbols :discouraged-namespace config)))
(is (= #{} (kondo-ratchet/discouraged-symbols :discouraged-var {})))))
(deftest ^:parallel discouraged-count-key-test
(are [sym expected] (= expected (kondo-ratchet/discouraged-count-key sym))
'metabase.entity-retrieval.core/search-unfiltered
:entity-retrieval.core/search-unfiltered
;; `ee.`, not dropped outright: an OSS namespace and its enterprise mirror of the same shape must
;; not collapse onto the same key
'metabase-enterprise.entity-retrieval.core/search-unfiltered
:ee.entity-retrieval.core/search-unfiltered
'clojure.core/println
:clojure.core/println
;; a bare namespace symbol (:discouraged-namespace) has no name segment to split out
'clojure.tools.logging
:clojure.tools.logging
'metabase.legacy-mbql.normalize
:legacy-mbql.normalize))
(deftest ^:parallel discouraged-count-keys-test
(let [config '{:linters {:discouraged-var {a/x {:message "no"}, a/y {:message "no"}}}}]
(is (= #{:a/x :a/y}
(kondo-ratchet/discouraged-count-keys :discouraged-var config)))))
(deftest ^:parallel discouraged-count-field-test
(is (= :discouraged-var-counts (kondo-ratchet/discouraged-count-field :discouraged-var)))
(is (= :discouraged-namespace-counts (kondo-ratchet/discouraged-count-field :discouraged-namespace))))
(deftest ^:parallel discouraged-in-ignore-counts-test
(is (= #{:discouraged-namespace :discouraged-var}
(kondo-ratchet/discouraged-in-ignore-counts {:discouraged-var 132, :a 1, :discouraged-namespace 68})))
(is (= #{} (kondo-ratchet/discouraged-in-ignore-counts {:a 1, :b 2}))))
(deftest resolve-seed-test
(mt/with-dynamic-fn-redefs [kondo-ratchet/discouraged-count-keys (constantly #{:a/x :ee.b/y})]
(testing "an ordinary linter seeds itself in :ignore-counts"
(is (= {:ignore-counts [:some-linter]}
(#'kondo-ratchet/resolve-seed ":some-linter"))))
(testing "a bare discouragement linter seeds every configured symbol in its own field"
(is (= {:discouraged-var-counts [:a/x :ee.b/y], :bulk #{:discouraged-var-counts}}
(#'kondo-ratchet/resolve-seed ":discouraged-var"))))
(testing "a symbol suffix seeds that symbol, named either way"
(is (= {:discouraged-var-counts [:ee.b/y]}
(#'kondo-ratchet/resolve-seed ":discouraged-var/metabase-enterprise.b/y")
(#'kondo-ratchet/resolve-seed ":discouraged-var/ee.b/y"))))
(testing "an unconfigured or empty symbol suffix throws instead of seeding"
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"\"a/nope\" is not a symbol configured under :discouraged-var"
(#'kondo-ratchet/resolve-seed ":discouraged-var/a/nope")))
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"\"\" is not a symbol configured under :discouraged-var"
(#'kondo-ratchet/resolve-seed ":discouraged-var/"))))
(testing "nil seeds nothing"
(is (= {} (#'kondo-ratchet/resolve-seed nil))))))
(deftest ^:parallel discouraged-count-keys-collision-test
(is (thrown-with-msg? clojure.lang.ExceptionInfo #":discouraged-var symbols a/x, metabase.a/x share the ratchet key :a/x"
(kondo-ratchet/discouraged-count-keys
:discouraged-var
'{:linters {:discouraged-var {a/x {}, metabase.a/x {}}}}))))
(defn- git-in
"Run git in `dir`, failing the test on a nonzero exit."
[dir & args]
@@ -775,9 +1033,11 @@
(deftest ^:parallel merge-ratchets-absent-base-test
(testing "with no base stage, each policy is a one-sided addition and shared linters take the stricter"
(is (= {:ignore-counts {:ours 2, :shared 3, :theirs 3}
:config-counts {}
:comment-exempt #{:ours :theirs}}
(is (= {:ignore-counts {:ours 2, :shared 3, :theirs 3}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{:ours :theirs}}
(kondo-ratchet/merge-ratchets
{}
{:ignore-counts {:ours 2, :shared :unlimited}, :comment-exempt #{:ours}}
@@ -791,9 +1051,11 @@
{:comment-exempt #{:kept :theirs-drop :ours-add}}
{:comment-exempt #{:kept :ours-drop :theirs-add}})))))
(testing "exemptions merge independently of the same linter's count policy"
(is (= {:ignore-counts {:a 3}
:config-counts {}
:comment-exempt #{:a}}
(is (= {:ignore-counts {:a 3}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{:a}}
(kondo-ratchet/merge-ratchets
{:ignore-counts {:a 5}, :comment-exempt #{}}
{:ignore-counts {:a 3}, :comment-exempt #{}}
@@ -809,13 +1071,15 @@
:one-sided 2
:new 2
:new-unlimited :unlimited}})]
(is (= {:ignore-counts {:a 3
:equal :unlimited
:one-sided :unlimited
:new 2
:new-unlimited :unlimited}
:config-counts {}
:comment-exempt #{}}
(is (= {:ignore-counts {:a 3
:equal :unlimited
:one-sided :unlimited
:new 2
:new-unlimited :unlimited}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{}}
merged
(edn/read-string (kondo-ratchet/render merged)))
"rendering the merged policies preserves bounded and unlimited linters")))
@@ -869,7 +1133,10 @@
(let [occurrences (concat (for [[linter n] {:lower 3, :over 7, :new 9, :same 4, :free 2}
i (range n)]
{:file "f.clj", :line (inc i), :linters [linter], :justified? false})
[{:file "g.clj", :line 1, :linters [:polite], :justified? true}])]
[{:file "g.clj", :line 1, :linters [:polite], :justified? true}])
attribution {:actual {:discouraged-var {:dv-lower 2, :dv-over 4, :dv-same 3, :dv-new 1}
:discouraged-namespace {:dn-over 3}}
:unattributed [{:file "h.clj", :line 9, :linters [:discouraged-var]}]}]
(is (= ["seeded :new at 9"
"WARNING: :void has no inline ignores -- nothing to seed"
"dropped :gone (no ignores left)"
@@ -877,33 +1144,55 @@
"WARNING: :over is over budget (5 recorded, 7 actual) -- remove ignores, or accept them all with `--seed :over`"
"dropped :zero (no ignores left)"
"WARNING: :unlimited policies with no ignores left: :empty -- delete an entry by hand once its linter no longer needs one"
"WARNING: :dn-over is over budget (1 recorded, 3 actual) -- remove ignores, or accept them all with `--seed :discouraged-namespace/dn-over`"
"dropped :dv-gone (no ignores left)"
"lowered :dv-lower 5 -> 2"
"WARNING: :dv-over is over budget (1 recorded, 4 actual) -- remove ignores, or accept them all with `--seed :discouraged-var/dv-over`"
"WARNING: :dv-new has 1 ignores but no budget entry -- seed one with `./bin/mage kondo-ratchets-shrink --seed :discouraged-var/dv-new`"
"WARNING: h.clj:9 ignores :discouraged-var but kondo reports no such finding under it -- probably stale, under a nested ignore for the same linter, or in a reader branch kondo skips"
"dropped config :cfg-gone (no suppressions left)"
"lowered config :cfg-lower 4 -> 2"
"WARNING: config suppressions for :cfg-over are over budget (1 recorded, 3 actual) -- remove one from .clj-kondo/config.edn or raise the budget by hand"
"lowered module :api-any 2 -> 1"
"WARNING: module :uses-any is over budget (1 recorded, 2 actual) -- remove one from .clj-kondo/config/modules/config.edn or raise the budget by hand"
"WARNING: :comment-exempt is no longer needed for these linters: :polite -- delete the stale entries by hand"]
(kondo-ratchet/change-report {:ignore-counts {:empty :unlimited
:free :unlimited
:gone 5
:lower 5
:over 5
:polite 1
:same 4
:zero 0}
:config-counts {:cfg-gone 2
:cfg-lower 4
:cfg-over 1
:cfg-same 6}
:comment-exempt #{:lower :polite}}
(kondo-ratchet/change-report {:ignore-counts {:empty :unlimited
:free :unlimited
:gone 5
:lower 5
:over 5
:polite 1
:same 4
:zero 0}
:discouraged-var-counts {:dv-gone 2
:dv-lower 5
:dv-over 1
:dv-same 3}
:discouraged-namespace-counts {:dn-over 1}
:config-counts {:cfg-gone 2
:cfg-lower 4
:cfg-over 1
:cfg-same 6}
:comment-exempt #{:lower :polite}}
{:api-any 2, :uses-any 1}
occurrences
attribution
{:cfg-lower 2, :cfg-over 3, :cfg-same 6}
{:api-any 1, :uses-any 2}
[:new :void]))
"untouched budgets (:same, :cfg-same), a used unlimited policy (:free), and a still-needed
{:ignore-counts [:new :void]}))
"untouched budgets (:same, :cfg-same, :dv-same), a used unlimited policy (:free), and a still-needed
exemption (:lower) earn no line; a hand-written 0 (:zero) is dropped like any bounded budget with no
ignores left; the empty unlimited policy (:empty) is kept and warned about")))
ignores left; the empty unlimited policy (:empty) is kept and warned about; per-symbol fields report
like :ignore-counts, suggesting a per-symbol seed; an unattributed ignore gets its own warning"))
(testing "a per-symbol seed reports under its own field only"
(is (= ["seeded :dv-new at 1"]
(kondo-ratchet/change-report {:ignore-counts {}, :config-counts {}, :comment-exempt #{}}
{}
[]
{:actual {:discouraged-var {:dv-new 1}}}
{}
{}
{:discouraged-var-counts [:dv-new]})))))
(deftest ^:parallel shrink-summary-test
(is (= (str "{:a 2 => 1\n"

1
mage/src/mage/escherize/pr_summary.clj
View File

@@ -2,7 +2,6 @@
(:require
[babashka.json :as json]
[babashka.process :as p]
^{:clj-kondo/ignore [:discouraged-namespace]}
[clojure.pprint :as pprint]
[clojure.string :as str]
[mage.util :as u]

215
mage/src/mage/kondo_ratchet.clj
View File

@@ -1,7 +1,10 @@
(ns mage.kondo-ratchet
"Tooling around the inline ignore ratchets that needs a real kondo run (slow: a minute or two):
report ignores kondo considers redundant, and bulk-insert ignores when landing a new linter."
report ignores kondo considers redundant, attribute discouraged-var and discouraged-namespace ignores to the
symbols they cover, and bulk-insert ignores when landing a new linter."
(:require
[babashka.fs :as fs]
[babashka.json :as json]
[clojure.edn :as edn]
[clojure.string :as str]
[dev.kondo-ratchet :as kondo-ratchet]
@@ -13,17 +16,213 @@
(def ^:private lint-roots
["src" "test" "enterprise/backend" "modules/drivers" "dev" "bin" "mage"])
(defn parse-kondo-output
"The map a kondo run printed as `out` lines in `output-format` (`:edn` or `:json`).
Throws when the run failed (an `exit` other than 0, 2 or 3) or its output isn't a map with a `:findings` list,
rather than returning an empty result."
[output-format {:keys [exit out err]}]
(let [;; 2 and 3 mean kondo ran and reported warnings or errors
parsed (when (#{0 2 3} exit)
(try
(case output-format
:edn (edn/read-string (str/join "\n" out))
:json (json/read-str (str/join "\n" out) {:key-fn keyword}))
(catch Exception _ nil)))]
(when-not (sequential? (:findings parsed))
(throw (ex-info (format "clj-kondo run failed or printed no findings list (exit %d):\n%s"
exit (str/join "\n" (take-last 20 err)))
{:exit exit})))
parsed))
(defn- run-kondo!
"Output of a `clojure -M:kondo` run over `paths`, with `config` merged over the repository's, as
[[parse-kondo-output]] reads it.
With `cache?` false the run neither reads nor writes kondo's cache."
([output-format config paths]
(run-kondo! output-format config paths true))
([output-format config paths cache?]
(parse-kondo-output output-format
(apply shell/sh* {:quiet? true}
"clojure" "-M:kondo"
"--config" (pr-str (assoc-in config [:output :format] output-format))
(concat (when-not cache? ["--cache" "false"])
["--lint"]
paths)))))
(defn- kondo-findings!
"Kondo run over `roots`, optionally with `linter` forced to `:warning`; returns findings as EDN maps
(all of them when `linter` is nil, else just that type)."
[linter roots]
(let [config (pr-str (cond-> {:output {:format :edn}}
linter (assoc :linters {linter {:level :warning}})))
{:keys [out]} (apply shell/sh* {:quiet? true}
"clojure" "-M:kondo" "--config" config "--lint" roots)
findings (:findings (edn/read-string (str/join "\n" out)))]
(cond->> findings
linter (filter #(= (:type %) linter)))))
(cond->> (:findings (run-kondo! :edn (cond-> {} linter (assoc :linters {linter {:level :warning}})) roots))
linter (filter #(= (:type %) linter))))
;;;; ---------------------------------------------------------------------------
;;;; Per-symbol attribution for :discouraged-var / :discouraged-namespace
;;;; ---------------------------------------------------------------------------
(defn- discouraged-ignores
"The [[kondo-ratchet/ignore-matches]] in `content` naming a [[kondo-ratchet/discouragement-linters]] entry."
[content]
(filterv #(some kondo-ratchet/discouragement-linters (:linters %))
(kondo-ratchet/ignore-matches content)))
(def ^:private ignore-key
(str ":clj-kondo" "/ignore"))
;; Renaming only the key leaves every form as the reader saw it: a `#_` still discards its map, and a `^` map
;; is still metadata. Matching the length keeps every row and column.
(def ^:private disabled-key
":ratchet/unignore")
(defn disable-ignores
"`content` with the key of each of `ignores` (from [[kondo-ratchet/ignore-matches]]) renamed so kondo no
longer honors it. Every character outside those keys keeps its offset."
[content ignores]
(reduce (fn [s {:keys [start]}]
(let [i (str/index-of s ignore-key start)]
(str (subs s 0 i) disabled-key (subs s (+ i (count ignore-key))))))
content
ignores))
(defn- offset-fn
"A function from a 1-based `row` and `col` in `content` to its character offset."
[content]
(let [line-starts (into [0] (keep-indexed (fn [i c] (when (= c \newline) (inc i)))) content)]
(fn [row col]
(+ (line-starts (dec row)) (dec col)))))
(def ^:private usages-key
"The kondo analysis bucket holding the usages each linter's findings report on."
{:discouraged-var :var-usages
:discouraged-namespace :namespace-usages})
(defn- usage-symbol
"The fully-qualified symbol a var-usage or namespace-usage names.
A var used through an alias takes its namespace from that alias in `alias-ns`, a map of `[file lang alias]` to
the namespace the file requires under it; a `.cljc` file can require a different one for each language."
[alias-ns {:keys [filename lang alias to name]}]
;; Kondo resolves a re-exported var to the namespace it learned the var came from, which depends on its cache
;; and on which files share the run. The namespace the file required under the alias doesn't move.
(if name
(symbol (str (get alias-ns [filename (some-> lang str) (str alias)] to)) (str name))
(symbol (str to))))
(defn attribute-discouraged
"Per-symbol actual counts for the [[kondo-ratchet/discouragement-linters]] ignores in `contents`, a map of
file to text.
`output` is kondo's JSON output, findings plus var and namespace usages, for the same files with those
ignores disabled by [[disable-ignores]] and its `:filename`s mapped back to the keys of `contents`.
`baseline` is kondo's findings for the files as they are, with the ignores in place.
`known` maps each linter to its configured symbols.
An ignore counts once for each distinct symbol among the findings it covers.
Returns `{:actual {linter {key count}}, :unattributed _, :unresolved _}`, with keys from
[[kondo-ratchet/discouraged-count-key]].
The `:unattributed` ignores cover no finding, and the `:unresolved` findings have no configured symbol; both
are `{:file _, :line _, :linters [linter]}` maps."
[contents output baseline known]
;; A finding missing from `baseline` was suppressed by one of the disabled ignores: the last one before it
;; naming its linter. Kondo has already applied `:config-in-ns` scopes, `:off` overrides, and inline ns
;; config, so only usages it flagged are counted.
;;
;; Known gap: nested ignores undercount. The last ignore before a finding isn't always the one whose form
;; holds it, so with an ignore inside another for the same linter, the inner one also takes the outer one's
;; later findings. The outer ignore then counts toward no budget, leaving its symbol's budget one short, and
;; is reported as unattributed even though removing it would surface a warning. So a new ignore wrapped
;; around an existing one needs no budget.
;; TODO: charge each finding to the ignore that covers it alone. Also lint copies of files with nested
;; ignores, one ignore disabled per copy, and charge a finding that only copy i reveals to ignore i.
(let [ignores (update-vals contents discouraged-ignores)
offsets (update-vals contents offset-fn)
reported? (set (map (juxt :filename :row :col :type) baseline))
alias-ns (into {}
(for [{:keys [filename lang alias to]} (get-in output [:analysis :namespace-usages])
:when alias]
[[filename (some-> lang str) (str alias)] to]))
;; kondo puts each finding at its usage's own :row/:col
usages-at (group-by (juxt :linter :filename :row :col)
(for [[linter k] usages-key
usage (get-in output [:analysis k])]
(assoc usage :linter linter)))
hits (for [{:keys [filename row col], :as finding} (:findings output)
:let [linter (keyword (:type finding))]
:when (and (usages-key linter)
(contains? contents filename)
(not (reported? [filename row col (:type finding)])))
:let [offset ((offsets filename) row col)]]
{:file filename
:line row
:linter linter
:ignore (last (filter #(and (<= (:end %) offset) (some #{linter} (:linters %)))
(ignores filename)))
:symbol (some (comp (known linter) (partial usage-symbol alias-ns))
(usages-at [linter filename row col]))})
counted (distinct (for [{:keys [file linter ignore symbol]} hits
:when (and ignore symbol)]
[file ignore linter (kondo-ratchet/discouraged-count-key symbol)]))
covered? (set (map (juxt :file :linter :ignore) hits))]
{:actual (into {}
(for [linter kondo-ratchet/discouragement-linters]
[linter (frequencies (for [[_ _ l k] counted :when (= l linter)] k))]))
:unattributed (vec (for [[file file-ignores] ignores
ignore file-ignores
linter (filter kondo-ratchet/discouragement-linters (:linters ignore))
:when (not (covered? [file linter ignore]))]
{:file file, :line (:line ignore), :linters [linter]}))
:unresolved (vec (for [{:keys [file line linter symbol]} hits
:when (nil? symbol)]
{:file file, :line line, :linters [linter]}))}))
(defn- lint-with-ignores-disabled!
"Kondo's JSON output, as [[attribute-discouraged]] takes it, for `contents` (a map of file to text) with
their [[kondo-ratchet/discouragement-linters]] ignores disabled, plus its findings for the files as they
are: `[output baseline]`."
[contents]
;; Lints the files and, in the same run, copies with the discouraged ignores disabled, under a temp
;; directory. Kondo configures a copy the same as the original because every ns-group matches on namespace
;; names, not paths.
(let [dir (fs/create-temp-dir {:prefix "kondo-ratchet-attribution"})
original (into {} (map (juxt #(str (fs/path dir %)) identity)) (keys contents))
;; keeps only the copies' entries, renamed to their originals
restore (partial keep #(some->> (original (:filename %)) (assoc % :filename)))]
(try
(doseq [[copy file] original
:let [content (contents file)]]
(fs/create-dirs (fs/parent copy))
(spit copy (disable-ignores content (discouraged-ignores content))))
;; Without the cache, a run's findings depend only on the files in it, so CI and a local run agree.
(let [output (run-kondo! :json
{:output {:analysis {:var-usages true, :namespace-usages true}}}
(concat (keys contents) (keys original))
false)]
[(-> output
(update :findings restore)
(update-in [:analysis :var-usages] restore)
(update-in [:analysis :namespace-usages] restore))
(filter (comp contents :filename) (:findings output))])
(finally
(fs/delete-tree dir)))))
(defn attribute-occurrences!
"Per-symbol attribution of the [[kondo-ratchet/discouragement-linters]] ignores in each of `groups`, a seq
of occurrence seqs, as [[attribute-discouraged]] returns it, in the same order; the `:attribute` that
[[dev.kondo-ratchet/check]] and `fix!` take."
[groups]
;; One kondo run covers every group, since a JVM start and a lint pass dominate the cost.
(let [discouraged-files (fn [occurrences]
(into (sorted-set)
(comp (filter #(some kondo-ratchet/discouragement-linters (:linters %)))
(map :file))
occurrences))
file-groups (mapv discouraged-files groups)
contents (into {} (map (juxt identity slurp)) (reduce into #{} file-groups))
config (kondo-ratchet/kondo-config)
known (into {}
(for [linter kondo-ratchet/discouragement-linters]
[linter (kondo-ratchet/discouraged-symbols linter config)]))
[output baseline] (if (empty? contents)
[{} []]
(lint-with-ignores-disabled! contents))]
(mapv #(attribute-discouraged (select-keys contents %) output baseline known) file-groups)))
(def keep-marker
"Comment token marking an ignore as a verified `:redundant-ignore` false positive.

156
mage/test/mage/kondo_ratchet_test.clj
View File

@@ -421,3 +421,159 @@
(remove-ignores-at'
"#_{:clj-kondo/ignore [:x]}\n(a)\n#_{:clj-kondo/ignore [:y]}\n(b)\n"
[1 2 3])))))
;;;; ---------------------------------------------------------------------------
;;;; Per-symbol attribution
;;;; ---------------------------------------------------------------------------
(deftest disable-ignores-test
(let [content (str "(ns a)\n"
"#_ ;; why\n"
"{:clj-kondo/ignore [:discouraged-var]}\n"
"(eval 1)\n"
"(defn f #^{:clj-kondo/ignore [:discouraged-var], :a {:b 1}} [] (eval 2))\n")
disabled (kondo-ratchet/disable-ignores content (dev-ratchet/ignore-matches content))]
(testing "only the ignore key changes, so every other character keeps its offset"
(is (= (str "(ns a)\n"
"#_ ;; why\n"
"{:ratchet/unignore [:discouraged-var]}\n"
"(eval 1)\n"
"(defn f #^{:ratchet/unignore [:discouraged-var], :a {:b 1}} [] (eval 2))\n")
disabled)))
(testing "the result no longer contains any ignore"
(is (empty? (dev-ratchet/ignore-matches disabled))))))
(def ^:private eval-and-println
"Kondo's JSON output, as [[kondo-ratchet/attribute-discouraged]] takes it, for `a.clj` below with its ignores
disabled: `println` and `eval` flagged on line 3, and `eval` again on line 5."
{:findings [{:filename "a.clj", :row 3, :col 1, :type "discouraged-var"}
{:filename "a.clj", :row 3, :col 10, :type "discouraged-var"}
{:filename "a.clj", :row 5, :col 1, :type "discouraged-var"}
{:filename "a.clj", :row 5, :col 1, :type "unused-binding"}]
:analysis {:var-usages [{:filename "a.clj", :row 3, :col 1, :to "clojure.core", :name "println"}
{:filename "a.clj", :row 3, :col 10, :to "clojure.core", :name "eval"}
{:filename "a.clj", :row 5, :col 1, :to "clojure.core", :name "eval"}]
:namespace-usages []}})
(def ^:private a-clj
(str "(ns a)\n"
"#_{:clj-kondo/ignore [:discouraged-var]}\n"
"(println (eval 1))\n"
"#_{:clj-kondo/ignore [:discouraged-var :unused-binding]}\n"
"(eval 2)\n"
"#_{:clj-kondo/ignore [:discouraged-var]}\n"
"(inc 3)\n"))
(defn- var-hits
"Kondo's JSON output, as [[kondo-ratchet/attribute-discouraged]] takes it, for a discouraged-var finding at each
`[file row col sym]` of `hits`, with the var usage it reports on."
[& hits]
{:findings (for [[file row col] hits]
{:filename file, :row row, :col col, :type "discouraged-var"})
:analysis {:var-usages (for [[file row col sym] hits]
{:filename file, :row row, :col col, :to (namespace sym), :name (name sym)})}})
(def ^:private known
{:discouraged-var #{'clojure.core/eval 'clojure.core/println}
:discouraged-namespace #{'clojure.tools.logging}})
(deftest attribute-discouraged-counting-test
(testing "each ignore counts once per distinct symbol it covers; one covering nothing is unattributed"
(is (= {:actual {:discouraged-var {:clojure.core/eval 2, :clojure.core/println 1}
:discouraged-namespace {}}
:unattributed [{:file "a.clj", :line 6, :linters [:discouraged-var]}]
:unresolved []}
(kondo-ratchet/attribute-discouraged {"a.clj" a-clj} eval-and-println [] known))))
(testing "two findings of one symbol under one ignore count once"
(is (= {:clojure.core/eval 1}
(-> (kondo-ratchet/attribute-discouraged
{"b.clj" "#_{:clj-kondo/ignore [:discouraged-var]}\n(do (eval 1) (eval 2))\n"}
(var-hits ["b.clj" 2 5 'clojure.core/eval]
["b.clj" 2 14 'clojure.core/eval])
[]
known)
(get-in [:actual :discouraged-var])))))
(testing "the same ignore at the same place in two files counts once in each"
(is (= {:clojure.core/eval 2}
(-> (kondo-ratchet/attribute-discouraged
{"b.clj" "#_{:clj-kondo/ignore [:discouraged-var]}\n(eval 1)\n"
"c.clj" "#_{:clj-kondo/ignore [:discouraged-var]}\n(eval 1)\n"}
(var-hits ["b.clj" 2 1 'clojure.core/eval]
["c.clj" 2 1 'clojure.core/eval])
[]
known)
(get-in [:actual :discouraged-var]))))))
(deftest attribute-discouraged-suppression-test
(testing "a finding kondo also reports with the ignores in place isn't charged to the ignore before it"
(is (= {:actual {:discouraged-var {:clojure.core/eval 1, :clojure.core/println 1}
:discouraged-namespace {}}
:unattributed [{:file "a.clj", :line 4, :linters [:discouraged-var]}
{:file "a.clj", :line 6, :linters [:discouraged-var]}]
:unresolved []}
(kondo-ratchet/attribute-discouraged {"a.clj" a-clj}
eval-and-println
[{:filename "a.clj", :row 5, :col 1, :type "discouraged-var"}]
known))))
(testing "findings in files outside `contents` are ignored"
(is (= {}
(get-in (kondo-ratchet/attribute-discouraged {} eval-and-println [] known) [:actual :discouraged-var])))))
(deftest attribute-discouraged-symbol-test
(testing "a namespace finding resolves through its namespace usage"
(is (= {:clojure.tools.logging 1}
(-> (kondo-ratchet/attribute-discouraged
{"c.clj" "(ns c\n (:require\n #_{:clj-kondo/ignore [:discouraged-namespace]}\n [clojure.tools.logging]))\n"}
{:findings [{:filename "c.clj", :row 4, :col 5, :type "discouraged-namespace"}]
:analysis {:namespace-usages [{:filename "c.clj", :row 4, :col 5, :to "clojure.tools.logging"}]}}
[]
known)
(get-in [:actual :discouraged-namespace])))))
(testing "a var used through an alias is keyed by the namespace the file requires, not where kondo resolved it"
(is (= {:lib.core/->legacy-MBQL 1}
(-> (kondo-ratchet/attribute-discouraged
{"d.clj" "#_{:clj-kondo/ignore [:discouraged-var]}\n(lib/->legacy-MBQL q)\n"}
{:findings [{:filename "d.clj", :row 2, :col 2, :type "discouraged-var"}]
:analysis {:var-usages [{:filename "d.clj"
:row 2
:col 2
:to "metabase.lib.convert"
:alias "lib"
:name "->legacy-MBQL"}]
:namespace-usages [{:filename "d.clj", :row 1, :col 1, :to "metabase.lib.core", :alias "lib"}]}}
[]
{:discouraged-var #{'metabase.lib.convert/->legacy-MBQL 'metabase.lib.core/->legacy-MBQL}})
(get-in [:actual :discouraged-var])))))
(testing "in a .cljc file, each language's usage takes the alias that language requires"
(is (= {:clojure.pprint/print-table 1}
(-> (kondo-ratchet/attribute-discouraged
{"e.cljc" "#_{:clj-kondo/ignore [:discouraged-var]}\n(pprint/print-table [])\n"}
{:findings [{:filename "e.cljc", :row 2, :col 2, :type "discouraged-var"}]
:analysis {:var-usages [{:filename "e.cljc", :row 2, :col 2, :lang "clj", :to "clojure.pprint", :alias "pprint", :name "print-table"}
{:filename "e.cljc", :row 2, :col 2, :lang "cljs", :to "cljs.pprint", :alias "pprint", :name "print-table"}]
:namespace-usages [{:filename "e.cljc", :lang "clj", :to "clojure.pprint", :alias "pprint"}
{:filename "e.cljc", :lang "cljs", :to "cljs.pprint", :alias "pprint"}]}}
[]
{:discouraged-var #{'clojure.pprint/print-table}})
(get-in [:actual :discouraged-var])))))
(testing "a finding whose usage isn't a configured symbol is unresolved"
(is (= [{:file "a.clj", :line 3, :linters [:discouraged-var]}]
(:unresolved
(kondo-ratchet/attribute-discouraged {"a.clj" a-clj}
eval-and-println
[]
(assoc known :discouraged-var #{'clojure.core/eval})))))))
(deftest parse-kondo-output-test
(testing "a run that reported findings still parses"
(is (= {:findings [{:type :x}]}
(kondo-ratchet/parse-kondo-output :edn {:exit 2, :out ["{:findings [{:type :x}]}"], :err []})))
(is (= {:findings [{:type "x"}]}
(kondo-ratchet/parse-kondo-output :json {:exit 3, :out ["{\"findings\": [{\"type\": \"x\"}]}"], :err []}))))
(testing "a failed run throws with its stderr rather than reading as no findings"
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"(?s)clj-kondo run failed or printed no findings list \(exit 1\):.*boom"
(kondo-ratchet/parse-kondo-output :edn {:exit 1, :out ["{}"], :err ["boom"]}))))
(testing "output that isn't a map with a findings list throws"
(doseq [out [["not json"] [""] ["[]"] ["{}"] ["{\"summary\": {}}"]]]
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"printed no findings list \(exit 0\)"
(kondo-ratchet/parse-kondo-output :json {:exit 0, :out out, :err []}))))))

22
mage/test/mage/merge_kondo_ratchets_test.clj
View File

@@ -117,9 +117,11 @@
(is (= 0 exit))
(is (str/includes? out "staged merged .clj-kondo/ratchets.edn")))
(testing "the finite budget beats :unlimited, one-sided changes win, and a concurrent removal stays gone"
(is (= {:ignore-counts {:a 4, :b 3}
:config-counts {:c 1}
:comment-exempt #{}}
(is (= {:ignore-counts {:a 4, :b 3}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {:c 1}
:comment-exempt #{}}
(ratchets-policies dir))))
(testing "only the ratchets file is staged; the other conflict is left alone"
(is (= #{"M .clj-kondo/ratchets.edn" "UU app.txt"}
@@ -137,9 +139,11 @@
(with-conflict [dir {:ours {ratchets-file "{:ignore-counts {:a 3, :shared :unlimited}}\n"}
:theirs {ratchets-file "{:ignore-counts {:b 4, :shared 2}}\n"}}]
(is (= 0 (:exit (run dir script))))
(is (= {:ignore-counts {:a 3, :b 4, :shared 2}
:config-counts {}
:comment-exempt #{}}
(is (= {:ignore-counts {:a 3, :b 4, :shared 2}
:discouraged-var-counts {}
:discouraged-namespace-counts {}
:config-counts {}
:comment-exempt #{}}
(ratchets-policies dir)))
(is (= #{"M .clj-kondo/ratchets.edn" "UU app.txt"}
(status dir)))))
@@ -152,8 +156,10 @@
(is (= 0 exit))
(is (str/includes? out (str "staged merged " test-ratchets-file))))
(let [text (slurp (str (fs/path dir test-ratchets-file)))]
(is (= {:ignore-counts {:a 4, :b 3}
:comment-exempt #{}}
(is (= {:ignore-counts {:a 4, :b 3}
:discouraged-namespace-counts {}
:discouraged-var-counts {}
:comment-exempt #{}}
(edn/read-string text))
"merged with the same rules as the prod file, and written without :config-counts")
(is (str/starts-with? text ";; Budgets for kondo suppressions in test code")))

1
src/metabase/config/core.clj
View File

@@ -1,6 +1,5 @@
(ns metabase.config.core
(:require
^{:clj-kondo/ignore [:discouraged-namespace]}
[clojure.java.io :as io]
[clojure.string :as str]
[environ.core :as env]

2
src/metabase/lib/metadata/cached_provider.cljc
View File

@@ -1,7 +1,7 @@
(ns metabase.lib.metadata.cached-provider
(:refer-clojure :exclude [update-keys get-in #?(:clj doseq)])
(:require
#?@(:clj (^{:clj-kondo/ignore [:discouraged-namespace]} [clj-yaml.core]
#?@(:clj ([clj-yaml.core]
[metabase.util.json :as json]
[pretty.core :as pretty]))
[clojure.set :as set]

2
src/metabase/lib/metadata/invocation_tracker.cljc
View File

@@ -1,7 +1,7 @@
(ns metabase.lib.metadata.invocation-tracker
(:require
#?@(:clj
(^{:clj-kondo/ignore [:discouraged-namespace]} [clj-yaml.core]
([clj-yaml.core]
[metabase.util.json :as json]
[pretty.core :as pretty]))
[metabase.lib.metadata.protocols :as lib.metadata.protocols]))

2
src/metabase/lib_be/metadata/jvm.clj
View File

@@ -2,7 +2,7 @@
"Implementation(s) of [[metabase.lib.metadata.protocols/MetadataProvider]] only for the JVM."
(:refer-clojure :exclude [get-in])
(:require
^{:clj-kondo/ignore [:discouraged-namespace]} [clj-yaml.core]
[clj-yaml.core]
[clojure.core.cache :as cache]
[clojure.core.cache.wrapped :as cache.wrapped]
[clojure.string :as str]

1
src/metabase/util.cljc
View File

@@ -9,7 +9,6 @@
[me.flowthing.pp :as pp]
[metabase.config.core :as config]
[clojure.pprint :as pprint]
^{:clj-kondo/ignore [:discouraged-namespace]}
[metabase.util.jvm :as u.jvm]
[metabase.util.string :as u.str]
[potemkin :as p]

1
src/metabase/util/yaml.clj
View File

@@ -2,7 +2,6 @@
"Convenience functions for parsing and generating YAML."
(:refer-clojure :exclude [load])
(:require
^{:clj-kondo/ignore [:discouraged-namespace]}
[clj-yaml.core :as yaml]
[clojure.java.io :as io]
[clojure.string :as str]