shipmonk-rnd / composer-dependency-analyser

🚀 Fast detection of composer dependency issues (unused dependencies, shadow dependencies, misplaced dependencies)
MIT License
332 stars 8 forks source link

Question about shadow dependencies #109

Closed llaville closed 3 months ago

llaville commented 3 months ago

Hello,

After checking a little the source code of this repo, I've at least one question about shadow dependencies (at end of this report).

Here is my anonimized example : Four packages developed on same time with a single responsability on each one !

├── package-a <-- main package
│   ├── src
│   ├── tests
│   └── vendor
├── package-b <- Command Line Interpreter with Symfony Console Component
│   ├── src
│   ├── tests
│   └── vendor
├── package-c
│   ├── src
│   ├── tests
│   └── vendor
└── package-d
    ├── src
    ├── tests
    └── vendor
vendor/
├── autoload.php
├── vendor-a
│   ├── package-b -> ../../../package-b/
│   ├── package-c -> ../../../package-c/
│   └── package-d -> ../../../package-d/
└── composer
{
    "repositories": [
        {
            "type": "path",
            "url": "../package-b",
            "options": {
                "reference": "config",
                "symlink": true
            }
        },
        {
            "type": "path",
            "url": "../package-c",
            "options": {
                "reference": "config",
                "symlink": true
            }
        },
        {
            "type": "path",
            "url": "../package-d",
            "options": {
                "reference": "config",
                "symlink": true
            }
        }
    ],
    "minimum-stability": "stable",
    "require": {
        "php": "^8.1",
        "vendor-a/package-b": "@dev",
        "vendor-a/package-c": "@dev",
        "vendor-a/package-d": "@dev"
    }
}
{
    "minimum-stability": "stable",
    "require": {
        "php": "^8.1",
        "symfony/console": "^6.4 || ^7.0",
        "symfony/stopwatch": "^6.4 || ^7.0"
    }
}

With Configuration like :

<?php declare(strict_types = 1);

use ShipMonk\ComposerDependencyAnalyser\Config\Configuration;

return (new Configuration())
    ->addPathToScan(__DIR__ . '/src', false)
    ->addPathToScan(__DIR__ . '/../package-b/src', false)
    ->addPathToScan(__DIR__ . '/../package-c/src', false)
    ->addPathToScan(__DIR__ . '/../package-d/src', false)
;

I got results as follows :

Found shadow dependencies!
(those are used, but not listed as dependency in composer.json)

  • symfony/console
      e.g. Symfony\Component\Console\Application in package-b/src/Console/Application.php:10 (+ 6 more)

  • symfony/stopwatch
      e.g. Symfony\Component\Stopwatch\Stopwatch in package-b/src/Console/Profiler.php:15 (+ 1 more)

As we are able to load source code with Composer Autoloaders, why dependencies of package-b are considered as shadow while there are declared as direct in package-b/composer.json ?

janedbal commented 3 months ago

Using ->addPathToScan as shown in your example does not use its vendor as "own", it just scans more php files for symbol usages (and actually causes this "invalid report"). I think you should rather run composer-dependency-analyser for each package you have (a,b,c,d) as those have own vendor dependencies to check against.

llaville commented 3 months ago

This is what I'll add as extra: when running on package-b I got expected results


No composer issues found
(scanned 4 files in 0.002 s)
llaville commented 3 months ago

@janedbal But if I don't add addPathToScan in config file, previous shadow dependencies was removed but I got

Found unused dependencies!
(those are listed in composer.json, but no usage was found in scanned paths)

  • vendor-a/package-b
  • vendor-a/package-c
  • vendor-a/package-d

(scanned 37 files in 0.008 s)
janedbal commented 3 months ago

That looks like some issue with local dependencies, I'll try to reproduce. Thanks.

janedbal commented 3 months ago

I just tried simulating your issue, but didn't encounter your problem.

Are you sure you really use stuff from vendor-a/package-b in your main package (in paths you have in autoload sections of its composer.json)?

Some minimal reproduction repository would be best here.

llaville commented 3 months ago

I've anonymised at least minimal two repositories for demo, and I 'll give you now the results :

From running bin/composer-dependency-analyser without any arguments on package-a

Found shadow dependencies!
(those are used, but not listed as dependency in composer.json)

  • symfony/console
      e.g. Symfony\Component\Console\Output\OutputInterface in src/Console/Command/CmdA.php:13

Found unused dependencies!
(those are listed in composer.json, but no usage was found in scanned paths)

  • vendor-a/package-b

(scanned 1 files in 0.001 s)

From vendor-a/package-a (main)

With composer.json as follow:

