Repository navigation
Add Printer::stripIgnoredCharacters() - #1985
Merged
Merged
Conversation
spawnia
added this pull request to stack #1986
October 3, 2026 16:40
Port of graphql-js stripIgnoredCharacters, including its test and fuzz suites. BlockString::print() gains a $minimize flag. 🤖 Generated with Claude Code
🤖 Generated with Claude Code
🤖 Generated with Claude Code
🤖 Generated with Claude Code
spawnia
force-pushed
the
strip-ignored-characters
branch
from
October 3, 2026 16:44
f1e0b5a to
eb1843f
Compare
…cters # Conflicts: # CHANGELOG.md
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 6395e14 | Previous: d260876 | Ratio |
|---|---|---|---|
BuildSchemaBench::benchBuildSchema |
24.932 ms |
13.672 ms |
1.82 |
OverlappingFieldsCanBeMergedBench::benchRepeatedFields100 |
3.252 ms |
1.755 ms |
1.85 |
OverlappingFieldsCanBeMergedBench::benchRepeatedFields500 |
15.684 ms |
8.357 ms |
1.88 |
OverlappingFieldsCanBeMergedBench::benchRepeatedFields1000 |
30.798 ms |
16.524 ms |
1.86 |
OverlappingFieldsCanBeMergedBench::benchRepeatedFields2000 |
61.849 ms |
32.289 ms |
1.92 |
OverlappingFieldsCanBeMergedBench::benchRepeatedFields3000 |
92.463 ms |
48.746 ms |
1.90 |
VisitorBench::benchVisitIntrospectionWithEnterLeave |
0.306 ms |
0.158 ms |
1.94 |
VisitorBench::benchVisitIntrospectionWithKindMap |
0.297 ms |
0.156 ms |
1.90 |
VisitorBench::benchVisitIntrospectionWithKindCallable |
0.279 ms |
0.145 ms |
1.92 |
VisitorBench::benchVisitIntrospectionWithEnterLeaveMap |
0.3 ms |
0.159 ms |
1.89 |
VisitorBench::benchVisitNestedWithEnterLeave |
0.071 ms |
0.035 ms |
2.03 |
ScalarOverrideBench::benchExecuteWithoutOverride |
0.161 ms |
0.079 ms |
2.04 |
ScalarOverrideBench::benchExecuteWithTypesOverride |
0.162 ms |
0.079 ms |
2.05 |
StarWarsBench::benchSchema |
0.005 ms |
0.002 ms |
2.50 |
StarWarsBench::benchHeroQuery |
0.326 ms |
0.163 ms |
2 |
StarWarsBench::benchNestedQuery |
0.715 ms |
0.361 ms |
1.98 |
StarWarsBench::benchQueryWithFragment |
0.759 ms |
0.383 ms |
1.98 |
StarWarsBench::benchQueryWithInterfaceFragment |
0.718 ms |
0.361 ms |
1.99 |
StarWarsBench::benchStarWarsIntrospectionQuery |
7.181 ms |
3.618 ms |
1.98 |
HugeSchemaBench::benchSchema |
12.89 ms |
6.532 ms |
1.97 |
HugeSchemaBench::benchSmallQuery |
14.609 ms |
7.235 ms |
2.02 |
HugeSchemaBench::benchSmallQueryLazy |
15.579 ms |
7.689 ms |
2.03 |
LexerBench::benchIntrospectionQuery |
0.294 ms |
0.152 ms |
1.93 |
LexerBench::benchDeeplyIndentedQuery |
1.897 ms |
1.004 ms |
1.89 |
DeferredBench::benchSingleDeferred |
0.001 ms |
0 ms |
+∞ |
DeferredBench::benchNestedDeferred |
0.003 ms |
0.001 ms |
3 |
DeferredBench::benchChain5 |
0.005 ms |
0.003 ms |
1.67 |
DeferredBench::benchChain100 |
0.081 ms |
0.039 ms |
2.08 |
DeferredBench::benchManyDeferreds |
0.455 ms |
0.221 ms |
2.06 |
DeferredBench::benchManyNestedDeferreds |
11.956 ms |
5.926 ms |
2.02 |
DeferredBench::bench1000Chains |
3.465 ms |
1.704 ms |
2.03 |
This comment was automatically generated by workflow using github-action-benchmark.
🤖 Generated with Claude Code
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation closely follows the reference utility and is supported by comprehensive semantic and fuzz testing.
Review effort: Balanced
Findings: None
What changed in this PR
Adds Printer::stripIgnoredCharacters() to compact GraphQL documents while preserving semantics.
Changes:
- Implements token-based compaction and minimized block-string printing.
- Adds comprehensive unit and fuzz coverage.
- Documents the API and updates static-analysis compatibility.
| File | Description |
|---|---|
src/Language/Printer.php |
Implements ignored-character stripping. |
src/Language/BlockString.php |
Adds minimized block-string printing. |
tests/Language/StripIgnoredCharactersTest.php |
Tests representative inputs and AST equivalence. |
tests/Language/StripIgnoredCharactersFuzzTest.php |
Tests generated token and block-string combinations. |
tests/Language/BlockStringTest.php |
Covers minimized block-string output. |
phpstan/php-below-8.0.neon |
Handles PHP 7.4 return-type analysis. |
docs/class-reference.md |
Documents the new public method. |
CHANGELOG.md |
Records the new API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Since the bulk string scan from #1948, the Lexer accepts invalid UTF-8 inside strings and keeps the raw bytes. Comments and block strings swallow up to three bytes after an invalid lead byte. The spec only allows Unicode scalar values as SourceCharacter. 🤖 Generated with Claude Code
🤖 Generated with Claude Code
Token positions count characters the way Lexer::readChar() does, so stepping by lead byte finds the byte offsets without converting the whole body. Relies on #1987 rejecting invalid UTF-8. 🤖 Generated with Claude Code
…ignored-characters # Conflicts: # CHANGELOG.md
The prefix regex hit pcre.backtrack_limit on about 1 million 3- or 4-byte characters and then reported the first byte of the document. mb_scrub() replaces only invalid bytes, so the first differing byte is the invalid one. 🤖 Generated with Claude Code
…ignored-characters
Test multibyte characters before string tokens, including a 4-byte character followed by a 3-byte one, which is the shortest input that catches a wrong 4-byte step. Add assert messages, fix the remainder typo carried over from graphql-js and tighten the fuzz length comment. 🤖 Generated with Claude Code
Collaborator
Author
|
Released as v15.38.0. |
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.
Resolves #1028
graphql-php has no equivalent of graphql-js
stripIgnoredCharacters, which clients use to send compact documents.This adds
Printer::stripIgnoredCharacters(), a port of https://github.com/graphql/graphql-js/blob/17.x.x/src/utilities/stripIgnoredCharacters.ts.Merge https://github.com//pull/1987 first, its commits show in this diff until then
String tokens are copied byte for byte, as in graphql-js.
Token positions count characters, so the byte offsets come from stepping lead bytes like
Lexer::readChar().That matches the Lexer only for valid UTF-8, which #1987 enforces.
GitHub refused to change the base branch, so this pull request targets master.
Parsing the output gives the same AST as parsing the input, apart from locations
Block strings are re-printed minimized, through a new
$minimizeflag onBlockString::print().Both
stripIgnoredCharacters-test.tsandstripIgnoredCharacters-fuzz.tsare ported, plus the minimized cases fromblockString-test.ts.The fuzz suite checks that every block string up to 7 characters, built from 6 tricky characters, keeps its value.
Runtime stays linear in document size
On a 664 KB document, stripping takes 0.85s and
Parser::parse()takes 1.07s.Slicing with
mb_substr()instead took 3.85s, because it scans from the start of the body for every string token.🤖 Generated with Claude Code