Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update PHPStan #1211

Merged
merged 8 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ jobs:
fail-fast: false
matrix:
php-version:
- "7.1"
- "7.4"
- "8.1"

steps:
- uses: actions/checkout@master
Expand Down Expand Up @@ -102,6 +104,12 @@ jobs:
path: ${{ steps.composer-cache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('composer.json') }}

# We run PHPStan in a separate job, and the version of PHPStan we use is not compatible
# with all the versions we support and run unit tests for. We need to temporarily remove it
# from composer.json here, else composer install will fail due the version incompatibility.
- name: Trim dependency
run: composer remove --dev phpstan/phpstan

- name: Validate composer.json and composer.lock
run: composer validate

Expand Down
6 changes: 1 addition & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ export PHPSTAN_VERSION := 0.12.59
vendor: composer.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want to remove the PHPSTAN_VERSION export above.

composer install

vendor/bin/phpstan: vendor
curl -sfL https://github.com/phpstan/phpstan/releases/download/$(PHPSTAN_VERSION)/phpstan.phar -o vendor/bin/phpstan
chmod +x vendor/bin/phpstan

vendor/bin/phpdoc: vendor
curl -sfL https://github.com/phpDocumentor/phpDocumentor/releases/download/$(PHPDOCUMENTOR_VERSION)/phpDocumentor.phar -o vendor/bin/phpdoc
chmod +x vendor/bin/phpdoc
Expand All @@ -27,7 +23,7 @@ fmtcheck: vendor
phpdoc: vendor/bin/phpdoc
vendor/bin/phpdoc

phpstan: vendor/bin/phpstan
phpstan: vendor
php -d memory_limit=512M vendor/bin/phpstan analyse lib tests
.PHONY: phpstan

Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"phpunit/phpunit": "^5.7 || ^9.0",
"php-coveralls/php-coveralls": "^2.1",
"squizlabs/php_codesniffer": "^3.3",
"friendsofphp/php-cs-fixer": "3.2.1 || 2.17.1"
"friendsofphp/php-cs-fixer": "3.2.1 || 2.17.1",
"phpstan/phpstan": "^1.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this cause issues installing the package with dev dependencies on earlier versions if we're saying it's not compatible with them?

Copy link
Contributor Author