{
    "name": "vendor-a/package-a",
    "type": "project",
    "license": "MIT",
    "autoload": {
        "psr-4": {
            "VendorA\\PackageA\\": "src/"
        }
    },
    "repositories": [
        {
            "type": "path",
            "url": "../package-b",
            "options": {
            "reference": "config",
            "symlink": true
            }
        }
    ],
    "minimum-stability": "stable",
    "require": {
        "php": "^8.1",
        "vendor-a/package-b": "@dev"
    }
}
composer update
Composer could not detect the root package (vendor-a/package-a) version, defaulting to '1.0.0'. See https://getcomposer.org/root-version
Loading composer repositories with package information
Updating dependencies
Lock file operations: 9 installs, 0 updates, 0 removals
  - Locking psr/container (2.0.2)
  - Locking symfony/console (v7.0.0)
  - Locking symfony/polyfill-ctype (v1.29.0)
  - Locking symfony/polyfill-intl-grapheme (v1.29.0)
  - Locking symfony/polyfill-intl-normalizer (v1.29.0)
  - Locking symfony/polyfill-mbstring (v1.29.0)
  - Locking symfony/service-contracts (v3.4.1)
  - Locking symfony/string (v7.0.4)
  - Locking vendor-a/package-b (dev-main)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 9 installs, 0 updates, 0 removals
  - Installing psr/container (2.0.2): Extracting archive
  - Installing symfony/service-contracts (v3.4.1): Extracting archive
  - Installing symfony/polyfill-mbstring (v1.29.0): Extracting archive
  - Installing symfony/polyfill-intl-normalizer (v1.29.0): Extracting archive
  - Installing symfony/polyfill-intl-grapheme (v1.29.0): Extracting archive
  - Installing symfony/polyfill-ctype (v1.29.0): Extracting archive
  - Installing symfony/string (v7.0.4): Extracting archive
  - Installing symfony/console (v7.0.0): Extracting archive
  - Installing vendor-a/package-b (dev-main): Symlinking from ../package-b
Generating autoload files
7 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
No security vulnerability advisories found.

IMPORTANT : vendor-a/package-b is symlinked

Directory structure once composer update is invoked

├── bin
│   └── console.php
├── composer.json
├── composer.lock
├── src
│   └── Console
│       └── Command
│           └── CmdA.php
└── vendor
    ├── autoload.php
    ├── composer
    │   ├── autoload_classmap.php
    │   ├── autoload_files.php
    │   ├── autoload_namespaces.php
    │   ├── autoload_psr4.php
    │   ├── autoload_real.php
    │   ├── autoload_static.php
    │   ├── ClassLoader.php
    │   ├── installed.json
    │   ├── installed.php
    │   ├── InstalledVersions.php
    │   ├── LICENSE
    │   └── platform_check.php
    ├── psr
    │   └── container
    ├── symfony
    │   ├── console
    │   ├── polyfill-ctype
    │   ├── polyfill-intl-grapheme
    │   ├── polyfill-intl-normalizer
    │   ├── polyfill-mbstring
    │   ├── service-contracts
    │   └── string
    └── vendor-a
        └── package-b -> ../../../package-b/

With launcher bin/console.php

#!/usr/bin/env php
<?php

declare(strict_types=1);

$rootDir = dirname(__DIR__);

require_once $rootDir . '/vendor/autoload.php';

$commands = [
    'cmd:a' => $rootDir . '/src/Console/Command/CmdA.php',
];
$descriptions = [
    'cmd:a' => 'Execute first command',
];

include $rootDir . '/vendor/vendor-a/package-b/bin/console.php';

With CmdA.php that call a class in package-b

<?php

declare(strict_types=1);

/**
 * @author Laurent Laville
 */

use VendorA\PackageB\Internal\DoSomething;

use Symfony\Component\Console\Output\OutputInterface;

return function (OutputInterface $output) {
    $output->writeln("Begin line >>>");

    (new DoSomeThing)->execute();
    $output->writeln("Cmd A process results");

    $output->writeln("End line <<<");
};

From vendor-a/package-b

With composer.json as follow:

{
    "name": "vendor-a/package-b",
    "type": "project",
    "license": "MIT",
    "autoload": {
        "psr-4": {
            "VendorA\\PackageB\\": "src/"
        }
    },
    "minimum-stability": "stable",
    "require": {
        "php": "^8.1",
        "symfony/console": "^6.4 || 7.0"
    }
}

Directory structure once composer update is invoked

├── bin
│   └── console.php
├── composer.json
├── composer.lock
├── src
│   ├── Console
│   │   ├── Application.php
│   │   └── Command
│   │       ├── Command.php
│   │       └── FactoryCommandLoader.php
│   └── Internal
│       └── DoSomething.php
└── vendor
    ├── autoload.php
    ├── composer
    │   ├── autoload_classmap.php
    │   ├── autoload_files.php
    │   ├── autoload_namespaces.php
    │   ├── autoload_psr4.php
    │   ├── autoload_real.php
    │   ├── autoload_static.php
    │   ├── ClassLoader.php
    │   ├── installed.json
    │   ├── installed.php
    │   ├── InstalledVersions.php
    │   ├── LICENSE
    │   └── platform_check.php
    ├── psr
    │   └── container
    └── symfony
        ├── console
        ├── polyfill-ctype
        ├── polyfill-intl-grapheme
        ├── polyfill-intl-normalizer
        ├── polyfill-mbstring
        ├── service-contracts
        └── string

