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

support @internal class annotations #538

Closed
staabm opened this issue Apr 27, 2021 · 5 comments · Fixed by #904
Closed

support @internal class annotations #538

staabm opened this issue Apr 27, 2021 · 5 comments · Fixed by #904

Comments

@staabm
Copy link
Contributor

staabm commented Apr 27, 2021

to formulate all restrictions we have in our current architecture we need roughly 120 lines within the depfile.
I try to find some ways so the depfile could get easier/simpler without loosing its explicitness(or rules).

we divided our source code in modules, and each of consist of several layers.
our file strucutre (simplified) looks like:


├── lib
│   ├── account
│   │   ├── business
│   │   │   └── UserAccount.php
│   │   ├── exception
│   │   │   └── DuplicateUserMailException.php
│   │   ├── persistence
│   │   │   ├── BlockedDeliveryAddressRecord.php
│   │   │   ├── DeliveryAddressAccountRepository.php
│   │   │   └── UserAccountRepository.php
│   ├── auth
│   │   ├── business
│   │   │   ├── EmailVO.php
│   │   │   ├── EncryptedPassword.php
│   │   ├── exception
│   │   │   ├── BlockedUserException.php
│   │   │   ├── InvalidEmailException.php
│   │   ├── persistence
│   │   │   ├── UserAuthRepository.php
│   │   │   ├── UserRecord.php
│   │   │   └── UserRepository.php

the file structure and class-names adhere to PSR4

in our persistence-layer we define repository classes, which internally use record classes for the mapping to the database.

  - name: Persistence
    collectors:
      - type: className
        regex: .*\\persistence\\.*

we have rules which layer is allowed to depend on persistence. but since - because of the namespace - record classes also are part of the persistence layer a service class is actually allowed to reference/use record classes.

ruleset:
  Service:
    - Persistence

our goal is to hide the record classes within the persistence layer and that other layers which depend on the persistence layer need to call into it thru the repository classes.

atm we need to blacklist every class within the persistence layer which should not be leaked out of the layer with a separate selector and/or rule.

wouldn't it be easier if deptrac would support a class-level annotation, e.g. @internal which defines that the given class is not allowed to be referenced outside the layer it resides...?

@dbrumann
Copy link
Collaborator

dbrumann commented May 7, 2021

I started working on this, but I don't think it will make a substantial difference in how complex your configuration will be unless you have many rules for excluded internal classes.

Right now you probably have something like:

  - name: Persistence
    collectors:
      - type: bool
         must:
           - type: className
             regex: .*\\persistence\\.*
         must_not:
           - type: className
             regex: .*Record$

When the PR is finished it could be something like:

  - name: Persistence
    collectors:
      - type: bool
         must:
           - type: className
             regex: .*\\persistence\\.*
         must_not:
           - type: internal_classes

Is this what you are looking for?

@staabm
Copy link
Contributor Author

staabm commented May 7, 2021

Thx for working on it.

My intention was that a class configured @internal (or another annotation) would automatically be handled like suggested, without further rules required

dbrumann pushed a commit to dbrumann/deptrac that referenced this issue May 7, 2021
@github-actions
Copy link

This issue has not seen activity in a while and will be closed automatically soon.

@staabm
Copy link
Contributor Author

staabm commented Nov 1, 2021

@dbrumann It seems the latest release contains this issue in the changelog even though its a ongoing and not yet finished task

@dbrumann
Copy link
Collaborator

dbrumann commented Nov 1, 2021

Yes, the changelog generator reports every closed ticket between releases, whether resolved or not. Maybe if there's an option for not showing those, I can use that with the next release

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