qossmic / deptrac

Keep your architecture clean.
https://qossmic.github.io/deptrac
MIT License
2.61k stars 135 forks source link

Test dependencies from method signatures #1328

Closed brightbyte closed 6 months ago

brightbyte commented 8 months ago

This adds a test case that checks for dependencies derived from method signatures.

brightbyte commented 8 months ago

@patrickkusebauch It looks to me like depeendencies from method signatures are not tracked at all right now. Is this expected?

In FileReferenceVisitor, the relevant code for handling parameters and returns types seems to be in enterFunction, which is not called for class methods...

brightbyte commented 8 months ago

Ok I'm clearly misunderstanding something here... Looking at the test failures, some dependencies are now tracked twice. What am I missing?

patrickkusebauch commented 8 months ago

@brightbyte Oh, they are tracked, you were just looking at the wrong file. :)

Think of the purpose of FileReferenceVisitor as setting up the context of where the parsing is happening - are we inside a class, inside a function? What are the currently applied use statements? What are the template types?

Once you are in the context of the class, the parsing of class methods is not done inside this class. Instead it is processed by https://github.com/qossmic/deptrac/blob/main/src/Core/Ast/Parser/Extractors/FunctionLikeExtractor.php

brightbyte commented 8 months ago

Once you are in the context of the class, the parsing of class methods is not done inside this class. Instead it is processed by https://github.com/qossmic/deptrac/blob/main/src/Core/Ast/Parser/Extractors/FunctionLikeExtractor.php

Ah, thank you for pointing me in the right direction!

I changed this PR to just add FunctionLikeExtractorTest. Having this test case makes it easier for me to investigate what is supported and what is not.

patrickkusebauch commented 8 months ago

I see. Transitive dependencies are not tracked as per https://github.com/qossmic/deptrac/issues/449. It is a very difficult problem to solve and likely would require plugging into something like PHPStan AST parser to get it done.

brightbyte commented 8 months ago

I see. Transitive dependencies are not tracked as per #449. It is a very difficult problem to solve and likely would require plugging into something like PHPStan AST parser to get it done.

Ah, thank you for pointing me to that discussion.

Yes, it's hard. The only thing I know that does this pretty exhaustively is phpstorm's analyzer. Phan also does type inference, but it was born into a world without tpye hints, and takes a very brute force approach...

I'd love to see this implemented, but I also realize that the benefits are limited when working with a reasonably clean code base with small classes and small methods. Unfortunately, the code base I'm trying to clean up isn't clean ;)

gennadigennadigennadi commented 6 months ago

@brightbyte thank you again for another contribution.