Feature: Handle name resolving with PHP-Parser's NameResolver

[eliashaeussler]

Hi @icanhazstring, sorry for the noise, but I've probably found a better solution than the one introduced with #91. Please let me know what you think about it :)

💡 The current implementation would also require a change in composer-unused since the constructor signature of SymbolNameParser has changed. I can take care of that as well in case you accept the changes in this PR :)

Suggested change

Name resolving of namespace-scoped symbols (such as classes, functions and constants) is now done through PHP-Parser's NameResolver. By using it as an additional node visitor, we add a safe additional layer on top of the collection strategies provided by the symbol-parser package. Usage of the NameResolver resolves the exact same issue I outlined in #91.

⚠️ This also makes the fix introduced with 13fd5e9 obsolete
now, as the NameResolver is more accurate. I reverted the change as part of this PR (see 0a2d5ab).

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

1. Does your submission pass tests?
2. Have you lint your code locally prior to submission?

Changes to Core Features:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your core changes, as applicable?
• Have you successfully ran tests with your changes locally?

[icanhazstring]

That looks like the TypedAttributeStrategy now also includes parts of the UseStrategy as well.
Before the TypedAttributeStrategy only took care of FullyQualified nodes.

As far as I understood the NameResolver now makes every type into a FullyQualified one?

[eliashaeussler]

I had the same feeling with this piece of code. From what I understand, the NameResolver does exactly what you said – it resolves all resolvable names to fully qualified resolved names.

[icanhazstring]

I really like the idea. Since my implementation is kind of "custom baked" :)
Using the NameResolver could resolve a lot of issues.

But it seems to be, that using this, makes some strategies to be kind of "duplicate" in logic.
For example, UseStrategy probably now also reports "partial" namespaces, as well as Symbols, which TypedAttributeStrategy also reports 🤔

Not saying that it's bad, we probably should just merge it :)
Maybe have later some amendments to clean up or refactor logic in some degree (make it simpler)

[eliashaeussler]

Thanks a lot for your feedback! Sure, I can take a look at the strategies and check which of them are probably obsolete now. I wanted to make sure you like the idea first before I go deeper into the codebase :)

[icanhazstring]

I merged it into main for now.
Maybe we can create PR in composer-unused and check with dev-main that everything still works.

After that we can still figure out some future changes 💪

[eliashaeussler]

Sounds like a good plan. Thanks for merging 🚀 Will provide a PR in the composer-unused repo.

[eliashaeussler]

Here we go: composer-unused/composer-unused#460