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

Make deptrac complain about namespaces that don't belong to any layers #231

Closed
jakzal opened this issue Jun 11, 2019 · 4 comments · Fixed by #285
Closed

Make deptrac complain about namespaces that don't belong to any layers #231

jakzal opened this issue Jun 11, 2019 · 4 comments · Fixed by #285
Assignees
Milestone

Comments

@jakzal
Copy link

jakzal commented Jun 11, 2019

Given the following config:

paths:
  - ./src
exclude_files: ~
layers:
  - name: Domain
    collectors:
      - type: className
        regex: ^Foo\\Domain\\.*
  - name: Infrastructure
    collectors:
      - type: className
        regex: ^Foo\\Infrastructure\\.*
  - name: Doctrine ORM
    collectors:
      - type: classname
        regex: ^Doctrine\\(Common|ORM)\\.*

ruleset:
  Domain:
  Infrastructure:
  - Doctrine ORM

Everything works as expected within the defined layers.

However, any layer can depend on namespaces that are not defined as layers yet. At the moment if someone introduced a dependency on Symfony\Component\DependencyInjection\ContainerInterface in Foo\Domain, deptrac wouldn't complain.

Things can be easily missed this way. Any newly introduced dependency needs to be defined as a layer. Someone needs to remember to do it, and people tend to forget.

For this reason I've been introducing a catch-all layer, that would catch any namespace that doesn't belong to other layers:

layers:
  # ...
  - name: Other Vendors
    collectors:
      - type: bool
        must:
          # must be outside of global namespace
          - type: className
            regex: '[\\]+'
        must_not:
          # must not be one of the known vendors or packages
          - type: className
            regex: ^Foo\\(Domain|Infrastructure)\\.*
          - type: className
            regex: ^Doctrine\\(Common|ORM)\\.*

Each time a layer is defined, the "Other Vendors" layer needs to be updated as well to exclude the new layer.

The advantage of this approach is that as soon as a new dependency or a package is introduced, deptrac will complain it doesn't know what to do with it.

The drawback of this approach is having to exclude any new layer from "Other Vendors" each time (so each regex is defined in two places).

My preference would be if deptrac either complained about namespaces that don't belong to any layers by default, or if there was a "catch-all-that-was-not-defined" collector. Perhaps authors have better ideas, or maybe I'm doing it wrong.

Real life examples:

@smoench
Copy link
Contributor

smoench commented Sep 17, 2019

It has nothing todo with collectors as all dependencies will be collected. The RulesetEngine ignores classes without any layer. We need to refactor the RulesetEngine to adapt violations, skipped violations and uncovered dependencies to the DependencyContext.
Per configuration we could choose a strategy (ignore, warning, error) how uncovered dependencies are exposed.

This refactoring would us allow resolve #205 as we could add allowed dependencies also to the context.

@smoench smoench self-assigned this Sep 17, 2019
@smoench smoench added this to the 0.6 milestone Sep 17, 2019
@smoench
Copy link
Contributor

smoench commented Sep 20, 2019

@jakzal I added an POC #266 how uncovered dependencies could be tracked

@smoench smoench modified the milestones: 0.6, 0.7 Oct 18, 2019
@smoench
Copy link
Contributor

smoench commented Feb 5, 2020

@jakzal can you have a look at #285 and give me some feedback if this fits your needs 😃

@jakzal
Copy link
Author

jakzal commented Feb 5, 2020

@smoench I will as soon as I can, thanks for working on this!

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

Successfully merging a pull request may close this issue.

2 participants