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

Fix/issue 7000 2 #1307

Merged
merged 9 commits into from May 12, 2022
Merged

Fix/issue 7000 2 #1307

merged 9 commits into from May 12, 2022

Conversation

rajyan
Copy link
Contributor

@rajyan rajyan commented May 12, 2022

fixes phpstan/phpstan#6508
fixes phpstan/phpstan#7000
fixes phpstan/phpstan#7190
see #1299

This is (a little hacky) solution to solve isset expression specifying now.

There are two problems in specifyExpressionType for ArrayDimFetch (that cannot be solved easily I think) now.

  1. Specifying ArrayDimFetch with the same type of the current value can change the type in some cases

ex. https://phpstan.org/r/941b7692-8141-4fee-a410-303706ad0abf

Class Foo {	
	/**
	 * @param 'id'|'name' $key
	 * @param array{id:int, name:string} $data
	 */
	public function test2(string $key, array $data): void {
		if (isset($data[$key])) {
			\PHPStan\Testing\assertType('array{id: int, name: string}', $data);
		}
	}
}

this case was skipped before c0bf915, but the current implementation of type specifying changes the type to 'array{id: int|string, name: int|string}'

  1. revertNonNullability cannot revert the non nullability correctly.
    /**
    * @param EnsuredNonNullabilityResultExpression[] $specifiedExpressions
    */
    private function revertNonNullability(MutatingScope $scope, array $specifiedExpressions): MutatingScope
    {
    foreach ($specifiedExpressions as $specifiedExpressionResult) {
    $scope = $scope->specifyExpressionType(
    $specifiedExpressionResult->getExpression(),
    $specifiedExpressionResult->getOriginalType(),
    $specifiedExpressionResult->getOriginalNativeType(),
    );
    }
    return $scope;
    }

A - (specify B) > -(specify A)> !== A

For example, we cannot know whether the offset was marked as required or not before the specification. (maybe phpstan/phpstan#7224 is related to this problem)

array{ 'optional'?: string|null } - (specify non null) > array{ 'optional': string } - (specify null) >
array{ 'optional': string|null } - (specify non null) > array{ 'optional': string } - (specify null) >

I had to skip $dimType with UnionType bd2eb4e because of these reasons (for now).

if (!$dimType instanceof UnionType) {
foreach ($constantArrays as $constantArray) {
$setArrays[] = $constantArray->setOffsetValueType(
TypeCombinator::intersect(ArrayType::castToArrayKeyType($dimType), $constantArray->getKeyType()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the cast before the loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! thanks

@rajyan
Copy link
Contributor Author

rajyan commented May 12, 2022

Checked the failing integration tests, but I think they're all correct to report. ;)

@ondrejmirtes
Copy link
Member

I love this! Thank you very much :)

@ondrejmirtes ondrejmirtes merged commit 21164d6 into phpstan:1.7.x May 12, 2022
@rajyan rajyan deleted the fix/issue-7000-2 branch May 12, 2022 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants