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

PHPStan level 4 #2080

Merged
merged 1 commit into from
Jun 20, 2022
Merged

PHPStan level 4 #2080

merged 1 commit into from
Jun 20, 2022

Conversation

franmomu
Copy link
Contributor

So this PR increases PHPstan level and sets treatPhpDocTypesAsCertain as false, this is because there are several errors like:

 ------ --------------------------------------------------------------------------------------------------------------------------------------------
  Line   src/Aggregation/GeohashGrid.php
 ------ --------------------------------------------------------------------------------------------------------------------------------------------
  48     Result of && is always false.
         💡 Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your phpstan.neon.
  49     Else branch is unreachable because ternary operator condition is always true.
 ------ --------------------------------------------------------------------------------------------------------------------------------------------

but the code is fine:

* @param int|string $precision an integer between 1 and 12, inclusive. Defaults to 5 or distance like 1km, 10m
*
* @return $this
*/
public function setPrecision($precision): self
{
if (!\is_int($precision) && !\is_string($precision)) {
throw new \TypeError(\sprintf('Argument 1 passed to "%s()" must be of type int|string, %s given.', __METHOD__, \is_object($precision) ? \get_class($precision) : \gettype($precision)));
}

About the ignored errors (all in tests), most of them are because of TestCase::markTestSkipped() call, so it warns about the code after that call not being executed.

Comment on lines +115 to +120
$innerHits = $collapse->getParam('inner_hits');

$this->assertCount(2, $innerHits);
$this->assertIsArray($innerHits);
$this->assertEquals($innerHits1, $innerHits[0]);
$this->assertEquals($innerHits2, $innerHits[1]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been changed to not confuse PHPStan, because below there is this call:

$this->assertInstanceOf(InnerHits::class, $collapse->getParam('inner_hits'));

So without this change, this was the error:

 ------ -------------------------------------------------------------------------------------------------------------------------------------------
  Line   tests/Collapse/CollapseTest.php
 ------ -------------------------------------------------------------------------------------------------------------------------------------------
  125    Call to method PHPUnit\Framework\Assert::assertInstanceOf() with 'Elastica\\Collapse\\InnerHits' and array will always evaluate to false.
 ------ -------------------------------------------------------------------------------------------------------------------------------------------

@franmomu franmomu closed this Jun 20, 2022
@franmomu franmomu reopened this Jun 20, 2022
@ruflin
Copy link
Owner

ruflin commented Jun 20, 2022

There is one build failing with:

Run vendor/bin/phpstan analyse --no-progress --error-format=github
Note: Using configuration file /home/runner/work/Elastica/Elastica/phpstan.neon.
Error: Expression on left side of ?? is not nullable.
Error: Process completed with exit code 1.

It seems it is also marked in the review. Not sure why?

Excited to see that we are climbing the phpstan levels :-) Thanks for getting us here.

@franmomu
Copy link
Contributor Author

Oops I missed that one, that is because in:

https://github.com/elastic/elasticsearch-php/blob/24c51c05103bf43fb9847acab039cba1d6e43173/src/Elasticsearch/Endpoints/AbstractEndpoint.php#L54-L57

there is no null as possible value. Since they released 8.x and there is no body property anymore, not sure if they would create a release for this change.

@ruflin ruflin merged commit c0bcdb3 into ruflin:master Jun 20, 2022
@franmomu franmomu deleted the phpstan_level_4 branch June 20, 2022 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants