Harden the registry dispatch-table parser against benign reformatting#29
Merged
Conversation
Summary Make the registry validator's shell dispatch scan tolerant of cosmetic formatting changes so a purely stylistic edit to the dispatchers can no longer brick every format/lint/plan run. `_shell_dispatch_functions` reconstructs the adapter-to-function dispatch table by scraping the `case` arms of `_format_dispatch`/`_lint_dispatch` in `autoformat.sh`/`autolint.sh`, and it runs on EVERY `load_registry` (so on every `checkrun` format/lint/plan/explain, including the commit hook). The old scan required the handler on the same line as the `pattern)` label and detected the function end with an exact, unindented `}`. A formatting-only change — moving a handler to the next line, indenting `esac`, a shell formatter splitting an arm — could silently drop arms or empty the table, which then raises `RegistryError` and bricks the tool. - rewrite the scan to accept the handler on the same or a following line, tolerate blank lines between a label and its handler, and stop at an `esac`/closing brace regardless of indentation - preserve the loud failure on an unreadable (empty) table: the parser never silently returns an empty mapping that would make every selector look undispatched, and a wrong function value is still caught loudly by the existing `_validate_invariants` dispatch-mismatch check - add a prominent WHY-comment: this still derives control flow from shell source (a coupling the durable fix removes by making the dispatch mapping data the registry owns in `registry.json`), documents the reformats that are tolerated, and notes that alternation (`a|b)`) and stacked bare labels are intentionally unsupported and fail loudly - leave `_shell_functions` (an existence-set check whose miss surfaces loudly as "adapter is not implemented") unchanged as lower-risk The new parser is byte-identical to the old one on the production dispatchers (21 format arms, 27 lint arms, same keys and values). Testing - new `registry-test` case: a reformatted-but-equivalent dispatcher (next-line handlers, blank lines, indented `esac`) parses to the exact expected table, and an unreadable/empty dispatcher raises `RegistryError` with the "could not be read" message - verified red-green: the old same-line-only parser yields an empty table (which raises) on the reformatted fixture - full suite green (`./test/checkrun-test`: ok); `checkrun lint` clean
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.
Summary
Make the registry validator's shell dispatch scan tolerant of cosmetic
formatting changes so a purely stylistic edit to the dispatchers can no
longer brick every format/lint/plan run.
_shell_dispatch_functionsreconstructs the adapter-to-function dispatchtable by scraping the
casearms of_format_dispatch/_lint_dispatchin
autoformat.sh/autolint.sh, and it runs on EVERYload_registry(so on every
checkrunformat/lint/plan/explain, including the commithook). The old scan required the handler on the same line as the
pattern)label and detected the function end with an exact, unindented}. A formatting-only change — moving a handler to the next line,indenting
esac, a shell formatter splitting an arm — could silentlydrop arms or empty the table, which then raises
RegistryErrorandbricks the tool.
line, tolerate blank lines between a label and its handler, and stop
at an
esac/closing brace regardless of indentationnever silently returns an empty mapping that would make every selector
look undispatched, and a wrong function value is still caught loudly by
the existing
_validate_invariantsdispatch-mismatch checksource (a coupling the durable fix removes by making the dispatch
mapping data the registry owns in
registry.json), documents thereformats that are tolerated, and notes that alternation (
a|b)) andstacked bare labels are intentionally unsupported and fail loudly
_shell_functions(an existence-set check whose miss surfacesloudly as "adapter is not implemented") unchanged as lower-risk
The new parser is byte-identical to the old one on the production
dispatchers (21 format arms, 27 lint arms, same keys and values).
Testing
registry-testcase: a reformatted-but-equivalent dispatcher(next-line handlers, blank lines, indented
esac) parses to the exactexpected table, and an unreadable/empty dispatcher raises
RegistryErrorwith the "could not be read" message(which raises) on the reformatted fixture
./test/checkrun-test: ok);checkrun lintclean