soullivaneuh
commented
6 years ago Another false positive not related to bundles:
class: SLLH\IsoCodesValidator\Constraints\CreditCard
library (required): sllh/iso-codes-validator
Open soullivaneuh opened 6 years ago
soullivaneuh
commented
6 years ago Another false positive not related to bundles:
class: SLLH\IsoCodesValidator\Constraints\CreditCard
library (required): sllh/iso-codes-validator
soullivaneuh
commented
6 years ago I updated the issue body. Except Composer\Script\Event and KERNEL_ROOT_DIR (I have to instigate about why), all are false postivives to me.
Ocramius
commented
6 years ago @maglnet do we have something to collect all defined symbols and the defining files? Would be interesting to have some debug output that dumps all files associated with the originating file.
Ocramius
commented
6 years ago @Soullivaneuh it seems that all the Bundle classes aren't there. Are they by chance in a classmap instead of a PSR-0 or PSR-4 namespace?
soullivaneuh
commented
6 years ago it seems that all the
Bundleclasses aren't there.
You are right, only some are reported.
Are they by chance in a classmap instead of a PSR-0 or PSR-4 namespace?
It looks like not. But I found the target-dir is used. Example: https://github.com/Gregwar/ImageBundle/blob/v2.1.3/composer.json
I check that on some bundle of this list, and they all use this keyword. I think it's a clue. :+1:
Ocramius
commented
6 years ago I don't think target-dir will really work out then... seems much more complicated than is needed...
Ocramius
commented
6 years ago target-dir seems to be deprecated, so I really wouldn't dig into this...
soullivaneuh
commented
6 years ago @Ocramius Well, is that really hard to manage target-dir? If not it would be a great fix. If yes, and because it's deprecated, in this case the bundles should be fixed.
Maybe this tool should also trow a warning about that?
Going back to the issue. Some other bundle does not work simply because composer.json is not present. For example: https://github.com/dmaicher/doctrine-test-bundle/blob/ccdea2ce9fec5048385d1b9b5bc7c4c3f32ab48f/.gitattributes
Ocramius
commented
6 years ago @Soullivaneuh most of these issues would be fixed by switching to BetterReflection, I reckon. It's just not gonna be nice for performance, but it would be much, much more reliable :-)
maglnet
commented
6 years ago @maglnet do we have something to collect all defined symbols and the defining files? Would be interesting to have some debug output that dumps all files associated with the originating file.
Sadly currently not, but this should be done soon as it would make debugging issues like this a lot easier and also gives a user hints where dependencies are found without the need to do a fulltext search within the code.
soullivaneuh
commented
6 years ago It's just not gonna be nice for performance, but it would be much, much more reliable :-)
It's a dev tool. IMHO, we can drop a bit of performance to make it more reliable. :wink:
soullivaneuh
commented
6 years ago Another case: Sensio\Bundle\GeneratorBundle\SensioGeneratorBundle
For this one, I don't know why composer-require-checker is yelling. The composer.json file is present and the deprecated target-dir is not used at all.
EDIT: Same thing for:
Sonata\EasyExtendsBundle\SonataEasyExtendsBundleDoctrine\Bundle\FixturesBundle\DoctrineFixturesBundleJoli\GifExceptionBundle\GifExceptionBundleDAMA\DoctrineTestBundle\DAMADoctrineTestBundleLiip\FunctionalTestBundle\LiipFunctionalTestBundlesoullivaneuh
commented
6 years ago Concerning Swift_* classes, they are correctly required but no psr-0/4 autoload on the composer file:
"autoload": {
"files": ["lib/swift_required.php"]
},Concerning KERNEL_ROOT_DIR, it's a constant I defined on a php file:
define('KERNEL_ROOT_DIR', __DIR__)And this php file is auto-loaded by composer, so it should not be an issue.
maglnet
commented
6 years ago Ok, I understand the problem with Swift classes: the mentioned file only registers an autoloader but does not define the symbols, so I think all Swift_* classes are unknown.
I think we cannot fix this, because it is afaik impossible to get all known classes from an autoloader.
Nevertheless, the Swift project could add an psr-0 path to its composer.json. This would probably fix the issues with classes from this project.
The second problem with the constant could be, that the autoloaded file requires another file and we currently do not follow require[_once] or include[_once] but this may be fixed.
If you could confirm this or could provide an simple to reproduce test case, that would be perfect.
Otherwise I can only hope that this would fix this issue ;)
soullivaneuh
commented
6 years ago I think we cannot fix this, because it is afaik impossible to get all known classes from an autoloader. Nevertheless, the Swift project could add an psr-0 path to its composer.json.
Please see https://github.com/swiftmailer/swiftmailer/issues/971#issuecomment-370377599.
The second problem with the constant could be, that the autoloaded file requires another file and we currently do not follow...
The define function is on the app/env.php and autoloaded by composer:
"autoload": {
"psr-4": { "": "src/" },
"files": [
"app/env.php",
"app/AppKernel.php"
]
},Another false positive since I split the Symfony fullstack requirement from my project:
Symfony\Bundle\DebugBundle\DebugBundle
Symfony\Bundle\WebProfilerBundle\WebProfilerBundle
Symfony\Bundle\WebServerBundle\WebServerBundleBut they are correctly required and installed under a dev env:
"require-dev": {
"symfony/debug-bundle": "^4.0",
"symfony/var-dumper": "^4.0",
"symfony/web-profiler-bundle": "^4.0",
"symfony/web-server-bundle": "^4.0"
},But they are correctly required and installed under a dev env:
Are they used in src? If so, it's not require-dev.
soullivaneuh
commented
6 years ago @Ocramius It's quite more complicated, it's only bundle instantiation on app/AppKernel.php and later on config/bundles.php according to Symfony Flex structure.
Ocramius
commented
6 years ago Most likely to be reported upstream then
soullivaneuh
commented
6 years ago Most likely to be reported upstream then
What do you mean?
config/bundles.php is a special file to activate or not the bundle according to environment. There is no reason to require debug-bundle on production.
Maybe not really a false positive according to the current behavior, but it would be great to have an option to specify files where both can be accepted (dev and not). :+1:
Ocramius
commented
6 years ago Well, if it's in the considered sources, then it should not be in require-dev, but in require. It really is just that.
If config/bundles.php does environment switching decisions and still relies on these symbols, the solution is to split it up to paths that aren't considered as "production-ish".
Ocramius
commented
6 years ago Btw, yes, I fully understand where this comes from: I'm just saying that the exit code with this particular tool will always be 1. We have similar "opt-in dependencies" in zendframework too, and they're really no fun when they land in production and things start to crash by accident.
soullivaneuh
commented
6 years ago I understand your point of view but I still think files like config/bundles.php are very specials.
I won't require packages on production if I don't need them.
Plus for that case, it will an issue for a lot of people following the Symfony standard.
Adding a simple option listing the specific files where the tool should not care if they are require on dev env or not and let the user take the risk (or not) still worth it IMHO! :-)
Shall I open a separate issue for that?
soullivaneuh
commented
6 years ago Well, the config/bundles.php may be not an issue as this file is not event autoloaded by composer.
Ocramius
commented
6 years ago Plus for that case, it will an issue for a lot of people following the Symfony standard.
I said it many, many, many many times: I don't care what symfony does. In this and in other contexts. I also don't care about what zendframework does either, since it's not the scope of this project. The tool simply reports if something doesn't respect the simple rule of "X used in Y but is not in require".
Shall I open a separate issue for that?
My simplistic suggestion (because every tool is opinionated, and these will always pop up a lot) is to suggest a feature in which we can map 'file-name.php' => ['Dependencies\\To', 'Exclude\\Here']. Yes, new issue please :-)
soullivaneuh
commented
6 years ago I don't care what
symfonydoes
Yeah, your choice, I'm not trying to convince you. But yes, an option to permit both would solve everything! :+1:
I'm on the issue.
maglnet
commented
6 years ago The problem regarding the constant from https://github.com/maglnet/ComposerRequireChecker/issues/55#issuecomment-368920763 should be fixed with 0.2.1
henri9813
commented
5 years ago Hello,
I had the same issue, with Cravler\MaxMindGeoIpBundle, ( https://packagist.org/packages/cravler/maxmind-geoip-bundle )
composer.json:
"require": {
"cravler/maxmind-geoip-bundle": ^1.2,>1.2.1"
}My command and his output:
henri@cffc6e0b5438:/code$ php -d error_reporting=0 -d xdebug.max_nesting_level=1500 `which composer-require-checker` --ignore-parse-errors | tail -n +6 | head -n -1 | sed 's/[| ]//g' | sort || true && git diff --exit-code .composer-require-checker.lock
Cravler\MaxMindGeoIpBundle\Service\GeoIpService
diff --git a/.composer-require-checker.lock b/.composer-require-checker.lock
index e69de29..792ade8 100644
--- a/.composer-require-checker.lock
+++ b/.composer-require-checker.lock
@@ -0,0 +1 @@
+Cravler\MaxMindGeoIpBundle\Service\GeoIpServiceMy usage:
<?php
declare(strict_types=1);
namespace App\Controller;
use Cravler\MaxMindGeoIpBundle\Service\GeoIpService;
final class HomeController
{
public function __construct(GeoIpService $geoIp)
{
$this->geoIp = $geoIp;
}
}Best regards, Henri Devigne
mmenozzi
commented
5 years ago Hi guys, I don't know if this is related to this issue but I tried to run this tool on https://github.com/Sylius/Sylius-Standard and I get this:
➜ composer-require-checker check composer.json
ComposerRequireChecker unknown-development
The following unknown symbols were found:
+----------------------------------------------------------------+--------------------+
| unknown symbol | guessed dependency |
+----------------------------------------------------------------+--------------------+
| Doctrine\Migrations\AbstractMigration | |
| Doctrine\DBAL\Schema\Schema | |
| Symfony\Component\DependencyInjection\ContainerAwareInterface | |
| Webmozart\Assert\Assert | |
| Symfony\Component\DependencyInjection\ContainerInterface | |
| Doctrine\Common\Persistence\ObjectRepository | |
| Symfony\Component\HttpKernel\Kernel | |
| Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait | |
| Symfony\Component\DependencyInjection\ContainerBuilder | |
| Symfony\Component\Config\Loader\LoaderInterface | |
| Symfony\Component\Config\Resource\FileResource | |
| Symfony\Component\Routing\RouteCollectionBuilder | |
| PSS\SymfonyMockerContainer\DependencyInjection\MockerContainer | |
| Symfony\Component\HttpKernel\Config\FileLocator | |
| Symfony\Component\Config\Loader\LoaderResolver | |
| Symfony\Component\DependencyInjection\Loader\XmlFileLoader | |
| Symfony\Component\DependencyInjection\Loader\YamlFileLoader | |
| Symfony\Component\DependencyInjection\Loader\IniFileLoader | |
| Symfony\Component\DependencyInjection\Loader\PhpFileLoader | |
| Symfony\Component\DependencyInjection\Loader\GlobFileLoader | |
| Symfony\Component\DependencyInjection\Loader\DirectoryLoader | |
| Symfony\Component\DependencyInjection\Loader\ClosureLoader | |
| Symfony\Component\Config\Loader\DelegatingLoader | |
+----------------------------------------------------------------+--------------------+I think that a lot of these are false positives. For sure Symfony\Component\DependencyInjection\Loader\YamlFileLoader (and other loaders) and also Webmozart\Assert\Assert. What am I missing?
Ocramius
commented
5 years ago I don't see webmozart/assert in your dependencies
mmenozzi
commented
5 years ago It's a dependency of sylius/sylius:
https://github.com/Sylius/Sylius/blob/master/composer.json#L97
kubawerlos
commented
5 years ago What am I missing?
@mmenozzi the idea of this tool ;) -> https://github.com/maglnet/ComposerRequireChecker#whats-it-about
mmenozzi
commented
5 years ago @mmenozzi the idea of this tool ;)
I swear that I've read it before submitting this comment. Now I just read it again twice and I understood. Sorry guys.
mpdude
commented
4 years ago Regarding bundles.php, one way to work around this is to put it into autoload.exclude-from-classmap in the composer.json file. Does not matter for Symfony (the file is not autoloaded anyway), and makes ComposerRequireChecker ignore it.
Ocramius
commented
4 years ago bundles.php should most likely contain only prod info: if you have an if (APP_ENV === 'dev') { switch in there, you can segregate it out to your test/ dir
mpdude
commented
4 years ago Unfortunately, the way suggested by Symfony is one big array with all bundle classes as keys and sub-arrays for each one choosing the environment where it should be used.
Ocramius
commented
4 years ago Yes, and you can split the development config into a separate file to be put somewhere outside your sources.
jnkowa-gfk
commented
3 years ago i might have found a false-positive for libXMLError.
test setup: https://github.com/jnkowa-gfk/test-composer-require-checker/tree/test/libXMLError
Sebbo94BY
commented
2 years ago One more false-positive:
+-----------------+--------------------+
| unknown symbol | guessed dependency |
+-----------------+--------------------+
| IMAP\Connection | |
+-----------------+--------------------+composer.json requires the required PHP extension:
"require": {
"php": "^7.2 || ^8.0",
"ext-fileinfo": "*",
"ext-iconv": "*",
"ext-imap": "*",
"ext-mbstring": "*"
},See https://github.com/barbushin/php-imap/blob/master/composer.json
Would be also helpful, if I would just be able to "ignore" specific unknown symbols using the config file.
maglnet
commented
2 years ago Hi,
I'm not sure if it's a false positive. If you're running the check with PHP versions < 8.1 the class is not available (seems like it was introduced with 8.1) and so it's a unknown symbol for the lower versions.
Ignoring symbols is already possible. Please have a look at the symbol-whitelist within the example configuration: https://github.com/maglnet/ComposerRequireChecker/blob/3.7.x/data/config.dist.json
SvenRtbg
commented
2 years ago Looks like the referenced project has never released a version that is compatible with PHP 8.1. I cannot identify what exactly is going on, but that link above currently reads
"php": "^7.2 || ^8.0 <8.1",which is true for all versions 4.2.x, any earlier 4.x is allowing php ^7.2, and version 3.x states php >=5.5 or >=5.6. Looking at their bugtracker, apparently being compatible with PHP 8.1 is an open issue right now.
tvdijen
commented
1 year ago i might have found a false-positive for
libXMLError. test setup: https://github.com/jnkowa-gfk/test-composer-require-checker/tree/test/libXMLError
This is still an issue.. This checker will only recognize it when spelled with a capital L (LibXMLError), but not when correctly spelled with a lowercase l as libXMLError
Ocramius
commented
1 year ago Sounds like a separate issue: perhaps send a patch with a test around case sensitivity?
Command result (extract):
But the corresponding bundles are required on the root composer.json file: