fix: name the rule when a configurable rule is missing its assertions block - #3040
fix: name the rule when a configurable rule is missing its assertions block#3040luantaraschi wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 57731ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for the contribution, @luantaraschi. Frankly, I'd prefer to silently skip incorrectly configured rules rather than throw an error, since there's already a lint check for the configuration itself that should flag the problem to the user. What do you think? |
|
That works for me on the CLI side, where The case I would flag is the programmatic one. There is also the sibling guard a few lines above: a missing I am happy to switch this to skipping either way, with or without the |
What/Why/How?
A configurable rule declared without its
assertionsblock dies with a message that says nothing about the config:getAssertsToApplyreadsassertion.assertions[assertName]for the first key ofasserts, which ispattern, so that is where theundefinedsurfaces.The config schema already declares the block as mandatory.
ConfigurableRuleandWhereinpackages/core/src/types/redocly-yaml.tsboth carryrequired: ['subject', 'assertions'], so no valid configuration is being turned away here. The runtime simply was not checking what the schema promises, andredocly lintstill ran the rule after the config struct warning.The change adds that check next to the
'type' (String) is requiredguards that were already there, in the four places a configurable rule definition gets built:rules/common/assertions/index.ts, the rule itselfrules/common/assertions/utils.ts, eachwhereentrygraphql/assertions.ts, both of the same two spotsThe GraphQL path keeps its own
if (!kind) continue, so a rule with no subject type is still skipped rather than rejected. Only the missingassertionsblock now raises, which is what the OpenAPI path already did.After:
Reference
Closes #2185.
Testing
Two unit tests in
packages/core/src/rules/common/assertions/__tests__/index.test.tsfor the rule and itswhereentry, and two inpackages/core/src/rules/graphql/__tests__/assertions.test.tsthat go throughlintFromStringagainst a schema. All four fail onmainwith theCannot read properties of undefinedmessage.The whole
packages/coresuite was run onmainand on this branch back to back, on Windows with Node 22.20.maingives 36 failed and 1015 passed, the branch gives 36 failed and 1019 passed, so the only difference is the four tests added here. Those 36 are pre-existing path and snapshot mismatches on Windows and none of them touch assertions.Check yourself
assertionsas part of the rule shape, so nothing there changes.Security
TypeErrorinto a named error before any document is read.Note
Low Risk
Small defensive validation that replaces an obscure TypeError with explicit config errors; no change to valid rule behavior.
Overview
Adds runtime validation so configurable rules without an
assertionsobject fail with a clear, rule-named error instead ofCannot read properties of undefined (reading 'pattern').The check mirrors existing
'type' (String) is requiredguards in the OpenAPI path (Assertionsand eachwhereentry inbuildVisitorObject) and the GraphQL path (GraphqlAssertionsandbuildWhereMatchers). Invalid configs are rejected beforegetAssertsToApplytouchesassertion.assertions.Unit and integration tests cover missing
assertionson the rule and onwhereentries for both stacks; a patch changeset documents the fix.Reviewed by Cursor Bugbot for commit 57731ab. Bugbot is set up for automated code reviews on this repo. Configure here.