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

Error when calling ReflectionAttribute::newInstance() #9877

Closed
enumag opened this issue Sep 13, 2023 · 16 comments
Closed

Error when calling ReflectionAttribute::newInstance() #9877

enumag opened this issue Sep 13, 2023 · 16 comments
Labels
Milestone

Comments

@enumag
Copy link
Contributor

enumag commented Sep 13, 2023

Bug report

I'm unsure where exactly the error is but most likely somewhere in https://github.com/ondrejmirtes/BetterReflection rather than phpstan.

What happened is this. I switched my app from annotations to attributes. And some of them are causing a nasty issue. For instance:

use OpenApi\Attributes as OA;
#[OA\Post(requestBody: new OA\RequestBody(content: [new OA\MediaType(mediaType: 'application/json')]))]

We have a custom phpstan rule which is analyzing these attributes. To do that it gets the ReflectionAttribute instance from reflection and calls ->newInstance(). Which causes an error like this:

Uncaught TypeError: OpenApi\Attributes\RequestBody::__construct(): Argument #1 ($ref) must be of type object|string|null, array given, called in phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/NodeCompiler/CompileNodeToValue.php on line 216 in /app/vendor/zircote/swagger-php/src/Attributes/RequestBody.php:21

Apparently it's trying to put the [new OA\MediaType(mediaType: 'application/json')] value into the $ref argument of RequestBody::__construct() instead of the $content argument. Somewhere somehow the args array passed to RequestBody lost the named arguments information and puts the value to the first argument (ref) instead of the one I specified (content).

Full stacktrace when I ran this with --debug:

