qossmic / deptrac

Keep your architecture clean.
https://qossmic.github.io/deptrac
MIT License
2.65k stars 134 forks source link

`\` vs `\\` in depfile #508

Closed clxmstaab closed 3 years ago

clxmstaab commented 3 years ago

my testing on windows suggests that the example mentioned in the readme

layers:
  - name: Foo
    collectors:
      - type: implements
        implements: 'App\SomeInterface'

does not work.

it works after adjusting it to

layers:
  - name: Foo
    collectors:
      - type: implements
        implements: 'App\\SomeInterface'

is this a windows only issue? is the example in the readme wrong?

smoench commented 3 years ago

The issue is not caused by windows this time but by YAML. The examples for implements etc. are all incorrect those backslashes need to be replaced with \\.

clxmstaab commented 3 years ago

I see, readme fixing PR incoming.

could/should deptrac validate the yaml? would this help?

xabbuh commented 3 years ago

@clxmstaab Are you sure that you used single and not double quotes?

clxmstaab commented 3 years ago

Are you sure that you used single and not double quotes?

yep

image

before the mentioned commit

image

smoench commented 3 years ago

Does your AuthenticationBusinessContext directly implements Behat\Behat\Context\Context or inherits an other class/interface which is located in your vendors?

clxmstaab commented 3 years ago
<?php

use Behat\Behat\Context\Context;
//...

class AuthenticationBusinessContext implements Context
smoench commented 3 years ago

Behat\Behat\Context\Context needs to be covered by a layer. Can you try following

layers:
 #...
 - name: Behat
    collectors:
      - type: className
        regex: ^Behat\\.*
ruleset:
  FeatureTest:
    - Behat
clxmstaab commented 3 years ago

thx for your feedback guys, really appreciate it.

I created a minimal repro for my problem:

https://github.com/clxmstaab/deptrac-issue508

behat does not report the wrong dependency from FeatureTest on Integration..

smoench commented 3 years ago

https://github.com/clxmstaab/deptrac-issue508 -> 404

clxmstaab commented 3 years ago

sorry, its public now.

smoench commented 3 years ago

I created a PR with a solution https://github.com/clxmstaab/deptrac-issue508/pull/1

clxmstaab commented 3 years ago

you are right guys, that adding the double backslashes was not correct. it turns out there were other problems within out config which were unnoticed.

it feels like there is some kind of underlying UX/DX issue, as its pretty easy to configure things wrong but don't be aware of it.

just to let you know, our depfile in the end looks like:

paths:
  - app/portal/controllers/
  - app/portal/features/
  - app/portal/lib/
  - app/portal/models/
  - app/portal/views/
layers:
  - name: Controller
    collectors:
      - type: className
        regex: .*Controller.*
  - name: View
    collectors:
      - type: directory
        regex: .*/app/portal/views/.*
  - name: ViewModel
    collectors:
      - type: className
        regex: ^(.*\\viewmodel\\.*|I18N|LanguageHelper)$
  - name: Service
    collectors:
      - type: className
        regex: .*\\service\\.*
  - name: Integration
    collectors:
      - type: className
        regex: .*\\integration\\.*
  - name: Persistence
    collectors:
      - type: className
        regex: .*\\persistence\\.*
  - name: Business
    collectors:
      - type: className
        regex: .*\\business\\.*
  - name: BusinessException
    collectors:
      - type: className
        regex: .*\\exception\\.*
  - name: FeatureTest
    collectors:
      - type: implements
        implements: 'Behat\\Behat\\Context\\Context'
  - name: ActiveRecord
    collectors:
      - type: className
        regex: ^.+(Record)$
  - name: ActionResults
    collectors:
      - type: className
        regex: ^.*(Result)$
  # list vendor dependencies
  # https://github.com/qossmic/deptrac/issues/506#issuecomment-790677433
  - name: Vendor
    collectors:
      - type: className
        regex: ^(ZxcvbnPhp\\.*|WebConnector\\.*|ActionMailer)$
ruleset:
  Controller:
    - View
    - Service
    - ViewModel
    - BusinessException
    - ActionResults
  Service:
    - Persistence
    - Business
    - BusinessException
    - Integration
    - ViewModel
  Business:
    - BusinessException
    - Vendor
  Integration:
    - Business
    - Vendor
  Persistence:
    - Business
    - ActiveRecord
  ActiveRecord:
    - Business
  View:
    - ViewModel
  ViewModel:
    - Business
    - BusinessException
  FeatureTest:
    - Business
    - BusinessException

its kind of convoluted.. some of the rules here are necessary because of the class-structures our framework is dictating us.

smoench commented 3 years ago

Would be https://github.com/qossmic/deptrac/issues/327 a possible solution to improve DX/UX?

clxmstaab commented 3 years ago

I guess having a list/visualization of which classes are part of which layer - like described in the linked issue - could be helpful.