How to correctly configure that exclude-from-classmap is kept in mind

[alexander-schranz]

I wanted to give this tool a try but I have some issues that it analyse "test" files:

Following error happening:

  [Roave\BetterReflection\Reflector\Exception\IdentifierNotFound]
  Roave\BetterReflection\Reflection\ReflectionClass "Sulu\Component\Util\Tests\Unit\FoobarTestClass" could not be found in the located source

The file is inside src/Sulu/Component/Util/Tests/Unit/SortUtilsTest.php.

Which should in my opinion not being analyzed as it not part of composer.json autoloader:

    "autoload": {
        "psr-0": {
            "Sulu\\": "src/"
        },
        "exclude-from-classmap": [
            "src/Sulu/Component/*/Tests/",
            "src/Sulu/Bundle/*/Tests/",
            "src/Sulu/Bundle/*/Resources/phpcr-migrations"
        ]
    },

Did I miss something. I run the check with following command, without a custom config:

vendor/bin/roave-backward-compatibility-check --from=2.6 --install-development-dependencies

[Ocramius]

The project seems to declare src/ to be PSR-0: you'd need to remove these classes from src/, as you have conflict autoload declarations.

Perhaps the wording in https://github.com/Roave/BackwardCompatibilityCheck/blob/a41387a409276b92c2b7a0388bf531bc8a0b96e1/README.md#pre-requisitesassumptions isn't clear, but the general idea is that sources declared in your composer.json do in fact work

[alexander-schranz]

Thx for the fast response 🚀

The project seems to declare src/ to be PSR-0: you'd need to remove these classes from src/, as you have conflict autoload declarations.

I think its not conflicting as exclude-from-classmap is specially designed to exclude test files composer docs:

If you want to exclude some files or folders from the classmap you can use the exclude-from-classmap property. This might be useful to exclude test classes in your live environment, for example, as those will be skipped from the classmap even when building an optimized autoloader.

I'm interpreting your comment correctly that currently Roave/BackwardCompatibilityCheck does not support ignore files of exclude-from-classmap currently or export-ignore in .gitattributes file?

[Ocramius]

@alexander-schranz the classmap exclusion is for dropping from the classmap: the classmap is a performance optimization, not an authoritative declaration.

exclude-from-classmap is not a "these classes don't exist in sources", but rather "these classes should not be considered for production optimizations".

.gitattributes is not considered at all: it is only affecting how Github exports the release archive, but it is not a declaration of sources vs non-sources.

[Ocramius]

To word this differently: does your codebase pass something like https://github.com/maglnet/ComposerRequireChecker/ ?

If not, then you will have issues with tools that traverse the entire sources

[alexander-schranz]

exclude-from-classmap is not a "these classes don't exist in sources", but rather "these classes should not be considered for production optimizations".

I can understand that interpretion. But real world case for libs (where bc make sense) its only used to exclude test / dev files as else the lib would not work with composer dump-autoload --classmap-authoritative flag if you would mark files as excluded which not longer could be then loaded by the autoloader. Symfony also exclude-from-classmap it this way here even it sure not best example as symfony/symfony isnt used longer a lot (Update subcomponents also using it same way: https://github.com/symfony/http-foundation/blob/0edbbac68bfa4963b819363ad63a22078d1cd297/composer.json#L38-L40).

.gitattributes is not considered at all: it is only affecting how Github exports the release archive, but it is not a declaration of sources vs non-sources.

Not only :), Composer keep it in mind (used the github packages) for the installs aslong as you not set prefer-source flag to get a whole git repo and so typical used to exclude test and unneeded files. You will not find files in dist install if its part of .gitattributes export-ignore. https://getcomposer.org/doc/02-libraries.md#light-weight-distribution-packages

To word this differently: does your codebase pass something like https://github.com/maglnet/ComposerRequireChecker/ ?

We use "shipmonk/composer-dependency-analyser" for that case: https://github.com/shipmonk-rnd/composer-dependency-analyser. So dependency are defined.

They keep the exclude-from-classmap parameter in mind: https://github.com/shipmonk-rnd/composer-dependency-analyser/blob/37cbe36d5a61f421962576c5355f117ef6c7687d/src/ComposerJson.php#L99-L102.

Same seems your linked library doing: https://github.com/maglnet/ComposerRequireChecker/blob/74f171cec6f2aa00061831ec19565021949ff7d3/src/ComposerRequireChecker/FileLocator/LocateComposerPackageSourceFiles.php#L34

