ComposerRequireChecker
ComposerRequireChecker copied to clipboard
maglnet
Add an option to check the development code.
The use case for this additional option is not immediately obvious to me. I'd very much read about a real world story where this additional check finds something - and what it would be.
I'm very happy this tool exists, and I want to emphasize that it's dedicated purpose makes it directly useful without using any command line options at all (once you know how it's output should trigger your next actions).
Originally, we had 1 code base and 1 list of dependencies, and they had to "match". Now we'd introduce 2 code bases and 2 lists of dependencies, and they add up to 4 relations, some of them probably must still "match", but others do not need to.
In fact, after painting it on my whiteboard, I'd say this: Prod code has to have all it's symbols be in the prod dependencies (with it's own code being one of them). Prod code must not have symbols in the dev dependencies. Dev code may have it's symbols in prod dependencies. Dev code may have it's symbols in dev dependencies.
And if we could get it coded like this, then the option would actually make sense to add, because the prod code check would remain the same and still be triggering errors in the same cases, thus removing the need to profusely explain the differences in the readme.
The use case is the following : finding indirect dependencies in test code, for very same reason you want to find them in the production one. Someone once told me to treat the test code the same way you treat the production one.
However, in this version, CRC --dev checks both the prod and dev code against both dev and production dependencies, which should not be an issue, but is surely confusing. I will try to make it clearer.
Ok, so I've tried to slap a couple of graphs together to explain the use cases.
The current CRC check ensures that the production code of the checked projects only depends on production code of direct, production dependencies. E.g.:
The checked project only requires "foo", so only production code from the foo package is allowed. bar is a dev dependency, and quz, tester, mouse and builder are indirect dependencies, so they should not be referenced from the code.
In the case of a dev check, the proposed change ensures that the dev code (e.g. test and builder code, mostly) only depends on the production code of direct production and dev packages. E.g.:
The checked project requires both "foo" and "bar" for its development, so production code from both there packages is allowed. quz, tester, mouse and builder are indirect dependencies, so they should not be referenced from the code.
As you can see, there are invariants:
- References to indirect dependencies are always invalid.
- References to development code of dependencies are always invalid. There are not included in Composer autoloader anyway, so they should not be possible in the first place.
@Ocramius, @SvenRtbg does this address your concern?
I would appreciate if this was supported, I arrived here because at first I thought I have a bug somewhere in the setup.
My realword usecase:
I know about this tool for a long time and when I recently encountered missing dependency in tests, which broke the build for a contributor and then I spent quite some time tracking where in the dependencies it broke. I thought: "This would never happened if I was using CRC".
Today, when I was adding CRC I wanted to test this case, to see it fail and nothing happened, which was not really obvious to me, since it is not really mentioned in the Readme. I think the usecase is exactly the same as with the "production" code, just as this issue describes: "you should put your requires for tests to require-dev to avoid breaking in the future".
Since should be about half of the code (maybe more), I think it would be beneficial to check them as well. Plus in case of libraries a lot of times, there are more dependencies for tests than src code.
And if we could get it coded like this, then the option would actually make sense to add, because the prod code check would remain the same and still be triggering errors in the same cases, thus removing the need to profusely explain the differences in the Readme.
Yes, this is exactly my situation :) I think it does not to be "opt-in" as described in the Readme change of this PR. It is only needed to separate them during evaluation and then present accordingly in the report - whether the missing symbols are required to be "prod" or "dev".
to avoid downstream having broken dependency graphs.
@Ocramius if implemented using the rules described in the comments above I do not understand how this could have this effect?
