checks: one listing per rule set, and list the validate rules (#927) (#1132)
* checks: list the validate rules, and describe them (#927)
Adds the rule listing #927 asked for:
- 'oasdiff checks validate' lists the validate rule IDs with a description
of what each reports.
- 'oasdiff checks changelog' is the explicit form of bare 'oasdiff checks',
so the two rule sets are addressed symmetrically. Bare 'checks' is
unchanged.
Both render through the shared formatters (text/json/yaml), so --format
validation and rendering stay consistent. Flags are registered per command
because viper binds a command's own persistent flags.
Descriptions are plain English, not localized: a validate finding takes its
message from the parser at runtime, so there is no localized message file
to hang them off, and they are oasdiff's own (kin's CodedError carries only
a code). The 30 version-gate rules are described from the ID rather than
listed, so a newly gated field upstream is described without a change here.
Supporting output fixes so an IDs-and-descriptions listing renders cleanly:
Check's optional fields are omitempty (no change to the existing checks
output, where all 506 rules populate every field), and the text renderer
includes a column only when some row populates it, so the listing no longer
prints an empty LEVEL column.
* checks: give the validate rules their severity
The listing left LEVEL empty on the grounds that a validate finding's
severity is decided per finding at runtime. It isn't: severityForKinError
switched on the error type, and each type carries its own code, so the
code already determined the severity. The one branch that looked
runtime-dependent, SchemaValueError splitting on ValueKind, is really two
types with two codes: ExampleViolatesSchema and DefaultViolatesSchema.
So severity is a property of the rule, and ruleLevels is now the single
source of truth for both ends. severityForKinError became a lookup on the
finding's own rule id, which drops the type switch that re-derived what
the code lookup had already established.
The version-gate rules (<field>-field-for-3-1-plus) are matched by the
same regex that describes them rather than listed one by one, so a field
gated upstream tomorrow is classified without a change here. All 29 of
kin's current gate codes match it.
oasdiff's own lints built findings directly and set checker.WARN inline,
so the listing showed them as errors while they emitted warnings. They
now take their level from the same map. The listing surfaced that, which
is the argument for having it.
With a level on every row, the text renderer needs no dynamic columns:
RenderChecks is back to main's version, and its tests with it. Check
keeps omitempty, which is still load-bearing for direction, area, kind
and action, absent on every validate rule.
Tests pin the map's keys against the registered ids (a typo would
silently fall through to ERR), and drive real specs through Validate for
an example, a default, a version gate, a native lint and a plain error,
asserting the finding's level is the one the listing shows. Each was
checked against a negative control.
* checks: require a subcommand, and give validate its own flags
`oasdiff checks` named one rule set by omission, which stopped being
tenable once `checks validate` existed. The parent now has no body: it
groups the two listings and prints the help when run bare, so a listing
always names its rule set.
An old `oasdiff checks --format json` fails with an unknown flag rather
than quietly producing nothing, since the listing flags moved to the
subcommands.
The flags now differ per listing, which they could not while one command
served both:
- --format and --severity on both. Severity filtering is new to the
validate listing and only possible because a validate rule has a
severity of its own.
- --lang and --tags on changelog only. The validate descriptions are
plain English rather than localized, so --lang would have nothing to
translate, and validate rules carry no tags.
The severity filter is now one helper rather than an inline chain, since
both listings need it. It switches on the level, not on Level.String(),
because the flag takes the short forms and "warn" is not "warning".
Existing tests move to `checks changelog`; the new ones in
internal/checks_test.go cover the parent's help, the loud failure of the
old invocation, and both listings reporting a severity for every rule.
Note for the website: the rule catalog is generated by w3's
generate-checks.sh, which calls the bare command, and the /docs page
names it in visitor-facing copy. Both need updating when w3 bumps
.oasdiff-version; nothing breaks before then, since that pin controls
which oasdiff CI installs.
* checks: fail on an unknown subcommand instead of exiting 0
`oasdiff checks bogus` printed the help and exited 0, while `oasdiff
bogus` exits 100. An unknown subcommand is a user error and nothing was
done, so the exit code has to say so: a script running `oasdiff checks
chnagelog --format json > out.json` was writing help text to its target
and seeing success.
The cause is cobra's execute(): a non-runnable command returns
flag.ErrHelp before Args is evaluated, so cobra.NoArgs on the parent
never ran. Giving the parent a RunE that prints the help keeps it
runnable, which is what makes Args reachable. The error now matches the
top-level one, `unknown command "bogus" for "oasdiff checks"`, and exits
100 like every other usage error.
DisableFlagsInUseLine keeps `[flags]` out of the usage line, since the
flags belong to the subcommands. Bare `oasdiff checks` still prints the
help and exits 0: no arguments is an ambiguous request, not a wrong one.
* checks: report an unknown subcommand the way the top level does
`oasdiff checks bogus` printed the error then the whole help block, while
`oasdiff bogus` prints the error and a one-line hint. Same mistake, two
different outputs depending on depth.
Cobra only prints the short form when it fails to *find* a command.
`checks bogus` resolves to `checks` with a leftover argument, so the
error surfaces from Args instead and cobra follows it with the usage
block. Reporting it in the Args validator gives the top-level shape:
Error: unknown command "bogus" for "oasdiff checks"
Run 'oasdiff checks --help' for usage.
The two lines are emitted the way cobra emits them, ErrPrefix plus the
hint as its own line rather than inside the error string, so the error
text carries no trailing punctuation (ST1005) and the output is
byte-identical to the top-level form modulo the command path. A test
pins both messages against each other.
An unknown *flag* on `checks` still prints the usage block, matching
`flatten --bogus` and every other subcommand: that is a different error,
and the usage is what the user needs to see for it.
* checks: document the cobra workaround and how to remove it
The three-part wiring on the `checks` parent (RunE, Args,
DisableFlagsInUseLine) reads as arbitrary without the reason, and the
reason is upstream: cobra returns flag.ErrHelp for a non-runnable command
before it validates Args, so a typo'd subcommand exits 0.
Records the mechanism with the cobra source lines it depends on
(command.go:956 vs 969 in v1.10.2), the upstream issue open since 2020
(spf13/cobra#1156, plus #706, #981, #2130) and the PR proposing
ErrorOnUnknownSubcommand awaiting review since 2024 (#2167), and the exact
removal: set that field, restore cobra.NoArgs, drop all three parts and
this function, at which point the usage block reads
`oasdiff checks [command]` alone.
Both upstream threads upvoted rather than opening a fifth duplicate.
* docs: name the checks subcommands in the command list
The command index still offered a single `checks` entry. It is two
listings now, so both get a line, and the comment calling `checks
validate` the counterpart of `oasdiff checks` names the changelog
subcommand instead.
* checks: give the changelog listing its own file
checks.go held both the parent command and the whole changelog listing,
while the validate listing already had checks_validate.go. Splitting it
leaves the three files with one job each and the two listings shaped
identically, so a third rule set would have an obvious template.
checks.go the parent command, and what both listings share:
--format, --severity, the severity filter, and the
unknown-subcommand handling
checks_changelog.go const, command, flags, runner, output
checks_validate.go the same five, in the same order
Names follow: runChecks became runChecksChangelog, matching
runChecksValidate, and outputChecks became outputChangelogRules.
outputValidateRuleIDs became outputValidateRules, since it stopped being
ids alone when it gained descriptions and levels. addChecksValidateFlags
moves next to the command it serves.
Also fixes an error identifier: a failed print in the changelog listing
reported "checks <format>" rather than "checks changelog <format>", the
only place still naming the parent.
Pure moves and renames, no behaviour change: 506 changelog rules and 97
validate rules before and after. R
Reuven Harrison committed
94c30cfe86aadb10ecb9ac8ca90d1e8b84ea3011
Parent: e5236e5
Committed by GitHub <noreply@github.com>
on 7/30/2026, 11:28:27 AM