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

Feature request: detect if identifiers from autoload-dev are used #29

Open
willemstuursma opened this issue Nov 8, 2018 · 6 comments
Open
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@willemstuursma
Copy link
Contributor

It would be great if this package could detect usages of classes from autoload-dev in regular code, just like it detects usage from dependencies.

For example, we have some base tests / testing traits we load using autoload-dev in composer.json. Of course we want to be alerted when these are used accidentally in production code.

@janmartenjongerius janmartenjongerius added the enhancement New feature or request label Nov 8, 2018
@janmartenjongerius
Copy link
Contributor

janmartenjongerius commented Nov 9, 2018

I really like this idea. After thinking it through for a bit, I suggest approaching it as follows:

Create a new implementation of DependencyGuardInterface, DevDependencyGuard. Create a third implementation, DependencyGuardChain.
Update the factory to create a chain with the two existing implementations.

If it so happens that this causes an alarming number of duplicate analysis calls, think about creating a symbol context that can be passed to both implementations and elevates analysis from one implementation to the next.

@willemstuursma, are you willing to implement this? I can't make any promises on clearing my schedule any time soon.

@willemstuursma
Copy link
Contributor Author

@johmanx10 I will discuss internally and report back.

@janmartenjongerius janmartenjongerius added this to the 1.1.0 milestone Nov 11, 2018
@willemstuursma
Copy link
Contributor Author

@johmanx10 can you give me some pointers on how we could add this functionality?

@janmartenjongerius
Copy link
Contributor

Okay @willemstuursma , so I have been reading your issue more carefully and dove into the code for a bit. There is a class called ViolationFinder, which is responsible for, as in the name, finding violations in source code. Under the hood it splits its responsibility between determineLockViolations and determineUnusedCodeViolations. The first one is to determine if candidates are or are not present in the vendor directory. The second one is responsible for finding installed packages that aren't used. This finder is what you would want to expand or maybe even break up. Without having to introduce too many changes in the code base, one could do this by creating multiple traits.

Then, now you've found this point of entry, you would want to create a list of files that are autoloaded because of the autoload-dev section. Now, this can be done by slightly souping up the \Mediact\DependencyGuard\Composer\Iterator\SourceFileIteratorFactory. This has a method create which receives the Composer instance. it currently has the following line of code in place to ensure you only get the files from autoload and not autoload-dev:

$autoloadGenerator->setDevMode(false);

That value, false, should become configurable, so you can reuse this class to determine a list of all autoloadable files and then create a difference between the production ready files. This is where you have the necessary information to let the violation finder determine whether or not the candidates hold a symbol that is encountered in code from autoload, yet originates in a file in the list from autoload-dev.

Please be aware that if directives in autoload-dev match (part of) files or directives in autoload, that is currently not really detectable.

I hope this was helpful to you :)

@willemstuursma
Copy link
Contributor Author

Thanks @johmanx10.

We're going to try to create an implementation for this with my team at Mollie. I expect it to be picked up somewhere between next week - end of this year.

@janmartenjongerius
Copy link
Contributor

Thanks in advance for the contribution! There are currently two pull requests actively trying to patch bugs. In regards with release planning, I'm looking to get those two merged ASAP, but at least before a new feature is introduced. Since developer availability tends to be pretty low at the end of the year; please understand that this release might happen as the first thing we do in 2019, depending on how things go.

@janmartenjongerius janmartenjongerius modified the milestones: 1.1.0, 1.2.0 Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants