Merge pull request #251 from stof/static_analysis

Add static analysis with phpstan
tijsverkoyen · Dec 21, 2024 · 0d72ac1 · 0d72ac1
2 parents e10791e + 9e85e72
commit 0d72ac1
Show file tree

Hide file tree

Showing 16 changed files with 146 additions and 93 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -7,6 +7,7 @@
 /.gitignore export-ignore
 /.travis.yml export-ignore
 /phpunit.xml.dist export-ignore
+/phpstan.dist.neon export-ignore
 /CHANGELOG.md export-ignore
 /README.md export-ignore
 /.github export-ignore
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -51,3 +51,18 @@ jobs:
       - name: Upload code coverage
         continue-on-error: true
         run: wget https://scrutinizer-ci.com/ocular.phar && php ocular.phar code-coverage:upload --format=php-clover coverage.clover
+
+  static_analysis:
+    name: Static analysis
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: shivammathur/setup-php@v2
+        with:
+          coverage: "none"
+          php-version: "8.3"
+          ini-file: development
+      - name: Install dependencies
+        run: composer update --ansi --no-progress
+      - name: Run phpstan
+        run: vendor/bin/phpstan analyse --ansi --no-progress
diff --git a/.gitignore b/.gitignore
@@ -1,3 +1,4 @@
 /vendor
 composer.lock
 /.phpunit.result.cache
