Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add static analysis with phpstan #251

Merged
merged 12 commits into from
Dec 21, 2024
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/vendor
composer.lock
/.phpunit.result.cache
/phpunit.neon
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
11 changes: 11 additions & 0 deletions phpstan.dist.neon
Original file line number Diff line number Diff line change
@@ -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
10 changes: 5 additions & 5 deletions src/Css/Processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/Css/Property/Processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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, ';');
Expand Down
4 changes: 2 additions & 2 deletions src/Css/Property/Property.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ final class Property
private $value;

/**
* @var Specificity
* @var Specificity|null
*/
private $originalSpecificity;

Expand Down Expand Up @@ -57,7 +57,7 @@ public function getValue()
/**
* Get originalSpecificity
*
* @return Specificity
* @return Specificity|null
*/
public function getOriginalSpecificity()
{
Expand Down
18 changes: 12 additions & 6 deletions src/Css/Rule/Processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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, '}');
Expand Down Expand Up @@ -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
Expand All @@ -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
|
Expand All @@ -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
);
}

Expand Down
32 changes: 25 additions & 7 deletions src/CssToInlineStyles.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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()
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
28 changes: 15 additions & 13 deletions tests/Css/ProcessorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -42,7 +42,7 @@ public function testCssWithOneRule()
$this->assertEquals(1, $rules[0]->getOrder());
}

public function testCssWithComments()
public function testCssWithComments(): void
{
$css = <<<CSS
a {
Expand All @@ -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) {
Expand All @@ -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));
Expand All @@ -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";
Expand All @@ -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(
Expand All @@ -159,7 +161,7 @@ public function testSimpleStyleTagsInHtml()
);
}

public function testStyleTagsWithAttributeInHtml()
public function testStyleTagsWithAttributeInHtml(): void
{
$expected = 'p { color: #F00; }' . "\n";
$this->assertEquals(
Expand All @@ -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(
Expand All @@ -206,7 +208,7 @@ public function testMultipleStyleTagsInHtml()
);
}

public function testWeirdTagsInHtml()
public function testWeirdTagsInHtml(): void
{
$expected = 'p { color: #F00; }' . "\n";
$this->assertEquals(
Expand All @@ -231,7 +233,7 @@ public function testWeirdTagsInHtml()

}

public function testStyleTagsInCommentInHtml()
public function testStyleTagsInCommentInHtml(): void
{
$expected = 'p { color: #F00; }' . "\n";
$this->assertEquals(
Expand Down
Loading
Loading