Repository: ampproject/amp-toolbox-php (php)

Issue:
PHP 8.4 Compatibility
I wanted to test PHP 8.4 compatibility in the Web Stories plugin a bit early and noticed a few deprecation warnings coming from the AMP PHP Toolbox:

```
PHP Deprecated:  Implicitly marking parameter $node as nullable is deprecated, the explicit nullable type must be used instead in /home/runner/work/web-stories-wp/web-stories-wp/third-party/vendor/ampproject/amp-toolbox/src/Dom/Document.php on line 343
PHP Deprecated:  Implicitly marking parameter $node as nullable is deprecated, the explicit nullable type must be used instead in /home/runner/work/web-stories-wp/web-stories-wp/third-party/vendor/ampproject/amp-toolbox/src/Dom/Document.php on line 353
PHP Deprecated:  Implicitly marking parameter $node as nullable is deprecated, the explicit nullable type must be used instead in /home/runner/work/web-stories-wp/web-stories-wp/vendor/ampproject/amp-toolbox/src/Dom/Document.php on line 416
PHP Deprecated:  Implicitly marking parameter $node as nullable is deprecated, the explicit nullable type must be used instead in /home/runner/work/web-stories-wp/web-stories-wp/vendor/ampproject/amp-toolbox/src/Dom/Document.php on line 427
PHP Deprecated:  Implicitly marking parameter $configuration as nullable is deprecated, the explicit nullable type must be used instead in /home/runner/work/web-stories-wp/web-stories-wp/third-party/vendor/ampproject/amp-toolbox/src/Optimizer/TransformationEngine.php on line 51
```

This is related to the [PHP RFC to deprecate implicitly nullable parameter types](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types). See https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated for a good explanation & ways to address this.

The problem is that nullable types were only added in PHP 7.1 but this library still supports PHP 5.6.

I suggest to just bump the minimum PHP requirement to PHP 7.4 (like the AMP plugin and the Web Stories plugin) in a 0.12 release and highlight this as a breaking change.

Pull request:
Improve PHP 8.4 compatibility
PHP 8.4 is slated to be released on November 21, 2024.

In the process of making the Web Stories plugin compatible, this addresses warnings about implicit nullable types.

PHP requirement is increased to 7.4 to make maintenance easier.

Fixes #549