Uncaught TypeError: OpenApi\Attributes\RequestBody::__construct(): Argument #1 ($ref) must be of type object|string|null, array given, called in phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/NodeCompiler/CompileNodeToValue.php on line 216 in /app/vendor/zircote/swagger-php/src/Attributes/RequestBody.php:21
#0 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/NodeCompiler/CompileNodeToValue.php(216): OpenApi\Attributes\RequestBody->__construct(Array)
#1 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/NodeCompiler/CompileNodeToValue.php(52): PHPStan\BetterReflection\NodeCompiler\CompileNodeToValue->compileNew(Object(PhpParser\Node\Expr\New_), Object(PHPStan\BetterReflection\NodeCompiler\CompilerContext))
#2 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/nikic/php-parser/lib/PhpParser/ConstExprEvaluator.php(131): PHPStan\BetterReflection\NodeCompiler\CompileNodeToValue->PHPStan\BetterReflection\NodeCompiler\{closure}(Object(PhpParser\Node\Expr\New_))
#3 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/nikic/php-parser/lib/PhpParser/ConstExprEvaluator.php(96): PhpParser\ConstExprEvaluator->evaluate(Object(PhpParser\Node\Expr\New_))
#4 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/NodeCompiler/CompileNodeToValue.php(99): PhpParser\ConstExprEvaluator->evaluateDirectly(Object(PhpParser\Node\Expr\New_))
#5 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionAttribute.php(79): PHPStan\BetterReflection\NodeCompiler\CompileNodeToValue->__invoke(Object(PhpParser\Node\Expr\New_), Object(PHPStan\BetterReflection\NodeCompiler\CompilerContext))
#6 [internal function]: PHPStan\BetterReflection\Reflection\ReflectionAttribute::PHPStan\BetterReflection\Reflection\{closure}(Object(PhpParser\Node\Expr\New_))
#7 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionAttribute.php(78): array_map(Object(Closure), Array)
#8 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/Adapter/ReflectionAttribute.php(41): PHPStan\BetterReflection\Reflection\ReflectionAttribute->getArguments()
#9 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/Adapter/ReflectionAttribute.php(52): PHPStan\BetterReflection\Reflection\Adapter\ReflectionAttribute->getArguments()
#10 /app/vendor/spaceflow/apidoc/src/ApiDocRules/PHPStan/Rules/ModelClassesAreResolvable.php(104): PHPStan\BetterReflection\Reflection\Adapter\ReflectionAttribute->newInstance()
#11 /app/vendor/spaceflow/apidoc/src/ApiDocRules/PHPStan/Rules/ModelClassesAreResolvable.php(51): Spaceflow\Library\ApiDocRules\PHPStan\Rules\ModelClassesAreResolvable->inspectMethod(Object(PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod), 'OpenApi\\Attribu...')
#12 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(107): Spaceflow\Library\ApiDocRules\PHPStan\Rules\ModelClassesAreResolvable->processNode(Object(PhpParser\Node\Stmt\ClassMethod), Object(PHPStan\Analyser\MutatingScope))
#13 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Node/ClassStatementsGatherer.php(108): PHPStan\Analyser\FileAnalyser->PHPStan\Analyser\{closure}(Object(PhpParser\Node\Stmt\ClassMethod), Object(PHPStan\Analyser\MutatingScope))
#14 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(441): PHPStan\Node\ClassStatementsGatherer->__invoke(Object(PhpParser\Node\Stmt\ClassMethod), Object(PHPStan\Analyser\MutatingScope))
#15 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(384): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\ClassMethod), Object(PHPStan\Analyser\MutatingScope), Object(PHPStan\Node\ClassStatementsGatherer), Object(PHPStan\Analyser\StatementContext))
#16 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(640): PHPStan\Analyser\NodeScopeResolver->processStmtNodes(Object(PhpParser\Node\Stmt\Class_), Array, Object(PHPStan\Analyser\MutatingScope), Object(PHPStan\Node\ClassStatementsGatherer), Object(PHPStan\Analyser\StatementContext))
#17 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(384): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\Class_), Object(PHPStan\Analyser\MutatingScope), Object(Closure), Object(PHPStan\Analyser\StatementContext))
#18 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(612): PHPStan\Analyser\NodeScopeResolver->processStmtNodes(Object(PhpParser\Node\Stmt\Namespace_), Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure), Object(PHPStan\Analyser\StatementContext))
#19 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(353): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\Namespace_), Object(PHPStan\Analyser\MutatingScope), Object(Closure), Object(PHPStan\Analyser\StatementContext))
#20 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(166): PHPStan\Analyser\NodeScopeResolver->processNodes(Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#21 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/Analyser.php(72): PHPStan\Analyser\FileAnalyser->analyseFile('/app/src/Module...', Array, Object(PHPStan\Rules\LazyRegistry), Object(PHPStan\Collectors\Registry), NULL)
#22 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyserRunner.php(62): PHPStan\Analyser\Analyser->analyse(Array, Object(Closure), NULL, true, Array)
#23 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyseApplication.php(209): PHPStan\Command\AnalyserRunner->runAnalyser(Array, Array, Object(Closure), NULL, true, true, '/app/phpstan.ne...', Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput))
#24 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyseApplication.php(101): PHPStan\Command\AnalyseApplication->runAnalyser(Array, Array, true, '/app/phpstan.ne...', Object(PHPStan\Command\Symfony\SymfonyOutput), Object(PHPStan\Command\Symfony\SymfonyOutput), Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput))
#25 phar:///app/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyseCommand.php(198): PHPStan\Command\AnalyseApplication->analyse(Array, true, Object(PHPStan\Command\Symfony\SymfonyOutput), Object(PHPStan\Command\Symfony\SymfonyOutput), false, true, '/app/phpstan.ne...', Array, Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput))
#26 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Command/Command.php(259): PHPStan\Command\AnalyseCommand->execute(Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput), Object(_PHPStan_95cdbe577\Symfony\Component\Console\Output\ConsoleOutput))
#27 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(870): _PHPStan_95cdbe577\Symfony\Component\Console\Command\Command->run(Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput), Object(_PHPStan_95cdbe577\Symfony\Component\Console\Output\ConsoleOutput))
#28 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(261): _PHPStan_95cdbe577\Symfony\Component\Console\Application->doRunCommand(Object(PHPStan\Command\AnalyseCommand), Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput), Object(_PHPStan_95cdbe577\Symfony\Component\Console\Output\ConsoleOutput))
#29 phar:///app/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(157): _PHPStan_95cdbe577\Symfony\Component\Console\Application->doRun(Object(_PHPStan_95cdbe577\Symfony\Component\Console\Input\ArgvInput), Object(_PHPStan_95cdbe577\Symfony\Component\Console\Output\ConsoleOutput))
#30 phar:///app/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan(124): _PHPStan_95cdbe577\Symfony\Component\Console\Application->run()
#31 phar:///app/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan(125): _PHPStan_95cdbe577\{closure}()
#32 /app/vendor/phpstan/phpstan/phpstan(8): require('phar:///app/ven...')
#33 /app/vendor/bin/phpstan(120): include('/app/vendor/php...')
#34 {main}

