deptrac icon indicating copy to clipboard operation
deptrac copied to clipboard

Need some help understanding new violation

Open barryvanveen opened this issue 3 years ago • 1 comments

Hi! First of all: thanks for building and maintaining this software, I have found it very useful 👍

This week I was updating one of my dependencies and ran into a new violation. This puzzles me because the kind of dependency that is now seen as a violation has been present in the same files for months, without any problems. Can you try to help me understand why this would suddenly be a problem?

It is a public repository, an example of a PR causing the errors can be found here: https://github.com/barryvanveen/blog/pull/305. The PR updates laravel/framework. In the PR you can also see my deptrac config file, which I have updated a bit once I noticed the new naming that was introduced.

Files in app/Infrastructure/Console/Commands/ extend a class vendor/laravel/framework/src/Illuminate/Console/Command.php. These are files in layer Infrastructure depending on a file in layer Framework. According to my config this should be allowed. Still it breaks.

deptrac

All 8 errors are variations on "Error: App\Infrastructure\Console\Commands\ImportData must not depend on Illuminate\Console\Signals (Infrastructure on Framework)".

And this is the bit that puzzles me. When I comment it out any usages of Illuminate\Console\Signals, I still have many other examples of Infrastructure depending on Framework code. And somehow, then it is fine and I have no violations.

deptrac

Any help would be greatly appreciate, I just can't see the problem 😅

barryvanveen avatar Sep 18 '22 18:09 barryvanveen

I am taking a look. Having some trouble getting a " non-compiled" version running so that I can step through the deptrac code as it is generating the violations. But I am on it. It just might take some time. Please be patient.

patrickkusebauch avatar Sep 21 '22 17:09 patrickkusebauch

@barryvanveen I got it. The Illuminate\Console\Signals class is marked with the @internal annotation. Therefore any reference to it outside of the layer itself is always marked as a violation.

patrickkusebauch avatar Sep 30 '22 17:09 patrickkusebauch

Hi @patrickkusebauch, thank you for investigating, I could not have figured that out by myself 🙏

If you don't mind, can you help me understand exactly why this is a violation of the layer boundaries?

I have a console command class called CreateUser that is part of the Infrastructure layer. It extends a class called Command that is part of the Framework layer. The Command class has trait InteractsWithSignals that uses code with the @internal annotation.

To me, it feels like my Infrastructure code uses Framework code, which is okay according to my configuration. Is the problem that my code extends the Framework code? I know for certain that no part of CreateUser explicitly interacts with the annotated code.

barryvanveen avatar Oct 02 '22 14:10 barryvanveen

I would discourage you from extending classes and using traits across layers. Why? Well, what does it mean to do either of those things? It means the code in the trait or parent class is also part of the child class. Therefore any calls to internal classes will fail.

Basically code in "InteractsWithSignals" is as good as code directly in "Command". And any code directly in "Command" is as good as in "CreateUser".

Now this is the puritan way that forces you into a certain design mindset that you might not like. The "problem" here is Symfony design and forcing upon you inheritance. That's just not a good design, if you want to separate layers like deptrac does.

It up to you if you want to either ignore deptrac or decouple Symfony from your application.

patrickkusebauch avatar Oct 02 '22 15:10 patrickkusebauch

I like the puritan way that deptrac helps to enforce. It makes it very visible how the dependencies run and I have to be explicit in when to allow a violation and when not. Thank you for your help!

barryvanveen avatar Oct 02 '22 19:10 barryvanveen