From 72ded76547762a804e543318b374b42cc26af24d Mon Sep 17 00:00:00 2001 From: Chris Jenkinson Date: Fri, 25 Sep 2026 10:14:35 +0000 Subject: [PATCH 1/3] Raise PHPStan to the max level and fix the errors it reports --- phpstan.neon | 2 +- src/Finder/RegexFinder.php | 7 ++++++- src/Lexer/Lexer.php | 5 ++++- src/Matcher/MatchedText.php | 3 +++ src/Node/AbstractNode.php | 2 +- src/NodeTraverser/NodeTraverser.php | 8 ++++++++ src/State/AmbiguousTokenFoundException.php | 16 +++++++++++----- src/Token/Token.php | 5 ++++- src/Token/TokenStream.php | 12 ++++++------ tests/NodeTraverser/NodeTraverserTest.php | 16 ++++++++++++++++ 10 files changed, 60 insertions(+), 16 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 1eb50eb..8315b5e 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 5 + level: max paths: - src - tests diff --git a/src/Finder/RegexFinder.php b/src/Finder/RegexFinder.php index 93e4f56..8cb3867 100644 --- a/src/Finder/RegexFinder.php +++ b/src/Finder/RegexFinder.php @@ -7,7 +7,7 @@ class RegexFinder { /** - * @var mixed[] + * @var array */ private array $matches = []; @@ -32,6 +32,11 @@ public function find(string $text): bool return false; } + /** + * @param string[] $keys + * + * @return array + */ public function getMatches(array $keys): array { return array_intersect_key($this->matches, array_flip($keys)); diff --git a/src/Lexer/Lexer.php b/src/Lexer/Lexer.php index 0eb3cb7..20b8ec7 100644 --- a/src/Lexer/Lexer.php +++ b/src/Lexer/Lexer.php @@ -93,7 +93,10 @@ private function tokeniseFromInitialState(string $text): TokenStream $tokens->add($token); - $length = mb_strlen($token->getValue('all')); + $all = $token->getValue('all'); + assert(is_string($all)); + + $length = mb_strlen($all); if (0 < $length) { $cursor->advance($length); diff --git a/src/Matcher/MatchedText.php b/src/Matcher/MatchedText.php index 5a7c5a9..ebb0091 100644 --- a/src/Matcher/MatchedText.php +++ b/src/Matcher/MatchedText.php @@ -25,6 +25,9 @@ public function get(string $key) return null; } + /** + * @return mixed[] + */ public function getAll(): array { return $this->matches; diff --git a/src/Node/AbstractNode.php b/src/Node/AbstractNode.php index 4230956..25ad772 100644 --- a/src/Node/AbstractNode.php +++ b/src/Node/AbstractNode.php @@ -25,7 +25,7 @@ public function __toString(): string } /** - * @return array{attributes: array, nodes: NodeInterface[]} + * @return array{attributes: array, nodes: NodeInterface[]} */ public function jsonSerialize(): array { diff --git a/src/NodeTraverser/NodeTraverser.php b/src/NodeTraverser/NodeTraverser.php index a14a141..2b2d8dc 100644 --- a/src/NodeTraverser/NodeTraverser.php +++ b/src/NodeTraverser/NodeTraverser.php @@ -65,6 +65,11 @@ public function traverseNode(NodeInterface $node): NodeInterface|NodeVisitorActi return $node; } + /** + * @param mixed[] $children + * + * @return mixed[] + */ public function traverseChildren(array $children): array { $isList = array_is_list($children); @@ -131,6 +136,9 @@ private function runTraverseNodeOnSubNodes(NodeInterface $node, array $children) }, $children); } + /** + * @param array $attributes + */ private function runTraverseChildrenOnAttributes(NodeInterface $node, array $attributes): void { array_walk($attributes, function ($attribute, $key) use (&$node): void { diff --git a/src/State/AmbiguousTokenFoundException.php b/src/State/AmbiguousTokenFoundException.php index 7776b47..de9b12c 100644 --- a/src/State/AmbiguousTokenFoundException.php +++ b/src/State/AmbiguousTokenFoundException.php @@ -24,11 +24,11 @@ public function __construct( int $code = 0, ?Throwable $previous = null ) { - $matches = array_map( - static fn (string $matcherName, MatchedText $matchedText): string => sprintf('%s (%s)', $matcherName, TextExcerpt::of($matchedText->getAll()['all'])), - $calledMatchers, - $matchedTokens - ); + $matches = array_map(static function (string $matcherName, MatchedText $matchedText): string { + $all = $matchedText->getAll()['all'] ?? null; + + return sprintf('%s (%s)', $matcherName, TextExcerpt::of(is_string($all) ? $all : '')); + }, $calledMatchers, $matchedTokens); $message = sprintf( 'Ambiguous token found with state %s at line %d, column %d: matchers %s', @@ -41,11 +41,17 @@ public function __construct( parent::__construct($message, $code, $previous); } + /** + * @return string[] + */ public function getCalledMatchers(): array { return $this->calledMatchers; } + /** + * @return MatchedText[] + */ public function getMatchedTokens(): array { return $this->matchedTokens; diff --git a/src/Token/Token.php b/src/Token/Token.php index 1089eaf..fd8b8e5 100644 --- a/src/Token/Token.php +++ b/src/Token/Token.php @@ -18,7 +18,10 @@ public function __construct( public function __toString(): string { - return sprintf('%s (%s)', $this->getType(), trim($this->getValue('all'))); + $all = $this->getValue('all'); + assert(is_string($all)); + + return sprintf('%s (%s)', $this->getType(), trim($all)); } /** diff --git a/src/Token/TokenStream.php b/src/Token/TokenStream.php index c602728..8ab1743 100644 --- a/src/Token/TokenStream.php +++ b/src/Token/TokenStream.php @@ -55,12 +55,12 @@ public function getCurrentToken(): ?TokenInterface */ public function expectTokenType(string $expectedType): bool { - if (0 === count($this)) { + $currentToken = $this->getCurrentToken(); + + if (null === $currentToken) { throw new RuntimeException(sprintf('No more tokens; expected %s', $expectedType)); } - $currentToken = $this->getCurrentToken(); - if ($expectedType !== $currentToken->getType()) { throw new RuntimeException( sprintf('Token type is %s; expected %s', $currentToken->getType(), $expectedType) @@ -77,12 +77,12 @@ public function expectTokenType(string $expectedType): bool */ public function expectTokenTypes(array $expectedTypes): bool { - if (0 === count($this)) { + $currentToken = $this->getCurrentToken(); + + if (null === $currentToken) { throw new RuntimeException(sprintf('No more tokens; expected any of %s', implode(', ', $expectedTypes))); } - $currentToken = $this->getCurrentToken(); - $expected = array_filter($expectedTypes, static fn ($expectedType) => $expectedType === $currentToken->getType()); if (count($expected) >= 1) { diff --git a/tests/NodeTraverser/NodeTraverserTest.php b/tests/NodeTraverser/NodeTraverserTest.php index f32b909..7694cad 100644 --- a/tests/NodeTraverser/NodeTraverserTest.php +++ b/tests/NodeTraverser/NodeTraverserTest.php @@ -29,6 +29,8 @@ public function testItReplacesANode(): void $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertFalse($node->hasNode('ChildNode')); Assert::assertInstanceOf(NodeFromVisitor::class, $node->getNode('NodeFromVisitor')); } @@ -52,6 +54,8 @@ public function testItCanChangeGrandchildrenNodes(): void $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertEquals('replacement', $node->getNode('ChildNode')->getNode('GrandchildNode')->getAttribute('testAttribute')); } @@ -66,6 +70,8 @@ public function testItRemovesAChildNode(): void $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertFalse($node->hasNode('ChildNode')); } @@ -80,6 +86,8 @@ public function testItRemovesAChildNodeWhenAVisitorReturnsTheRemoveNodeAction(): $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertFalse($node->hasNode('ChildNode')); } @@ -109,6 +117,8 @@ public function testItPreservesKeysOfArrayAttributesWithoutNodes(): void $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertSame([5 => 'five', 9 => 'nine'], $node->getAttribute('values')); } @@ -124,6 +134,8 @@ public function testItPreservesKeysWhenRemovingANodeFromAKeyedArrayAttribute(): $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertSame(['second' => $keep], $node->getAttribute('children')); } @@ -139,6 +151,8 @@ public function testItReindexesAListAttributeWhenRemovingANode(): void $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertSame([$keep], $node->getAttribute('children')); } @@ -154,6 +168,8 @@ public function testItVisitsNodesInNestedArrayAttributes(): void $node = $traverser->traverse($node); + Assert::assertNotNull($node); + Assert::assertSame(['group' => [$keep]], $node->getAttribute('children')); } } From 73ffaa406d4b1c767daf43d86f45b2561c2dd1e1 Mon Sep 17 00:00:00 2001 From: Chris Jenkinson Date: Fri, 25 Sep 2026 10:17:38 +0000 Subject: [PATCH 2/3] Add TokenInterface::getText() and validate the matched text when building a Token instead of asserting --- spec/Token/TokenSpec.php | 20 ++++++++++++++++++++ src/Lexer/Lexer.php | 5 +---- src/Token/Token.php | 17 ++++++++++++++--- src/Token/TokenInterface.php | 5 +++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/spec/Token/TokenSpec.php b/spec/Token/TokenSpec.php index 3879389..167ef1a 100644 --- a/spec/Token/TokenSpec.php +++ b/spec/Token/TokenSpec.php @@ -6,6 +6,7 @@ use chrisjenkinson\StructuredDocumentParser\Token\NonexistentKeyException; use chrisjenkinson\StructuredDocumentParser\Token\TokenPosition; +use InvalidArgumentException; use PhpSpec\ObjectBehavior; class TokenSpec extends ObjectBehavior @@ -49,4 +50,23 @@ public function it_casts_to_a_string(): void { $this->__toString()->shouldReturn('Something (value)'); } + + public function it_has_the_matched_text(): void + { + $this->getText()->shouldReturn('value'); + } + + public function it_requires_the_matched_text_to_be_a_string(): void + { + $this->beConstructedWith('Something', ['all' => 13], new TokenPosition(3, 7)); + + $this->shouldThrow(InvalidArgumentException::class)->duringInstantiation(); + } + + public function it_requires_the_matched_text(): void + { + $this->beConstructedWith('Something', ['heading' => 'value'], new TokenPosition(3, 7)); + + $this->shouldThrow(InvalidArgumentException::class)->duringInstantiation(); + } } diff --git a/src/Lexer/Lexer.php b/src/Lexer/Lexer.php index 20b8ec7..27fa4aa 100644 --- a/src/Lexer/Lexer.php +++ b/src/Lexer/Lexer.php @@ -93,10 +93,7 @@ private function tokeniseFromInitialState(string $text): TokenStream $tokens->add($token); - $all = $token->getValue('all'); - assert(is_string($all)); - - $length = mb_strlen($all); + $length = mb_strlen($token->getText()); if (0 < $length) { $cursor->advance($length); diff --git a/src/Token/Token.php b/src/Token/Token.php index fd8b8e5..bf86214 100644 --- a/src/Token/Token.php +++ b/src/Token/Token.php @@ -4,8 +4,12 @@ namespace chrisjenkinson\StructuredDocumentParser\Token; +use InvalidArgumentException; + class Token implements TokenInterface { + private readonly string $text; + /** * @param mixed[] $value */ @@ -14,14 +18,21 @@ public function __construct( private readonly array $value, private readonly TokenPosition $position ) { + if (!isset($value['all']) || !is_string($value['all'])) { + throw new InvalidArgumentException(sprintf('Token %s needs a string "all" value', $type)); + } + + $this->text = $value['all']; } public function __toString(): string { - $all = $this->getValue('all'); - assert(is_string($all)); + return sprintf('%s (%s)', $this->getType(), trim($this->getText())); + } - return sprintf('%s (%s)', $this->getType(), trim($all)); + public function getText(): string + { + return $this->text; } /** diff --git a/src/Token/TokenInterface.php b/src/Token/TokenInterface.php index 93a24a7..35cbca6 100644 --- a/src/Token/TokenInterface.php +++ b/src/Token/TokenInterface.php @@ -17,6 +17,11 @@ public function getValue(string $key): mixed; public function getType(): string; + /** + * The text the token consumed from the input. + */ + public function getText(): string; + public function getPosition(): TokenPosition; public function hasKey(string $key): bool; From 3bd32479b67ec51d3841b9be568bfeb8118bd6d7 Mon Sep 17 00:00:00 2001 From: Chris Jenkinson Date: Fri, 25 Sep 2026 10:18:18 +0000 Subject: [PATCH 3/3] Show an explicit instanceof check instead of assert() in the VisitableInterface example --- src/Visitor/VisitableInterface.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Visitor/VisitableInterface.php b/src/Visitor/VisitableInterface.php index 8d7b485..91e784a 100644 --- a/src/Visitor/VisitableInterface.php +++ b/src/Visitor/VisitableInterface.php @@ -12,7 +12,9 @@ * * public function accept(VisitorInterface $visitor): mixed * { - * assert($visitor instanceof MarkdownVisitor); + * if (!$visitor instanceof MarkdownVisitor) { + * throw new InvalidArgumentException('Heading can only be visited by a MarkdownVisitor'); + * } * * return $visitor->visitHeading($this); * }