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

Valinor may crash when encountering a PHPStan/Psalm type syntax it doesn't support yet #448

Open
arnaud-lb opened this issue Nov 2, 2023 · 3 comments

Comments

@arnaud-lb
Copy link

It appears that Valinor may crash when encountering a type syntax it doesn't support yet. This can be an issue as this prevents from using some PHPStan/Psalm features on objects that may be mapped by Valinor.

For example, the following code:

<?php

require 'vendor/autoload.php';

class B {}

class A
{
    /**
     * @phpstan-param ($a is 1 ? B : null) $b
     * @param B|null $b
     */
    public function __construct(
        public readonly int $a,
        public readonly ?B $b,
    ) {
    }
}

return (new \CuyZ\Valinor\MapperBuilder())
    ->mapper()
    ->map(
        A::class,
        new \CuyZ\Valinor\Mapper\Source\JsonSource('{"a":0,"b":null}')
    );

leads to this assertion failure:

PHP Fatal error:  Uncaught AssertionError: assert(!$type instanceof UnresolvableType) in src/Mapper/Tree/Shell.php:31
Stack trace:
#0 src/Mapper/Tree/Shell.php(31): assert()
#1 src/Mapper/Tree/Shell.php(41): CuyZ\Valinor\Mapper\Tree\Shell->__construct()
#2 src/Mapper/Tree/Builder/ObjectNodeBuilder.php(50): CuyZ\Valinor\Mapper\Tree\Shell->child()
#3 src/Mapper/Tree/Builder/ObjectNodeBuilder.php(23): CuyZ\Valinor\Mapper\Tree\Builder\ObjectNodeBuilder->children()
#4 src/Mapper/Tree/Builder/NativeClassNodeBuilder.php(39): CuyZ\Valinor\Mapper\Tree\Builder\ObjectNodeBuilder->build()
#5 src/Mapper/Tree/Builder/CasterNodeBuilder.php(24): CuyZ\Valinor\Mapper\Tree\Builder\NativeClassNodeBuilder->build()
#6 src/Mapper/Tree/Builder/UnionNodeBuilder.php(37): CuyZ\Valinor\Mapper\Tree\Builder\CasterNodeBuilder->build()
#7 src/Mapper/Tree/Builder/InterfaceNodeBuilder.php(51): CuyZ\Valinor\Mapper\Tree\Builder\UnionNodeBuilder->build()
#8 src/Mapper/Tree/Builder/CasterProxyNodeBuilder.php(24): CuyZ\Valinor\Mapper\Tree\Builder\InterfaceNodeBuilder->build()
#9 src/Mapper/Tree/Builder/IterableNodeBuilder.php(26): CuyZ\Valinor\Mapper\Tree\Builder\CasterProxyNodeBuilder->build()
#10 src/Mapper/Tree/Builder/StrictNodeBuilder.php(36): CuyZ\Valinor\Mapper\Tree\Builder\IterableNodeBuilder->build()
#11 src/Mapper/Tree/Builder/ErrorCatcherNodeBuilder.php(33): CuyZ\Valinor\Mapper\Tree\Builder\StrictNodeBuilder->build()
#12 src/Mapper/Tree/Builder/RootNodeBuilder.php(16): CuyZ\Valinor\Mapper\Tree\Builder\ErrorCatcherNodeBuilder->build()
#13 src/Mapper/TypeTreeMapper.php(44): CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder->build()
#14 src/Mapper/TypeTreeMapper.php(25): CuyZ\Valinor\Mapper\TypeTreeMapper->node()
#15 test.php(24): CuyZ\Valinor\Mapper\TypeTreeMapper->map()
#16 {main}
  thrown in src/Mapper/Tree/Shell.php on line 31

Should Valinor fallback to the unprefixed @param in this case?

@stiflerbox
Copy link

stiflerbox commented Dec 8, 2023

Having similar problem

<?php

final class LoginResult
{
	public const OK     = 'Ok';
	public const FAILED = 'Failed';
}

final class LoginResponse
{
	public function __construct(
		/** @var LoginResult::* */  // <-------------- problem with this annotation with ::*
		public readonly string $LoginResult
	)
	{
	}
}

return (new \CuyZ\Valinor\MapperBuilder())
	->allowSuperfluousKeys()
	->mapper()
	->map(
		LoginResponse::class,
		\CuyZ\Valinor\Mapper\Source\Source::json('{"LoginResult":"Ok"}')
	);

The above scenario throws exception AssertionError: assert(!$type instanceof UnresolvableType).

Works correctly when annotated explicitly with:
/** @var LoginResult::OK|LoginResult::FAILED */.

@romm
Copy link
Member

romm commented Apr 7, 2024

Hey there, thanks for the report!

I think that the fallback option is not the best as it cannot be guaranteed that the @param annotation works as well.

I was thinking about adding a @valinor-* annotation support that has precedence over all the others, for the following cases: @valinor-var, @valinor-param, @valinor-return, @valinor-extends, @valinor-template, @valinor-type.

WDYT?

@romm
Copy link
Member

romm commented Apr 7, 2024

@stiflerbox your usecase was actually a "bug" (rather an unwanted feature actually 😛 ), see #520.

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

No branches or pull requests

3 participants