[BUGFIX] Fix PhpStorm code inspection warnings (#770)

[BUGFIX] Fix PhpStorm code inspection warnings

Fixes #729

Also update regular expression for CSS rule names

CHANGELOG.md

@@ -105,6 +105,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
  ([#690](https://github.com/MyIntervals/emogrifier/pull/690))
 
 ### Fixed
+- Fix PhpStorm code inspection warnings
+ ([#729](https://github.com/MyIntervals/emogrifier/issues/729),
+ [#770](https://github.com/MyIntervals/emogrifier/pull/770))
 - Uppercase type combined with class or ID in selector
  ([#590](https://github.com/MyIntervals/emogrifier/issues/590),
  [#769](https://github.com/MyIntervals/emogrifier/pull/769))

src/Emogrifier.php

@@ -587,7 +587,7 @@ private function getAllNodesWithStyleAttribute()
  * "media" (the media query string, e.g. "@media screen and (max-width: 480px)",
  * or an empty string if not from a `@media` rule),
  * "selector" (the CSS selector, e.g., "*" or "header h1"),
- * "hasUnmatchablePseudo" (true if that selector contains psuedo-elements or dynamic pseudo-classes
+ * "hasUnmatchablePseudo" (true if that selector contains pseudo-elements or dynamic pseudo-classes
  * such that the declarations cannot be applied inline),
  * "declarationsBlock" (the semicolon-separated CSS declarations for that selector,
  * e.g., "color: red; height: 4px;"),
@@ -611,8 +611,7 @@ private function parseCssRules($css)
  continue;
  }
 
- $selectors = \explode(',', $cssRule['selectors']);
- foreach ($selectors as $selector) {
+ foreach (\explode(',', $cssRule['selectors']) as $selector) {
  // don't process pseudo-elements and behavioral (dynamic) pseudo-classes;
  // only allow structural pseudo-classes
  $hasPseudoElement = \strpos($selector, '::') !== false;
@@ -752,6 +751,7 @@ public function removeUnprocessableHtmlTag($tagName)
  {
  $key = \array_search($tagName, $this->unprocessableHtmlTags, true);
  if ($key !== false) {
+ /** @var int|string $key */
  unset($this->unprocessableHtmlTags[$key]);
  }
  }
@@ -843,7 +843,7 @@ private function normalizeStyleAttributesOfAllNodes()
  private function normalizeStyleAttributes(\DOMElement $node)
  {
  $normalizedOriginalStyle = \preg_replace_callback(
- '/[A-z\\-]+(?=\\:)/S',
+ '/-?+[_a-zA-Z][\\w\\-]*+(?=:)/S',
  static function (array $m) {
  return \strtolower($m[0]);
  },
@@ -1004,7 +1004,7 @@ private function copyUninlinableCssToStyleNode(array $cssRules, $cssImportRules)
  // avoid including unneeded class dependency if there are no rules
  if ($cssRulesRelevantForDocument !== []) {
  // support use without autoload
- if (!\class_exists('Pelago\\Emogrifier\\Utilities\\CssConcatenator')) {
+ if (!\class_exists(CssConcatenator::class)) {
  require_once __DIR__ . '/Emogrifier/Utilities/CssConcatenator.php';
  }
 
@@ -1058,7 +1058,7 @@ private function removeUnmatchablePseudoComponents($selector)
  // The regex allows nested brackets via `(?2)`.
  // A space is temporarily prepended because the callback can't determine if the match was at the very start.
  $selectorWithoutNots = \ltrim(\preg_replace_callback(
- '/(\\s?+):not(\\([^\\(\\)]*+(?:(?2)[^\\(\\)]*+)*+\\))/i',
+ '/(\\s?+):not(\\([^()]*+(?:(?2)[^()]*+)*+\\))/i',
  [$this, 'replaceUnmatchableNotComponent'],
  ' ' . $selector
  ));
@@ -1169,6 +1169,8 @@ protected function addStyleElementToDocument($css)
  * Checks that $this->domDocument has a BODY element and adds it if it is missing.
  *
  * @return void
+ *
+ * @throws \UnexpectedValueException
  */
  private function ensureExistenceOfBodyElement()
  {
@@ -1177,6 +1179,9 @@ private function ensureExistenceOfBodyElement()
  }
 
  $htmlElement = $this->domDocument->getElementsByTagName('html')->item(0);
+ if ($htmlElement === null) {
+ throw new \UnexpectedValueException('There is no HTML element although there should be one.', 1569930874);
+ }
  $htmlElement->appendChild($this->domDocument->createElement('body'));
  }
 
@@ -1230,7 +1235,7 @@ private function extractImportAndCharsetRules($css)
  }
 
  /**
- * Splits input CSS code into an array of parts for different media querues, in order.
+ * Splits input CSS code into an array of parts for different media queries, in order.
  * Each part is an array where:
  *
  * - key "css" will contain clean CSS code (for @media rules this will be the group rule body within "{...}")
@@ -1268,7 +1273,7 @@ private function splitCssAndMediaQuery($css)
  $mediaRuleBodyMatcher = '[^{]*+{(?:[^{}]*+{.*})?\\s*+}\\s*+';
 
  $cssSplitForAllowedMediaTypes = \preg_split(
- '#(@media\\s++(?:only\\s++)?+(?:(?=[{\\(])' . $mediaTypesExpression . ')' . $mediaRuleBodyMatcher
+ '#(@media\\s++(?:only\\s++)?+(?:(?=[{(])' . $mediaTypesExpression . ')' . $mediaRuleBodyMatcher
  . ')#misU',
  $css,
  -1,
@@ -1318,8 +1323,7 @@ private function removeUnprocessableTags()
  }
  /** @var \DOMNode $node */
  foreach ($nodes as $node) {
- $hasContent = $node->hasChildNodes() || $node->hasChildNodes();
- if (!$hasContent) {
+ if (!$node->hasChildNodes()) {
  $node->parentNode->removeChild($node);
  }
  }
@@ -1468,13 +1472,13 @@ static function (array $matches) {
  $trimmedLowercaseSelector,
  $matches
  );
- if (!$hasNotSelector) {
- $xPath = '//' . $this->translateCssToXpathPass($trimmedLowercaseSelector);
- } else {
+ if ($hasNotSelector) {
  /** @var string[] $matches */
  list(, $partBeforeNot, $notContents) = $matches;
  $xPath = '//' . $this->translateCssToXpathPass($partBeforeNot) .
  '[not(' . $this->translateCssToXpathPassInline($notContents) . ')]';
+ } else {
+ $xPath = '//' . $this->translateCssToXpathPass($trimmedLowercaseSelector);
  }
  $this->caches[self::CACHE_KEY_SELECTOR][$xPathKey] = $xPath;
 
@@ -1579,10 +1583,8 @@ private function matchClassAttributes(array $match)
  private function matchClassAttributesInline(array $match)
  {
  return 'contains(concat(" ",@class," "),concat(" ","' .
- \implode(
- '"," "))][contains(concat(" ",@class," "),concat(" ","',
- \explode('.', \substr($match[2], 1))
- ) . '"," "))';
+ \str_replace('.', '"," "))][contains(concat(" ",@class," "),concat(" ","', \substr($match[2], 1)) .
+ '"," "))';
  }
 
  /**
@@ -1720,9 +1722,7 @@ private function parseCssDeclarationsBlock($cssDeclarationsBlock)
  }
 
  $properties = [];
- $declarations = \preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock);
-
- foreach ($declarations as $declaration) {
+ foreach (\preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock) as $declaration) {
  $matches = [];
  if (!\preg_match('/^([A-Za-z\\-]+)\\s*:\\s*(.+)$/s', \trim($declaration), $matches)) {
  continue;

src/Emogrifier/CssInliner.php

@@ -380,7 +380,7 @@ private function getAllNodesWithStyleAttribute()
  private function normalizeStyleAttributes(\DOMElement $node)
  {
  $normalizedOriginalStyle = \preg_replace_callback(
- '/[A-z\\-]+(?=\\:)/S',
+ '/-?+[_a-zA-Z][\\w\\-]*+(?=:)/S',
  static function (array $m) {
  return \strtolower($m[0]);
  },
@@ -424,9 +424,7 @@ private function parseCssDeclarationsBlock($cssDeclarationsBlock)
  }
 
  $properties = [];
- $declarations = \preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock);
-
- foreach ($declarations as $declaration) {
+ foreach (\preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock) as $declaration) {
  $matches = [];
  if (!\preg_match('/^([A-Za-z\\-]+)\\s*:\\s*(.+)$/s', \trim($declaration), $matches)) {
  continue;
@@ -562,7 +560,7 @@ private function getCssSelectorConverter()
  * "media" (the media query string, e.g. "@media screen and (max-width: 480px)",
  * or an empty string if not from a `@media` rule),
  * "selector" (the CSS selector, e.g., "*" or "header h1"),
- * "hasUnmatchablePseudo" (true if that selector contains psuedo-elements or dynamic pseudo-classes
+ * "hasUnmatchablePseudo" (true if that selector contains pseudo-elements or dynamic pseudo-classes
  * such that the declarations cannot be applied inline),
  * "declarationsBlock" (the semicolon-separated CSS declarations for that selector,
  * e.g., "color: red; height: 4px;"),
@@ -589,8 +587,7 @@ private function parseCssRules($css)
  continue;
  }
 
- $selectors = \explode(',', $cssRule['selectors']);
- foreach ($selectors as $selector) {
+ foreach (\explode(',', $cssRule['selectors']) as $selector) {
  // don't process pseudo-elements and behavioral (dynamic) pseudo-classes;
  // only allow structural pseudo-classes
  $hasPseudoElement = \strpos($selector, '::') !== false;
@@ -699,7 +696,7 @@ private function getCssRuleMatches($css)
  }
 
  /**
- * Splits input CSS code into an array of parts for different media querues, in order.
+ * Splits input CSS code into an array of parts for different media queries, in order.
  * Each part is an array where:
  *
  * - key "css" will contain clean CSS code (for @media rules this will be the group rule body within "{...}")
@@ -737,7 +734,7 @@ private function splitCssAndMediaQuery($css)
  $mediaRuleBodyMatcher = '[^{]*+{(?:[^{}]*+{.*})?\\s*+}\\s*+';
 
  $cssSplitForAllowedMediaTypes = \preg_split(
- '#(@media\\s++(?:only\\s++)?+(?:(?=[{\\(])' . $mediaTypesExpression . ')' . $mediaRuleBodyMatcher
+ '#(@media\\s++(?:only\\s++)?+(?:(?=[{(])' . $mediaTypesExpression . ')' . $mediaRuleBodyMatcher
  . ')#misU',
  $css,
  -1,
@@ -1015,7 +1012,7 @@ private function removeUnmatchablePseudoComponents($selector)
  // The regex allows nested brackets via `(?2)`.
  // A space is temporarily prepended because the callback can't determine if the match was at the very start.
  $selectorWithoutNots = \ltrim(\preg_replace_callback(
- '/(\\s?+):not(\\([^\\(\\)]*+(?:(?2)[^\\(\\)]*+)*+\\))/i',
+ '/(\\s?+):not(\\([^()]*+(?:(?2)[^()]*+)*+\\))/i',
  [$this, 'replaceUnmatchableNotComponent'],
  ' ' . $selector
  ));

src/Emogrifier/HtmlProcessor/AbstractHtmlProcessor.php

@@ -295,6 +295,8 @@ private function ensurePhpUnrecognizedSelfClosingTagsAreXml($html)
  * Checks that $this->domDocument has a BODY element and adds it if it is missing.
  *
  * @return void
+ *
+ * @throws \UnexpectedValueException
  */
  private function ensureExistenceOfBodyElement()
  {
@@ -303,6 +305,9 @@ private function ensureExistenceOfBodyElement()
  }
 
  $htmlElement = $this->domDocument->getElementsByTagName('html')->item(0);
+ if ($htmlElement === null) {
+ throw new \UnexpectedValueException('There is no HTML element although there should be one.', 1569930853);
+ }
  $htmlElement->appendChild($this->domDocument->createElement('body'));
  }
 }

src/Emogrifier/HtmlProcessor/CssToAttributeConverter.php

@@ -99,9 +99,7 @@ private function parseCssDeclarationsBlock($cssDeclarationsBlock)
  }
 
  $properties = [];
- $declarations = \preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock);
-
- foreach ($declarations as $declaration) {
+ foreach (\preg_split('/;(?!base64|charset)/', $cssDeclarationsBlock) as $declaration) {
  $matches = [];
  if (!\preg_match('/^([A-Za-z\\-]+)\\s*:\\s*(.+)$/s', \trim($declaration), $matches)) {
  continue;
@@ -219,9 +217,9 @@ private function mapComplexCssProperty($property, $value, \DOMElement $node)
  private function mapBackgroundProperty(\DOMElement $node, $value)
  {
  // parse out the color, if any
- $styles = \explode(' ', $value);
+ $styles = \explode(' ', $value, 2);
  $first = $styles[0];
- if (\is_numeric($first[0]) || \strpos($first, 'url') === 0) {
+ if (\is_numeric($first[0]) || \strncmp($first, 'url', 3) === 0) {
  return;
  }
 
@@ -304,6 +302,7 @@ private function isTableOrImageNode(\DOMElement $node)
  */
  private function parseCssShorthandValue($value)
  {
+ /** @var string[] $values */
  $values = \preg_split('/\\s+/', $value);
 
  $css = [];

src/Emogrifier/HtmlProcessor/HtmlPruner.php

@@ -88,6 +88,7 @@ private function removeClassesFromElements(\DOMNodeList $elements, array $classe
  {
  $classesToKeepIntersector = new ArrayIntersector($classesToKeep);
 
+ /** @var \DOMNode $element */
  foreach ($elements as $element) {
  $elementClasses = \preg_split('/\\s++/', \trim($element->getAttribute('class')));
  $elementClassesToKeep = $classesToKeepIntersector->intersectWith($elementClasses);
@@ -108,6 +109,7 @@ private function removeClassesFromElements(\DOMNodeList $elements, array $classe
  */
  private function removeClassAttributeFromElements(\DOMNodeList $elements)
  {
+ /** @var \DOMNode $element */
  foreach ($elements as $element) {
  $element->removeAttribute('class');
  }

tests/Support/Traits/AssertCss.php

@@ -11,7 +11,7 @@ trait AssertCss
 {
  /**
  * Processing of @media rules may involve removal of some unnecessary whitespace from the CSS placed in the <style>
- * element added to the docuemnt, due to the way that certain parts are `trim`med. Notably, whitespace either side
+ * element added to the document, due to the way that certain parts are `trim`med. Notably, whitespace either side
  * of "{", "}" and "," or at the beginning of the CSS may be removed.
  *
  * This method helps takes care of that, by converting a search needle for an exact match into a regular expression

tests/Unit/CssInlinerTest.php

@@ -693,8 +693,7 @@ public function inlineCssAppliesCssToMatchingElements($css, $expectedHtml)
 
  $result = $subject->render();
  $selector = \trim(\strtok($css, '{'));
- $cssSelectorConverter = new CssSelectorConverter();
- $xPathExpression = $cssSelectorConverter->toXPath($selector);
+ $xPathExpression = (new CssSelectorConverter())->toXPath($selector);
  $message = 'with converted XPath expression `' . $xPathExpression . '`';
  self::assertContains($needleExpected, $result, $message);
  }

tests/Unit/Emogrifier/HtmlProcessor/AbstractHtmlProcessorTest.php

@@ -641,6 +641,7 @@ public function getDomDocumentVoidElementNotHasChildNodes($htmlWithNonXmlSelfClo
  $domDocument = $subject->getDomDocument();
 
  $voidElements = $domDocument->getElementsByTagName($tagName);
+ /** @var \DOMElement $element */
  foreach ($voidElements as $element) {
  self::assertFalse($element->hasChildNodes());
  }

tests/Unit/EmogrifierTest.php

@@ -2586,6 +2586,7 @@ public function getDomDocumentVoidElementNotHasChildNodes($htmlWithNonXmlSelfClo
  $domDocument = $this->subject->getDomDocument();
 
  $voidElements = $domDocument->getElementsByTagName($tagName);
+ /** @var \DOMElement $element */
  foreach ($voidElements as $element) {
  self::assertFalse($element->hasChildNodes());
  }
@@ -3145,20 +3146,6 @@ public function emogrifyKeepsInlineStylePriorityVersusStyleBlockRules()
  self::assertContains('<p style="padding: 10px; padding-left: 20px;">', $result);
  }
 
- /**
- * Asserts that $html contains a $tagName tag with the $attribute attribute.
- *
- * @param string $html the HTML string we are searching in
- * @param string $tagName the HTML tag we are looking for
- * @param string $attribute the attribute we are looking for (with or even without a value)
- */
- private function assertHtmlStringContainsTagWithAttribute($html, $tagName, $attribute)
- {
- self::assertTrue(
- \preg_match('/<' . \preg_quote($tagName, '/') . '[^>]+' . \preg_quote($attribute, '/') . '/', $html) > 0
- );
- }
-
  /**
  * @return string[][]
  */