diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 77b3aa5..9b0e461 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -30,6 +30,36 @@ jobs: - name: Run PHPCSFixer run: php-cs-fixer fix --dry-run --diff + phpstan: + name: PHP Static Analysis + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + php: + - '7.1' + - '7.2' + - '7.3' + - '7.4' + - '8.0' + - '8.1' + - '8.2' + - '8.3' + steps: + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + extensions: xml + + - uses: actions/checkout@v2 + + - name: Composer Install + run: composer install --ansi --prefer-dist --no-interaction --no-progress + + - name: Run phpstan + run: ./vendor/bin/phpstan analyse -c phpstan.neon.dist + phpunit: name: PHPUnit runs-on: ubuntu-latest diff --git a/composer.json b/composer.json index 7637fb8..9532352 100644 --- a/composer.json +++ b/composer.json @@ -22,6 +22,7 @@ "friendsofphp/php-cs-fixer": "^2.1" }, "require-dev": { - "phpunit/phpunit": ">=7.0" + "phpunit/phpunit": ">=7.0", + "phpstan/phpstan": "^0.12.88 || ^1.0.0" } } diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..9521cfa --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,12 @@ +parameters: + level: 7 + bootstrapFiles: + - vendor/autoload.php + paths: + - src + - tests + reportUnmatchedIgnoredErrors: false + ignoreErrors: + + ## Remove after remove ArrayObject + treatPhpDocTypesAsCertain: false \ No newline at end of file diff --git a/src/Math/Element/AbstractElement.php b/src/Math/Element/AbstractElement.php index be7d4a8..3888de9 100644 --- a/src/Math/Element/AbstractElement.php +++ b/src/Math/Element/AbstractElement.php @@ -2,13 +2,18 @@ namespace PhpOffice\Math\Element; +use PhpOffice\Math\Math; + abstract class AbstractElement { /** - * @var string + * @var Math|AbstractGroupElement|null */ protected $parent; + /** + * @param Math|AbstractGroupElement|null $parent + */ public function setParent($parent): self { $this->parent = $parent; @@ -16,6 +21,9 @@ public function setParent($parent): self return $this; } + /** + * @return Math|AbstractGroupElement|null + */ public function getParent() { return $this->parent; diff --git a/src/Math/Math.php b/src/Math/Math.php index 962d628..0eaeb2b 100644 --- a/src/Math/Math.php +++ b/src/Math/Math.php @@ -2,6 +2,8 @@ namespace PhpOffice\Math; +use PhpOffice\Math\Element\AbstractElement; + class Math { /** @@ -32,7 +34,7 @@ public function remove(Element\AbstractElement $element): self $this->elements = array_filter($this->elements, function ($child) use ($element) { return $child != $element; }); - $component->setParent(null); + $element->setParent(null); return $this; } diff --git a/src/Math/Reader/MathML.php b/src/Math/Reader/MathML.php index eaa348f..8a370d9 100644 --- a/src/Math/Reader/MathML.php +++ b/src/Math/Reader/MathML.php @@ -4,6 +4,7 @@ use DOMDocument; use DOMElement; +use DOMNode; use DOMXPath; use Exception; use PhpOffice\Math\Element; @@ -41,10 +42,13 @@ public function read(string $content): ?Math return $this->math; } - protected function parseNode(?DOMElement $nodeRowElement, $parent): void + /** + * @param Math|Element\AbstractGroupElement $parent + */ + protected function parseNode(?DOMNode $nodeRowElement, $parent): void { $this->xpath = new DOMXpath($this->dom); - foreach ($this->xpath->query('*', $nodeRowElement) as $nodeElement) { + foreach ($this->xpath->query('*', $nodeRowElement) ?: [] as $nodeElement) { $element = $this->getElement($nodeElement); $parent->add($element); @@ -54,14 +58,14 @@ protected function parseNode(?DOMElement $nodeRowElement, $parent): void } } - protected function getElement(DOMElement $nodeElement): Element\AbstractElement + protected function getElement(DOMNode $nodeElement): Element\AbstractElement { $nodeValue = trim($nodeElement->nodeValue); switch ($nodeElement->nodeName) { case 'mfrac': $element = new Element\Fraction(); $nodeList = $this->xpath->query('*', $nodeElement); - if ($nodeList->length == 2) { + if ($nodeList && $nodeList->length == 2) { $element ->setNumerator($this->getElement($nodeList->item(0))) ->setDenominator($this->getElement($nodeList->item(1))); @@ -71,13 +75,15 @@ protected function getElement(DOMElement $nodeElement): Element\AbstractElement case 'mi': return new Element\Identifier($nodeValue); case 'mn': - return new Element\Numeric($nodeValue); + return new Element\Numeric(floatval($nodeValue)); case 'mo': if (empty($nodeValue)) { $nodeList = $this->xpath->query('*', $nodeElement); if ( - $nodeList->length == 1 + $nodeList + && $nodeList->length == 1 && $nodeList->item(0)->nodeName == 'mchar' + && $nodeList->item(0) instanceof DOMElement && $nodeList->item(0)->hasAttribute('name') ) { $nodeValue = $nodeList->item(0)->getAttribute('name'); @@ -90,7 +96,7 @@ protected function getElement(DOMElement $nodeElement): Element\AbstractElement case 'msup': $element = new Element\Superscript(); $nodeList = $this->xpath->query('*', $nodeElement); - if ($nodeList->length == 2) { + if ($nodeList && $nodeList->length == 2) { $element ->setBase($this->getElement($nodeList->item(0))) ->setSuperscript($this->getElement($nodeList->item(1))); diff --git a/src/Math/Reader/OfficeMathML.php b/src/Math/Reader/OfficeMathML.php index a6a7ee6..0f46874 100644 --- a/src/Math/Reader/OfficeMathML.php +++ b/src/Math/Reader/OfficeMathML.php @@ -3,7 +3,7 @@ namespace PhpOffice\Math\Reader; use DOMDocument; -use DOMElement; +use DOMNode; use DOMXPath; use Exception; use PhpOffice\Math\Element; @@ -11,12 +11,12 @@ class OfficeMathML implements ReaderInterface { + /** @var DOMDocument */ + protected $dom; + /** @var Math */ protected $math; - /** @var XMLReader */ - protected $xmlReader; - /** @var DOMXpath */ protected $xpath; @@ -46,11 +46,13 @@ public function read(string $content): ?Math /** * @see https://devblogs.microsoft.com/math-in-office/officemath/ * @see https://learn.microsoft.com/fr-fr/archive/blogs/murrays/mathml-and-ecma-math-omml + * + * @param Math|Element\AbstractGroupElement $parent */ - protected function parseNode(?DOMElement $nodeRowElement, $parent): void + protected function parseNode(?DOMNode $nodeRowElement, $parent): void { $this->xpath = new DOMXpath($this->dom); - foreach ($this->xpath->query('*', $nodeRowElement) as $nodeElement) { + foreach ($this->xpath->query('*', $nodeRowElement) ?: [] as $nodeElement) { $element = $this->getElement($nodeElement); $parent->add($element); @@ -60,27 +62,27 @@ protected function parseNode(?DOMElement $nodeRowElement, $parent): void } } - protected function getElement(DOMElement $nodeElement): Element\AbstractElement + protected function getElement(DOMNode $nodeElement): Element\AbstractElement { switch ($nodeElement->nodeName) { case 'm:f': $element = new Element\Fraction(); // Numerator $nodeNumerator = $this->xpath->query('m:num/m:r/m:t', $nodeElement); - if ($nodeNumerator->length == 1) { + if ($nodeNumerator && $nodeNumerator->length == 1) { $value = $nodeNumerator->item(0)->nodeValue; if (is_numeric($value)) { - $element->setNumerator(new Element\Numeric($value)); + $element->setNumerator(new Element\Numeric(floatval($value))); } else { $element->setNumerator(new Element\Identifier($value)); } } // Denominator $nodeDenominator = $this->xpath->query('m:den/m:r/m:t', $nodeElement); - if ($nodeDenominator->length == 1) { + if ($nodeDenominator && $nodeDenominator->length == 1) { $value = $nodeDenominator->item(0)->nodeValue; if (is_numeric($value)) { - $element->setDenominator(new Element\Numeric($value)); + $element->setDenominator(new Element\Numeric(floatval($value))); } else { $element->setDenominator(new Element\Identifier($value)); } @@ -89,18 +91,19 @@ protected function getElement(DOMElement $nodeElement): Element\AbstractElement return $element; case 'm:r': $nodeText = $this->xpath->query('m:t', $nodeElement); - if ($nodeText->length == 1) { + if ($nodeText && $nodeText->length == 1) { $value = trim($nodeText->item(0)->nodeValue); if (in_array($value, $this->operators)) { return new Element\Operator($value); } if (is_numeric($value)) { - return new Element\Numeric($value); + return new Element\Numeric(floatval($value)); } return new Element\Identifier($value); } - break; + + return new Element\Identifier(''); case 'm:oMath': return new Element\Row(); default: diff --git a/src/Math/Writer/MathML.php b/src/Math/Writer/MathML.php index 374131b..76d1a64 100644 --- a/src/Math/Writer/MathML.php +++ b/src/Math/Writer/MathML.php @@ -69,10 +69,21 @@ protected function writeElementItem(Element\AbstractElement $element): void return; } - // Element\AbstractElement - $this->output->startElement($this->getElementTagName($element)); - $this->output->text((string) $element->getValue()); - $this->output->endElement(); + if ($element instanceof Element\Identifier + || $element instanceof Element\Numeric + || $element instanceof Element\Operator) { + $this->output->startElement($this->getElementTagName($element)); + $this->output->text((string) $element->getValue()); + $this->output->endElement(); + + return; + } + + throw new Exception(sprintf( + '%s : The class `%s` is not implemented', + __METHOD__, + get_class($element) + )); } protected function getElementTagName(Element\AbstractElement $element): string diff --git a/src/Math/Writer/OfficeMathML.php b/src/Math/Writer/OfficeMathML.php index 3d986d4..11153ab 100644 --- a/src/Math/Writer/OfficeMathML.php +++ b/src/Math/Writer/OfficeMathML.php @@ -62,12 +62,23 @@ protected function writeElementItem(Element\AbstractElement $element): void return; } - // Element\AbstractElement - $this->output->startElement('m:r'); - $this->output->startElement('m:t'); - $this->output->text((string) $element->getValue()); - $this->output->endElement(); - $this->output->endElement(); + if ($element instanceof Element\Identifier + || $element instanceof Element\Numeric + || $element instanceof Element\Operator) { + $this->output->startElement('m:r'); + $this->output->startElement('m:t'); + $this->output->text((string) $element->getValue()); + $this->output->endElement(); + $this->output->endElement(); + + return; + } + + throw new Exception(sprintf( + '%s : The class `%s` is not implemented', + __METHOD__, + get_class($element) + )); } protected function getElementTagName(Element\AbstractElement $element): string diff --git a/tests/Math/Reader/MathMLTest.php b/tests/Math/Reader/MathMLTest.php index f0dc326..617a746 100644 --- a/tests/Math/Reader/MathMLTest.php +++ b/tests/Math/Reader/MathMLTest.php @@ -34,39 +34,64 @@ public function testReadBasic(): void $this->assertCount(1, $elements); $this->assertInstanceOf(Element\Row::class, $elements[0]); + /** @var Element\Row $element */ $element = $elements[0]; $subElements = $element->getElements(); $this->assertCount(9, $subElements); - $this->assertInstanceOf(Element\Identifier::class, $subElements[0]); - $this->assertEquals('a', $subElements[0]->getValue()); + /** @var Element\Identifier $subElement */ + $subElement = $subElements[0]; + $this->assertInstanceOf(Element\Identifier::class, $subElement); + $this->assertEquals('a', $subElement->getValue()); - $this->assertInstanceOf(Element\Operator::class, $subElements[1]); - $this->assertEquals('InvisibleTimes', $subElements[1]->getValue()); + /** @var Element\Identifier $subElement */ + $subElement = $subElements[1]; + $this->assertInstanceOf(Element\Operator::class, $subElement); + $this->assertEquals('InvisibleTimes', $subElement->getValue()); + /** @var Element\Superscript $subElement */ + $subElement = $subElements[2]; $this->assertInstanceOf(Element\Superscript::class, $subElements[2]); - $this->assertInstanceOf(Element\Identifier::class, $subElements[2]->getBase()); - $this->assertEquals('x', $subElements[2]->getBase()->getValue()); - $this->assertInstanceOf(Element\Numeric::class, $subElements[2]->getSuperscript()); - $this->assertEquals(2, $subElements[2]->getSuperscript()->getValue()); - $this->assertInstanceOf(Element\Operator::class, $subElements[3]); - $this->assertEquals('+', $subElements[3]->getValue()); - - $this->assertInstanceOf(Element\Identifier::class, $subElements[4]); - $this->assertEquals('b', $subElements[4]->getValue()); - - $this->assertInstanceOf(Element\Operator::class, $subElements[5]); - $this->assertEquals('InvisibleTimes', $subElements[5]->getValue()); - - $this->assertInstanceOf(Element\Identifier::class, $subElements[6]); - $this->assertEquals('x', $subElements[6]->getValue()); - - $this->assertInstanceOf(Element\Operator::class, $subElements[7]); - $this->assertEquals('+', $subElements[7]->getValue()); - - $this->assertInstanceOf(Element\Identifier::class, $subElements[8]); - $this->assertEquals('c', $subElements[8]->getValue()); + /** @var Element\Identifier $base */ + $base = $subElement->getBase(); + $this->assertInstanceOf(Element\Identifier::class, $base); + $this->assertEquals('x', $base->getValue()); + + /** @var Element\Numeric $superscript */ + $superscript = $subElement->getSuperscript(); + $this->assertInstanceOf(Element\Numeric::class, $superscript); + $this->assertEquals(2, $superscript->getValue()); + + /** @var Element\Operator $subElement */ + $subElement = $subElements[3]; + $this->assertInstanceOf(Element\Operator::class, $subElement); + $this->assertEquals('+', $subElement->getValue()); + + /** @var Element\Identifier $subElement */ + $subElement = $subElements[4]; + $this->assertInstanceOf(Element\Identifier::class, $subElement); + $this->assertEquals('b', $subElement->getValue()); + + /** @var Element\Operator $subElement */ + $subElement = $subElements[5]; + $this->assertInstanceOf(Element\Operator::class, $subElement); + $this->assertEquals('InvisibleTimes', $subElement->getValue()); + + /** @var Element\Identifier $subElement */ + $subElement = $subElements[6]; + $this->assertInstanceOf(Element\Identifier::class, $subElement); + $this->assertEquals('x', $subElement->getValue()); + + /** @var Element\Operator $subElement */ + $subElement = $subElements[7]; + $this->assertInstanceOf(Element\Operator::class, $subElement); + $this->assertEquals('+', $subElement->getValue()); + + /** @var Element\Identifier $subElement */ + $subElement = $subElements[8]; + $this->assertInstanceOf(Element\Identifier::class, $subElement); + $this->assertEquals('c', $subElement->getValue()); } /** @@ -97,20 +122,33 @@ public function testReadFraction(): void $this->assertCount(1, $elements); $this->assertInstanceOf(Element\Fraction::class, $elements[0]); + /** @var Element\Fraction $element */ $element = $elements[0]; $this->assertInstanceOf(Element\Fraction::class, $element->getNumerator()); + /** @var Element\Fraction $subElement */ $subElement = $element->getNumerator(); - $this->assertInstanceOf(Element\Identifier::class, $subElement->getNumerator()); - $this->assertEquals('a', $subElement->getNumerator()->getValue()); - $this->assertInstanceOf(Element\Identifier::class, $subElement->getDenominator()); - $this->assertEquals('b', $subElement->getDenominator()->getValue()); + + /** @var Element\Identifier $numerator */ + $numerator = $subElement->getNumerator(); + $this->assertInstanceOf(Element\Identifier::class, $numerator); + $this->assertEquals('a', $numerator->getValue()); + /** @var Element\Identifier $denominator */ + $denominator = $subElement->getDenominator(); + $this->assertInstanceOf(Element\Identifier::class, $denominator); + $this->assertEquals('b', $denominator->getValue()); $this->assertInstanceOf(Element\Fraction::class, $element->getDenominator()); + /** @var Element\Fraction $subElement */ $subElement = $element->getDenominator(); - $this->assertInstanceOf(Element\Identifier::class, $subElement->getNumerator()); - $this->assertEquals('c', $subElement->getNumerator()->getValue()); - $this->assertInstanceOf(Element\Identifier::class, $subElement->getDenominator()); - $this->assertEquals('d', $subElement->getDenominator()->getValue()); + + /** @var Element\Identifier $numerator */ + $numerator = $subElement->getNumerator(); + $this->assertInstanceOf(Element\Identifier::class, $numerator); + $this->assertEquals('c', $numerator->getValue()); + /** @var Element\Identifier $denominator */ + $denominator = $subElement->getDenominator(); + $this->assertInstanceOf(Element\Identifier::class, $denominator); + $this->assertEquals('d', $denominator->getValue()); } } diff --git a/tests/Math/Reader/OfficeMathMLTest.php b/tests/Math/Reader/OfficeMathMLTest.php index 0d17dd1..c88bf60 100644 --- a/tests/Math/Reader/OfficeMathMLTest.php +++ b/tests/Math/Reader/OfficeMathMLTest.php @@ -33,15 +33,24 @@ public function testRead(): void $this->assertCount(1, $elements); $this->assertInstanceOf(Element\Row::class, $elements[0]); - $subElements = $elements[0]->getElements(); + /** @var Element\Row $element */ + $element = $elements[0]; + $subElements = $element->getElements(); $this->assertCount(1, $subElements); $this->assertInstanceOf(Element\Fraction::class, $subElements[0]); - $this->assertInstanceOf(Element\Identifier::class, $subElements[0]->getNumerator()); - $this->assertEquals('π', $subElements[0]->getNumerator()->getValue()); + /** @var Element\Fraction $subElement */ + $subElement = $subElements[0]; - $this->assertInstanceOf(Element\Numeric::class, $subElements[0]->getDenominator()); - $this->assertEquals(2, $subElements[0]->getDenominator()->getValue()); + /** @var Element\Identifier $numerator */ + $numerator = $subElement->getNumerator(); + $this->assertInstanceOf(Element\Identifier::class, $numerator); + $this->assertEquals('π', $numerator->getValue()); + + /** @var Element\Numeric $denominator */ + $denominator = $subElement->getDenominator(); + $this->assertInstanceOf(Element\Numeric::class, $denominator); + $this->assertEquals(2, $denominator->getValue()); } /** @@ -89,22 +98,36 @@ public function testReadWithWTag(): void $elements = $math->getElements(); $this->assertCount(5, $elements); - $this->assertInstanceOf(Element\Fraction::class, $elements[0]); - $this->assertInstanceOf(Element\Identifier::class, $elements[0]->getNumerator()); - $this->assertEquals('π', $elements[0]->getNumerator()->getValue()); - $this->assertInstanceOf(Element\Numeric::class, $elements[0]->getDenominator()); - $this->assertEquals(2, $elements[0]->getDenominator()->getValue()); - - $this->assertInstanceOf(Element\Operator::class, $elements[1]); - $this->assertEquals('+', $elements[1]->getValue()); - - $this->assertInstanceOf(Element\Identifier::class, $elements[2]); - $this->assertEquals('a', $elements[2]->getValue()); - - $this->assertInstanceOf(Element\Operator::class, $elements[3]); - $this->assertEquals('∗', $elements[3]->getValue()); - - $this->assertInstanceOf(Element\Numeric::class, $elements[4]); - $this->assertEquals(2, $elements[4]->getValue()); + /** @var Element\Fraction $element */ + $element = $elements[0]; + $this->assertInstanceOf(Element\Fraction::class, $element); + /** @var Element\Identifier $numerator */ + $numerator = $element->getNumerator(); + $this->assertInstanceOf(Element\Identifier::class, $numerator); + $this->assertEquals('π', $numerator->getValue()); + /** @var Element\Numeric $denominator */ + $denominator = $element->getDenominator(); + $this->assertInstanceOf(Element\Numeric::class, $denominator); + $this->assertEquals(2, $denominator->getValue()); + + /** @var Element\Operator $element */ + $element = $elements[1]; + $this->assertInstanceOf(Element\Operator::class, $element); + $this->assertEquals('+', $element->getValue()); + + /** @var Element\Identifier $element */ + $element = $elements[2]; + $this->assertInstanceOf(Element\Identifier::class, $element); + $this->assertEquals('a', $element->getValue()); + + /** @var Element\Operator $element */ + $element = $elements[3]; + $this->assertInstanceOf(Element\Operator::class, $element); + $this->assertEquals('∗', $element->getValue()); + + /** @var Element\Numeric $element */ + $element = $elements[4]; + $this->assertInstanceOf(Element\Numeric::class, $element); + $this->assertEquals(2, $element->getValue()); } } diff --git a/tests/Math/Writer/WriterTestCase.php b/tests/Math/Writer/WriterTestCase.php index 778fe26..482019f 100644 --- a/tests/Math/Writer/WriterTestCase.php +++ b/tests/Math/Writer/WriterTestCase.php @@ -5,6 +5,7 @@ namespace Tests\PhpOffice\Math\Writer; use DOMDocument; +use LibXMLError; use PHPUnit\Framework\TestCase; class WriterTestCase extends TestCase @@ -15,21 +16,27 @@ public function assertIsSchemaMathMLValid(string $content): void $dom->loadXML($content); $xmlSource = $dom->saveXML(); - $dom->loadXML($xmlSource); - $dom->schemaValidate(dirname(__DIR__, 2) . '/resources/schema/mathml3/mathml3.xsd'); + if (is_string($xmlSource)) { + $dom->loadXML($xmlSource); + $dom->schemaValidate(dirname(__DIR__, 2) . '/resources/schema/mathml3/mathml3.xsd'); - $error = libxml_get_last_error(); - if ($error instanceof LibXMLError) { - $this->failXmlError($error, $fileName, $xmlSource); - } else { - $this->assertTrue(true); + $error = libxml_get_last_error(); + if ($error instanceof LibXMLError) { + $this->failXmlError($error, $xmlSource); + } else { + $this->assertTrue(true); + } + + return; } + + $this->fail(sprintf('The XML is not valid : %s', $content)); } /** * @param array $params */ - protected function failXmlError(LibXMLError $error, string $fileName, string $source, array $params = []): void + protected function failXmlError(LibXMLError $error, string $source, array $params = []): void { switch ($error->level) { case LIBXML_ERR_WARNING: @@ -65,9 +72,8 @@ protected function failXmlError(LibXMLError $error, string $fileName, string $so } } $this->fail(sprintf( - "Validation %s :\n - File : %s\n - Line : %s\n - Message : %s - Lines :\n%s%s", + "Validation %s :\n - - Line : %s\n - Message : %s - Lines :\n%s%s", $errorType, - $fileName, $error->line, $error->message, implode(PHP_EOL, $lines),