@richardm-stripe richardm-stripe Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I think we should go with a policy of composer install is only supported on php 7.3+. I think this policy is fine because the only users of composer install are

  • CI - which we have already handled with the composer remove workaround in this PR.
  • maintainers/contributors to stripe-php:
    • I am happy to be restricted to php7.2+ when doing everyday development on stripe-php
    • When I am explicitly trying to test compatibility things with <php7.2 it will be definitely an inconvenience to not be able to composer install without manually working around, and beyond that it will be inconvenient to not be able to utilize phpstan/php-cs-fixer, but I think this inconvenience is far outweighed by the additional complexity it would take to continuously maintain compatibility with multiple versions of PHPStan and php-cs-fixer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - let's leave a comment explaining this and steps for running <php 7.2 so that this doesn't come back to haunt us down the road once we've forgotten :)

},
"autoload": {
"psr-4": {
Expand Down
2 changes: 0 additions & 2 deletions lib/ApiOperations/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ protected function _request($method, $url, $params = [], $options = null)
* @param null|array|string $options
*
* @throws \Stripe\Exception\ApiErrorException if the request fails
*
* @return array tuple containing (the JSON response, $options)
*/
protected function _requestStream($method, $url, $readBodyChunk, $params = [], $options = null)
{
Expand Down
2 changes: 0 additions & 2 deletions lib/ApiRequestor.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,6 @@ public function request($method, $url, $params = null, $headers = null)
* @param null|array $headers
*
* @throws Exception\ApiErrorException
*
* @return array tuple containing (ApiReponse, API key)
*/
public function requestStream($method, $url, $readBodyChunkCallable, $params = null, $headers = null)
{
Expand Down
2 changes: 2 additions & 0 deletions lib/ApiResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public static function classUrl()
{
// Replace dots with slashes for namespaced resources, e.g. if the object's name is
// "foo.bar", then its URL will be "/v1/foo/bars".

/** @phpstan-ignore-next-line */
$base = \str_replace('.', '/', static::OBJECT_NAME);

return "/v1/{$base}s";
Expand Down
2 changes: 2 additions & 0 deletions lib/Collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public function retrieve($id, $params = null, $opts = null)
/**
* @return int the number of objects in the current page
*/
#[\ReturnTypeWillChange]
public function count()
{
return \count($this->data);
Expand All @@ -116,6 +117,7 @@ public function count()
* @return \ArrayIterator an iterator that can be used to iterate
* across objects in the current page
*/
#[\ReturnTypeWillChange]
public function getIterator()
{
return new \ArrayIterator($this->data);
Expand Down
2 changes: 2 additions & 0 deletions lib/Customer.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public function deleteDiscount($params = null, $opts = null)
$url = $this->instanceUrl() . '/discount';
list($response, $opts) = $this->_request('delete', $url, $params, $opts);
$this->refreshFrom(['discount' => null], $opts, true);

return $this;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need a codegen change right?

}

/**
Expand Down
2 changes: 0 additions & 2 deletions lib/Quote.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ class Quote extends ApiResource
* @param null|array|string $opts
*
* @throws \Stripe\Exception\ApiErrorException if the request fails
*
* @return \Stripe\File the created file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagging this will also need a codegen change.

*/
public function pdf($readBodyChunkCallable, $params = null, $opts = null)
{
Expand Down
2 changes: 2 additions & 0 deletions lib/SingletonApiResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public static function classUrl()
{
// Replace dots with slashes for namespaced resources, e.g. if the object's name is
// "foo.bar", then its URL will be "/v1/foo/bar".

/** @phpstan-ignore-next-line */
$base = \str_replace('.', '/', static::OBJECT_NAME);

return "/v1/{$base}";
Expand Down
4 changes: 4 additions & 0 deletions lib/StripeObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,19 @@ public function __debugInfo()
}

// ArrayAccess methods
#[\ReturnTypeWillChange]
public function offsetSet($k, $v)
{
$this->{$k} = $v;
}

#[\ReturnTypeWillChange]
public function offsetExists($k)
{
return \array_key_exists($k, $this->_values);
}

#[\ReturnTypeWillChange]
public function offsetUnset($k)
{
unset($this->{$k});
Expand All @@ -215,6 +218,7 @@ public function offsetGet($k)
}

// Countable method
#[\ReturnTypeWillChange]
public function count()
{
return \count($this->_values);
Expand Down
2 changes: 2 additions & 0 deletions lib/Subscription.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,7 @@ public function deleteDiscount($params = null, $opts = null)
$url = $this->instanceUrl() . '/discount';
list($response, $opts) = $this->_request('delete', $url, $params, $opts);
$this->refreshFrom(['discount' => null], $opts, true);

return $this;
}
}
5 changes: 5 additions & 0 deletions lib/Util/CaseInsensitiveArray.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,19 @@ public function __construct($initial_array = [])
$this->container = \array_change_key_case($initial_array, \CASE_LOWER);
}

#[\ReturnTypeWillChange]
public function count()
{
return \count($this->container);
}

#[\ReturnTypeWillChange]
public function getIterator()
{
return new \ArrayIterator($this->container);
}

#[\ReturnTypeWillChange]
public function offsetSet($offset, $value)
{
$offset = static::maybeLowercase($offset);
Expand All @@ -41,13 +44,15 @@ public function offsetSet($offset, $value)
}
}

#[\ReturnTypeWillChange]
public function offsetExists($offset)
{
$offset = static::maybeLowercase($offset);

return isset($this->container[$offset]);
}

#[\ReturnTypeWillChange]
public function offsetUnset($offset)
{
$offset = static::maybeLowercase($offset);
Expand Down
1 change: 1 addition & 0 deletions lib/Util/Set.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public function toArray()
return \array_keys($this->_elts);
}

#[\ReturnTypeWillChange]
public function getIterator()
{
return new ArrayIterator($this->toArray());
Expand Down
12 changes: 0 additions & 12 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -1,12 +0,0 @@
parameters:
ignoreErrors:
-
message: "#^Access to undefined constant Stripe\\\\ApiResource\\:\\:OBJECT_NAME\\.$#"
count: 1
path: lib/ApiResource.php

-
message: "#^Access to undefined constant Stripe\\\\SingletonApiResource\\:\\:OBJECT_NAME\\.$#"
count: 1
path: lib/SingletonApiResource.php

2 changes: 2 additions & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ parameters:

ignoreErrors:
- '#Unsafe usage of new static\(\).#'

reportUnmatchedIgnoredErrors: false
6 changes: 6 additions & 0 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class TestCase extends \PHPUnit\Framework\TestCase
public static function compatAssertIsArray($obj)
{
if (method_exists(static::class, 'assertIsArray')) {
// @phpstan-ignore-next-line
static::assertIsArray($obj);
} else {
// @phpstan-ignore-next-line
Expand All @@ -26,6 +27,7 @@ public static function compatAssertIsArray($obj)
public function compatExpectExceptionMessageMatches($msg)
{
if (method_exists($this, 'expectExceptionMessageMatches')) {
// @phpstan-ignore-next-line
$this->expectExceptionMessageMatches($msg);
} else {
// @phpstan-ignore-next-line
Expand All @@ -36,15 +38,18 @@ public function compatExpectExceptionMessageMatches($msg)
public static function compatAssertStringContainsString($a, $b)
{
if (method_exists(static::class, 'assertStringContainsString')) {
// @phpstan-ignore-next-line
static::assertStringContainsString($a, $b);
} else {
// @phpstan-ignore-next-line
static::assertContains($a, $b);
}
}

public function compatAssertIsString($x)
{
if (method_exists($this, 'assertIsString')) {
// @phpstan-ignore-next-line
static::assertIsString($x);
} else {
// @phpstan-ignore-next-line
Expand All @@ -55,6 +60,7 @@ public function compatAssertIsString($x)
public static function compatWarningClass()
{
if (class_exists('\PHPUnit\Framework\Error\Warning')) {
// @phpstan-ignore-next-line
return \PHPUnit\Framework\Error\Warning::class;
}

Expand Down
2 changes: 0 additions & 2 deletions tests/TestHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,6 @@ function ($method, $absUrl, $readBodyChunkCallable, $headers, $params, $hasFile)
* @param array $response
* @param int $rcode
* @param null|string $base
*
* @return array
*/
protected function stubRequest(
$method,
Expand Down