From a5f45a9b04a8d9b8f07fe878853bd688d3f14806 Mon Sep 17 00:00:00 2001 From: Toon Verwerft Date: Tue, 16 Apr 2024 16:05:05 +0200 Subject: [PATCH] Fix static analysis --- .github/workflows/analyzers.yaml | 1 + .github/workflows/autoloader.yaml | 1 + .github/workflows/code-style.yaml | 1 + .github/workflows/stress.yaml | 1 + .github/workflows/tests.yaml | 2 +- psalm.xml | 1 + src/Xml/Dom/Collection/NodeList.php | 6 +- src/Xml/Dom/Locator/Node/detect_document.php | 6 +- .../Xsd/locate_namespaced_xsd_schemas.php | 7 +- .../Xsd/locate_no_namespaced_xsd_schemas.php | 7 +- src/Xml/Dom/Locator/document_element.php | 3 +- .../elements_with_namespaced_tagname.php | 6 +- src/Xml/Dom/Locator/elements_with_tagname.php | 5 +- src/Xml/Dom/Locator/root_namespace.php | 2 +- src/Xml/Dom/Manipulator/Attribute/rename.php | 5 +- .../Document/optimize_namespaces.php | 5 +- .../Element/copy_named_xmlns_attributes.php | 2 +- src/Xml/Dom/Manipulator/Element/rename.php | 4 +- .../Traverser/Visitor/RemoveNamespaces.php | 3 +- .../Internal/Decoder/Builder/element.php | 4 +- .../Internal/Decoder/Builder/namespaces.php | 2 +- stubs/DOM.phpstub | 48 +++++-- stubs/php_xsl.stub.phpstub | 131 ++++++++++++++++++ .../Document/OptimizeNamespacesTest.php | 24 ++++ 24 files changed, 238 insertions(+), 39 deletions(-) create mode 100644 stubs/php_xsl.stub.phpstub diff --git a/.github/workflows/analyzers.yaml b/.github/workflows/analyzers.yaml index 7ade8b2..3735eea 100644 --- a/.github/workflows/analyzers.yaml +++ b/.github/workflows/analyzers.yaml @@ -13,6 +13,7 @@ jobs: matrix: operating-system: [ubuntu-latest] php-versions: ['8.4'] + composer-options: ['--ignore-platform-req=php+'] fail-fast: false name: PHP ${{ matrix.php-versions }} @ ${{ matrix.operating-system }} steps: diff --git a/.github/workflows/autoloader.yaml b/.github/workflows/autoloader.yaml index 39e7ec9..db62f8f 100644 --- a/.github/workflows/autoloader.yaml +++ b/.github/workflows/autoloader.yaml @@ -8,6 +8,7 @@ jobs: matrix: operating-system: [ubuntu-latest] php-versions: ['8.4'] + composer-options: ['--ignore-platform-req=php+'] fail-fast: false name: PHP ${{ matrix.php-versions }} @ ${{ matrix.operating-system }} steps: diff --git a/.github/workflows/code-style.yaml b/.github/workflows/code-style.yaml index dadf1e6..f2ea74d 100644 --- a/.github/workflows/code-style.yaml +++ b/.github/workflows/code-style.yaml @@ -12,6 +12,7 @@ jobs: matrix: operating-system: [ubuntu-latest] php-versions: ['8.4'] + composer-options: ['--ignore-platform-req=php+'] fail-fast: false name: PHP ${{ matrix.php-versions }} @ ${{ matrix.operating-system }} steps: diff --git a/.github/workflows/stress.yaml b/.github/workflows/stress.yaml index 4141f4f..82ebbdf 100644 --- a/.github/workflows/stress.yaml +++ b/.github/workflows/stress.yaml @@ -8,6 +8,7 @@ jobs: matrix: operating-system: [ubuntu-latest] php-versions: ['8.4'] + composer-options: ['--ignore-platform-req=php+'] fail-fast: false name: PHP ${{ matrix.php-versions }} @ ${{ matrix.operating-system }} steps: diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 013c038..65df9e7 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -13,7 +13,7 @@ jobs: matrix: operating-system: [ubuntu-latest] php-versions: ['8.4'] - experimental: [false] + composer-options: ['--ignore-platform-req=php+'] fail-fast: false name: PHP ${{ matrix.php-versions }} @ ${{ matrix.operating-system }} steps: diff --git a/psalm.xml b/psalm.xml index 95604c8..8cf9405 100644 --- a/psalm.xml +++ b/psalm.xml @@ -45,6 +45,7 @@ + diff --git a/src/Xml/Dom/Collection/NodeList.php b/src/Xml/Dom/Collection/NodeList.php index fa8191e..a78ddc1 100644 --- a/src/Xml/Dom/Collection/NodeList.php +++ b/src/Xml/Dom/Collection/NodeList.php @@ -8,7 +8,7 @@ use \DOM\Element; use \DOM\Node; use \DOM\NodeList as DOMNodeList; -use DOMXpath as DOMXpath; +use \DOM\XPath as DOMXPath; use Generator; use InvalidArgumentException; use IteratorAggregate; @@ -193,7 +193,7 @@ public function reduce(callable $reducer, mixed $initial): mixed } /** - * @param list $configurators + * @param list $configurators * @throws RuntimeException * @return NodeList<\DOM\Node> */ @@ -211,7 +211,7 @@ public function query(string $xpath, callable ... $configurators): self /** * @template X - * @param list $configurators + * @param list $configurators * @param TypeInterface $type * @return list */ diff --git a/src/Xml/Dom/Locator/Node/detect_document.php b/src/Xml/Dom/Locator/Node/detect_document.php index 86a5c7a..d3a3188 100644 --- a/src/Xml/Dom/Locator/Node/detect_document.php +++ b/src/Xml/Dom/Locator/Node/detect_document.php @@ -5,16 +5,14 @@ namespace VeeWee\Xml\Dom\Locator\Node; use \DOM\XMLDocument; -use \DOM\Node; use InvalidArgumentException; -use Webmozart\Assert\Assert; +use function VeeWee\Xml\Dom\Assert\assert_document; use function VeeWee\Xml\Dom\Predicate\is_document; /** * @throws InvalidArgumentException - * @psalm-suppress RedundantCondition - node->ownerDocument can also be null... */ function detect_document(\DOM\Node $node): \DOM\XMLDocument { - return is_document($node) ? $node : $node->ownerDocument; + return is_document($node) ? $node : assert_document($node->ownerDocument); } diff --git a/src/Xml/Dom/Locator/Xsd/locate_namespaced_xsd_schemas.php b/src/Xml/Dom/Locator/Xsd/locate_namespaced_xsd_schemas.php index 47d45e4..e4b2ffe 100644 --- a/src/Xml/Dom/Locator/Xsd/locate_namespaced_xsd_schemas.php +++ b/src/Xml/Dom/Locator/Xsd/locate_namespaced_xsd_schemas.php @@ -10,6 +10,7 @@ use VeeWee\Xml\Xsd\Schema\Schema; use VeeWee\Xml\Xsd\Schema\SchemaCollection; use function Psl\Regex\split; +use function VeeWee\Xml\Dom\Locator\document_element; /** * @throws RuntimeException @@ -17,15 +18,15 @@ function locate_namespaced_xsd_schemas(\DOM\XMLDocument $document): SchemaCollection { $schemaNs = Xmlns::xsi()->value(); - $attributes = $document->documentElement->attributes; + $documentElement = document_element()($document); + $attributes = $documentElement->attributes; $collection = new SchemaCollection(); if (!$schemaLocation = $attributes->getNamedItemNS($schemaNs, 'schemaLocation')) { return $collection; } - /** @psalm-suppress MissingThrowsDocblock - Covered the runtime exception! */ - $parts = split(trim($schemaLocation->textContent), '/\s+/'); + $parts = split(trim($schemaLocation->textContent ?? ''), '/\s+/'); $partsCount = count($parts); for ($k = 0; $k < $partsCount; $k += 2) { $collection = $collection->add(Schema::withNamespace($parts[$k], $parts[$k + 1])); diff --git a/src/Xml/Dom/Locator/Xsd/locate_no_namespaced_xsd_schemas.php b/src/Xml/Dom/Locator/Xsd/locate_no_namespaced_xsd_schemas.php index 2a9600f..b03abf5 100644 --- a/src/Xml/Dom/Locator/Xsd/locate_no_namespaced_xsd_schemas.php +++ b/src/Xml/Dom/Locator/Xsd/locate_no_namespaced_xsd_schemas.php @@ -11,6 +11,7 @@ use VeeWee\Xml\Xsd\Schema\SchemaCollection; use function Psl\Dict\map; use function Psl\Regex\split; +use function VeeWee\Xml\Dom\Locator\document_element; /** * @throws RuntimeException @@ -18,13 +19,13 @@ function locate_no_namespaced_xsd_schemas(\DOM\XMLDocument $document): SchemaCollection { $schemaNs = Xmlns::xsi()->value(); - $attributes = $document->documentElement->attributes; + $documentElement = document_element()($document); + $attributes = $documentElement->attributes; if (!$schemaLocNoNamespace = $attributes->getNamedItemNS($schemaNs, 'noNamespaceSchemaLocation')) { return new SchemaCollection(); } - /** @psalm-suppress MissingThrowsDocblock - Covered the runtime exception! */ - $parts = split(trim($schemaLocNoNamespace->textContent), '/\s+/'); + $parts = split(trim($schemaLocNoNamespace->textContent ?? ''), '/\s+/'); return new SchemaCollection( ...map( diff --git a/src/Xml/Dom/Locator/document_element.php b/src/Xml/Dom/Locator/document_element.php index 821b997..ace573b 100644 --- a/src/Xml/Dom/Locator/document_element.php +++ b/src/Xml/Dom/Locator/document_element.php @@ -7,11 +7,12 @@ use Closure; use \DOM\XMLDocument; use \DOM\Element; +use function VeeWee\Xml\Dom\Assert\assert_element; /** * @return Closure(\DOM\XMLDocument): \DOM\Element */ function document_element(): Closure { - return static fn (\DOM\XMLDocument $document): \DOM\Element => $document->documentElement; + return static fn (\DOM\XMLDocument $document): \DOM\Element => assert_element($document->documentElement); } diff --git a/src/Xml/Dom/Locator/elements_with_namespaced_tagname.php b/src/Xml/Dom/Locator/elements_with_namespaced_tagname.php index 951f9a7..eefbf3e 100644 --- a/src/Xml/Dom/Locator/elements_with_namespaced_tagname.php +++ b/src/Xml/Dom/Locator/elements_with_namespaced_tagname.php @@ -20,5 +20,9 @@ function elements_with_namespaced_tagname(string $namespace, string $localTagNam * @return NodeList<\DOM\Element> */ static fn (\DOM\XMLDocument $document): NodeList - => locate_by_namespaced_tag_name($document->documentElement, $namespace, $localTagName); + => locate_by_namespaced_tag_name( + document_element()($document), + $namespace, + $localTagName + ); } diff --git a/src/Xml/Dom/Locator/elements_with_tagname.php b/src/Xml/Dom/Locator/elements_with_tagname.php index c2773fb..6159b73 100644 --- a/src/Xml/Dom/Locator/elements_with_tagname.php +++ b/src/Xml/Dom/Locator/elements_with_tagname.php @@ -20,5 +20,8 @@ function elements_with_tagname(string $tagName): Closure * @return NodeList<\DOM\Element> */ static fn (\DOM\XMLDocument $document): NodeList - => locate_by_tag_name($document->documentElement, $tagName); + => locate_by_tag_name( + document_element()($document), + $tagName + ); } diff --git a/src/Xml/Dom/Locator/root_namespace.php b/src/Xml/Dom/Locator/root_namespace.php index d182f0a..719c6bb 100644 --- a/src/Xml/Dom/Locator/root_namespace.php +++ b/src/Xml/Dom/Locator/root_namespace.php @@ -12,5 +12,5 @@ */ function root_namespace_uri(): Closure { - return static fn (\DOM\XMLDocument $document): ?string => $document->documentElement->namespaceURI; + return static fn (\DOM\XMLDocument $document): ?string => document_element()($document)->namespaceURI; } diff --git a/src/Xml/Dom/Manipulator/Attribute/rename.php b/src/Xml/Dom/Manipulator/Attribute/rename.php index e3b2ea7..cac7cbd 100644 --- a/src/Xml/Dom/Manipulator/Attribute/rename.php +++ b/src/Xml/Dom/Manipulator/Attribute/rename.php @@ -18,6 +18,9 @@ function rename(\DOM\Attr $target, string $newQName, ?string $newNamespaceURI = { return disallow_issues(static fn (): \DOM\Attr => match(true) { is_xmlns_attribute($target) => rename_xmlns_attribute($target, $newQName), - default => tap(fn () => $target->rename($newNamespaceURI, $newQName))($target) + default => (function() use ($target, $newNamespaceURI, $newQName): \DOM\Attr { + $target->rename($newNamespaceURI, $newQName); + return $target; + })() }); } diff --git a/src/Xml/Dom/Manipulator/Document/optimize_namespaces.php b/src/Xml/Dom/Manipulator/Document/optimize_namespaces.php index 1f351f2..0f1a95e 100644 --- a/src/Xml/Dom/Manipulator/Document/optimize_namespaces.php +++ b/src/Xml/Dom/Manipulator/Document/optimize_namespaces.php @@ -10,6 +10,7 @@ use function Psl\Vec\sort; use function Psl\Vec\values; use function VeeWee\Xml\Dom\Builder\xmlns_attribute; +use function VeeWee\Xml\Dom\Locator\document_element; use function VeeWee\Xml\Dom\Locator\Xmlns\recursive_linked_namespaces; use function VeeWee\Xml\Dom\Manipulator\Xmlns\rename_element_namespace; @@ -18,10 +19,10 @@ */ function optimize_namespaces(\DOM\XMLDocument $document, string $prefix = 'ns'): void { - $documentElement = $document->documentElement; + $documentElement = document_element()($document); $namespaceURIs = values(unique(map( recursive_linked_namespaces($documentElement), - static fn (\DOM\NamespaceInfo $info): string => $info->namespaceURI + static fn (\DOM\NamespaceInfo $info): string => $info->namespaceURI ?? '' ))); foreach (sort($namespaceURIs) as $index => $namespaceURI) { diff --git a/src/Xml/Dom/Manipulator/Element/copy_named_xmlns_attributes.php b/src/Xml/Dom/Manipulator/Element/copy_named_xmlns_attributes.php index b2a32f7..9eb8136 100644 --- a/src/Xml/Dom/Manipulator/Element/copy_named_xmlns_attributes.php +++ b/src/Xml/Dom/Manipulator/Element/copy_named_xmlns_attributes.php @@ -14,7 +14,7 @@ function copy_named_xmlns_attributes(\DOM\Element $target, \DOM\Element $source): void { xmlns_attributes_list($source)->forEach(static function (\DOM\Attr $xmlns) use ($target) { - if ($xmlns->prefix && !$target->hasAttribute($xmlns->nodeName)) { + if ($xmlns->prefix !== null && !$target->hasAttribute($xmlns->nodeName)) { xmlns_attribute($xmlns->localName, $xmlns->value)($target); } }); diff --git a/src/Xml/Dom/Manipulator/Element/rename.php b/src/Xml/Dom/Manipulator/Element/rename.php index 58b4aff..b220cd3 100644 --- a/src/Xml/Dom/Manipulator/Element/rename.php +++ b/src/Xml/Dom/Manipulator/Element/rename.php @@ -16,7 +16,7 @@ function rename(\DOM\Element $target, string $newQName, ?string $newNamespaceURI $newPrefix = $parts[0] ?? ''; /* - * To make sure the new namespace prefix is being used, we need to apply an additional xmlns declaration chech: + * To make sure the new namespace prefix is being used, we need to apply an additional xmlns declaration check: * This is due to a particular rule in the XML serialization spec, * that enforces that a namespaceURI on an element is only associated with exactly one prefix. * See the note of bullet point 2 of https://www.w3.org/TR/DOM-Parsing/#dfn-concept-serialize-xml. @@ -24,7 +24,7 @@ function rename(\DOM\Element $target, string $newQName, ?string $newNamespaceURI * If you rename a:xx to b:xx an xmlns:b="xx" attribute gets added at the end, but prefix a: will still be serialized. * So in this case, we need to remove the xmlns declaration first. */ - if ($newPrefix && $newPrefix !== $target->prefix && $target->hasAttribute('xmlns:'.$target->prefix)) { + if ($newPrefix && $target->prefix !== null && $newPrefix !== $target->prefix && $target->hasAttribute('xmlns:'.$target->prefix)) { $target->removeAttribute('xmlns:'.$target->prefix); } diff --git a/src/Xml/Dom/Traverser/Visitor/RemoveNamespaces.php b/src/Xml/Dom/Traverser/Visitor/RemoveNamespaces.php index 0da7c9a..adfd2b6 100644 --- a/src/Xml/Dom/Traverser/Visitor/RemoveNamespaces.php +++ b/src/Xml/Dom/Traverser/Visitor/RemoveNamespaces.php @@ -18,7 +18,7 @@ final class RemoveNamespaces extends AbstractVisitor private $filter; /** - * @param null | callable(\DOM\Attr): bool $filter + * @param null | callable(\DOM\Attr | \DOM\Element): bool $filter */ public function __construct( ?callable $filter = null @@ -81,6 +81,7 @@ public function onNodeEnter(\DOM\Node $node): Action return new Action\Noop(); } + /** @var \DOM\Element | \DOM\Attr $node */ return new Action\RenameNode($node->localName, null); } diff --git a/src/Xml/Encoding/Internal/Decoder/Builder/element.php b/src/Xml/Encoding/Internal/Decoder/Builder/element.php index d9bf991..c79047c 100644 --- a/src/Xml/Encoding/Internal/Decoder/Builder/element.php +++ b/src/Xml/Encoding/Internal/Decoder/Builder/element.php @@ -22,7 +22,7 @@ function element(\DOM\Element $element): array $namespaces = namespaces($element); if (!count($children) && !count($attributes) && !count($namespaces)) { - return [$name => $element->textContent]; + return [$name => $element->textContent ?? '']; } return [ @@ -30,7 +30,7 @@ function element(\DOM\Element $element): array merge( $namespaces, $attributes, - $children ?: ['@value' => $element->textContent] + $children ?: ['@value' => $element->textContent ?? ''] ), static fn (mixed $data): bool => $data !== [] ) diff --git a/src/Xml/Encoding/Internal/Decoder/Builder/namespaces.php b/src/Xml/Encoding/Internal/Decoder/Builder/namespaces.php index ddf5a5a..05232af 100644 --- a/src/Xml/Encoding/Internal/Decoder/Builder/namespaces.php +++ b/src/Xml/Encoding/Internal/Decoder/Builder/namespaces.php @@ -22,7 +22,7 @@ function namespaces(\DOM\Element $element): array static fn (array $namespaces, \DOM\Attr $node) => $node->value ? merge($namespaces, [ - ($node->prefix ? $node->localName : '') => $node->value + ($node->prefix !== null ? $node->localName : '') => $node->value ]) : $namespaces, [] diff --git a/stubs/DOM.phpstub b/stubs/DOM.phpstub index 80923b0..1208b34 100644 --- a/stubs/DOM.phpstub +++ b/stubs/DOM.phpstub @@ -1027,6 +1027,9 @@ namespace namespace DOM { + + use Psl\Iter\Iterator; + /** * @var int * @cvalue INDEX_SIZE_ERR @@ -1163,7 +1166,10 @@ namespace DOM public ?Element $parentElement; /** @implementation-alias DOMNode::hasChildNodes */ public function hasChildNodes(): bool {} - /** @readonly */ + /** + * @readonly + * @var NodeList + */ public NodeList $childNodes; /** @readonly */ public ?Node $firstChild; @@ -1218,7 +1224,11 @@ namespace DOM public function __wakeup(): void {} } - class NodeList implements IteratorAggregate, Countable + /** + * @template-covariant TNode as \DOM\Node + * @template-implements \IteratorAggregate + */ + class NodeList implements \IteratorAggregate, \Countable { /** @readonly */ public int $length; @@ -1226,14 +1236,23 @@ namespace DOM /** @implementation-alias DOMNodeList::count */ public function count(): int {} - /** @implementation-alias DOMNodeList::getIterator */ + /** + * @implementation-alias DOMNodeList::getIterator + * @psalm-return iterable + */ public function getIterator(): \Iterator {} - /** @implementation-alias DOMNodeList::item */ + /** + * @implementation-alias DOMNodeList::item + * @psalm-return TNode + */ public function item(int $index): ?Node {} } - class NamedNodeMap implements IteratorAggregate, Countable + /** + * @implements \IteratorAggregate + */ + class NamedNodeMap implements \IteratorAggregate, \Countable { /** @readonly */ public int $length; @@ -1248,11 +1267,14 @@ namespace DOM /** @implementation-alias DOMNamedNodeMap::count */ public function count(): int {} - /** @implementation-alias DOMNamedNodeMap::getIterator */ + /** + * @implementation-alias DOMNamedNodeMap::getIterator + * @psalm-return iterable + */ public function getIterator(): \Iterator {} } - class DTDNamedNodeMap implements IteratorAggregate, Countable + class DTDNamedNodeMap implements \IteratorAggregate, \Countable { /** @readonly */ public int $length; @@ -1271,7 +1293,10 @@ namespace DOM public function getIterator(): \Iterator {} } - class HTMLCollection implements IteratorAggregate, Countable + /** + * @implements \IteratorAggregate + */ + class HTMLCollection implements \IteratorAggregate, \Countable { /** @readonly */ public int $length; @@ -1284,7 +1309,10 @@ namespace DOM /** @implementation-alias DOMNodeList::count */ public function count(): int {} - /** @implementation-alias DOMNodeList::getIterator */ + /** + * @implementation-alias DOMNodeList::getIterator + * @psalm-return iterable + */ public function getIterator(): \Iterator {} } @@ -1528,9 +1556,7 @@ namespace DOM { /** @readonly */ public Implementation $implementation; - /** @readonly */ public string $URL; - /** @readonly */ public string $documentURI; public string $characterSet; public string $charset; diff --git a/stubs/php_xsl.stub.phpstub b/stubs/php_xsl.stub.phpstub new file mode 100644 index 0000000..c01e218 --- /dev/null +++ b/stubs/php_xsl.stub.phpstub @@ -0,0 +1,131 @@ + EOXML, ]; + yield 'only-inline-namespace' => [ + << + + + EOXML, + << + + + EOXML, + ]; + yield 'empty-namespace' => [ + << + + + EOXML, + << + + + EOXML, + ]; } }