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

Irrelevant transient dependencies reported #1218

Open
lstrojny opened this issue Jun 6, 2023 · 3 comments
Open

Irrelevant transient dependencies reported #1218

lstrojny opened this issue Jun 6, 2023 · 3 comments

Comments

@lstrojny
Copy link

lstrojny commented Jun 6, 2023

class VO {}

abstract class A {
    final public function getVO(): VO { return new VO($this->generate()); }
    abstract protected function generate(): string;
}

class B extends A {
    protected function generate(): string { return "from B"; }
}

Now assume each class lives in its own layer called like the class wit this ruleset:

VO: []
A: [VO]
B: [A]

deptrac will complain that B depends on VO even though I would say it does not. Bug or feature?

@lstrojny lstrojny changed the title Irrelevant transient dependencies not ignored Irrelevant transient dependencies reported Jun 6, 2023
@patrickkusebauch
Copy link
Collaborator

patrickkusebauch commented Jun 6, 2023

This is absolutely a feature. Looks what happens when you "squash" B into the final shape the class will have at runtime:

class B {
    final public function getVO(): VO { return new VO($this->generate()); }
    protected function generate(): string { return "from B"; }
}

You absolutely depend on VO. You can look at it another way:

If I deleted VO, would B lint?

Obviously, it would not, it would complain about non-existent VO.

Deptrac should provide you with a relatively nice message about this being a dependency coming from A via inheritance as well. At least with the default console formatter.

@patrickkusebauch
Copy link
Collaborator

Thinking about it a bit more.

Is the argument there that since final public function getVO(): VO { return new VO($this->generate()); } is final it is always executed in the context of A and therefore not B should not be dependent on VO? And if the method was not final it would be?

That would be an interesting case.

If that would be the case what would happen if I had:

class C {
  public function foo(): void {
    $var = new B(); //This is clear dependency on B
    $vo = $var->getVO(); //Is this dependency on B or on A?
    var_dump($vo); //This could be a dependency on VO, but it is not yet implemented in deptrac
  }
}

It creates another conundrum. I am not opposed to this, just wondering what were your expectations. It is important that deptrac behaves the way people expect it do behave.

@lstrojny
Copy link
Author

lstrojny commented Jun 6, 2023

Hah, interesting indeed. I love the "if you delete it, will it compile" heuristic, since it’s super easy to grasp. That being said, the final case is not atypical in a ports-and-adapters kinda situation.
I guess this needs more long showers and long walks before I can say something more insightful here. But I will come back.

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

No branches or pull requests

2 participants