Improve type inference for public method return types

Also marks the class soft-final in preparation for the next major. Because the `encoding` constructor argument is now marked as `non-empty-string`, defensive type checks now emit a psalm issue which has been added to the baseline Signed-off-by: George Steel <[email protected]>
laminas · Dec 17, 2024 · ef058a3 · ef058a3
1 parent 9baa082
commit ef058a3
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 21 deletions.
diff --git a/composer.json b/composer.json
@@ -34,12 +34,12 @@
         "ext-mbstring": "*"
     },
     "require-dev": {
-        "infection/infection": "^0.27.9",
-        "laminas/laminas-coding-standard": "~3.0.0",
+        "infection/infection": "^0.27.11",
+        "laminas/laminas-coding-standard": "~3.0.1",
         "maglnet/composer-require-checker": "^3.8.0",
-        "phpunit/phpunit": "^9.6.16",
+        "phpunit/phpunit": "^9.6.22",
         "psalm/plugin-phpunit": "^0.19.0",
-        "vimeo/psalm": "^5.21.1"
+        "vimeo/psalm": "^5.26.1"
     },
     "autoload": {
         "psr-4": {

diff --git a/composer.lock b/composer.lock
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,2 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.9.0@8b9ad1eb9e8b7d3101f949291da2b9f7767cd163"/>
+<files psalm-version="5.26.1@d747f6500b38ac4f7dfc5edbcae6e4b637d7add0">
+  <file src="src/Escaper.php">
+    <TypeDoesNotContainType>
+      <code><![CDATA[$encoding === '']]></code>
+    </TypeDoesNotContainType>
+  </file>
+</files>
diff --git a/src/Escaper.php b/src/Escaper.php
@@ -25,6 +25,8 @@
 
 /**
  * Context specific methods for use in secure output escaping
+ *
+ * @final
  */
 class Escaper
 {
@@ -49,7 +51,7 @@ class Escaper
      * Current encoding for escaping. If not UTF-8, we convert strings from this encoding
      * pre-escaping and back to this encoding post-escaping.
      *
-     * @var string
+     * @var non-empty-string
      */
     protected $encoding = 'utf-8';
 
@@ -88,7 +90,7 @@ class Escaper
     /**
      * List of all encoding supported by this class
      *
-     * @var array
+     * @var list<non-empty-string>
      */
     protected $supportedEncodings = [
         'iso-8859-1',
@@ -131,6 +133,7 @@ class Escaper
      * Constructor: Single parameter allows setting of global encoding for use by
      * the current object.
      *
+     * @param non-empty-string|null $encoding
      * @throws Exception\InvalidArgumentException
      */
     public function __construct(?string $encoding = null)
@@ -171,7 +174,7 @@ public function __construct(?string $encoding = null)
     /**
      * Return the encoding that all output/input is expected to be encoded in.
      *
-     * @return string
+     * @return non-empty-string
      */
     public function getEncoding()
     {
@@ -182,7 +185,7 @@ public function getEncoding()
      * Escape a string for the HTML Body context where there are very few characters
      * of special meaning. Internally this will use htmlspecialchars().
      *
-     * @return string
+     * @return ($string is non-empty-string ? non-empty-string : string)
      */
     public function escapeHtml(string $string)
     {
@@ -194,7 +197,7 @@ public function escapeHtml(string $string)
      * to escape that are not covered by htmlspecialchars() to cover cases where an attribute
      * might be unquoted or quoted illegally (e.g. backticks are valid quotes for IE).
      *
-     * @return string
+     * @return ($string is non-empty-string ? non-empty-string : string)
      */
     public function escapeHtmlAttr(string $string)
     {
@@ -216,7 +219,7 @@ public function escapeHtmlAttr(string $string)
      * Backslash escaping is not used as it still leaves the escaped character as-is and so
      * is not useful in a HTML context.
      *
-     * @return string
+     * @return ($string is non-empty-string ? non-empty-string : string)
      */
     public function escapeJs(string $string)
     {
@@ -234,7 +237,7 @@ public function escapeJs(string $string)
      * an entire URI - only a subcomponent being inserted. The function is a simple proxy
      * to rawurlencode() which now implements RFC 3986 since PHP 5.3 completely.
      *
-     * @return string
+     * @return ($string is non-empty-string ? non-empty-string : string)
      */
     public function escapeUrl(string $string)
     {
@@ -245,7 +248,7 @@ public function escapeUrl(string $string)
      * Escape a string for the CSS context. CSS escaping can be applied to any string being
      * inserted into CSS and escapes everything except alphanumerics.
      *
-     * @return string
+     * @return ($string is non-empty-string ? non-empty-string : string)
      */
     public function escapeCss(string $string)
     {

diff --git a/test/EscaperTest.php b/test/EscaperTest.php
@@ -25,10 +25,11 @@ protected function setUp(): void
     public function testSettingEncodingToEmptyStringShouldThrowException(): void
     {
         $this->expectException(InvalidArgumentException::class);
+        /** @psalm-suppress InvalidArgument */
         new Escaper('');
     }
 
-    /** @return array<array-key, array{0: string}> */
+    /** @return array<array-key, array{0: non-empty-string}> */
     public function supportedEncodingsProvider(): array
     {
         return [
@@ -70,6 +71,7 @@ public function supportedEncodingsProvider(): array
     }
 
     /**
+     * @param non-empty-string $encoding
      * @dataProvider supportedEncodingsProvider
      */
     public function testSettingValidEncodingShouldNotThrowExceptions(string $encoding): void

diff --git a/test/StaticAnalysis/EscaperReturnTypeChecks.php b/test/StaticAnalysis/EscaperReturnTypeChecks.php
@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+namespace LaminasTest\Escaper\StaticAnalysis;
+
+use Laminas\Escaper\Escaper;
+
+/** @psalm-suppress UnusedClass */
+final class EscaperReturnTypeChecks
+{
+    public function __construct(private readonly Escaper $escaper)
+    {
+    }
+
+    /** @return non-empty-string */
+    public function escapeHtmlReturnsNonEmptyString(): string
+    {
+        return $this->escaper->escapeHtml('Not Empty');
+    }
+
+    public function escapeHtmlReturnsEmptyString(): string
+    {
+        return $this->escaper->escapeHtml('');
+    }
+
+    /** @return non-empty-string */
+    public function escapeJsReturnsNonEmptyString(): string
+    {
+        return $this->escaper->escapeJs('Not Empty');
+    }
+
+    public function escapeJsReturnsEmptyString(): string
+    {
+        return $this->escaper->escapeJs('');
+    }
+
+    /** @return non-empty-string */
+    public function escapeCssReturnsNonEmptyString(): string
+    {
+        return $this->escaper->escapeCss('Not Empty');
+    }
+
+    public function escapeCssReturnsEmptyString(): string
+    {
+        return $this->escaper->escapeCss('');
+    }
+
+    /** @return non-empty-string */
+    public function escapeAttributeReturnsNonEmptyString(): string
+    {
+        return $this->escaper->escapeHtmlAttr('Not Empty');
+    }
+
+    public function escapeAttributeReturnsEmptyString(): string
+    {
+        return $this->escaper->escapeHtmlAttr('');
+    }
+}