+/phpunit.neon
diff --git a/composer.json b/composer.json
@@ -18,7 +18,9 @@
     "symfony/css-selector": "^5.4 || ^6.0 || ^7.0"
   },
   "require-dev": {
-    "phpunit/phpunit": "^8.5.21 || ^9.5.10"
+    "phpunit/phpunit": "^8.5.21 || ^9.5.10",
+    "phpstan/phpstan": "^2.0",
+    "phpstan/phpstan-phpunit": "^2.0"
   },
   "autoload": {
     "psr-4": {

diff --git a/phpstan.dist.neon b/phpstan.dist.neon
@@ -0,0 +1,11 @@
+parameters:
+    level: 10
+    treatPhpDocTypesAsCertain: false
+    paths:
+        - ./src/
+        - ./tests/
+
+includes:
+    - vendor/phpstan/phpstan/conf/bleedingEdge.neon
+    - vendor/phpstan/phpstan-phpunit/extension.neon
+    - vendor/phpstan/phpstan-phpunit/rules.neon
diff --git a/src/Css/Processor.php b/src/Css/Processor.php
@@ -35,7 +35,7 @@ public function getCssFromStyleTags($html)
     {
         $css = '';
         $matches = array();
-        $htmlNoComments = preg_replace('|<!--.*?-->|s', '', $html);
+        $htmlNoComments = preg_replace('|<!--.*?-->|s', '', $html) ?? $html;
         preg_match_all('|<style(?:\s.*)?>(.*)</style>|isU', $htmlNoComments, $matches);
 
         if (!empty($matches[1])) {
@@ -55,15 +55,15 @@ public function getCssFromStyleTags($html)
     private function doCleanup($css)
     {
         // remove charset
-        $css = preg_replace('/@charset "[^"]++";/', '', $css);
+        $css = preg_replace('/@charset "[^"]++";/', '', $css) ?? $css;
         // remove media queries
-        $css = preg_replace('/@media [^{]*+{([^{}]++|{[^{}]*+})*+}/', '', $css);
+        $css = preg_replace('/@media [^{]*+{([^{}]++|{[^{}]*+})*+}/', '', $css) ?? $css;
 
         $css = str_replace(array("\r", "\n"), '', $css);
         $css = str_replace(array("\t"), ' ', $css);
         $css = str_replace('"', '\'', $css);
-        $css = preg_replace('|/\*.*?\*/|', '', $css);
-        $css = preg_replace('/\s\s++/', ' ', $css);
+        $css = preg_replace('|/\*.*?\*/|', '', $css) ?? $css;
+        $css = preg_replace('/\s\s++/', ' ', $css) ?? $css;
         $css = trim($css);
 
         return $css;

diff --git a/src/Css/Property/Processor.php b/src/Css/Property/Processor.php
@@ -50,8 +50,8 @@ private function cleanup($string)
         $string = str_replace(array("\r", "\n"), '', $string);
         $string = str_replace(array("\t"), ' ', $string);
         $string = str_replace('"', '\'', $string);
-        $string = preg_replace('|/\*.*?\*/|', '', $string);
-        $string = preg_replace('/\s\s+/', ' ', $string);
+        $string = preg_replace('|/\*.*?\*/|', '', $string) ?? $string;
+        $string = preg_replace('/\s\s+/', ' ', $string) ?? $string;
 
         $string = trim($string);
         $string = rtrim($string, ';');

diff --git a/src/Css/Property/Property.php b/src/Css/Property/Property.php
@@ -17,7 +17,7 @@ final class Property
     private $value;
 
     /**
-     * @var Specificity
+     * @var Specificity|null
      */
     private $originalSpecificity;
 
@@ -57,7 +57,7 @@ public function getValue()
     /**
      * Get originalSpecificity
      *
-     * @return Specificity
+     * @return Specificity|null
      */
     public function getOriginalSpecificity()
     {

diff --git a/src/Css/Rule/Processor.php b/src/Css/Rule/Processor.php
@@ -31,8 +31,8 @@ private function cleanup($string)
         $string = str_replace(array("\r", "\n"), '', $string);
         $string = str_replace(array("\t"), ' ', $string);
         $string = str_replace('"', '\'', $string);
-        $string = preg_replace('|/\*.*?\*/|', '', $string);
-        $string = preg_replace('/\s\s+/', ' ', $string);
+        $string = preg_replace('|/\*.*?\*/|', '', $string) ?? $string;
+        $string = preg_replace('/\s\s+/', ' ', $string) ?? $string;
 
         $string = trim($string);
         $string = rtrim($string, '}');
@@ -88,7 +88,7 @@ public function convertToObjects($rule, $originalOrder)
      */
     public function calculateSpecificityBasedOnASelector($selector)
     {
-        $idSelectorsPattern = "  \#";
+        $idSelectorCount = preg_match_all("/  \#/ix", $selector, $matches);
         $classAttributesPseudoClassesSelectorsPattern = "  (\.[\w]+)                     # classes
                         |
                         \[(\w+)                       # attributes
@@ -105,6 +105,7 @@ public function calculateSpecificityBasedOnASelector($selector)
                           |only-child|only-of-type
                           |empty|contains
                         ))";
+        $classAttributesPseudoClassesSelectorCount = preg_match_all("/{$classAttributesPseudoClassesSelectorsPattern}/ix", $selector, $matches);
 
         $typePseudoElementsSelectorPattern = "  ((^|[\s\+\>\~]+)[\w]+       # elements
                         |
@@ -114,11 +115,16 @@ public function calculateSpecificityBasedOnASelector($selector)
                           |selection
                         )
                       )";
+        $typePseudoElementsSelectorCount = preg_match_all("/{$typePseudoElementsSelectorPattern}/ix", $selector, $matches);
+
+        if ($idSelectorCount === false || $classAttributesPseudoClassesSelectorCount === false || $typePseudoElementsSelectorCount === false) {
+            throw new \RuntimeException('Failed to calculate specificity based on selector.');
+        }
 
         return new Specificity(
-            preg_match_all("/{$idSelectorsPattern}/ix", $selector, $matches),
-            preg_match_all("/{$classAttributesPseudoClassesSelectorsPattern}/ix", $selector, $matches),
-            preg_match_all("/{$typePseudoElementsSelectorPattern}/ix", $selector, $matches)
+            $idSelectorCount,
+            $classAttributesPseudoClassesSelectorCount,
+            $typePseudoElementsSelectorCount
         );
     }
 

diff --git a/src/CssToInlineStyles.php b/src/CssToInlineStyles.php
@@ -6,6 +6,7 @@
 use Symfony\Component\CssSelector\Exception\ExceptionInterface;
 use TijsVerkoyen\CssToInlineStyles\Css\Processor;
 use TijsVerkoyen\CssToInlineStyles\Css\Property\Processor as PropertyProcessor;
+use TijsVerkoyen\CssToInlineStyles\Css\Property\Property;
 use TijsVerkoyen\CssToInlineStyles\Css\Rule\Processor as RuleProcessor;
 
 class CssToInlineStyles
@@ -51,10 +52,10 @@ public function convert($html, $css = null)
     }
 
     /**
-     * Inline the given properties on an given DOMElement
+     * Inline the given properties on a given DOMElement
      *
      * @param \DOMElement             $element
-     * @param Css\Property\Property[] $properties
+     * @param Property[] $properties
      *
      * @return \DOMElement
      */
@@ -91,7 +92,7 @@ public function inlineCssOnElement(\DOMElement $element, array $properties)
      *
      * @param \DOMElement $element
      *
-     * @return Css\Property\Property[]
+     * @return Property[]
      */
     public function getInlineStyles(\DOMElement $element)
     {
@@ -130,12 +131,25 @@ protected function getHtmlFromDocument(\DOMDocument $document)
         // retrieve the document element
         // we do it this way to preserve the utf-8 encoding
         $htmlElement = $document->documentElement;
+
+        if ($htmlElement === null) {
+            throw new \RuntimeException('Failed to get HTML from empty document.');
+        }
+
         $html = $document->saveHTML($htmlElement);
+
+        if ($html === false) {
+            throw new \RuntimeException('Failed to get HTML from document.');
+        }
+
         $html = trim($html);
 
         // retrieve the doctype
         $document->removeChild($htmlElement);
         $doctype = $document->saveHTML();
+        if ($doctype === false) {
+            $doctype = '';
+        }
         $doctype = trim($doctype);
 
         // if it is the html5 doctype convert it to lowercase
@@ -158,6 +172,7 @@ protected function inline(\DOMDocument $document, array $rules)
             return $document;
         }
 
+        /** @var \SplObjectStorage<\DOMElement, array<string, Property>> $propertyStorage */
         $propertyStorage = new \SplObjectStorage();
 
         $xPath = new \DOMXPath($document);
@@ -178,6 +193,7 @@ protected function inline(\DOMDocument $document, array $rules)
             }
 
             foreach ($elements as $element) {
+                \assert($element instanceof \DOMElement);
                 $propertyStorage[$element] = $this->calculatePropertiesToBeApplied(
                     $rule->getProperties(),
                     $propertyStorage->contains($element) ? $propertyStorage[$element] : array()
@@ -195,12 +211,12 @@ protected function inline(\DOMDocument $document, array $rules)
     /**
      * Merge the CSS rules to determine the applied properties.
      *
-     * @param Css\Property\Property[] $properties
-     * @param Css\Property\Property[] $cssProperties existing applied properties indexed by name
+     * @param Property[] $properties
+     * @param array<string, Property> $cssProperties existing applied properties indexed by name
      *
-     * @return Css\Property\Property[] updated properties, indexed by name
+     * @return array<string, Property> updated properties, indexed by name
      */
-    private function calculatePropertiesToBeApplied(array $properties, array $cssProperties)
+    private function calculatePropertiesToBeApplied(array $properties, array $cssProperties): array
     {
         if (empty($properties)) {
             return $cssProperties;
@@ -218,6 +234,8 @@ private function calculatePropertiesToBeApplied(array $properties, array $cssPro
                 //overrule if current property is important and existing is not, else check specificity
                 $overrule = !$existingProperty->isImportant() && $property->isImportant();
                 if (!$overrule) {
+                    \assert($existingProperty->getOriginalSpecificity() !== null, 'Properties created for parsed CSS always have their associated specificity.');
+                    \assert($property->getOriginalSpecificity() !== null, 'Properties created for parsed CSS always have their associated specificity.');
                     $overrule = $existingProperty->getOriginalSpecificity()->compareTo($property->getOriginalSpecificity()) <= 0;
                 }
 

diff --git a/tests/Css/ProcessorTest.php b/tests/Css/ProcessorTest.php
@@ -15,12 +15,12 @@ class ProcessorTest extends TestCase
     /**
      * @before
      */
-    protected function prepare()
+    protected function prepare(): void
     {
         $this->processor = new Processor();
     }
 
-    public function testCssWithOneRule()
+    public function testCssWithOneRule(): void
     {
         $css = <<<EOF
             a {
@@ -42,7 +42,7 @@ public function testCssWithOneRule()
         $this->assertEquals(1, $rules[0]->getOrder());
     }
 
-    public function testCssWithComments()
+    public function testCssWithComments(): void
     {
         $css = <<<CSS
 a {
@@ -67,7 +67,7 @@ public function testCssWithComments()
         $this->assertEquals(2, $rules[1]->getOrder());
     }
 
-    public function testCssWithMediaQueries()
+    public function testCssWithMediaQueries(): void
     {
         $css = <<<EOF
 @media (max-width: 600px) {
@@ -92,14 +92,16 @@ public function testCssWithMediaQueries()
         $this->assertEquals(1, $rules[0]->getOrder());
     }
 
-    public function testCssWithBigMediaQueries()
+    public function testCssWithBigMediaQueries(): void
     {
-        $rules = $this->processor->getRules(file_get_contents(__DIR__.'/test.css'));
+        $css = file_get_contents(__DIR__.'/test.css');
+        $this->assertIsString($css, 'The fixture CSS file should be readable.');
+        $rules = $this->processor->getRules($css);
 
         $this->assertCount(414, $rules);
     }
 
-    public function testMakeSureMediaQueriesAreRemoved()
+    public function testMakeSureMediaQueriesAreRemoved(): void
     {
         $css = '@media tv and (min-width: 700px) and (orientation: landscape) {.foo {display: none;}}';
         $this->assertEmpty($this->processor->getRules($css));
@@ -117,7 +119,7 @@ public function testMakeSureMediaQueriesAreRemoved()
         $this->assertEmpty($this->processor->getRules($css));
     }
 
-    public function testCssWithCharset()
+    public function testCssWithCharset(): void
     {
         $css = <<<EOF
 @charset "UTF-8";
@@ -137,7 +139,7 @@ public function testCssWithCharset()
         $this->assertEquals(1, $rules[0]->getOrder());
     }
 
-    public function testSimpleStyleTagsInHtml()
+    public function testSimpleStyleTagsInHtml(): void
     {
         $expected = 'p { color: #F00; }' . "\n";
         $this->assertEquals(
@@ -159,7 +161,7 @@ public function testSimpleStyleTagsInHtml()
         );
     }
 
-    public function testStyleTagsWithAttributeInHtml()
+    public function testStyleTagsWithAttributeInHtml(): void
     {
         $expected = 'p { color: #F00; }' . "\n";
         $this->assertEquals(
@@ -181,7 +183,7 @@ public function testStyleTagsWithAttributeInHtml()
         );
     }
 
-    public function testMultipleStyleTagsInHtml()
+    public function testMultipleStyleTagsInHtml(): void
     {
         $expected = 'p { color: #F00; }' . "\n" . 'p { color: #0F0; }' . "\n";
         $this->assertEquals(
@@ -206,7 +208,7 @@ public function testMultipleStyleTagsInHtml()
         );
     }
 
-    public function testWeirdTagsInHtml()
+    public function testWeirdTagsInHtml(): void
     {
         $expected = 'p { color: #F00; }' . "\n";
         $this->assertEquals(
@@ -231,7 +233,7 @@ public function testWeirdTagsInHtml()
 
     }
 
-    public function testStyleTagsInCommentInHtml()
+    public function testStyleTagsInCommentInHtml(): void
     {
         $expected = 'p { color: #F00; }' . "\n";
         $this->assertEquals(