False positives on some bundle and classes about composerrequirechecker HOT 44 OPEN

from composerrequirechecker.

@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.

from composerrequirechecker.

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). 👍

from composerrequirechecker.

I don't care what symfony does

Yeah, your choice, I'm not trying to convince you. But yes, an option to permit both would solve everything! 👍

I'm on the issue.

from composerrequirechecker.

Another false positive not related to bundles:

class: SLLH\IsoCodesValidator\Constraints\CreditCard
library (required): sllh/iso-codes-validator

from composerrequirechecker.

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.

from composerrequirechecker.

@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?

from composerrequirechecker.

it seems that all the Bundle classes 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. 👍

from composerrequirechecker.

I don't think target-dir will really work out then... seems much more complicated than is needed...

from composerrequirechecker.

target-dir seems to be deprecated, so I really wouldn't dig into this...

from composerrequirechecker.

@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

from composerrequirechecker.

@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 :-)

from composerrequirechecker.

@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.

from composerrequirechecker.

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. 😉

from composerrequirechecker.

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\SonataEasyExtendsBundle
• Doctrine\Bundle\FixturesBundle\DoctrineFixturesBundle
• Joli\GifExceptionBundle\GifExceptionBundle
• DAMA\DoctrineTestBundle\DAMADoctrineTestBundle
• Liip\FunctionalTestBundle\LiipFunctionalTestBundle

from composerrequirechecker.

Concerning Swift_* classes, they are correctly required but no psr-0/4 autoload on the composer file:

"autoload": {
    "files": ["lib/swift_required.php"]
},

from composerrequirechecker.

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.

from composerrequirechecker.

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 ;)

from composerrequirechecker.

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 swiftmailer/swiftmailer#971 (comment).

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"
    ]
},

from composerrequirechecker.

Another false positive since I split the Symfony fullstack requirement from my project:

Symfony\Bundle\DebugBundle\DebugBundle
Symfony\Bundle\WebProfilerBundle\WebProfilerBundle
Symfony\Bundle\WebServerBundle\WebServerBundle

But 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"
},

from composerrequirechecker.

But they are correctly required and installed under a dev env:

Are they used in src? If so, it's not require-dev.

from composerrequirechecker.

@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.

from composerrequirechecker.

Most likely to be reported upstream then

from composerrequirechecker.

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".

from composerrequirechecker.

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.

from composerrequirechecker.

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?

from composerrequirechecker.

Well, the config/bundles.php may be not an issue as this file is not event autoloaded by composer.

from composerrequirechecker.

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 :-)

from composerrequirechecker.

The problem regarding the constant from #55 (comment) should be fixed with 0.2.1

from composerrequirechecker.

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\GeoIpService

My 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

from composerrequirechecker.

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?

from composerrequirechecker.

I don't see webmozart/assert in your dependencies

from composerrequirechecker.

It's a dependency of sylius/sylius:
https://github.com/Sylius/Sylius/blob/master/composer.json#L97

from composerrequirechecker.

What am I missing?

@mmenozzi the idea of this tool ;) -> https://github.com/maglnet/ComposerRequireChecker#whats-it-about

from composerrequirechecker.

@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.

from composerrequirechecker.

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.

from composerrequirechecker.

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

from composerrequirechecker.

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.

https://symfony.com/doc/current/bundles.html

from composerrequirechecker.

Yes, and you can split the development config into a separate file to be put somewhere outside your sources.

from composerrequirechecker.

i might have found a false-positive for libXMLError.
test setup: https://github.com/jnkowa-gfk/test-composer-require-checker/tree/test/libXMLError

from composerrequirechecker.

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.

from composerrequirechecker.

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.

from composerrequirechecker.

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

from composerrequirechecker.

Sounds like a separate issue: perhaps send a patch with a test around case sensitivity?

from composerrequirechecker.