Skip to content

Commit d06041d

Browse files
JakeQZJakeQZ
authored
[TASK] Make DeclarationBlockParser methods static (#1599)
The class provides two utility functions and a static cache. It does not represent anything, and has no properties. Thus it does not make sense for instances to have to be instantiated.
1 parent 8471be1 commit d06041d

File tree

8 files changed

+23
-38
lines changed

8 files changed

+23
-38
lines changed

‎config/phpmd.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
<rule ref="rulesets/cleancode.xml/BooleanArgumentFlag"/>
1010
<rule ref="rulesets/cleancode.xml/StaticAccess">
1111
<properties>
12-
<!-- Avoid false positives when calling factory and cache-clearing methods -->
13-
<property name="ignorepattern" value="/^(from|create|clear)/" />
12+
<!-- Avoid false positives when calling methods that are intentionally static -->
13+
<property name="ignorepattern" value="/^(from|create|clear|parse$|normalizePropertyName$)/" />
1414
</properties>
1515
</rule>
1616

‎config/phpstan-baseline.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ parameters:
77
path: ../src/CssInliner.php
88

99
-
10-
message: '#^Parameter \#1 \$name of method Pelago\\Emogrifier\\Utilities\\DeclarationBlockParser\:\:normalizePropertyName\(\) expects non\-empty\-string, mixed given\.$#'
10+
message: '#^Parameter \#1 \$name of static method Pelago\\Emogrifier\\Utilities\\DeclarationBlockParser\:\:normalizePropertyName\(\) expects non\-empty\-string, mixed given\.$#'
1111
identifier: argument.type
1212
count: 1
1313
path: ../src/CssInliner.php

‎src/CssInliner.php‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -435,12 +435,10 @@ private function getAllNodesWithStyleAttribute(): \DOMNodeList
435435
*/
436436
private function normalizeStyleAttributes(\DOMElement $node): void
437437
{
438-
$declarationBlockParser = new DeclarationBlockParser();
439-
440438
$pattern = '/-{0,2}+[_a-zA-Z][\\w\\-]*+(?=:)/S';
441439
/** @param array<array-key, string> $propertyNameMatches */
442-
$callback = static function (array $propertyNameMatches) use ($declarationBlockParser): string {
443-
return $declarationBlockParser->normalizePropertyName($propertyNameMatches[0]);
440+
$callback = static function (array $propertyNameMatches): string {
441+
return DeclarationBlockParser::normalizePropertyName($propertyNameMatches[0]);
444442
};
445443
if (\function_exists('Safe\\preg_replace_callback')) {
446444
$normalizedOriginalStyle = preg_replace_callback($pattern, $callback, $node->getAttribute('style'));
@@ -453,7 +451,7 @@ private function normalizeStyleAttributes(\DOMElement $node): void
453451
// In order to not overwrite existing style attributes in the HTML, we have to save the original HTML styles.
454452
$nodePath = $node->getNodePath();
455453
if (\is_string($nodePath) && ($nodePath !== '') && !isset($this->styleAttributesForNodes[$nodePath])) {
456-
$this->styleAttributesForNodes[$nodePath] = $declarationBlockParser->parse($normalizedOriginalStyle);
454+
$this->styleAttributesForNodes[$nodePath] = DeclarationBlockParser::parse($normalizedOriginalStyle);
457455
$this->visitedNodes[$nodePath] = $node;
458456
}
459457

@@ -759,16 +757,15 @@ private function getCssSelectorPrecedence(string $selector): int
759757
private function copyInlinableCssToStyleAttribute(\DOMElement $node, array $cssRule): void
760758
{
761759
$declarationsBlock = $cssRule['declarationsBlock'];
762-
$declarationBlockParser = new DeclarationBlockParser();
763-
$newStyleDeclarations = $declarationBlockParser->parse($declarationsBlock);
760+
$newStyleDeclarations = DeclarationBlockParser::parse($declarationsBlock);
764761
if ($newStyleDeclarations === []) {
765762
return;
766763
}
767764

768765
// if it has a style attribute, get it, process it, and append (overwrite) new stuff
769766
if ($node->hasAttribute('style')) {
770767
// break it up into an associative array
771-
$oldStyleDeclarations = $declarationBlockParser->parse($node->getAttribute('style'));
768+
$oldStyleDeclarations = DeclarationBlockParser::parse($node->getAttribute('style'));
772769
} else {
773770
$oldStyleDeclarations = [];
774771
}
@@ -816,14 +813,13 @@ private function generateStyleStringFromDeclarationsArrays(array $oldStyles, arr
816813

817814
$combinedStyles = \array_merge($oldStyles, $newStyles);
818815

819-
$declarationBlockParser = new DeclarationBlockParser();
820816
$style = '';
821817
foreach ($combinedStyles as $attributeName => $attributeValue) {
822818
$trimmedAttributeName = \trim($attributeName);
823819
if ($trimmedAttributeName === '') {
824820
throw new \UnexpectedValueException('An empty property name was encountered.', 1727046078);
825821
}
826-
$propertyName = $declarationBlockParser->normalizePropertyName($trimmedAttributeName);
822+
$propertyName = DeclarationBlockParser::normalizePropertyName($trimmedAttributeName);
827823
$propertyValue = \trim($attributeValue);
828824
$style .= $propertyName . ': ' . $propertyValue . '; ';
829825
}
@@ -847,10 +843,9 @@ private function attributeValueIsImportant(string $attributeValue): bool
847843
*/
848844
private function fillStyleAttributesWithMergedStyles(): void
849845
{
850-
$declarationBlockParser = new DeclarationBlockParser();
851846
foreach ($this->styleAttributesForNodes as $nodePath => $styleAttributesForNode) {
852847
$node = $this->visitedNodes[$nodePath];
853-
$currentStyleAttributes = $declarationBlockParser->parse($node->getAttribute('style'));
848+
$currentStyleAttributes = DeclarationBlockParser::parse($node->getAttribute('style'));
854849
$node->setAttribute(
855850
'style',
856851
$this->generateStyleStringFromDeclarationsArrays(
@@ -887,7 +882,7 @@ private function removeImportantAnnotationFromAllInlineStyles(): void
887882
private function removeImportantAnnotationFromNodeInlineStyle(\DOMElement $node): void
888883
{
889884
$style = $node->getAttribute('style');
890-
$inlineStyleDeclarations = (new DeclarationBlockParser())->parse((bool) $style ? $style : '');
885+
$inlineStyleDeclarations = DeclarationBlockParser::parse((bool) $style ? $style : '');
891886
/** @var array<string, string> $regularStyleDeclarations */
892887
$regularStyleDeclarations = [];
893888
/** @var array<string, string> $importantStyleDeclarations */

‎src/HtmlProcessor/CssToAttributeConverter.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,8 @@ final class CssToAttributeConverter extends AbstractHtmlProcessor
6262
*/
6363
public function convertCssToVisualAttributes(): self
6464
{
65-
$declarationBlockParser = new DeclarationBlockParser();
6665
foreach ($this->getAllNodesWithStyleAttribute() as $node) {
67-
$inlineStyleDeclarations = $declarationBlockParser->parse($node->getAttribute('style'));
66+
$inlineStyleDeclarations = DeclarationBlockParser::parse($node->getAttribute('style'));
6867
$this->mapCssToHtmlAttributes($inlineStyleDeclarations, $node);
6968
}
7069

‎src/HtmlProcessor/CssVariableEvaluator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function evaluateVariables(): self
4545

4646
// Avoid parsing declarations if none use or define a variable
4747
if (preg_match('/(?<![\\w\\-])--[\\w\\-]/', $style) !== 0) {
48-
$declarations = (new DeclarationBlockParser())->parse($style);
48+
$declarations = DeclarationBlockParser::parse($style);
4949
$variableDefinitions =
5050
$this->getVariableDefinitionsFromDeclarations($declarations) + $currentAncestorDefinitions;
5151
$this->currentVariableDefinitions = $variableDefinitions;

‎src/Utilities/DeclarationBlockParser.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public static function clearCache(): void
4141
*
4242
* @return non-empty-string
4343
*/
44-
public function normalizePropertyName(string $name): string
44+
public static function normalizePropertyName(string $name): string
4545
{
4646
if (\substr($name, 0, 2) === '--') {
4747
return $name;
@@ -77,7 +77,7 @@ public function normalizePropertyName(string $name): string
7777
*
7878
* @throws \UnexpectedValueException if an empty property name is encountered (which cannot happen)
7979
*/
80-
public function parse(string $declarationBlock): array
80+
public static function parse(string $declarationBlock): array
8181
{
8282
$trimmedDeclarationBlock = \trim($declarationBlock, "; \n\r\t\v\x00");
8383
if ($trimmedDeclarationBlock === '') {
@@ -103,7 +103,7 @@ public function parse(string $declarationBlock): array
103103

104104
$propertyName = $matches[1];
105105
$propertyValue = $matches[2];
106-
$properties[$this->normalizePropertyName($propertyName)] = $propertyValue;
106+
$properties[self::normalizePropertyName($propertyName)] = $propertyValue;
107107
}
108108
self::$cache[$trimmedDeclarationBlock] = $properties;
109109

‎tests/Unit/CssInlinerTest.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3922,8 +3922,7 @@ public function copyUninlinableCssToStyleNodeHasNoSideEffects(): void
39223922
*/
39233923
public function inlineCssClearsDeclarationBlockParserCache(): void
39243924
{
3925-
$subject = new DeclarationBlockParser();
3926-
$subject->parse('color: green;');
3925+
DeclarationBlockParser::parse('color: green;');
39273926

39283927
$cacheProperty = new \ReflectionProperty(DeclarationBlockParser::class, 'cache');
39293928
if (\PHP_VERSION_ID < 80100) {

‎tests/Unit/Utilities/DeclarationBlockParserTest.php‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ public function providePropertyNameAndExpectedNormalization(): array
6363
*/
6464
public function normalizesPropertyName(string $name, string $expectedNormalization): void
6565
{
66-
$subject = new DeclarationBlockParser();
67-
68-
$result = $subject->normalizePropertyName($name);
66+
$result = DeclarationBlockParser::normalizePropertyName($name);
6967

7068
self::assertSame($expectedNormalization, $result);
7169
}
@@ -264,9 +262,7 @@ public function provideDeclarationBlockAsStringAndArray(): array
264262
*/
265263
public function parses(string $declarationBlockAsString, array $declarationBlockAsArray): void
266264
{
267-
$subject = new DeclarationBlockParser();
268-
269-
$result = $subject->parse($declarationBlockAsString);
265+
$result = DeclarationBlockParser::parse($declarationBlockAsString);
270266

271267
self::assertSame($declarationBlockAsArray, $result);
272268
}
@@ -276,9 +272,7 @@ public function parses(string $declarationBlockAsString, array $declarationBlock
276272
*/
277273
public function overridesEarlierDeclarationWithLaterOne(): void
278274
{
279-
$subject = new DeclarationBlockParser();
280-
281-
$result = $subject->parse('color: red; color: green;');
275+
$result = DeclarationBlockParser::parse('color: red; color: green;');
282276

283277
self::assertSame(['color' => 'green'], $result);
284278
}
@@ -291,11 +285,10 @@ public function overridesEarlierDeclarationWithLaterOne(): void
291285
*/
292286
public function providesConsistentResults(): void
293287
{
294-
$subject = new DeclarationBlockParser();
295288
$declarationBlock = 'color: green;';
296289

297-
$firstResult = $subject->parse($declarationBlock);
298-
$secondResult = $subject->parse($declarationBlock);
290+
$firstResult = DeclarationBlockParser::parse($declarationBlock);
291+
$secondResult = DeclarationBlockParser::parse($declarationBlock);
299292

300293
self::assertSame($firstResult, $secondResult);
301294
}
@@ -305,8 +298,7 @@ public function providesConsistentResults(): void
305298
*/
306299
public function clearCacheEmptiesStaticCache(): void
307300
{
308-
$subject = new DeclarationBlockParser();
309-
$subject->parse('color: green;');
301+
DeclarationBlockParser::parse('color: green;');
310302

311303
DeclarationBlockParser::clearCache();
312304

0 commit comments

Comments
 (0)