Skip to content

Fix/issue 7000 2#1307

Merged
ondrejmirtes merged 9 commits into
phpstan:1.7.xfrom
rajyan:fix/issue-7000-2
May 12, 2022
Merged

Fix/issue 7000 2#1307
ondrejmirtes merged 9 commits into
phpstan:1.7.xfrom
rajyan:fix/issue-7000-2

Conversation

@rajyan

@rajyan rajyan commented May 12, 2022

Copy link
Copy Markdown
Contributor

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).

@rajyan rajyan force-pushed the fix/issue-7000-2 branch from 47ae33f to f2e511b Compare May 12, 2022 17:27
@rajyan rajyan force-pushed the fix/issue-7000-2 branch from c81bcb1 to 152694e Compare May 12, 2022 17:39
Comment thread src/Analyser/MutatingScope.php Outdated
if (!$dimType instanceof UnionType) {
foreach ($constantArrays as $constantArray) {
$setArrays[] = $constantArray->setOffsetValueType(
TypeCombinator::intersect(ArrayType::castToArrayKeyType($dimType), $constantArray->getKeyType()),

Copy link
Copy Markdown
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
Copy Markdown
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

rajyan commented May 12, 2022

Copy link
Copy Markdown
Contributor Author

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

@ondrejmirtes

Copy link
Copy Markdown
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