---

I understand your points and I think the definitions of exclude-from-classmap and .gitattributes can be interpreted in different ways and used in different ways. And at the end its your tool and need to match your usecases not ours.

Is there any way I can ignore specific files being analysed by the tool?

The exception is thrown out uncatched before $baseline->ignores is called:

In IdentifierNotFound.php line 26:

  [Roave\BetterReflection\Reflector\Exception\IdentifierNotFound]
  Roave\BetterReflection\Reflection\ReflectionClass "Sulu\Component\Util\Tests\Unit\FoobarTestClass" could not be found in the located source


Exception trace:
  at vendor/sulu/sulu/vendor/roave/better-reflection/src/Reflector/Exception/IdentifierNotFound.php:26
 Roave\BetterReflection\Reflector\Exception\IdentifierNotFound::fromIdentifier() at vendor/sulu/sulu/vendor/roave/better-reflection/src/Reflector/DefaultReflector.php:35
 Roave\BetterReflection\Reflector\DefaultReflector->reflectClass() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/src/CompareClasses.php:61
 Roave\BackwardCompatibility\CompareClasses->makeSymbolsIterator() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/src/Changes.php:98
 Roave\BackwardCompatibility\Changes->getIterator() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/src/Changes.php:75
 Roave\BackwardCompatibility\Changes->{closure:Roave\BackwardCompatibility\Changes::applyBaseline():74}() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/src/Changes.php:98
 Roave\BackwardCompatibility\Changes->getIterator() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/src/Formatter/SymfonyConsoleTextFormatter.php:18
 Roave\BackwardCompatibility\Formatter\SymfonyConsoleTextFormatter->write() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/src/Command/AssertBackwardsCompatible.php:175
 Roave\BackwardCompatibility\Command\AssertBackwardsCompatible->execute() at vendor/sulu/sulu/vendor/symfony/console/Command/Command.php:341
 Symfony\Component\Console\Command\Command->run() at vendor/sulu/sulu/vendor/symfony/console/Application.php:1079
 Symfony\Component\Console\Application->doRunCommand() at vendor/sulu/sulu/vendor/symfony/console/Application.php:356
 Symfony\Component\Console\Application->doRun() at vendor/sulu/sulu/vendor/symfony/console/Application.php:195
 Symfony\Component\Console\Application->run() at vendor/sulu/sulu/vendor/roave/backward-compatibility-check/bin/roave-backward-compatibility-check.php:360

So something like:

<?xml version="1.0" encoding="UTF-8"?>
<roave-bc-check xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <baseline>
        <!-- Ignore all messages coming from Bundle/Tests -->
        <ignored-regex>#^.*\\Sulu\\Bundle\\.*\\Tests.*$#</ignored-regex>

        <!-- Ignore all messages coming from Component/Tests -->
        <ignored-regex>#^.*\\Sulu\\Component\\.*\\Tests.*$#</ignored-regex>
    </baseline>
</roave-bc-check>

Does not work. Do you have any idea how we could maybe still use the tool without refactoring / move classes?

[Ocramius]

• for .gitattributes: I'm very familiar with them, and they are also only an optimization, not a declaration of sources vs non-sources
• for exclude-from-classmap, it is not a disallow-list: it's still an optimization, not an exclusion from the original autoloader

It is semantically correct for this tool to not consider .gitattributes nor exclude-from-classmap.

From my PoV, I'd say that we need to adjust ignores in a way that exceptions can also be ignored: that's the most flexible way of operating here.

[alexander-schranz]

From my PoV, I'd say that we need to adjust ignores in a way that exceptions can also be ignored: that's the most flexible way of operating here.

That would be sure awesome. I tried to understand the code, but was not able to catch things correctly to get it work. So sadly I'm not able to contribute it.

[Ocramius]

I think this code needs to become resistant to crashes:

BackwardCompatibilityCheck/src/Changes.php

Lines 98 to 102 in 4d6a7e5

	foreach ($this->unBufferedChanges ?? [] as $change) {
	$this->bufferedChanges[] = $change;
	yield $change;
	}

Thinking that we need a new state for it called CRASHED or such:

BackwardCompatibilityCheck/src/Change.php

Lines 13 to 16 in 4d6a7e5

	private const string ADDED = 'added';
	private const string CHANGED = 'changed';
	private const string REMOVED = 'removed';
	private const string SKIPPED = 'skipped';

A lot of designing around this will be needed, sadly :-\