The main launcher bin/console.php :

#!/usr/bin/env php
<?php

declare(strict_types=1);

use VendorA\PackageB\Console;

gc_disable(); // performance boost

$rootDir = dirname(__DIR__);

require_once $rootDir . '/vendor/autoload.php';

$application = new Console\Application();

$commands ??= [];
$descriptions ??= [];
$factories = [];

foreach ($commands as $name => $resource) {
    $factories[$name] = fn() => new Console\Command\Command($name, $resource);
}
// @link https://symfony.com/doc/current/console/lazy_commands.html#factorycommandloader
$application->setCommandLoader(new Console\Command\FactoryCommandLoader($factories, $descriptions));

$application->run();
<?php

declare(strict_types=1);

namespace VendorA\PackageB\Console;

/**
 * @author Laurent Laville
 */
class Application extends \Symfony\Component\Console\Application
{
}
<?php

declare(strict_types=1);

namespace VendorA\PackageB\Console\Command;

use Symfony\Component\Console\Command\Command as SymfonyConsoleCommand;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

use Closure;
use function file_exists;
use function is_callable;
use function sprintf;

/**
 * @author Laurent Laville
 */
class Command extends SymfonyConsoleCommand
{
    public const SUCCESS = 0;
    public const FAILURE = 1;

    protected Closure $callable;
    protected ?string $callableResource = null;

    public function __construct(string $name, ?string $pathToCallable = null)
    {
        parent::__construct($name);

        $closure = function () {};

        if (null !== $pathToCallable && file_exists($pathToCallable)) {
            $this->callableResource = $pathToCallable;
            $callable = require $pathToCallable;
            if (is_callable($callable)) {
                $closure = $callable(...);
            }
        }
        $this->callable = $closure;
    }

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $section1 = $output->section();

        $section1->writeln([
            sprintf('Running: <info>%s</info> with <comment>%s</comment> resource.', $input, $this->callableResource),
            ''
        ]);

        $do = $this->callable;
        $do($section1);

        return self::SUCCESS;
    }
}
<?php

declare(strict_types=1);

namespace VendorA\PackageB\Console\Command;

use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\CommandLoader\FactoryCommandLoader as SymfonyFactoryCommandLoader;

/**
 * @author Laurent Laville
 */
final class FactoryCommandLoader extends SymfonyFactoryCommandLoader
{
    /**
     * @var string[]
     */
    private array $descriptions;

    public function __construct(array $factories, array $descriptions = [])
    {
        parent::__construct($factories);
        $this->descriptions = $descriptions;
    }

    public function get(string $name): Command
    {
        $command = parent::get($name);
        if (isset($this->descriptions[$name])) {
            $command->setDescription($this->descriptions[$name]);
        }
        return $command;
    }
}
<?php

declare(strict_types=1);

namespace VendorA\PackageB\Internal;

class DoSomething
{
    public function execute(): void
    {
        echo "Run " . __METHOD__ . PHP_EOL;
    }
}

Package-B is for CLI features only :

php bin/console.php
Console Tool

Usage:
  command [options] [arguments]

Options:
  -h, --help            Display help for the given command. When no command is given display help for the list command
  -q, --quiet           Do not output any message
  -V, --version         Display this application version
      --ansi|--no-ansi  Force (or disable --no-ansi) ANSI output
  -n, --no-interaction  Do not ask any interactive question
  -v|vv|vvv, --verbose  Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Available commands:
  completion  Dump the shell completion script
  help        Display help for a command
  list        List commands

NOTE : Command resources are declared in sandbox/main Package-A

Run the bin/console.php main launcher from package-a to see the demo :

Running: 'cmd:a' with /path/to/vendor-a/package-a/src/Console/Command/CmdA.php resource.

Begin line >>>
Run VendorA\PackageB\Internal\DoSomething::execute
Cmd A process results
End line <<<
janedbal commented 3 months ago

I was able to simulate this issue with symlink, but only when the classes in package-b were not autoloadable by composer (and fallback package detection via reflection was used), but that should not happen in your case.

I made a draft of a fix here: #112, can you check if it resolves your issue?

llaville commented 3 months ago

Just tested your branch detect-symlinks, but sorry got same results !

janedbal commented 3 months ago

Would you mind pushing the anonymized example somewhere public where I can clone it and debug it locally?

llaville commented 3 months ago

Sorry, I've already gave you all info yesterday. But If you're able to retrieve this two temporary files (40 Kb each one) before 1 hour now, you 'll be able to recreate two repo easily

https://tmpfiles.org/dl/4524727/package-a.tar https://tmpfiles.org/dl/4524729/package-b.tar

janedbal commented 3 months ago

The problem is that you have case mismatch in CmdA.php:

This caused the issue with unused dependency being reported. I'll think if this can be fixed, but I'd consider this an error. PHPStan reports this: https://phpstan.org/r/d1b9d2ea-ce7b-4624-852d-fc7c7e391e48

llaville commented 3 months ago

@janedbal Thanks for your analysis (I appreciate a lot)