Question about shadow dependencies #109

llaville · 2024-03-22T09:18:42Z

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

main package vendor-a/package-a vendor directory structure :

vendor/
├── autoload.php
├── vendor-a
│   ├── package-b -> ../../../package-b/
│   ├── package-c -> ../../../package-c/
│   └── package-d -> ../../../package-d/
└── composer

composer.json of main package with additional packages (linked) as follows :

{
    "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"
    }
}

composer.json of package-b :

{
    "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 ?

The text was updated successfully, but these errors were encountered:

janedbal · 2024-03-22T09:35:37Z

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 · 2024-03-22T09:37:30Z

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 · 2024-03-22T09:49:24Z

@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 · 2024-03-22T10:22:01Z

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

janedbal · 2024-03-25T13:55:49Z

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 · 2024-03-25T17:08:35Z

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 · 2024-03-26T11:27:48Z

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 · 2024-03-26T12:52:49Z

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

janedbal · 2024-03-26T14:10:49Z

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

llaville · 2024-03-26T15:50:38Z

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 · 2024-04-04T09:01:16Z

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

use VendorA\PackageB\Internal\\DoSomething
(new DoSomeThing)->execute();

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 · 2024-04-04T09:26:48Z

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

janedbal mentioned this issue Mar 26, 2024

Analyser: support local symlinked packages even when not present in composer autoloader #112

Open

llaville closed this as completed Apr 4, 2024

janedbal added the question Further information is requested label Apr 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Question about shadow dependencies #109

Question about shadow dependencies #109

llaville commented Mar 22, 2024

janedbal commented Mar 22, 2024

llaville commented Mar 22, 2024

llaville commented Mar 22, 2024 •

edited

janedbal commented Mar 22, 2024

janedbal commented Mar 25, 2024

llaville commented Mar 25, 2024

janedbal commented Mar 26, 2024

llaville commented Mar 26, 2024

janedbal commented Mar 26, 2024

llaville commented Mar 26, 2024

janedbal commented Apr 4, 2024

llaville commented Apr 4, 2024

Question about shadow dependencies #109

Question about shadow dependencies #109

Comments

llaville commented Mar 22, 2024

janedbal commented Mar 22, 2024

llaville commented Mar 22, 2024

llaville commented Mar 22, 2024 • edited

janedbal commented Mar 22, 2024

janedbal commented Mar 25, 2024

llaville commented Mar 25, 2024

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

From vendor-a/package-a (main)

From vendor-a/package-b

janedbal commented Mar 26, 2024

llaville commented Mar 26, 2024

janedbal commented Mar 26, 2024

llaville commented Mar 26, 2024

janedbal commented Apr 4, 2024

llaville commented Apr 4, 2024

llaville commented Mar 22, 2024 •

edited

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

From `vendor-a/package-a` (main)

From `vendor-a/package-b`