Skip to content

Conversation

hgraca
Copy link
Contributor

@hgraca hgraca commented May 20, 2025

It seems the php parser is using phpdocs and deciding that
something like
/**

  • @return array<int, string|int>
  • /
    is a class with FQCN My\Namespace\array<int, string|int> which then ofc fails to create a FullyQualifiedClassName.
@micheleorselli
Copy link
Member

Hey @hgraca, thank you for reporting that 🙇. I guess this depends on the new DocblockParser and related classes. We'll try to look into it asap, in the meantime, if you want to give it a try the fix should happen in the DocblockParser and/or DocblockTypesResolver classes.

@micheleorselli
Copy link
Member

#510 should fix the problem

Copy link
Member

@AlessandroMinoccheri AlessandroMinoccheri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hgraca
Copy link
Contributor Author

hgraca commented May 28, 2025

@micheleorselli thank you for fixing the bug, is this PR still worth merging or shall I close it?

It seems the php parser is using phpdocs and deciding that something like /** * @return array<int, string|int> */ is a class with FQCN `My\Namespace\array<int, string|int>` which then ofc fails to create a FullyQualifiedClassName.
@hgraca
Copy link
Contributor Author

hgraca commented May 28, 2025

@micheleorselli
Actually I tested now with your latest and without my commit and I still get this error

 2731/3413 [======================>-----] 80%Parse Error: PHPUnit\Framework\Attributes\psalm-var is not a valid namespace definition#0 /app/no-vcs/lib/arkitect/src/Analyzer/ClassDependency.php(17): Arkitect\Analyzer\FullyQualifiedClassName::fromString('PHPUnit\\Framewo...') #1 /app/no-vcs/lib/arkitect/src/Analyzer/FileVisitor.php(251): Arkitect\Analyzer\ClassDependency->__construct('PHPUnit\\Framewo...', 25) #2 /app/no-vcs/lib/arkitect/src/Analyzer/FileVisitor.php(57): Arkitect\Analyzer\FileVisitor->handleTypedProperty(Object(PhpParser\Node\Stmt\Property)) #3 /app/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(196): Arkitect\Analyzer\FileVisitor->enterNode(Object(PhpParser\Node\Stmt\Property)) #4 /app/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(98): PhpParser\NodeTraverser->traverseArray(Array) #5 /app/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(227): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Class_)) #6 /app/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(98): PhpParser\NodeTraverser->traverseArray(Array) #7 /app/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(227): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Namespace_)) #8 /app/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(76): PhpParser\NodeTraverser->traverseArray(Array) #9 /app/no-vcs/lib/arkitect/src/Analyzer/FileParser.php(72): PhpParser\NodeTraverser->traverse(Array) 

I fixed the merge conflicts and force pushed.

@hgraca hgraca force-pushed the fix/fix-file-vistor-not-a-class branch from 0d65be6 to 9376332 Compare May 28, 2025 14:32
Copy link

codecov bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 97.53%. Comparing base (12f0a78) to head (9376332).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/Analyzer/FileVisitor.php 50.00% 4 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@ ## main #508 +/- ## ============================================ - Coverage 97.75% 97.53% -0.22%  - Complexity 615 619 +4  ============================================ Files 80 80 Lines 1778 1786 +8 ============================================ + Hits 1738 1742 +4  - Misses 40 44 +4 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@micheleorselli
Copy link
Member

Hey @hgraca thank you for testing it! Based on the error it seems this is related to something different rather than the @return tag. It seems the problem is caused by @psalm-var, right? Anyway we should exclude all the docblock tags we are not interested in parsing

@micheleorselli
Copy link
Member

I'm trying to reproduce it but with no luck, would you mind sharing the snippet of the code causing problems? thanks!

@hgraca
Copy link
Contributor Author

hgraca commented Jun 8, 2025

@micheleorselli

I finally had some time to look into what situations still give problems, I found only this:

https://github.com/ramsey/uuid/blob/4.x/src/Uuid.php#L232-L234

We updated our project from php8.2 to 8.4 this week, including some libraries ofc so I guess that's why this is not exactly the same issue as before, but at the root it seems to be the same, it looks like it interprets @my-custom-doc-tag as a class FQCN.

Unfortunately I dont know what shoulod be done to solve this on phparkitect side, except for what is in this PR already :(

let me know if I can be of any further assistance.

@micheleorselli
Copy link
Member

I was able to reproduce it, this #517 should fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants