diff --git a/.gitattributes b/.gitattributes index 3dc7af676..3f9ce563b 100644 --- a/.gitattributes +++ b/.gitattributes @@ -18,6 +18,8 @@ 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 # # 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 538875b08..1909eae8c 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: 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/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/phpstan-bootstrap.php b/phpstan-bootstrap.php new file mode 100644 index 000000000..3bd12dc75 --- /dev/null +++ b/phpstan-bootstrap.php @@ -0,0 +1,14 @@ + 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/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 11fe2f6f6..6668d0bf7 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -829,6 +829,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-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`) */ 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..87d24985a 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]); @@ -825,10 +824,22 @@ public function testBadIP() { Requests::get('http://256.256.256.0/', [], $this->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.'); - return; } $request = Requests::get($this->httpbin('/get', true), [], $this->getOptions()); @@ -841,7 +852,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 +863,6 @@ public function testRevokedHTTPS() { if ($this->skip_https) { $this->markTestSkipped('SSL support is not available.'); - return; } $this->expectException(Exception::class); @@ -866,7 +875,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 +884,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 +900,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 +931,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;