[NodeTypeResolver] Use Scope->getType() on ArrayDimFetch on NodeTypeResolver::getNativeType()

[samsonasik]

@staabm this is what I mean for using Scope->getType() on ArrayDimFetch, as using getNativeType() will got mixed on this code:

        $parts = parse_url($url);

        if (!empty($parts['host'])) {
            return $parts['host'];
        }

        return null;

checking with getNativeType() always got MixedType on $parts['host'] which always string by getType(), see fixture:

rector-src/tests/Issues/EmptyBooleanCompare/Fixture/fixture.php.inc

Lines 3 to 37 in 8ba3d90

	namespace Rector\Core\Tests\Issues\EmptyBooleanCompare\Fixture;
	final class SomeFixture
	{
	public function checkUrl(string $url)
	{
	$parts = parse_url($url);
	if (!empty($parts['host'])) {
	return $parts['host'];
	}
	return null;
	}
	}
	?>
	-----
	<?php
	namespace Rector\Core\Tests\Issues\EmptyBooleanCompare\Fixture;
	final class SomeFixture
	{
	public function checkUrl(string $url)
	{
	$parts = parse_url($url);
	if (isset($parts['host']) && $parts['host'] !== '') {
	return $parts['host'];
	}
	return null;
	}
	}

[staabm]

Please add tests which show why this is required

[samsonasik]

The test case already exists

rector-src/tests/Issues/EmptyBooleanCompare/Fixture/fixture.php.inc

Lines 3 to 37 in 8ba3d90

	namespace Rector\Core\Tests\Issues\EmptyBooleanCompare\Fixture;
	final class SomeFixture
	{
	public function checkUrl(string $url)
	{
	$parts = parse_url($url);
	if (!empty($parts['host'])) {
	return $parts['host'];
	}
	return null;
	}
	}
	?>
	-----
	<?php
	namespace Rector\Core\Tests\Issues\EmptyBooleanCompare\Fixture;
	final class SomeFixture
	{
	public function checkUrl(string $url)
	{
	$parts = parse_url($url);
	if (isset($parts['host']) && $parts['host'] !== '') {
	return $parts['host'];
	}
	return null;
	}
	}

[samsonasik]

All checks have passed 🎉 @TomasVotruba it is ready for review.

[samsonasik]

@staabm the specific type of $parts['host'] from docblock will be detected as host: StringType instead of MixedType

that can be feature or bug :)

[staabm]

if the shape was returned by a native extension, I would say its a feature.

if the shape is declared as a phpdoc, I would say its a bug :)

[samsonasik]

how to detect if it is from native extension ?

[staabm]

I think we can't detect it. but maybe we can enhance our own getNativeType() method to apply the phpstan-src native extensions (if thats not the case already).

[samsonasik]

For summary,

1. the ArrayDimFetch on parse_url() is valid use case of getType(), see https://getrector.com/demo/860056e8-3533-4868-9ef0-16b99b2944e0 , with getNativeType() will goes MixedType

2. on docblock, probably yes or not, see https://getrector.com/demo/0d518afe-01ac-4b9d-8c29-b703664f1987 , but when user define {host: string}, type type should follow?

3. on direct value assigned is also valid use case (equal type both ->getType() vs getNativeType() ) see https://getrector.com/demo/3fc527af-1aa0-4e80-8f65-102b6925ef86

[staabm]

the ArrayDimFetch on parse_url() is valid use case of getType(), see https://getrector.com/demo/860056e8-3533-4868-9ef0-16b99b2944e0 , with getNativeType() will goes MixedType

Are you sure this problem is related to ArrayDimFetch alone and not return type extensions or array-shapes or similar?

[samsonasik]

There is no docblock definition on parse_url(), so it seems PHPStan detect by its NativeFunctionReflection->getType() but I probably missing something :)

[staabm]

on docblock, probably yes or not, see https://getrector.com/demo/0d518afe-01ac-4b9d-8c29-b703664f1987 , but when user define {host: string}, type type should follow?

We may follow only phpdocs which can be validated by the lie detector

https://phpstan.org/blog/phpstan-1-10-comes-with-lie-detector

[samsonasik]

ArrayDimFetch is special imo, which the data can by passed from different method which the user that define it should understand the risk and know what the data they want/will to pass.

Most of use cases, when user define @param string[] $param, it will always collection of string

/** @param string[] $param */
function foo(array $param)
{
     var_dump($param[0]); // string... 
}

instead of collection of mixed

[TomasVotruba]

Thank you 🙏

[staabm]

after applying rector-dev on our codebase we run about all the issues I mentioned above. I think we need to revert it, because atm this PR does apply phpdoc-based array-shape type (if it would/could only apply array-shape types coming from phpstan native extensions it would be fine)

see

• https://github.com/complex-gmbh/daiber/pull/4987#discussion_r1329986084
• https://github.com/complex-gmbh/mbw/pull/4294#discussion_r1329992228
• https://github.com/complex-gmbh/fare/pull/5286#discussion_r1329981597

[samsonasik]

If we can check what variable came from, eg from assign to native function, I think we can get win win solution

[staabm]

yeah, my main motivation is making sure this doesn't get into a release in the form it is right now.

if we can improve/iterate on it, even better.

[samsonasik]

see #5056