From ef058a37bb5909262ecd4f93e1e4532fe4bb4bd6 Mon Sep 17 00:00:00 2001 From: George Steel Date: Tue, 17 Dec 2024 17:11:14 +0000 Subject: [PATCH] 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 --- composer.json | 8 +-- composer.lock | 14 ++--- psalm-baseline.xml | 8 ++- src/Escaper.php | 19 +++--- test/EscaperTest.php | 4 +- .../EscaperReturnTypeChecks.php | 59 +++++++++++++++++++ 6 files changed, 91 insertions(+), 21 deletions(-) create mode 100644 test/StaticAnalysis/EscaperReturnTypeChecks.php diff --git a/composer.json b/composer.json index 38f38695..c9e65710 100644 --- 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 index 82b639c2..80c65532 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d386c6ea579a206d78011080a93d2e39", + "content-hash": "54f8e2257bf338268cf98993b23565f9", "packages": [], "packages-dev": [ { @@ -3784,16 +3784,16 @@ }, { "name": "spatie/array-to-xml", - "version": "3.3.0", + "version": "3.4.0", "source": { "type": "git", "url": "https://github.com/spatie/array-to-xml.git", - "reference": "f56b220fe2db1ade4c88098d83413ebdfc3bf876" + "reference": "7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/spatie/array-to-xml/zipball/f56b220fe2db1ade4c88098d83413ebdfc3bf876", - "reference": "f56b220fe2db1ade4c88098d83413ebdfc3bf876", + "url": "https://api.github.com/repos/spatie/array-to-xml/zipball/7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67", + "reference": "7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67", "shasum": "" }, "require": { @@ -3836,7 +3836,7 @@ "xml" ], "support": { - "source": "https://github.com/spatie/array-to-xml/tree/3.3.0" + "source": "https://github.com/spatie/array-to-xml/tree/3.4.0" }, "funding": [ { @@ -3848,7 +3848,7 @@ "type": "github" } ], - "time": "2024-05-01T10:20:27+00:00" + "time": "2024-12-16T12:45:15+00:00" }, { "name": "squizlabs/php_codesniffer", diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 09f782db..5fba7ffb 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,2 +1,8 @@ - + + + + + + + diff --git a/src/Escaper.php b/src/Escaper.php index a20120eb..1e5bc7f9 100644 --- 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 */ 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 index b97404db..496205ca 100644 --- 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 */ + /** @return array */ 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 new file mode 100644 index 00000000..d937f05a --- /dev/null +++ b/test/StaticAnalysis/EscaperReturnTypeChecks.php @@ -0,0 +1,59 @@ +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(''); + } +}