@mergeable
Copy link

mergeable bot commented Sep 13, 2023

This bug report is missing a link to reproduction at phpstan.org/try.

It will most likely be closed after manual review.

@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

The bug appeared when a custom rule was used so it's impossible to reproduce on phpstan.org.

@ondrejmirtes
Copy link
Member

Please link or post how the constructors of attribute classes under the OpenApi\Attributes as OA look like.

@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

https://github.com/zircote/swagger-php/blob/master/src/Attributes/RequestBody.php

(and other files within that directory)

@ondrejmirtes
Copy link
Member

You can send a bugfix to ondrejmirtes/BetterReflection if you reset --hard away the last two [GENERATED] commits from 6.11.x and base your PR on top of the last "manual" commit which is Fix newInstance on ReflectionAttribute adapter with enum argument.

The problem is that the "named argument" part is lost here: https://github.com/ondrejmirtes/BetterReflection/blob/851396550f122394fab96de837cf2f19db8278f7/src/NodeCompiler/CompileNodeToValue.php#L275-L291

Because the argument name isn't present as the array key, but inside Node\Arg as $name property.

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Sep 13, 2023
@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

Yeah I did find that place. I don't think array_map throws away keys though so they must have been lost somewhere earlier I think. I'll see if I have time to look more into it tomorrow.

@ondrejmirtes
Copy link
Member

No, the issue is that $node->args is always a list. It's not indexed by argument name.

@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

Ah I see. Where can I get the keys then?

Nevermind you answered that above.

@ondrejmirtes
Copy link
Member

Well you have Node\Arg $arg when iterating over $node->args. The name of the argument is in Node\Arg::$name.

You need to do a similar job that's done here: https://github.com/ondrejmirtes/BetterReflection/blob/851396550f122394fab96de837cf2f19db8278f7/src/Reflection/ReflectionAttribute.php#L49

@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

I'm unable to run the tests locally. I cloned the project, checked out the commit you linked, ran composer install and then vendor/bin/phpunit. But literally all the tests are failing with this error (which makes no sense):

This test did not perform any assertions

On second try they still fail but with different errors that don't make any sense either.

PHPUnit\Framework\MockObject\Generator\UnknownTypeException: Class or interface "Roave\BetterReflection\Reflector\Reflector" does not exist

@ondrejmirtes
Copy link
Member

Yes, working with this fork isn't straightforward. You need to:

  1. Get rid of the last two commits.
  2. Make the change you want to the source.
  3. Commit that change.
  4. Run make rename-inner.
  5. Now you can run the tests.

If the tests fail, you need to git reset --hard to get rid of the renames, and continue from step 2) again.

@ondrejmirtes
Copy link
Member

But honestly I'd accept this change without a new test. We can test it integration-wise in phpstan-src instead.

@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

Ah so it's better to make the change on top of the latest two commits, test it and then cherry pick it to a branch without those commits. Okay. :)

@ondrejmirtes
Copy link
Member

You're gonna get conflicts that way because those generated commits change the source quite significantly.

@enumag
Copy link
Contributor Author

enumag commented Sep 13, 2023

The patch is simple enough that I didn't get any conflicts that way. 😆

ondrejmirtes/BetterReflection#34

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants