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

False positive: How can this be a violation? #351

Closed
josefsabl opened this issue Aug 6, 2020 · 2 comments
Closed

False positive: How can this be a violation? #351

josefsabl opened this issue Aug 6, 2020 · 2 comments

Comments

@josefsabl
Copy link

josefsabl commented Aug 6, 2020

The PHP code:

<?php

declare(strict_types=1);

final class Foo extends Bar
{
}

class Bar
{
    private function getBaz(): Baz {
        return new Baz();
    }
}

final class Baz
{
}

depfile.yml

paths:
  - /

layers:
  - name: Foo
    collectors:
      - type: className
        regex: ^Foo$

  - name: Bar
    collectors:
      - type: className
        regex: ^Bar$

  - name: Baz
    collectors:
      - type: className
        regex: ^Baz$

ruleset:
  Foo:
    - Bar

  Bar:
    - Baz

  Baz: ~

Result of analysis:

Foo must not depend on Baz (Foo on Baz)
/sandbox/Bar.php::8
	Bar::5 -> 
	Baz::8
Foo must not depend on Baz (Foo on Baz)
/sandbox/Bar.php::7
	Bar::5 -> 
	Baz::7

Looks like the fact that the method is private is ignored. I would understand this as violation if it was protected.

@josefsabl
Copy link
Author

josefsabl commented Aug 6, 2020

What is worse, the Bar class don't need to expose the dependency in its interface at all. It looks like it just need to use it internally to trigger the violation.

<?php

declare(strict_types=1);

final class Foo extends Bar
{
}

class Bar
{
    private function whatever() {
        $theBaz = new Baz();
    }
}

final class Baz
{
}

This also triggers the violation for same configuration.

@smoench
Copy link
Contributor

smoench commented Dec 22, 2020

This is an indirect dependency. Deptrac does is only a static analyser and doesn't know about the runtime path. I could imagine the private function is being used by a protected or public method in Bar which itself is being used by Foo or will be promoted to the public methods of Foo. If none of this is true this method is probably unused and can be safely deleted or you have might have a bad abstraction.

@smoench smoench closed this as completed Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants