RFC: permissive whitelist formatter — from-scratch rewrite (research + PoC)#19
Draft
JanTvrdik wants to merge 13 commits into
Draft
RFC: permissive whitelist formatter — from-scratch rewrite (research + PoC)#19JanTvrdik wants to merge 13 commits into
JanTvrdik wants to merge 13 commits into
Conversation
…list formatter - notes/00-04: goal, existing-standard analysis, no-width decision, core invariant (vertical free / horizontal strict), whitelist template design - notes/10-18: formatter research (oxc, ruff, dprint, biome, mago, csharpier, php-cs-fixer, PHP-Parser, pretty-php); notes/20,21,30,40: synthesis, decisions, engine recommendation, architecture plan - poc/: dependency-free PHP engine — lexer (tokens+gaps), layout parser (default-deny totality gate), per-construct templates, MATCH/REPAIR/FATAL formatter, re-tokenize verifier, golden-file test suite (12 fixtures) Co-Authored-By: Claude Code
…tributes Grammar: namespace/use, class-likes (const/property/method/enum case/trait use, promoted ctor params), if/elseif/else, while, do-while, for, foreach, switch, try/catch/finally, match, closures/arrow fns, ternary + binary chains with per-joint break preservation (leading operators, existing standard's multiline condition/ternary shapes), access chains with ->-joint breaks, attributes, casts, clone/yield/include, static/global vars, named args, heredoc + interpolated strings as verbatim spans, trailing comments, docblock reindent (verifier compares comments modulo leading whitespace). Structural triggers implemented: 2+ params => one-per-line signature; trailing comma iff broken; match always expanded. Corpus: old coding-standard repo => 0 fatals, all 7 compliant sniff sources byte-identical (MATCH). php-cs-fixer/PHP-Parser/pretty-php (1036 files) => ~95% parse coverage, 0 verifier failures, 0 idempotency failures, 0 unsafe fatals, ~4ms/file. Co-Authored-By: Claude Code
…s callables Validated against oss-doctrine-entity-preloader: all 73 files (src + tests) now pass through 100% BYTE-IDENTICAL — 0 repairs, 0 fatals — the permissive-formatter ideal on a real shipmonk-standard codebase. - Lexer: PhpToken::tokenize(TOKEN_PARSE) — semi-reserved identifiers (method named new, Config::DEFAULT) tokenize correctly; invalid PHP throws upfront - Line-targeted comments (// @PHPStan-Ignore ...) must never move lines; new allowed placements: trailing after block-opening brace, after method signature before Allman brace, mid access chain, inside broken condition before ')', on a collection opener line - Cond object replaces CondLayout (owns condition trailing comment) - first-class callable foo(...), throw-as-expression, yield k => v - empty class body: both {\n} and {\n\n} allowed (old standard's canonical form) Co-Authored-By: Claude Code
notes/50-architecture-review.md: external review of the PoC. Verdict: core
whitelist/project-then-render model sound, graduate without rethink; the
mechanical substrate needs work before scaling grammar (render(): string ->
Emitter/token-writer, RenderCtx{line,cont}, generalized trailing-trivia model,
per-node recovery/violations). Template IR refuted.
Verifier fixes (both confirmed by repro, failed safe as designed):
- trailing comma added before '}' (match arms) was rejected
- trailing comma before '=>' in match condition lists was rejected
Comma allowances now: before ) ] } and =>. Fixture 17 locks both repairs.
Co-Authored-By: Claude Code
…y; unit tests Replaces pairwise skip rules with named normalization applied to both streams (layout commas before ) ] } =>; comments modulo per-line leading whitespace). Formatter catches Throwable at the file boundary so a template bug can never crash a batch run or corrupt a file. tests/unit.php covers the gate directly. Co-Authored-By: Claude Code
…s, row projection
The notes/50 substrate rework (priorities 2-5), replacing render(): string:
- Emitter (token writer): the only way templates emit output; indentation +
blank-line clamping live in ONE place (newline/lineBreak); per-construct
string concatenation gone
- RenderCtx {line, cont}: the two indentation coordinates made explicit;
renderAt() hacks and Cond's special dispatch removed
- Trailing trivia: Lexer attaches same-line comments to the preceding token;
the Emitter enforces one invariant (pending comment must be followed by a
line break, else fatal) — replaces ~8 bespoke comment placements, deletes
TrailingComment wrapper + mutable Segment field + Cond/Block/FunctionDecl
header-comment fields; line-targeted directives can never change lines
- Statement-level error recovery: unsupported constructs rewind + freeze as
VerbatimStmt (byte-exact incl. original indentation) + Violation, instead
of whole-file fatal. Foreign-corpus fatals: php-cs-fixer 27->6,
PHP-Parser 4->1, pretty-php 14->2
- Per-statement outcome spans: StmtSeries compares each statement's output
slice to its source slice -> line-precise 'formatting does not match any
allowed form' violations (innermost wins); --check prints file:line
- CollectionLayout: pure projectRows() projection (Row = the choice
assignment as data) replaces the closure-state row machine; source commas
re-emitted as tokens so their trivia survives
All 17 fixtures pass; oss-doctrine-entity-preloader stays 100% byte-identical
(73/73 MATCH); 0 verifier/idempotency failures across all corpora.
Co-Authored-By: Claude Code
…ser oracle - tests/corpus.php: MATCH/REPAIR/FATAL summary + fatal-message histogram; enforces the safety invariants per file (fatal => byte-identical input, repair => idempotent, verifier failures fail the run); --expect-all-match pins the shipmonk-standard corpus at 100% MATCH - tests/fuzz.php: randomizes horizontal whitespace (spacing + indentation, never line structure) of cleanly-formatting files and asserts convergence to the same output — targets exactly the repair paths and RenderCtx logic - tests/oracle.php: differential oracle vs nikic/PHP-Parser (CI-only) — whole-file fatals on nikic-parseable files are reported as coverage gaps - tests/ci.sh: full gate; currently ALL GREEN. Remaining whole-file fatals across 1036 foreign files: 9, all comment-placement (render-time), the next recovery refinement Co-Authored-By: Claude Code
Co-Authored-By: Claude Code
…d-forms catalog Ran the formatter over fresh worktrees of every oss-* repo (all depend on shipmonk/coding-standard; src/ dirs = compliant gold). Result after fixes: 14 of 16 src corpora 100% byte-identical; 566 files, 5 repairs total, all deliberate tightenings (indent drift, missing trailing commas on repos pinning standard <0.3, half-broken if-conditions the old sniff misses). Engine bugs the corpus caught: - by-ref param & swallowed by the type parser (amp token-id discrimination: only T_AMPERSAND_NOT_FOLLOWED_BY_VAR_OR_VARARG is an intersection operator) - trailing comments dropped on attribute ']' (store+emit close token) and on docblocks (Emitter::carryTrivia for verbatim-emitted comment text) — both were verifier-caught VERIFY-FAILs, gate worked - match arm condition ROWS joined (author grouping now preserved, e.g. CBOR byte tables); param attributes on their own line joined (now a choice) - anonymous-class recovery splitting '};' Design lesson (notes/60 principle 2): allowing BOTH +1 and aligned operator continuation indents was tried and REVERTED — the perturbation fuzz caught that reading a choice point off indentation (horizontal whitespace) breaks format(perturb(x)) == format(x). Choice points must come from line structure only; aligned chains are repaired to +1. notes/60-allowed-forms-catalog.md: the per-construct D7 catalog, corpus- grounded, with open questions for the RFC. Fixture 18 locks harvested forms. Co-Authored-By: Claude Code
Co-Authored-By: Claude Code
… corpus stats
query-guard (monorepo, 215 files): now 100% byte-identical MATCH with zero
recovered statements and zero repairs.
New syntax support (query-guard + oss-fleet driven):
- typed class constants (PHP 8.3): private const string ROUTE = ...
- asymmetric visibility (PHP 8.4): public private(set) int $x (token-id
polyfills in bootstrap for PHP < 8.4 runtimes)
- anonymous classes: new class (args) extends A implements B { members }
(space before args parens — corpus-unanimous, PER-CS)
- skipped destructuring slots: [, $second] = ..., foreach (... as [, $x])
- (semi-)reserved constant names: const PUBLIC = ...
- parseType hardened: two adjacent names never form a type (was swallowing
the constant name after a const type)
tests/corpus.php: files whose byte-identical output hides recovered
(verbatim) statements are now counted as 'recovered', never MATCH — this
exposed 10 hidden gaps across the oss fleet, all now fixed: recovered=0
in every oss src/ dir. Fixture 19 locks the new forms.
Co-Authored-By: Claude Code
…: 0 fatals)
Scaled the corpus to monorepo/backend/src (18,096 files). Baseline was 181
fatals, 8 verifier failures, 29 recovered; now 0 fatals, 0 verifier failures,
1 recovery (a genuine won't-fix) and 17,606 byte-identical matches (97.3%).
New allowed forms (all corpus-grounded, locked in fixtures 20/21):
- Multi-line `catch` type lists: `(` on the catch line, one type per line with a
TRAILING ` |`, `)` on its own line (519 unanimous corpus lines; were being
flattened, ~500 repairs became matches). A comment on `(` also forces it.
- Comment rows inside collections may be multi-line docblocks/block comments.
- Match-arm condition lists are now a full collection (rows, blank grouping,
own-line comment rows between conditions, per-condition trailing `//` notes),
modelled via a MatchCondItem ListItem; no mandatory trailing comma before `=>`.
- Own-line comment rows after `(` / before `)` in conditions (and before a binary
operator), preserved at +1.
- `return`/`throw`/... with a trailing comment or leading comment rows push the
expression to +1 with comments preserved.
- Own-line comment rows between a class header and `{` (the phpcs:enable pattern).
- ParenExpr gained the broken form (was flat-only).
Fixes:
- Verifier: a dropped layout comma's OWN trailing comment was lost, so
`[] //note` vs the repaired `[], //note` diverged — re-insert it on both sides.
- MatchArm: condition-comma trivia was synthesized, dropping source `//` notes.
- Emitter::layoutComma() writes a synthesized comma before a pending trailing
comment (the one text emission that bypasses the trailing-trivia guard).
- A comment INSIDE a type (`A | //note\n B`) has no flat-DNF position → clean
statement recovery instead of a whole-file render fatal.
Co-Authored-By: Claude Code
…rule)
Blank lines were previously always the author's choice (0-1 clamp). This adds the
first construct where vertical spacing is MANDATED — class-like member spacing —
via a new MemberSpacing policy consulted by StmtSeries (blocks/switch/file series
keep the author's blanks). Corpus-grounded on monorepo backend/src (18,096 files):
- blank after `{` before the first member (18,073 have it, 0 without)
- blank before `}` after the last member, non-empty body (18,093 vs 28)
- a blank surrounds every method — mandatory when either side of a boundary is a
method, and placed BEFORE the method's leading docblock/attribute block (the
tokenizer emits those as separate members; the blank must not split them)
- fields stay the author's choice — consecutive properties/constants/enum cases
may sit with or without a blank (813 adjacent consts, 974 adjacent cases with none)
Applies to class/interface/trait/enum and anonymous classes. StmtSeries takes an
optional (list, index) -> ?bool policy (true=mandate / false=forbid / null=clamp);
MemberSpacing::renderClose handles the pre-`}` blank. Line-precise --check: a
mandated/forbidden blank that the source doesn't match records a violation pinned
to the member line (the boundary lives between two statement slices, which the
existing content comparison never sees).
Cost: +46 repairs on backend/src (files missing a mandated blank), 0 on the OSS
gold corpora — the convention is effectively universal. entity-preloader stays
100% MATCH; fuzz + full CI green; idempotent. Fixture 22 covers it.
Co-Authored-By: Claude Code
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
1 participant
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.
What this is
A draft/RFC, not a mergeable change: research notes + a working proof of concept for rewriting this coding standard from scratch, without
squizlabs/php_codesnifferandslevomat/coding-standard.The core idea — a permissive formatter
Unlike Prettier-style formatters, this one does not enforce one canonical way to write code. For any formatting choice it defines a set of allowed forms and only reformats code that matches none of them. E.g. all of these are equally valid and each is preserved byte-exactly as written:
Governing invariant: vertical layout (line breaks, element grouping, blank lines) is the author's choice and is preserved; horizontal layout (spacing, indentation, trailing commas) is strictly enforced. There are deliberately no width-based rules — no max-line-length ever forces a wrap.
Design (see
notes/, esp.04,40,50)TOKEN_PARSE), composer-only.// @phpstan-ignore ...) can never change lines.notes/10-18: architecture research of oxc, ruff, dprint, biome, mago, csharpier, php-cs-fixer, nikic/PHP-Parser, pretty-php that led to this design.notes/50: independent architecture review.Status / numbers
poc/tests/ci.sh: unit + golden fixtures + corpora + whitespace-perturbation fuzz + nikic differential oracle — all green--checkreports line-precise violationsNote: corpus/oracle CI steps reference local sibling checkouts (
../repos/...); wiring proper CI is future work, as are: remaining grammar (anonymous classes,list(), …), the per-construct allowed-forms catalog, config surface, pragma/baseline.Feedback wanted
=>— currently repaired)Co-Authored-By: Claude Code