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

Constant scalar types (and compositions thereof) are not currently considered #1

Closed
Ocramius opened this issue Nov 29, 2021 · 2 comments · Fixed by #13
Closed

Constant scalar types (and compositions thereof) are not currently considered #1

Ocramius opened this issue Nov 29, 2021 · 2 comments · Fixed by #13

Comments

@Ocramius
Copy link
Contributor

While experimenting with the library, I noticed that constant scalar types are not currently working:

<?php

namespace My\App;

use CuyZ\Valinor\Attribute\StaticMethodConstructor;
use CuyZ\Valinor\Mapper\MappingError;
use CuyZ\Valinor\MapperBuilder;

require_once __DIR__ . '/Valinor/vendor/autoload.php';

class MyFlagWrapper
{
    /** @var 0|2|4|8|16 */
    public int $value;
}

$mapper = (new MapperBuilder())
    ->withCacheDir(__DIR__ . '/cache')
    ->mapper();

var_dump($mapper->map(MyFlagWrapper::class, ['value' => 2]));

produces:

php example.php 

Fatal error: Uncaught CuyZ\Valinor\Type\Parser\Exception\Stream\WrongTokenType: Wrong token type `CuyZ\Valinor\Type\Parser\Lexer\Token\UnionToken`, it should be an instance of `CuyZ\Valinor\Type\Parser\Lexer\Token\TraversingToken`. in /app/Valinor/src/Type/Parser/Lexer/TokenStream.php:36
Stack trace:
#0 /app/Valinor/src/Type/Parser/LexingParser.php(39): CuyZ\Valinor\Type\Parser\Lexer\TokenStream->read()
#1 /app/Valinor/src/Definition/Repository/Reflection/ReflectionPropertyDefinitionBuilder.php(132): CuyZ\Valinor\Type\Parser\LexingParser->parse('0|2|4|8|16 ')
#2 /app/Valinor/src/Definition/Repository/Reflection/ReflectionPropertyDefinitionBuilder.php(110): CuyZ\Valinor\Definition\Repository\Reflection\ReflectionPropertyDefinitionBuilder->parseType('0|2|4|8|16 ', Object(ReflectionProperty), Object(CuyZ\Valinor\Type\Parser\LexingParser))
#3 /app/Valinor/src/Definition/Repository/Reflection/ReflectionPropertyDefinitionBuilder.php(70): CuyZ\Valinor\Definition\Repository\Reflection\ReflectionPropertyDefinitionBuilder->typeFromDocBlock(Object(CuyZ\Valinor\Definition\ClassSignature), Object(ReflectionProperty))
#4 /app/Valinor/src/Definition/Repository/Reflection/ReflectionPropertyDefinitionBuilder.php(43): CuyZ\Valinor\Definition\Repository\Reflection\ReflectionPropertyDefinitionBuilder->resolveType(Object(CuyZ\Valinor\Definition\ClassSignature), Object(ReflectionProperty))
#5 /app/Valinor/src/Definition/Repository/Reflection/ReflectionClassDefinitionRepository.php(41): CuyZ\Valinor\Definition\Repository\Reflection\ReflectionPropertyDefinitionBuilder->for(Object(CuyZ\Valinor\Definition\ClassSignature), Object(ReflectionProperty))
#6 [internal function]: CuyZ\Valinor\Definition\Repository\Reflection\ReflectionClassDefinitionRepository->CuyZ\Valinor\Definition\Repository\Reflection\{closure}(Object(ReflectionProperty))
#7 /app/Valinor/src/Definition/Repository/Reflection/ReflectionClassDefinitionRepository.php(42): array_map(Object(Closure), Array)
#8 /app/Valinor/src/Definition/Repository/Cache/CacheClassDefinitionRepository.php(36): CuyZ\Valinor\Definition\Repository\Reflection\ReflectionClassDefinitionRepository->for(Object(CuyZ\Valinor\Definition\ClassSignature))
#9 /app/Valinor/src/Mapper/Tree/Builder/ClassNodeBuilder.php(35): CuyZ\Valinor\Definition\Repository\Cache\CacheClassDefinitionRepository->for(Object(CuyZ\Valinor\Definition\ClassSignature))
#10 /app/Valinor/src/Mapper/Tree/Builder/CasterNodeBuilder.php(35): CuyZ\Valinor\Mapper\Tree\Builder\ClassNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell), Object(CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder))
#11 /app/Valinor/src/Mapper/Tree/Builder/VisitorNodeBuilder.php(28): CuyZ\Valinor\Mapper\Tree\Builder\CasterNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell), Object(CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder))
#12 /app/Valinor/src/Mapper/Tree/Builder/ValueAlteringNodeBuilder.php(31): CuyZ\Valinor\Mapper\Tree\Builder\VisitorNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell), Object(CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder))
#13 /app/Valinor/src/Mapper/Tree/Builder/ShellVisitorNodeBuilder.php(30): CuyZ\Valinor\Mapper\Tree\Builder\ValueAlteringNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell), Object(CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder))
#14 /app/Valinor/src/Mapper/Tree/Builder/ErrorCatcherNodeBuilder.php(23): CuyZ\Valinor\Mapper\Tree\Builder\ShellVisitorNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell), Object(CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder))
#15 /app/Valinor/src/Mapper/Tree/Builder/RootNodeBuilder.php(21): CuyZ\Valinor\Mapper\Tree\Builder\ErrorCatcherNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell), Object(CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder))
#16 /app/Valinor/src/Mapper/TreeMapperContainer.php(57): CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder->build(Object(CuyZ\Valinor\Mapper\Tree\Shell))
#17 /app/Valinor/src/Mapper/TreeMapperContainer.php(31): CuyZ\Valinor\Mapper\TreeMapperContainer->node('My\\App\\MyFlagWr...', Array)
#18 /app/example.php(21): CuyZ\Valinor\Mapper\TreeMapperContainer->map('My\\App\\MyFlagWr...', Array)
#19 {main}
  thrown in /app/Valinor/src/Type/Parser/Lexer/TokenStream.php on line 36

The same union done with string values behaves incorrectly too, but without crash:

<?php

namespace My\App;

use CuyZ\Valinor\Attribute\StaticMethodConstructor;
use CuyZ\Valinor\Mapper\MappingError;
use CuyZ\Valinor\MapperBuilder;

require_once __DIR__ . '/Valinor/vendor/autoload.php';

class MyFlagWrapper
{
    /** @var 'foo'|'bar' */
    public string $value;
}

$mapper = (new MapperBuilder())
    ->withCacheDir(__DIR__ . '/cache')
    ->mapper();

var_dump($mapper->map(MyFlagWrapper::class, ['value' => 2]));

The output should be an exception, but 2 is cast to string.

php example.php 
object(My\App\MyFlagWrapper)#74 (1) {
  ["value"]=>
  string(1) "2"
}

This is probably just a limitation of how the library operates right now

romm added a commit that referenced this issue Nov 29, 2021
Previously, `array_filter` would remove the integer value `0` from the
array.

See #1
romm added a commit that referenced this issue Nov 29, 2021
Previously, `array_filter` would remove the integer value `0` from the
array.

See #1
@romm romm closed this as completed in #13 Nov 29, 2021
@romm
Copy link
Member

romm commented Nov 29, 2021

@Ocramius Thanks!

The cases you mentioned should work now, can you try again on your side and keep me updated? :)

@Ocramius
Copy link
Contributor Author

Seems to work as expected now, thanks!

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 a pull request may close this issue.

2 participants