-
Notifications
You must be signed in to change notification settings - Fork 13
Introduce detection of dev source code used in production #236
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
base: master
Are you sure you want to change the base?
Conversation
1be6249 to
c8180ef
Compare
|
@JanTvrdik @janedbal I'm curious to hear what you think about this change. |
|
@janedbal I know you're probably very busy, but do you have advice to move this forward? Thanks. Happy to make any changes. |
This adds a new error type DEV_SOURCE_IN_PROD that detects when
production code imports classes from dev-only autoload paths.
This prevents runtime failures when for example these dev directories
are left out of the deployment to production.
The feature detects cases like:
- Production code in src/ importing from src-dev/
- Production code importing from tests/ directory
Users can ignore these errors using:
$config->ignoreErrorsOnPackage('src-dev', [ErrorType::DEV_SOURCE_IN_PROD]);
c8180ef to
b57f588
Compare
|
Sorry about late response. This idea opens quite a big topic we already discussed multiple times internally: Basically, composer-dependency-analyser could be generalized to any-dependency-analyser while reusing its internal logic. The any-dependency-analyser would probably work just by specifying modules/layers and allowed dependencies among those. Basically the same thing as deptrac is doing. We already implemented internally such generic tool - mainly because deptrac (which we used before) was too painful to use and we were not satisfied with the development happening there in recent year. But this new tool (we call it dependency-enforcer internally) is using AST for simplicity (thus is less performant) and is not ready to be open-sourced yet. We were discussing possible CDA v2 which would somehow transition to this generic tool (maybe even offering two internal engines: token based + AST based) or about just open-sourcing the depenendecy-enforcer. Both those option would allow those kind of checks you tried to implement, but obviously even much more. We did not yet decide how to proceed. Personally, I lean toward keeping CDA related to composer dependency issues only. I feel that your contribution more belongs to this generic part. If I would forget all that, I like the value this MR brings (the problem it solves), but I dislike the fact that you abuse "package" abstraction for some src|src-dev "modules". If we could figure out some better naming for this new abstraction (maybe And I believe you can use deptrac for this. |
|
Thanks @janedbal for taking the time to look into this. Personally, I hope that this amazing project doesn't turn into a Deptrac 2.0. I'll try to make some changes to this PR according to your feedback. Thanks! |
This adds a new error type
DEV_SOURCE_IN_PRODthat detects when production code imports classes from dev-only autoload paths.This prevents runtime failures when for example these dev directories are left out of the deployment to production.
The feature detects cases like:
src/importing fromsrc-dev/tests/directoryUsers can ignore these errors using: