Undesired namespace consolidation

[llaville]

Describe the bug

If you have two namespaces that have common parts, they are treated like partial imports and are combined into one.

Additional information

This issue superseded to PR #102. Thanks to @ComiR for reporting by its unit tests !

Context

<?php
use My\Space\Using\ForeignUtility;
use ForeignUtility\SpecialClass;

Explains

Now, it's time to explains origin of this issue. (Thanks to git blame to help in search of origin)

This problem was related long time ago by composer-unused/composer-unused#287 and fixed by PR #26.

But consolidation made by PR 26 fixes is not acceptable if you use only UseStrategy in context above.

How To Fix

I'll propose a new PR that will continue to consolidate namespaces in context of composer-unused/composer-unused#287, but also fix this context (even if I don't think there is a real use case: @ComiR feel free to publish a full example where you have imports without classes/objects used)

My PR will apply this https://github.com/composer-unused/symbol-parser/blob/0.2.2/src/Parser/PHP/ConsumedSymbolCollector.php#L59-L88 resolution only if UseStrategy is combined with another one.

And it will apply this https://github.com/composer-unused/symbol-parser/blob/0.1.7/src/Parser/PHP/ConsumedSymbolCollector.php#L58 resolution in other combinations.

PR will follows soon.

[llaville]

As PR #143 passed all tests now, it's time to @ComiR to give real use cases (I want to know ;-)

[ComiR]

@llaville The imports were used. I only wrote the test without the code as it wasn't relevant for the UseStrategy but unfortunately, the problem persists. Here a more complete example:

<?php

use My\Space\Using\ForeignUtility;
use ForeignUtility\SpecialClass;

new SpecialClass(ForeignUtility::CONFIGURATION);

The package I actually use is amzn/amazon-pay-api-sdk-php (especially Amazon\Pay\API\Client) the problematic part is Amazon.

[llaville]

@ComiR I'm sorry, but I don't understand what you're expected here with this code snippet. Could you gave us more explains please ?

[ComiR]

@llaville sure. I currently don't have the time to trace this, so I'll just post the files to reproduce. I guess the real reason for this is something completely different, as it even occurs without the extra import (i.e. like in my example where I use the class name instead of self).

src/Amazon.php:

<?php

namespace My\Space;

use Amazon\Pay\API\Client;

class Amazon
{
	private const CONFIG = [];

	public static function createClient(): Client
	{
		return new Client(Amazon::CONFIG);
	}
}

composer.json:

{
    "name": "my/test",
    "type": "project",
    "require": {
        "amzn/amazon-pay-api-sdk-php": "^2.6"
    },
    "require-dev": {
        "icanhazstring/composer-unused": "^0.8.11"
    },
    "autoload": {
        "psr-4": {
            "My\\Space\\": "src/"
        }
    }
}

./vendor/bin/composer-unused output:

Results
-------

Found 0 used, 1 unused, 0 ignored and 0 zombie packages

 Used packages

 Unused packages
 ✗ amzn/amazon-pay-api-sdk-php (https://github.com/amzn/amazon-pay-api-sdk-php)

 Ignored packages

 Zombies exclusions (did not match any package)

When I change Amazon::CONFIG to self::CONFIG, the usage is detected correctly.

[llaville]

I already gave my answer at #143 (comment)

[llaville]

I've found a solution.
It's based on a custom version of NameContext and NameResolver components of PHP-Parser.
See official Name Resolution documentation for more details.

I have to write unit tests before to propose a new PR !

PS: As PHP-Parser itself there is a limitation : only self and parent special keywords may be able to be resolved. For static keyword it's impossible.

[llaville]

@icanhazstring BTW, are you agree if I change properties visibility on ConsumedSymbolCollector
https://github.com/composer-unused/symbol-parser/blob/0.2.2/src/Parser/PHP/ConsumedSymbolCollector.php#L23-L26
from private to protected

[llaville]

A teaser before upcoming PR (will TRUE unit/regression tests), here is how I've tested easily

How To use the demo script

• Change value of $useDefaultSymbolParserErrorHandler to see error as soon at it raised, or just collect them all.

i.e : When $useDefaultSymbolParserErrorHandler = false, following error

>>> Failure occurred (PhpParser\Error) : Cannot use My\Space\Foo as self because the name is already in use on unknown line
==> Will not stop demo here !
>>> Symbols found :
array (
)

is raised by default NameContext component.

TIP : to avoid such problem, use the default ComposerUnused\SymbolParser\Parser\PHP\ParserErrorCollector as non-blocking error handler.
$useDefaultSymbolParserErrorHandler = true

And when useDefaultSymbolParserErrorHandler = true, demo script will ouput :

>>> Symbols found :
array (
  0 => 'My\\Space\\Base::BAR',
  1 => 'static::BAR',
  2 => 'My\\Space\\Foo::BAR',
)

Sample script to demonstrate usage of Node type Expr_ClassConstFetch (ClassConstStrategy)

<?php
namespace My\Space;

class Base
{
    protected const BAR = 'bar_base';
}

class Foo extends Base
{
    protected const BAR = 'bar_foo';

    public function getParentBar()
    {
        return parent::BAR;
    }

    public function getSelfBar()
    {
        return self::BAR;
    }

    public function getStaticBar()
    {
        return static::BAR;
    }

    public function getFooBar()
    {
        return Foo::BAR;
    }
}

$foo = new Foo();
var_export([$foo->getParentBar(), $foo->getSelfBar(), $foo->getStaticBar(), $foo->getFooBar()]);
echo PHP_EOL;

The above example will output:

array (
  0 => 'bar_base',
  1 => 'bar_foo',
  2 => 'bar_foo',
  3 => 'bar_foo',
)

Sample script to test ClassConstStrategy

<?php

declare(strict_types=1);

use ComposerUnused\SymbolParser\Parser\PHP\ConsumedSymbolCollector;
use ComposerUnused\SymbolParser\Parser\PHP\NameResolver;
use ComposerUnused\SymbolParser\Parser\PHP\Strategy\ClassConstStrategy;
use ComposerUnused\SymbolParser\Parser\PHP\SymbolNameParser;
use PhpParser\ErrorHandler;
use PhpParser\ParserFactory;

require_once __DIR__ . '/vendor/autoload.php';

// @var bool $useDefaultSymbolParserErrorHandler
//           - true to use the new (default) \ComposerUnused\SymbolParser\Parser\PHP\ParserErrorCollector non-blocking error collector
//           - false to use the default PHP-Parser error handler that handles all errors by throwing them as exception.
$useDefaultSymbolParserErrorHandler = true;

        $code = <<<CODE
        <?php
        namespace My\Space;

        class Base
        {
            protected const BAR = 'bar_base';
        }

        class Foo extends Base
        {
            protected const BAR = 'bar_foo';

            public function getParentBar()
            {
                return parent::BAR;
            }

            public function getSelfBar()
            {
                return self::BAR;
            }

            public function getStaticBar()
            {
                return static::BAR;
            }

            public function getFooBar()
            {
                return Foo::BAR;
            }
        }
        CODE;

$strategies = [
    new ClassConstStrategy(),
];

$errorHandler = $useDefaultSymbolParserErrorHandler ? null : new ErrorHandler\Throwing();

$symbolCollector = new ConsumedSymbolCollector($strategies);

$symbolNameParser = new SymbolNameParser(
    (new ParserFactory())->createForNewestSupportedVersion(),
    new NameResolver($errorHandler),
    $symbolCollector,
    $errorHandler
);

try {
    $symbols = iterator_to_array($symbolNameParser->parseSymbolNames($code));
} catch (\PhpParser\Error $e) {
    printf('>>> Failure occurred (%s) : %s' . PHP_EOL, \get_class($e), $e->getMessage());
    printf('==> Will not stop demo here !' . PHP_EOL);
    $symbols = [];
} catch (Throwable $e) {
    printf('>>> Failure occurred (%s) : %s' . PHP_EOL, \get_class($e), $e->getMessage());
    printf('==> Demo stopped immediately !' . PHP_EOL);
    exit(1);
}

printf('>>> Symbols found :' . PHP_EOL);
var_export($symbols);
echo PHP_EOL;

[icanhazstring]

Yeah let's do that.
Looks fine to me 👍

[llaville]

PR 146 is on way !

[llaville]

@icanhazstring FYI : I've already prepared a smooth transition patch for composer-unused/composer-unused 0.8.11 to why not 0.9.0 that is able to support all add-ons integrated to main branch of composer-unused/symbol-parser, but also support PHP-Parser v5 too !

If you want it, tell me !

Tested with current content of PR 146 give some nice diffs (more class constants are available on debug:consumed-symbols output)

[llaville]

@ComiR When you test your context (explained by #142 (comment)), with this current version of composer-unused/symbol-parser (branch fix/issue-142) you'll have results whater you use Amazon or self prefixed class constant.

Difference is displayed with vendor/bin/composer-unused debug:consumed-symbols command

On version 0.8.11, you 'll get only

Amazon\Pay\API\Client

While with this fix (PR 146), you'll get

Amazon\Pay\API\Client
My\Space\Amazon::CONFIG

PS: with recent commit 4cf61a6 you'll get

Amazon\Pay\API\Client
My\Space\Amazon\CONFIG

[ComiR]

@llaville I can confirm that (with composer-unused 0.8.11 and symbol-parser dev-fix/issue-142) the problem is fixed! Thanks for your work!

[icanhazstring]

solved with #146