Fix #16: add strict mode to reject unknown ViewDefinition select directives#17
Open
piotrszul wants to merge 1 commit into
Open
Fix #16: add strict mode to reject unknown ViewDefinition select directives#17piotrszul wants to merge 1 commit into
piotrszul wants to merge 1 commit into
Conversation
Unknown directives on a ViewDefinition `select` element (e.g. `repeat`, which flatquack does not implement) were silently ignored: validateElement only inspected the keys it knew, so an unrecognized key on an element that also had a `column` array passed as an ordinary column block, and parseNode dropped the directive — compiling a valid-but-wrong query (one all-NULL row per resource) instead of failing. Add a `strict` option (default false, preserving current lenient behavior) threaded through parseVd -> validateVd and a --strict CLI flag. When strict, validateElement rejects any key on a select element that is not a recognized directive (column/select/forEach/forEachOrNull/unionAll), naming the offending key: "unsupported select directive: 'repeat'". The root ViewDefinition keys (resource/name/where/...) are unaffected — the check applies only to select elements. Implementing `repeat` itself remains a separate, larger task; this makes the unsupported subset fail loudly when callers opt in. Tests (tests/view.test.js): strict rejects `repeat`; non-strict (default) ignores it; strict accepts a view using only known directives. All 194 tests pass; tests/spec-tests/ untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a24bd02 to
709479b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16.
Problem
Unknown directives on a
selectelement (e.g.repeat, which flatquack doesn't implement) were silently ignored.validateElementonly inspected keys it recognized, so an element with an unknown key plus acolumnarray passed as an ordinary column block, andparseNodedropped the directive entirely — producing a valid-but-wrong query (one all-NULL row per resource) instead of failing.Fix
Add a
strictoption, defaultfalse(current lenient behavior unchanged), threadedparseVd → validateVd, plus a--strictCLI flag.When
strictis on,validateElementrejects any key on a select element that isn't a recognized directive (column/select/forEach/forEachOrNull/unionAll), naming the offender:The root ViewDefinition keys (
resource/name/where/…) are unaffected — the strict check is applied only to select elements (the rootvalidateElement(vd)call is not flagged as a select element).Verification
End-to-end via the CLI on the issue's
repeatview:--strict:error: unsupported select directive: 'repeat'Tests (
tests/view.test.js)repeat)All 194 tests pass.
tests/spec-tests/(reference fixtures) untouched.