Patch:
diff --git a/composer.json b/composer.json
index f8d52ee9a..c7a0ae994 100644
--- a/composer.json
+++ b/composer.json
@@ -4,7 +4,7 @@
   "description": "A collection of AMP tools making it easier to publish and host AMP pages with PHP.",
   "license": "Apache-2.0",
   "require": {
-    "php": "^5.6 || ^7.0 || ^8.0",
+    "php": "^7.4 || ^8.0",
     "ext-dom": "*",
     "ext-filter": "*",
     "ext-iconv": "*",
diff --git a/src/Cli/Executable.php b/src/Cli/Executable.php
index f17da6d00..31b6b0aa2 100644
--- a/src/Cli/Executable.php
+++ b/src/Cli/Executable.php
@@ -77,7 +77,7 @@ abstract class Executable
      * @param Colors|null  $colors    Optional. Instance of the Colors object to use. Defaults to null to instantiate a
      *                                new one.
      */
-    public function __construct($autocatch = true, Options $options = null, Colors $colors = null)
+    public function __construct($autocatch = true, ?Options $options = null, ?Colors $colors = null)
     {
         if ($autocatch) {
             set_exception_handler([$this, 'fatal']);
diff --git a/src/Cli/Options.php b/src/Cli/Options.php
index ec2051d4d..8b7821119 100644
--- a/src/Cli/Options.php
+++ b/src/Cli/Options.php
@@ -77,7 +77,7 @@ class Options
      * @param Colors $colors Optional. Configured color object.
      * @throws InvalidArgument When arguments can't be read.
      */
-    public function __construct(Colors $colors = null)
+    public function __construct(?Colors $colors = null)
     {
         $this->colors = $colors instanceof Colors ? $colors : new Colors();
 
diff --git a/src/Cli/TableFormatter.php b/src/Cli/TableFormatter.php
index 49a7960c6..6ed660465 100644
--- a/src/Cli/TableFormatter.php
+++ b/src/Cli/TableFormatter.php
@@ -74,7 +74,7 @@ class TableFormatter
      *
      * @param Colors|null $colors Optional. Instance of the Colors helper object.
      */
-    public function __construct(Colors $colors = null)
+    public function __construct(?Colors $colors = null)
     {
         // Try to get terminal width.
         $width = $this->getTerminalWidth();
diff --git a/src/CssLength.php b/src/CssLength.php
index a4b3821c8..0af3fe269 100644
--- a/src/CssLength.php
+++ b/src/CssLength.php
@@ -112,7 +112,7 @@ public function validate($allowAuto, $allowFluid)
         if (preg_match($pattern, $this->attrValue, $match)) {
             $this->isValid = true;
             $this->numeral = isset($match['numeral']) ? (float)$match['numeral'] : $this->numeral;
-            $this->unit    = isset($match['unit']) ? $match['unit'] : $this->unit;
+            $this->unit    = $match['unit'] ?? $this->unit;
         }
     }
 
diff --git a/src/Dom/Document.php b/src/Dom/Document.php
index 4f0a46fcf..f9a7a6a38 100644
--- a/src/Dom/Document.php
+++ b/src/Dom/Document.php
@@ -227,7 +227,7 @@ public static function fromHtml($html, $options = [])
             $options = [Option::ENCODING => $options];
         }
 
-        $encoding = isset($options[Option::ENCODING]) ? $options[Option::ENCODING] : null;
+        $encoding = $options[ Option::ENCODING ] ?? null;
 
         $dom = new self('', $encoding);
 
@@ -258,7 +258,7 @@ public static function fromHtmlFragment($html, $options = [])
             $options = [Option::ENCODING => $options];
         }
 
-        $encoding = isset($options[Option::ENCODING]) ? $options[Option::ENCODING] : null;
+        $encoding = $options[ Option::ENCODING ] ?? null;
 
         $dom = new self('', $encoding);
 
@@ -413,7 +413,7 @@ public function loadHTMLFragment($source, $options = [])
      * @return string The HTML, or false if an error occurred.
      */
     #[\ReturnTypeWillChange]
-    public function saveHTML(DOMNode $node = null)
+    public function saveHTML(?DOMNode $node = null)
     {
         return $this->saveHTMLFragment($node);
     }
@@ -424,7 +424,7 @@ public function saveHTML(DOMNode $node = null)
      * @param DOMNode|null $node Optional. Parameter to output a subset of the document.
      * @return string The HTML fragment, or false if an error occurred.
      */
-    public function saveHTMLFragment(DOMNode $node = null)
+    public function saveHTMLFragment(?DOMNode $node = null)
     {
         $filtersInReverse = array_reverse($this->filters);
 
@@ -551,7 +551,7 @@ private function normalizeDocumentStructure($content)
             $content   = preg_replace(self::HTML_STRUCTURE_HTML_START_TAG, '', $content, 1);
 
             preg_match(self::HTML_STRUCTURE_HTML_END_TAG, $content, $matches);
-            $htmlEnd = isset($matches['html_end']) ? $matches['html_end'] : $htmlEnd;
+            $htmlEnd = $matches['html_end'] ?? $htmlEnd;
             $content = preg_replace(self::HTML_STRUCTURE_HTML_END_TAG, '', $content, 1);
         }
 
diff --git a/src/Dom/Document/Filter/AmpBindAttributes.php b/src/Dom/Document/Filter/AmpBindAttributes.php
index 73ce896ae..7a5ffd2d3 100644
--- a/src/Dom/Document/Filter/AmpBindAttributes.php
+++ b/src/Dom/Document/Filter/AmpBindAttributes.php
@@ -215,7 +215,7 @@ public function afterSave($html)
                     ||
                     in_array($attrName, $this->convertedAmpBindAttributes, true)
                 ) {
-                    $attrValue = isset($attrMatches['value']) ? $attrMatches['value'] : '=""';
+                    $attrValue = $attrMatches['value'] ?? '=""';
                     $newAttrs .= " [{$attrName}]{$attrValue}";
                 } else {
                     $newAttrs .= $attrMatches[0];
diff --git a/src/Dom/Options.php b/src/Dom/Options.php
index 522337ef2..7bf611b54 100644
--- a/src/Dom/Options.php
+++ b/src/Dom/Options.php
@@ -72,7 +72,7 @@ public function offsetUnset($option)
     #[\ReturnTypeWillChange]
     public function offsetGet($option)
     {
-        return isset($this->options[$option]) ? $this->options[$option] : null;
+        return $this->options[ $option ] ?? null;
     }
 
     /**
diff --git a/src/Optimizer/Error/CannotPreloadImage.php b/src/Optimizer/Error/CannotPreloadImage.php
index e6d5058ec..e1df4fc33 100644
--- a/src/Optimizer/Error/CannotPreloadImage.php
+++ b/src/Optimizer/Error/CannotPreloadImage.php
@@ -25,7 +25,7 @@ final class CannotPreloadImage implements Error
      * @param Element|null $element Optional. Image element that has the srcset attribute, or null if no element.
      * @return self
      */
-    public static function fromImageWithSrcsetAttribute(Element $element = null)
+    public static function fromImageWithSrcsetAttribute(?Element $element = null)
     {
         $message = self::SRCSET_STRING;
 
diff --git a/src/Optimizer/TransformationEngine.php b/src/Optimizer/TransformationEngine.php
index 512aab7c1..192470a86 100644
--- a/src/Optimizer/TransformationEngine.php
+++ b/src/Optimizer/TransformationEngine.php
@@ -51,14 +51,14 @@ final class TransformationEngine
      * @param Configuration|null    $configuration Optional. Configuration data to use for setting up the transformers.
      * @param RemoteGetRequest|null $remoteRequest Optional. Transport to use for remote requests. Defaults to the
      *                                             CurlRemoteGetRequest implementation shipped with the library.
-     * @param Spec                  $spec          Optional. Validator spec instance to use.
+     * @param Spec|null             $spec          Optional. Validator spec instance to use.
      */
     public function __construct(
-        Configuration $configuration = null,
-        RemoteGetRequest $remoteRequest = null,
-        Spec $spec = null
+        ?Configuration $configuration = null,
+        ?RemoteGetRequest $remoteRequest = null,
+        ?Spec $spec = null
     ) {
-        $this->configuration = isset($configuration) ? $configuration : new DefaultConfiguration();
+        $this->configuration = $configuration ?? new DefaultConfiguration();
         $this->remoteRequest = $remoteRequest;
         $this->spec          = $spec;
 
diff --git a/src/Url.php b/src/Url.php
index 0b30c09b3..76228c6be 100644
--- a/src/Url.php
+++ b/src/Url.php
@@ -138,7 +138,7 @@ final class Url
      * @param Url|null    $baseUrl Base URL.
      * @throws FailedToParseUrl Exception when the URL or Base URL is malformed.
      */
-    public function __construct($url = null, Url $baseUrl = null)
+    public function __construct($url = null, ?Url $baseUrl = null)
     {
         $parsedUrl = [];
 
diff --git a/src/Validator/ValidationHandler.php b/src/Validator/ValidationHandler.php
index 339c635d2..1a098f570 100644
--- a/src/Validator/ValidationHandler.php
+++ b/src/Validator/ValidationHandler.php
@@ -56,7 +56,7 @@ final class ValidationHandler implements HtmlSaxHandlerWithLocation
      */
     private $validationResult;
 
-    public function __construct($htmlFormat = Format::AMP, Spec $spec = null)
+    public function __construct($htmlFormat = Format::AMP, ?Spec $spec = null)
     {
         $this->htmlFormat       = $htmlFormat;
         $this->spec             = $spec instanceof Spec ? $spec : new Spec();
diff --git a/src/Validator/ValidationResult.php b/src/Validator/ValidationResult.php
index d301ae2ea..96313c4e4 100644
--- a/src/Validator/ValidationResult.php
+++ b/src/Validator/ValidationResult.php
@@ -85,8 +85,8 @@ final class ValidationResult
      *                                                        array.
      */
     public function __construct(
-        ValidationStatus $status = null,
-        ValidationErrorCollection $errors = null,
+        ?ValidationStatus $status = null,
+        ?ValidationErrorCollection $errors = null,
         $specRevision = -1,
         $transformerVersion = 0,
         $typeIdentifiers = [],
diff --git a/src/Validator/ValidatorRules.php b/src/Validator/ValidatorRules.php
index 1ad0a507d..2cc8aeb90 100644
--- a/src/Validator/ValidatorRules.php
+++ b/src/Validator/ValidatorRules.php
@@ -72,7 +72,7 @@ final class ValidatorRules
      * @param string    $htmlFormat Optional. AMP HTML format to validate against.
      * @param Spec|null $spec       Optional. Validator specification to use.
      */
-    public function __construct($htmlFormat = Format::AMP, Spec $spec = null)
+    public function __construct($htmlFormat = Format::AMP, ?Spec $spec = null)
     {
         $this->htmlFormat = $htmlFormat;
         $this->spec       = $spec instanceof Spec ? $spec : new Spec();


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 681320f99..7650042be 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -12,15 +12,15 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php: ['8.2', '8.1', '7.4', '7.3', '7.2', '7.1', '7.0', '5.6']
+        php: ['8.3', '8.1', '8.0', '7.4']
         coverage: [false]
         random: [false]
         include:
-          - php: '8.0'
+          - php: '8.2'
             coverage: true
-          - php: '8.0'
+          - php: '8.2'
             random: true
-          - php: '8.3'
+          - php: '8.4'
             experimental: true
 
     steps:
diff --git a/tests/Optimizer/TransformationEngineTest.php b/tests/Optimizer/TransformationEngineTest.php
index 3b8ae3f43..a0d2237d6 100644
--- a/tests/Optimizer/TransformationEngineTest.php
+++ b/tests/Optimizer/TransformationEngineTest.php
@@ -132,7 +132,7 @@ public function testDependencyResolution($transformerClass)
      * @param Configuration|null $configuration Optional. Configuration object to use.
      * @return TransformationEngine Transformation engine instance to test against.
      */
-    private function getTransformationEngine(Configuration $configuration = null)
+    private function getTransformationEngine(?Configuration $configuration = null)
     {
         return new TransformationEngine(
             $configuration,