ci: judge rewritten files with zsh, not only our parser - #1452
Merged
Conversation
The autofix gate measured corruption as an increase in this linter own parser-error count. Our parser is more permissive than zsh: it accepts a dangling `>`, a stray `]]`, and an unterminated block, so a fix that produced any of those scored zero errors and passed. Corrupted output was also idempotent, so the convergence invariant stayed quiet too. The gate now also asks zsh. A file zsh accepted before the rewrite must still be accepted after it. Two traps the check accounts for. `zsh -n` exits 0 while still printing some diagnostics, so it reads stderr rather than the exit status. And `$(<file)` makes `zsh -n` open the file, reporting a missing input that is not a syntax error, so that message is ignored. Where zsh is absent the check is skipped with a warning rather than failing, so the sweep still runs off CI. The job that runs it is ubuntu-only, where zsh is preinstalled. Against the binary from before the recent fixer repairs the gate now reports 9 corrupted files; it reported none. Signed-off-by: afadesigns <afadesign.official@gmail.com>
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.
The hole
The autofix gate measured corruption as an increase in this linter's own parser-error count. Our parser is more permissive than zsh — it accepts a dangling
>, a stray]], an unterminated block — so a fix producing any of those scored zero errors and passed. The corrupted output was also idempotent, so the convergence invariant stayed quiet as well.That is how four separate corrupting autofixes reached the corpus while the gate reported
corruptions=0on every run.The check
A file zsh accepted before the rewrite must still be accepted after it.
Two traps it accounts for:
zsh -nexits 0 while still printing some diagnostics.zinit/share/rpm2cpio.zshprints "redirection with no command" and returns 0. The check reads stderr, not the exit status.$(<file)makeszsh -nopen the file, reporting a missing input. That is not a syntax error, so the message is ignored — otherwise the legitimate$(cat f)→$(<f)rewrite would look like damage.Where zsh is absent the check is skipped with a warning rather than failing, so the sweep still runs off CI and on other platforms. The job that runs it is ubuntu-only, where zsh is present; the summary line now ends with
zsh=zshorzsh=noneso a silent degradation is visible.Proof it works
Same script, two binaries:
The nine are the files this work repaired — fzf/completion.zsh, prezto/gpg/init.zsh, spaceship ×3, zgen, zinit-autoload, zsh-utils/editor, zsh-vimode-visual — each reported with its line and zsh's own reason, for example:
Before this change that same run reported
corruptions=0.Severity: Error — the gate that was supposed to catch destructive fixes could not see them.