From 9f55a625b523eb91c3c9e86358435cc22de01c6b Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Fri, 25 Sep 2026 23:23:27 -0700 Subject: [PATCH 1/7] Add PHPStan configuration at level 5 for the src directory Adds a minimal phpstan.neon.dist (level 5, paths: src, treatPhpDocTypesAsCertain: false) and a `composer phpstan` script. PHPStan is deliberately not added to require-dev: it needs PHP 7.4+ while this package supports PHP 5.6+, and the lint, test and quicktest workflows run `composer install` on PHP 5.6 and 7.0, where the dependency cannot resolve. The script therefore expects `phpstan` on the PATH (setup-php's `tools:` in CI, phive or a global Composer install locally), the same way the docs workflow provides phpdoc. Co-Authored-By: Claude Fable 5.1 --- composer.json | 6 +++++- phpstan.neon.dist | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 phpstan.neon.dist diff --git a/composer.json b/composer.json index bb441a1c3..788355c72 100644 --- a/composer.json +++ b/composer.json @@ -99,6 +99,9 @@ ], "coverage10": [ "@php ./vendor/phpunit/phpunit/phpunit -c phpunit10.xml.dist" + ], + "phpstan": [ + "phpstan analyse --memory-limit=1G" ] }, "scripts-descriptions": { @@ -108,6 +111,7 @@ "test": "Run the unit tests on PHPUnit 5.x - 9.x without code coverage.", "test10": "Run the unit tests on PHPUnit 10.x without code coverage.", "coverage": "Run the unit tests on PHPUnit 5.x - 9.x with code coverage.", - "coverage10": "Run the unit tests on PHPUnit 10.x with code coverage." + "coverage10": "Run the unit tests on PHPUnit 10.x with code coverage.", + "phpstan": "Run PHPStan static analysis on the src directory. Requires the phpstan executable on the PATH (PHPStan needs PHP 7.4+, so it is not a Composer dev dependency of this PHP 5.6+ package)." } } diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 000000000..3dfef054f --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,5 @@ +parameters: + level: 5 + paths: + - src + treatPhpDocTypesAsCertain: false From 2249e3c3328cabb2973e40ea377f96e86336a398 Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Fri, 25 Sep 2026 23:23:27 -0700 Subject: [PATCH 2/7] PHPStan: fix four type findings and baseline the remainder Fixes reported at level 5: - Iri::get_authority() is documented as returning string but returns null when the IRI has no authority, matching get_iauthority(). The docblock and the `authority` / `iauthority` magic properties now say string|null. - Iri::$port holds an int (set_port() casts, tests assert 80), not the string its @var and the `port` magic property claimed. - Iri::set_authority() compared the substr() result with false, which can only happen on PHP < 8.0; cast to string so the empty-port check works the same on every supported version. - Requests::parse_multiple() documented its by-reference $response as string, but overwrites it with a Response or the parsing Exception. The remaining 35 findings are docblock drift, PHP-version stub mismatches for cURL resources on PHP < 8.0, and undefined-variable false positives in Iri's UTF-8 decoding loop. They are recorded in phpstan-baseline.neon so the run is clean and new findings surface. Co-Authored-By: Claude Fable 5.1 --- phpstan-baseline.neon | 121 ++++++++++++++++++++++++++++++++++++++++++ phpstan.neon.dist | 3 ++ src/Iri.php | 15 +++--- src/Requests.php | 2 +- 4 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 phpstan-baseline.neon diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon new file mode 100644 index 000000000..b2f56ae22 --- /dev/null +++ b/phpstan-baseline.neon @@ -0,0 +1,121 @@ +parameters: + ignoreErrors: + - + message: '#^Parameter \#1 \$callback of function spl_autoload_register expects \(callable\(string\)\: void\)\|null, array\{''WpOrg\\\\Requests\\\\Autoload'', ''load''\} given\.$#' + identifier: argument.type + count: 1 + path: src/Autoload.php + + - + message: '#^Strict comparison using \!\=\= between true and true will always evaluate to false\.$#' + identifier: notIdentical.alwaysFalse + count: 1 + path: src/Autoload.php + + - + message: '#^Unsafe usage of new static\(\)\.$#' + identifier: new.static + count: 1 + path: src/Cookie.php + + - + message: '#^WpOrg\\Requests\\Utility\\CaseInsensitiveDictionary does not accept string\|true\.$#' + identifier: offsetAssign.valueType + count: 1 + path: src/Cookie.php + + - + message: '#^PHPDoc type bool\|int of property WpOrg\\Requests\\Exception\\Http\\StatusUnknown\:\:\$code is not covariant with PHPDoc type int of overridden property WpOrg\\Requests\\Exception\\Http\:\:\$code\.$#' + identifier: property.phpDocType + count: 1 + path: src/Exception/Http/StatusUnknown.php + + - + message: '#^PHPDoc tag @internal has invalid value \(\(Testing found regex was the fastest implementation\)\)\: Unexpected token "found", expected ''\)'' at offset 100 on line 4$#' + identifier: phpDoc.parseError + count: 1 + path: src/IdnaEncoder.php + + - + message: '#^Left side of && is always true\.$#' + identifier: booleanAnd.leftAlwaysTrue + count: 1 + path: src/Iri.php + + - + message: '#^Property WpOrg\\Requests\\Iri\:\:\$ifragment \(string\) does not accept null\.$#' + identifier: assign.propertyType + count: 3 + path: src/Iri.php + + - + message: '#^Variable \$character might not be defined\.$#' + identifier: variable.undefined + count: 5 + path: src/Iri.php + + - + message: '#^Variable \$length might not be defined\.$#' + identifier: variable.undefined + count: 3 + path: src/Iri.php + + - + message: '#^Variable \$start might not be defined\.$#' + identifier: variable.undefined + count: 3 + path: src/Iri.php + + - + message: '#^Variable \$valid might not be defined\.$#' + identifier: variable.undefined + count: 1 + path: src/Iri.php + + - + message: '#^Strict comparison using \!\=\= between mixed and false will always evaluate to true\.$#' + identifier: notIdentical.alwaysTrue + count: 1 + path: src/Requests.php + + - + message: '#^Property WpOrg\\Requests\\Response\:\:\$cookies \(WpOrg\\Requests\\Cookie\\Jar\) does not accept default value of type array\.$#' + identifier: property.defaultValue + count: 1 + path: src/Response.php + + - + message: '#^Property WpOrg\\Requests\\Response\:\:\$headers \(WpOrg\\Requests\\Response\\Headers\) does not accept default value of type array\.$#' + identifier: property.defaultValue + count: 1 + path: src/Response.php + + - + message: '#^Parameter \#1 \$handle of function curl_close expects CurlHandle, resource given\.$#' + identifier: argument.type + count: 2 + path: src/Transport/Curl.php + + - + message: '#^Parameter \#3 \$value of function curl_setopt expects bool, int given\.$#' + identifier: argument.type + count: 2 + path: src/Transport/Curl.php + + - + message: '#^Parameter \#3 \$value of function curl_setopt expects int, float given\.$#' + identifier: argument.type + count: 4 + path: src/Transport/Curl.php + + - + message: '#^Property WpOrg\\Requests\\Transport\\Curl\:\:\$handle \(CurlHandle\|resource\) is never assigned resource so it can be removed from the property type\.$#' + identifier: property.unusedType + count: 1 + path: src/Transport/Curl.php + + - + message: '#^Right side of && is always true\.$#' + identifier: booleanAnd.rightAlwaysTrue + count: 1 + path: src/Transport/Fsockopen.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 3dfef054f..29da6a7eb 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,3 +1,6 @@ +includes: + - phpstan-baseline.neon + parameters: level: 5 paths: diff --git a/src/Iri.php b/src/Iri.php index ac5ecbeb5..eb6ac8bf5 100644 --- a/src/Iri.php +++ b/src/Iri.php @@ -57,13 +57,13 @@ * @property string $iri IRI we're working with * @property-read string $uri IRI in URI form, {@see \WpOrg\Requests\Iri::to_uri()} * @property string $scheme Scheme part of the IRI - * @property string $authority Authority part, formatted for a URI (userinfo + host + port) - * @property string $iauthority Authority part of the IRI (userinfo + host + port) + * @property string|null $authority Authority part, formatted for a URI (userinfo + host + port) + * @property string|null $iauthority Authority part of the IRI (userinfo + host + port) * @property string $userinfo Userinfo part, formatted for a URI (after '://' and before '@') * @property string $iuserinfo Userinfo part of the IRI (after '://' and before '@') * @property string $host Host part, formatted for a URI * @property string $ihost Host part of the IRI - * @property string $port Port part of the IRI (after ':') + * @property int|null $port Port part of the IRI (after ':') * @property string $path Path part, formatted for a URI (after first '/') * @property string $ipath Path part of the IRI (after first '/') * @property string $query Query part, formatted for a URI (after '?') @@ -96,7 +96,7 @@ class Iri { /** * Port * - * @var string|null + * @var int|null */ protected $port = null; @@ -830,8 +830,9 @@ protected function set_authority($authority) { } if (($port_start = strpos($remaining, ':', (strpos($remaining, ']') ?: 0))) !== false) { - $port = substr($remaining, $port_start + 1); - if ($port === false || $port === '') { + // substr() returns false instead of '' on PHP < 8.0, hence the cast. + $port = (string) substr($remaining, $port_start + 1); + if ($port === '') { $port = null; } $remaining = substr($remaining, 0, $port_start); @@ -1092,7 +1093,7 @@ protected function get_iauthority() { /** * Get the complete authority * - * @return string + * @return string|null Null when the IRI has no authority component. */ protected function get_authority() { $iauthority = $this->get_iauthority(); diff --git a/src/Requests.php b/src/Requests.php index 11fe2f6f6..d2018ee21 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -828,7 +828,7 @@ protected static function parse_response($headers, $url, $req_headers, $req_data * * `$response` is either set to a \WpOrg\Requests\Response instance, or a \WpOrg\Requests\Exception object * - * @param string $response Full response text including headers and body (will be overwritten with Response instance) + * @param string|\WpOrg\Requests\Response|\WpOrg\Requests\Exception $response Full response text including headers and body (will be overwritten with a Response instance, or with the Exception thrown while parsing) * @param array $request Request data as passed into {@see \WpOrg\Requests\Requests::request_multiple()} * @return void */ From e753e78fdf52c96b29710a2f7d170519f06e39f2 Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Fri, 25 Sep 2026 23:23:27 -0700 Subject: [PATCH 3/7] CI: run PHPStan in the CS workflow Adds a `phpstan` job alongside PHPCS. PHPStan itself comes from setup-php's `tools:` input, since it cannot be a Composer dev dependency of a PHP 5.6+ package; Composer dependencies are still installed so PHPStan can use the project autoloader. The github error format annotates findings inline on pull requests. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/cs.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.github/workflows/cs.yml b/.github/workflows/cs.yml index 538875b08..2431ebafd 100644 --- a/.github/workflows/cs.yml +++ b/.github/workflows/cs.yml @@ -79,3 +79,36 @@ jobs: - name: Show PHPCS results in PR if: ${{ always() && steps.phpcs.outcome == 'failure' }} run: cs2pr ./phpcs-report.xml + + phpstan: #---------------------------------------------------------------------- + name: 'PHPStan' + runs-on: ubuntu-latest + permissions: + contents: read # Needed to clone the repo. + + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install PHP + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 + with: + php-version: 'latest' + coverage: none + # PHPStan needs PHP 7.4+, so it is installed as a tool rather than + # as a Composer dev dependency of this PHP 5.6+ package. + tools: phpstan + + # Install dependencies and handle caching in one go. + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer + - name: Install Composer dependencies + uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # 4.0.0 + with: + # Bust the cache at least once a month - output format: YYYY-MM. + custom-cache-suffix: $(date -u "+%Y-%m") + + # Run static analysis. The github error format annotates findings inline in PRs. + - name: Run PHPStan + run: composer phpstan -- --no-progress --error-format=github From 4375c5fcc3b1f1a6347b68a2119f92b84394a7ef Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Sat, 26 Sep 2026 15:26:51 -0700 Subject: [PATCH 4/7] PHPStan: drop the baseline, analyse all code, list ignores explicitly Addresses the review feedback on the initial setup: - Remove phpstan-baseline.neon. Every remaining finding is either fixed (following commits) or listed under `ignoreErrors` in phpstan.neon.dist with an explanation of why it is ignored. - Analyse build/ghpages, examples, library, src and tests with tests/bootstrap.php as a bootstrap file, excluding build/ghpages/vendor. - Exclude src/Iri.php from analysis: it is an externally maintained file, as its class docblock notes. The earlier docblock and comparison changes to it are reverted. - Remove the `composer phpstan` script: PHPStan is not installed via Composer, so the script would only fail. CONTRIBUTING.md now documents downloading the PHAR and running it locally instead. - Run `phpstan analyse` directly in the CI job. - Add phpstan.neon.dist to the .gitattributes export-ignore list and the phpstan.neon local override to .gitignore. Co-Authored-By: Claude Fable 5.1 --- .gitattributes | 1 + .github/CONTRIBUTING.md | 16 ++++ .github/workflows/cs.yml | 2 +- .gitignore | 3 + composer.json | 6 +- phpstan-baseline.neon | 121 ----------------------------- phpstan.neon.dist | 161 ++++++++++++++++++++++++++++++++++++++- src/Iri.php | 15 ++-- 8 files changed, 187 insertions(+), 138 deletions(-) delete mode 100644 phpstan-baseline.neon diff --git a/.gitattributes b/.gitattributes index 3dc7af676..aecb70b5e 100644 --- a/.gitattributes +++ b/.gitattributes @@ -18,6 +18,7 @@ tests/ export-ignore phpdoc.dist.xml export-ignore phpunit.xml.dist export-ignore phpunit10.xml.dist export-ignore +phpstan.neon.dist export-ignore # # Auto detect text files and perform LF normalization diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index c107acbe5..eb32a8b12 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -72,6 +72,22 @@ This project uses [PHP_CodeSniffer][] to detect coding standard violations and a [PHP_CodeSniffer]: https://github.com/PHPCSStandards/PHP_CodeSniffer +## Static Analysis + +This project uses [PHPStan][] for static analysis. The configuration lives in `phpstan.neon.dist`; findings which are known false positives or deliberate are listed under `ignoreErrors` in that file, each with an explanation. + +PHPStan requires PHP 7.4 or higher, while this library supports PHP 5.6 and higher, so it is not installed via Composer. +To run it locally, download the PHAR file and run it from the root of the repository: + +```sh +curl -sSLo phpstan.phar https://github.com/phpstan/phpstan/releases/latest/download/phpstan.phar +php phpstan.phar analyse +``` + +A `phpstan.neon` file can be used for local overrides; it is ignored by Git. + +[PHPStan]: https://phpstan.org/ + ## Unit Tests PRs should include unit tests for all changes. diff --git a/.github/workflows/cs.yml b/.github/workflows/cs.yml index 2431ebafd..1909eae8c 100644 --- a/.github/workflows/cs.yml +++ b/.github/workflows/cs.yml @@ -111,4 +111,4 @@ jobs: # Run static analysis. The github error format annotates findings inline in PRs. - name: Run PHPStan - run: composer phpstan -- --no-progress --error-format=github + run: phpstan analyse --error-format=github diff --git a/.gitignore b/.gitignore index 055b75f0c..1cc495a4d 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,9 @@ phpcs.xml phpunit.xml phpunit10.xml +# Ignore local overrides of the PHPStan config file. +phpstan.neon + # Ignore temporary files for ghpages builds. phpdoc.xml build/ghpages/.phpdoc diff --git a/composer.json b/composer.json index 788355c72..bb441a1c3 100644 --- a/composer.json +++ b/composer.json @@ -99,9 +99,6 @@ ], "coverage10": [ "@php ./vendor/phpunit/phpunit/phpunit -c phpunit10.xml.dist" - ], - "phpstan": [ - "phpstan analyse --memory-limit=1G" ] }, "scripts-descriptions": { @@ -111,7 +108,6 @@ "test": "Run the unit tests on PHPUnit 5.x - 9.x without code coverage.", "test10": "Run the unit tests on PHPUnit 10.x without code coverage.", "coverage": "Run the unit tests on PHPUnit 5.x - 9.x with code coverage.", - "coverage10": "Run the unit tests on PHPUnit 10.x with code coverage.", - "phpstan": "Run PHPStan static analysis on the src directory. Requires the phpstan executable on the PATH (PHPStan needs PHP 7.4+, so it is not a Composer dev dependency of this PHP 5.6+ package)." + "coverage10": "Run the unit tests on PHPUnit 10.x with code coverage." } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon deleted file mode 100644 index b2f56ae22..000000000 --- a/phpstan-baseline.neon +++ /dev/null @@ -1,121 +0,0 @@ -parameters: - ignoreErrors: - - - message: '#^Parameter \#1 \$callback of function spl_autoload_register expects \(callable\(string\)\: void\)\|null, array\{''WpOrg\\\\Requests\\\\Autoload'', ''load''\} given\.$#' - identifier: argument.type - count: 1 - path: src/Autoload.php - - - - message: '#^Strict comparison using \!\=\= between true and true will always evaluate to false\.$#' - identifier: notIdentical.alwaysFalse - count: 1 - path: src/Autoload.php - - - - message: '#^Unsafe usage of new static\(\)\.$#' - identifier: new.static - count: 1 - path: src/Cookie.php - - - - message: '#^WpOrg\\Requests\\Utility\\CaseInsensitiveDictionary does not accept string\|true\.$#' - identifier: offsetAssign.valueType - count: 1 - path: src/Cookie.php - - - - message: '#^PHPDoc type bool\|int of property WpOrg\\Requests\\Exception\\Http\\StatusUnknown\:\:\$code is not covariant with PHPDoc type int of overridden property WpOrg\\Requests\\Exception\\Http\:\:\$code\.$#' - identifier: property.phpDocType - count: 1 - path: src/Exception/Http/StatusUnknown.php - - - - message: '#^PHPDoc tag @internal has invalid value \(\(Testing found regex was the fastest implementation\)\)\: Unexpected token "found", expected ''\)'' at offset 100 on line 4$#' - identifier: phpDoc.parseError - count: 1 - path: src/IdnaEncoder.php - - - - message: '#^Left side of && is always true\.$#' - identifier: booleanAnd.leftAlwaysTrue - count: 1 - path: src/Iri.php - - - - message: '#^Property WpOrg\\Requests\\Iri\:\:\$ifragment \(string\) does not accept null\.$#' - identifier: assign.propertyType - count: 3 - path: src/Iri.php - - - - message: '#^Variable \$character might not be defined\.$#' - identifier: variable.undefined - count: 5 - path: src/Iri.php - - - - message: '#^Variable \$length might not be defined\.$#' - identifier: variable.undefined - count: 3 - path: src/Iri.php - - - - message: '#^Variable \$start might not be defined\.$#' - identifier: variable.undefined - count: 3 - path: src/Iri.php - - - - message: '#^Variable \$valid might not be defined\.$#' - identifier: variable.undefined - count: 1 - path: src/Iri.php - - - - message: '#^Strict comparison using \!\=\= between mixed and false will always evaluate to true\.$#' - identifier: notIdentical.alwaysTrue - count: 1 - path: src/Requests.php - - - - message: '#^Property WpOrg\\Requests\\Response\:\:\$cookies \(WpOrg\\Requests\\Cookie\\Jar\) does not accept default value of type array\.$#' - identifier: property.defaultValue - count: 1 - path: src/Response.php - - - - message: '#^Property WpOrg\\Requests\\Response\:\:\$headers \(WpOrg\\Requests\\Response\\Headers\) does not accept default value of type array\.$#' - identifier: property.defaultValue - count: 1 - path: src/Response.php - - - - message: '#^Parameter \#1 \$handle of function curl_close expects CurlHandle, resource given\.$#' - identifier: argument.type - count: 2 - path: src/Transport/Curl.php - - - - message: '#^Parameter \#3 \$value of function curl_setopt expects bool, int given\.$#' - identifier: argument.type - count: 2 - path: src/Transport/Curl.php - - - - message: '#^Parameter \#3 \$value of function curl_setopt expects int, float given\.$#' - identifier: argument.type - count: 4 - path: src/Transport/Curl.php - - - - message: '#^Property WpOrg\\Requests\\Transport\\Curl\:\:\$handle \(CurlHandle\|resource\) is never assigned resource so it can be removed from the property type\.$#' - identifier: property.unusedType - count: 1 - path: src/Transport/Curl.php - - - - message: '#^Right side of && is always true\.$#' - identifier: booleanAnd.rightAlwaysTrue - count: 1 - path: src/Transport/Fsockopen.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 29da6a7eb..1bf9d2519 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,8 +1,163 @@ -includes: - - phpstan-baseline.neon - parameters: level: 5 + bootstrapFiles: + - tests/bootstrap.php paths: + - build/ghpages + - examples + - library - src + - tests + excludePaths: + analyse: + - build/ghpages/vendor (?) + # Externally maintained file (based on the SimplePie IRI class), not held to this project's standards. + - src/Iri.php treatPhpDocTypesAsCertain: false + + ignoreErrors: + # The library supports PHP 5.6+, where cURL handles are resources. PHPStan analyses against the + # PHP 8 stubs, in which curl_close() only accepts a CurlHandle object and the resource type can + # never occur. + - + identifier: argument.type + message: '#function curl_close expects CurlHandle, resource given#' + path: src/Transport/Curl.php + - + identifier: argument.type + message: '#function curl_close expects CurlHandle, resource given#' + path: tests/Utility/InputValidator/IsCurlHandleTest.php + - + identifier: property.unusedType + message: '#is never assigned resource#' + path: src/Transport/Curl.php + - + identifier: property.unusedType + message: '#is never assigned resource#' + path: tests/Utility/InputValidator/IsCurlHandleTest.php + + # OPENSSL_TLSEXT_SERVER_NAME is only defined when OpenSSL is compiled with SNI support; the guard + # is needed on older PHP builds even though it is always true on the PHP version running PHPStan. + - + identifier: booleanAnd.rightAlwaysTrue + path: src/Transport/Fsockopen.php + + # PHPStan's stub for spl_autoload_register() declares the callback as returning void. PHP ignores + # the return value, and the Requests autoloaders return bool to report whether a class was loaded. + - + identifier: argument.type + message: '#function spl_autoload_register expects#' + path: src/Autoload.php + - + identifier: argument.type + message: '#function spl_autoload_register expects#' + path: tests/bootstrap.php + + # REQUESTS_SILENCE_PSR0_DEPRECATIONS is defined by the integrator before the library loads. PHPStan + # resolves it from the define() call in library/Requests.php and treats it as always true. + - + identifier: notIdentical.alwaysFalse + message: '#REQUESTS_SILENCE_PSR0_DEPRECATIONS|between true and true#' + path: src/Autoload.php + - + identifier: notIdentical.alwaysFalse + message: '#between true and true#' + path: library/Requests.php + + # The `cookies` option accepts false per the documentation, but set_defaults() replaces false with an + # empty Jar via the empty() check above, so the guard can never see false. Kept as-is pending a + # decision on whether `cookies => false` should disable cookie handling. + - + identifier: notIdentical.alwaysTrue + path: src/Requests.php + + # The REQUESTS_TEST_SERVER_*_AVAILABLE constants are defined by tests/bootstrap.php from the environment; + # PHPStan sees the values the bootstrap produced on this machine and treats the checks as constant. + - + identifier: booleanAnd.alwaysFalse + path: tests/TestCase.php + - + identifier: identical.alwaysFalse + path: tests/TestCase.php + - + identifier: booleanAnd.rightAlwaysFalse + path: tests/Proxy/Http/HttpTest.php + - + identifier: booleanNot.alwaysTrue + path: tests/Proxy/Http/HttpTest.php + + # PHPUnit version shim: setMethods() exists on PHPUnit < 10 and addMethods() on PHPUnit >= 8. + # PHPStan only sees the installed PHPUnit version. + - + identifier: function.alreadyNarrowedType + path: tests/TestCase.php + - + identifier: method.notFound + message: '#setMethods\(\)#' + path: tests/TestCase.php + + # Tests which deliberately pass invalid input to verify the exception thrown or the resulting behaviour. + - + identifier: argument.type + path: tests/Cookie/ParseTest.php + - + identifier: argument.type + path: tests/Hooks/RegisterTest.php + - + identifier: argument.type + path: tests/Proxy/Http/HttpTest.php + - + identifier: argument.type + path: tests/Utility/FilteredIterator/SerializationTest.php + - + identifier: array.invalidKey + path: tests/Utility/CaseInsensitiveDictionary/* + - + identifier: offsetAssign.dimType + path: tests/* + - + identifier: offsetAssign.valueType + path: tests/Response/Headers/* + - + identifier: assign.propertyType + message: '#Iri::\$host#' + path: tests/Iri/IriTest.php + + # Tests of magic property access (__get/__set/__isset) on properties which intentionally do not exist. + - + identifier: property.notFound + path: tests/Iri/IriTest.php + - + identifier: property.notFound + path: tests/Session/MagicPropertyAccessTest.php + + # Session exposes request options as magic properties through __get()/__set(). + - + identifier: property.notFound + message: '#Session::\$useragent#' + path: examples/session.php + + # The example uses a placeholder path the reader is expected to replace. + - + identifier: requireOnce.fileNotFound + path: examples/preload-aliases.php + + # Assertions on values PHPStan can prove at analysis time. They document the test environment + # (extension availability, constant definitions) rather than exercise logic. + - + identifier: method.alreadyNarrowedType + path: tests/* + + # Tests disabled at the top with markTestSkipped() while their body is kept for reference + # (see issues #966 and #1077). + - + identifier: deadCode.unreachable + path: tests/Transport/BaseTestCase.php + + # Minimal ArrayAccess fixture used only to satisfy type checks; it is never read from. + - + identifier: property.onlyWritten + path: tests/Fixtures/ArrayAccessibleObject.php + - + identifier: return.missing + path: tests/Fixtures/ArrayAccessibleObject.php diff --git a/src/Iri.php b/src/Iri.php index eb6ac8bf5..ac5ecbeb5 100644 --- a/src/Iri.php +++ b/src/Iri.php @@ -57,13 +57,13 @@ * @property string $iri IRI we're working with * @property-read string $uri IRI in URI form, {@see \WpOrg\Requests\Iri::to_uri()} * @property string $scheme Scheme part of the IRI - * @property string|null $authority Authority part, formatted for a URI (userinfo + host + port) - * @property string|null $iauthority Authority part of the IRI (userinfo + host + port) + * @property string $authority Authority part, formatted for a URI (userinfo + host + port) + * @property string $iauthority Authority part of the IRI (userinfo + host + port) * @property string $userinfo Userinfo part, formatted for a URI (after '://' and before '@') * @property string $iuserinfo Userinfo part of the IRI (after '://' and before '@') * @property string $host Host part, formatted for a URI * @property string $ihost Host part of the IRI - * @property int|null $port Port part of the IRI (after ':') + * @property string $port Port part of the IRI (after ':') * @property string $path Path part, formatted for a URI (after first '/') * @property string $ipath Path part of the IRI (after first '/') * @property string $query Query part, formatted for a URI (after '?') @@ -96,7 +96,7 @@ class Iri { /** * Port * - * @var int|null + * @var string|null */ protected $port = null; @@ -830,9 +830,8 @@ protected function set_authority($authority) { } if (($port_start = strpos($remaining, ':', (strpos($remaining, ']') ?: 0))) !== false) { - // substr() returns false instead of '' on PHP < 8.0, hence the cast. - $port = (string) substr($remaining, $port_start + 1); - if ($port === '') { + $port = substr($remaining, $port_start + 1); + if ($port === false || $port === '') { $port = null; } $remaining = substr($remaining, 0, $port_start); @@ -1093,7 +1092,7 @@ protected function get_iauthority() { /** * Get the complete authority * - * @return string|null Null when the IRI has no authority component. + * @return string */ protected function get_authority() { $iauthority = $this->get_iauthority(); From 1581dbe7d1fb24dbd3e916f3d870e7dbb0fe1500 Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Sat, 26 Sep 2026 15:26:51 -0700 Subject: [PATCH 5/7] PHPStan: fix reported type issues in the library - Transport\Curl: pass bool and int values to curl_setopt() for options which expect them (CURLOPT_RETURNTRANSFER, CURLOPT_SSL_VERIFYPEER, and the timeout options, where ceil()/round() return float). - Response: drop the array defaults on $headers and $cookies; the constructor always assigns a Headers and a Jar object, which is what the docblocks declare. - CaseInsensitiveDictionary::offsetSet(): the value is mixed, not string (Cookie stores boolean flag attributes in it). - Cookie: declare @phpstan-consistent-constructor, the contract that `new static()` in Cookie::parse() relies on. - StatusUnknown: the constructor casts the code to int, so document it as int. - IdnaEncoder: @internal takes no parenthesised value; move the note to the description. - Requests::parse_multiple(): document the by-reference overwrite with @param-out instead of widening the input type; the input is always the response string. - library/Requests.php: @var requires a variable name; the constant is described without the tag. Co-Authored-By: Claude Fable 5.1 --- library/Requests.php | 2 -- src/Cookie.php | 2 ++ src/Exception/Http/StatusUnknown.php | 2 +- src/IdnaEncoder.php | 4 +++- src/Requests.php | 3 ++- src/Response.php | 4 ++-- src/Transport/Curl.php | 12 ++++++------ src/Utility/CaseInsensitiveDictionary.php | 2 +- 8 files changed, 17 insertions(+), 14 deletions(-) diff --git a/library/Requests.php b/library/Requests.php index 6d4fc14f7..e7847ace2 100644 --- a/library/Requests.php +++ b/library/Requests.php @@ -25,8 +25,6 @@ /** * Constant to silence deprecation notices about use of the old PSR-0 based class names. - * - * @var bool */ define('REQUESTS_SILENCE_PSR0_DEPRECATIONS', true); } diff --git a/src/Cookie.php b/src/Cookie.php index 9075c25e7..21548d47c 100644 --- a/src/Cookie.php +++ b/src/Cookie.php @@ -20,6 +20,8 @@ * Cookie storage object * * @package Requests\Cookies + * + * @phpstan-consistent-constructor */ class Cookie { /** diff --git a/src/Exception/Http/StatusUnknown.php b/src/Exception/Http/StatusUnknown.php index e142978c3..d97e78986 100644 --- a/src/Exception/Http/StatusUnknown.php +++ b/src/Exception/Http/StatusUnknown.php @@ -21,7 +21,7 @@ final class StatusUnknown extends Http { /** * HTTP status code * - * @var int|bool Code if available, false if an error occurred + * @var int Code if available, 0 if an error occurred */ protected $code = 0; diff --git a/src/IdnaEncoder.php b/src/IdnaEncoder.php index d79846a83..fd90f9a9a 100644 --- a/src/IdnaEncoder.php +++ b/src/IdnaEncoder.php @@ -142,7 +142,9 @@ public static function to_ascii($text) { /** * Check whether a given text string contains only ASCII characters * - * @internal (Testing found regex was the fastest implementation) + * @internal + * + * Testing found regex was the fastest implementation. * * @param string $text Text to examine. * @return bool Is the text string ASCII-only? diff --git a/src/Requests.php b/src/Requests.php index d2018ee21..6668d0bf7 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -828,7 +828,8 @@ protected static function parse_response($headers, $url, $req_headers, $req_data * * `$response` is either set to a \WpOrg\Requests\Response instance, or a \WpOrg\Requests\Exception object * - * @param string|\WpOrg\Requests\Response|\WpOrg\Requests\Exception $response Full response text including headers and body (will be overwritten with a Response instance, or with the Exception thrown while parsing) + * @param string $response Full response text including headers and body (will be overwritten with Response instance) + * @param-out \WpOrg\Requests\Response|\WpOrg\Requests\Exception $response * @param array $request Request data as passed into {@see \WpOrg\Requests\Requests::request_multiple()} * @return void */ diff --git a/src/Response.php b/src/Response.php index 549d0a140..4f518c4f0 100644 --- a/src/Response.php +++ b/src/Response.php @@ -42,7 +42,7 @@ class Response { * * @var \WpOrg\Requests\Response\Headers Array-like object representing headers */ - public $headers = []; + public $headers; /** * Status code, false if non-blocking @@ -91,7 +91,7 @@ class Response { * * @var \WpOrg\Requests\Cookie\Jar Array-like object representing a cookie jar */ - public $cookies = []; + public $cookies; /** * Constructor diff --git a/src/Transport/Curl.php b/src/Transport/Curl.php index 215b77e97..c655322eb 100644 --- a/src/Transport/Curl.php +++ b/src/Transport/Curl.php @@ -108,7 +108,7 @@ public function __construct() { $this->handle = curl_init(); curl_setopt($this->handle, CURLOPT_HEADER, false); - curl_setopt($this->handle, CURLOPT_RETURNTRANSFER, 1); + curl_setopt($this->handle, CURLOPT_RETURNTRANSFER, true); if ($this->version >= self::CURL_7_10_5) { curl_setopt($this->handle, CURLOPT_ENCODING, ''); } @@ -200,7 +200,7 @@ public function request($url, $headers = [], $data = [], $options = []) { if (isset($options['verify'])) { if ($options['verify'] === false) { curl_setopt($this->handle, CURLOPT_SSL_VERIFYHOST, 0); - curl_setopt($this->handle, CURLOPT_SSL_VERIFYPEER, 0); + curl_setopt($this->handle, CURLOPT_SSL_VERIFYPEER, false); } elseif (is_string($options['verify'])) { curl_setopt($this->handle, CURLOPT_CAINFO, $options['verify']); } @@ -451,17 +451,17 @@ private function setup_handle($url, $headers, $data, $options) { $timeout = max($options['timeout'], 1); if (is_int($timeout) || $this->version < self::CURL_7_16_2) { - curl_setopt($this->handle, CURLOPT_TIMEOUT, ceil($timeout)); + curl_setopt($this->handle, CURLOPT_TIMEOUT, (int) ceil($timeout)); } else { // phpcs:ignore PHPCompatibility.Constants.NewConstants.curlopt_timeout_msFound - curl_setopt($this->handle, CURLOPT_TIMEOUT_MS, round($timeout * 1000)); + curl_setopt($this->handle, CURLOPT_TIMEOUT_MS, (int) round($timeout * 1000)); } if (is_int($options['connect_timeout']) || $this->version < self::CURL_7_16_2) { - curl_setopt($this->handle, CURLOPT_CONNECTTIMEOUT, ceil($options['connect_timeout'])); + curl_setopt($this->handle, CURLOPT_CONNECTTIMEOUT, (int) ceil($options['connect_timeout'])); } else { // phpcs:ignore PHPCompatibility.Constants.NewConstants.curlopt_connecttimeout_msFound - curl_setopt($this->handle, CURLOPT_CONNECTTIMEOUT_MS, round($options['connect_timeout'] * 1000)); + curl_setopt($this->handle, CURLOPT_CONNECTTIMEOUT_MS, (int) round($options['connect_timeout'] * 1000)); } curl_setopt($this->handle, CURLOPT_URL, $url); diff --git a/src/Utility/CaseInsensitiveDictionary.php b/src/Utility/CaseInsensitiveDictionary.php index 6e40873d9..6c1d0b84e 100644 --- a/src/Utility/CaseInsensitiveDictionary.php +++ b/src/Utility/CaseInsensitiveDictionary.php @@ -85,7 +85,7 @@ public function offsetGet($offset) { * Set the given item * * @param string $offset Item name - * @param string $value Item value + * @param mixed $value Item value * * @throws \WpOrg\Requests\Exception On attempting to use dictionary as list (`invalidset`) */ From 377aa343719fb20b0614d1130ab7208dcc357c68 Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Sat, 26 Sep 2026 15:26:51 -0700 Subject: [PATCH 6/7] PHPStan: fix reported issues in the test suite - Document the lazily initialised static resource properties in TypeProviderHelper and IsCurlHandleTest as nullable, which is what the isset() guards rely on. - FsockopenTest: query the current locale with the documented '0' string argument. - BaseTestCase: remove the `return` statements after markTestSkipped(), which always throws. - StatusCodeTest: fix a `$` missing from a @param tag. Co-Authored-By: Claude Fable 5.1 --- tests/Exception/Http/StatusCodeTest.php | 2 +- tests/Transport/BaseTestCase.php | 8 -------- tests/Transport/Fsockopen/FsockopenTest.php | 4 ++-- tests/TypeProviderHelper.php | 4 ++-- tests/Utility/InputValidator/IsCurlHandleTest.php | 2 +- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/tests/Exception/Http/StatusCodeTest.php b/tests/Exception/Http/StatusCodeTest.php index 62c993870..bcc3a259a 100644 --- a/tests/Exception/Http/StatusCodeTest.php +++ b/tests/Exception/Http/StatusCodeTest.php @@ -113,7 +113,7 @@ public static function dataUnknownStatusCodes() { * * @dataProvider dataKnownStatusCodes * - * @param int status_code HTTP status code. + * @param int $status_code HTTP status code. * @param string $expected_exception_class Exception class to expect. * * @return void diff --git a/tests/Transport/BaseTestCase.php b/tests/Transport/BaseTestCase.php index bbe6cf40e..40f290206 100644 --- a/tests/Transport/BaseTestCase.php +++ b/tests/Transport/BaseTestCase.php @@ -37,7 +37,6 @@ public function set_up() { if (!$supported) { $this->markTestSkipped($this->transport . ' is not available'); - return; } $ssl_supported = $test_method([Capability::SSL => true]); @@ -828,7 +827,6 @@ public function testBadIP() { public function testHTTPS() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $request = Requests::get($this->httpbin('/get', true), [], $this->getOptions()); @@ -841,7 +839,6 @@ public function testHTTPS() { public function testExpiredHTTPS() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $this->expectException(Exception::class); @@ -853,7 +850,6 @@ public function testRevokedHTTPS() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $this->expectException(Exception::class); @@ -866,7 +862,6 @@ public function testRevokedHTTPS() { public function testBadDomain() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $this->expectException(Exception::class); @@ -876,7 +871,6 @@ public function testBadDomain() { public function testBadDomainNoVerify() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $response = Requests::head('https://wrong.host.badssl.com/', [], $this->getOptions(['verify' => false])); @@ -893,7 +887,6 @@ public function testBadDomainNoVerify() { public function testAlternateNameSupport() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $request = Requests::head('https://badssl.com/', [], $this->getOptions()); @@ -925,7 +918,6 @@ public function testSNISupport($options) { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $request = Requests::head('https://humanmade.com/', [], $this->getOptions($options)); diff --git a/tests/Transport/Fsockopen/FsockopenTest.php b/tests/Transport/Fsockopen/FsockopenTest.php index 36ba7c1dc..60cbb75dc 100644 --- a/tests/Transport/Fsockopen/FsockopenTest.php +++ b/tests/Transport/Fsockopen/FsockopenTest.php @@ -59,13 +59,13 @@ public function checkContentLengthHeader($headers) { */ public function testHTTPVersionHeader() { // Remember the original locale. - $locale = setlocale(LC_NUMERIC, 0); + $locale = setlocale(LC_NUMERIC, '0'); // Set the locale to one using commas for the decimal point. setlocale(LC_NUMERIC, 'de_DE@euro', 'de_DE.utf8', 'de_DE', 'de', 'ge'); // Make sure the locale was changed. - $this->assertNotSame($locale, setlocale(LC_NUMERIC, 0), 'Changing the locale failed'); + $this->assertNotSame($locale, setlocale(LC_NUMERIC, '0'), 'Changing the locale failed'); $hooks = new Hooks(); $hooks->register('fsockopen.after_headers', [$this, 'checkHTTPVersionHeader']); diff --git a/tests/TypeProviderHelper.php b/tests/TypeProviderHelper.php index 0ef907bf7..be770ee30 100644 --- a/tests/TypeProviderHelper.php +++ b/tests/TypeProviderHelper.php @@ -168,14 +168,14 @@ final class TypeProviderHelper { /** * File handle to local memory (open resource). * - * @var resource + * @var resource|null */ private static $memory_handle_open; /** * File handle to local memory (closed resource). * - * @var resource + * @var resource|null */ private static $memory_handle_closed; diff --git a/tests/Utility/InputValidator/IsCurlHandleTest.php b/tests/Utility/InputValidator/IsCurlHandleTest.php index 7c79373b4..8a88be144 100644 --- a/tests/Utility/InputValidator/IsCurlHandleTest.php +++ b/tests/Utility/InputValidator/IsCurlHandleTest.php @@ -14,7 +14,7 @@ final class IsCurlHandleTest extends TestCase { /** * Curl handle. * - * @var resource|\CurlHandle + * @var resource|\CurlHandle|null */ private static $curl_handle; From aa667616937d7257fc2a5e557e942861f8518c45 Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Sat, 26 Sep 2026 20:14:37 -0700 Subject: [PATCH 7/7] PHPStan: silence the PSR-0 deprecation during analysis; cover float timeouts The Composer autoloader registers the autoloader for the deprecated PSR-0 `Requests_*` class names. When PHPStan reflects on those names, which the autoloader tests reference, it triggers an E_USER_DEPRECATED notice that PHPStan reports as an internal error, so the CI job exited non-zero on a fresh result cache. A PHPStan-only bootstrap file now defines REQUESTS_SILENCE_PSR0_DEPRECATIONS, the documented way to silence that notice, before tests/bootstrap.php runs. Also add a transport test with fractional `timeout` and `connect_timeout` values, which exercises the CURLOPT_TIMEOUT_MS and CURLOPT_CONNECTTIMEOUT_MS branches touched in the previous commit and which had no coverage. Co-Authored-By: Claude Fable 5.1 --- .gitattributes | 1 + phpstan-bootstrap.php | 14 ++++++++++++++ phpstan.neon.dist | 1 + tests/Transport/BaseTestCase.php | 13 +++++++++++++ 4 files changed, 29 insertions(+) create mode 100644 phpstan-bootstrap.php diff --git a/.gitattributes b/.gitattributes index aecb70b5e..3f9ce563b 100644 --- a/.gitattributes +++ b/.gitattributes @@ -18,6 +18,7 @@ tests/ export-ignore phpdoc.dist.xml export-ignore phpunit.xml.dist export-ignore phpunit10.xml.dist export-ignore +phpstan-bootstrap.php export-ignore phpstan.neon.dist export-ignore # diff --git a/phpstan-bootstrap.php b/phpstan-bootstrap.php new file mode 100644 index 000000000..3bd12dc75 --- /dev/null +++ b/phpstan-bootstrap.php @@ -0,0 +1,14 @@ +getOptions()); } + /** + * Verify that fractional timeout values are accepted and the request still succeeds. + */ + public function testFloatTimeoutOptions() { + $options = [ + 'timeout' => 2.5, + 'connect_timeout' => 2.5, + ]; + $request = Requests::get($this->httpbin('/get'), [], $this->getOptions($options)); + + $this->assertSame(200, $request->status_code); + } + public function testHTTPS() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.');