Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to check the development code. #163

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@ If this is already done, run it like this:
composer-require-checker check /path/to/your/project/composer.json
```

### Check development code and dependencies (optional)

By default, Composer require checker only checks the source code listed in the `autoload` section of `composer.json`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
By default, Composer require checker only checks the source code listed in the `autoload` section of `composer.json`
By default, Composer Require Checker only checks the source code listed in the `autoload` section of `composer.json`

against the dependencies listed in the `require` section. This checks that there are no indirect dependencies in the
*production* code.

To check the *development* code, use the `--dev` option. This will scan the source code from the `autoload-dev` section and will search for symbols in dependencies from both the `require` and `require-dev`
section.
Comment on lines +60 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If max-width is somehow applied (looking at lines above), then it should be rather:

Suggested change
To check the *development* code, use the `--dev` option. This will scan the source code from the `autoload-dev` section and will search for symbols in dependencies from both the `require` and `require-dev`
section.
To check the *development* code, use the `--dev` option. This will scan the source code from the `autoload-dev` section
and will search for symbols in dependencies from both the `require` and `require-dev` section.


Please note these two checks are really different: a successful `--dev` check does not guarantee there are no
no indirect dependencies in the *production* code. You probably want to do both.

## Configuration

Composer require checker is configured to whitelist some symbols per default. Have a look at the
Expand Down
30 changes: 30 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
build: false
platform:
- x64

environment:
COMPOSER_NO_INTERACTION: "1"
ANSICON: '121x90 (121x90)'
matrix:
- PHP_VERSION: '7.2'
COMPOSER_FLAGS: '--prefer-stable --prefer-lowest'
- PHP_VERSION: '7.3'
- PHP_VERSION: '7.4'
Comment on lines +8 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP version matrix needs to be updated.

Or maybe AppVeyor is not required at this point? Github Actions support Windows 🤔.


matrix:
fast_finish: true

init:
- SET PATH=C:\Program Files\OpenSSL;C:\tools\php;%PATH%

install:
- ps: Invoke-WebRequest "https://raw.githubusercontent.com/ChadSikorra/ps-install-php/master/Install-PHP.ps1" -OutFile "Install-PHP.ps1"
- ps: .\Install-PHP.ps1 -Version $Env:PHP_VERSION -Highest -Arch x64 -Extensions mbstring,fileinfo,openssl
- echo zend_extension=php_opcache.dll >> C:\tools\php\php.ini
- refreshenv
- php -r "readfile('https://getcomposer.org/installer');" | php
- php composer.phar update %COMPOSER_FLAGS% --no-interaction --no-progress -vvv
- php composer.phar info -D | sort

test_script:
- vendor/bin/phpunit -c phpunit.xml.dist
1 change: 1 addition & 0 deletions bin/composer-require-checker.bat
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@php %~dp0\composer-require-checker.php %*
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"nikic/php-parser": "^4.3",
"ocramius/package-versions": "^1.4.2",
"symfony/console": "^4.4.0",
"webmozart/assert": "^1.6",
"webmozart/glob": "^4.1"
},
"require-dev": {
Expand Down
20 changes: 17 additions & 3 deletions src/ComposerRequireChecker/Cli/CheckCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ protected function configure()
InputOption::VALUE_NONE,
'this will cause ComposerRequireChecker to ignore errors when files cannot be parsed, otherwise'
. ' errors will be thrown'
)
->addOption(
'dev',
null,
InputOption::VALUE_NONE,
'check that the development sources (i.e. tests) have not indirect dependencies'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'check that the development sources (i.e. tests) have not indirect dependencies'
'check that the development sources (i.e. tests) does not have indirect dependencies'

);
}

Expand All @@ -63,19 +69,27 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
$composerData = $this->getComposerData($composerJson);

$checkDevSources = (bool)$input->getOption('dev');

$options = $this->getCheckOptions($input);

$getPackageSourceFiles = new LocateComposerPackageSourceFiles();
$getAdditionalSourceFiles = new LocateFilesByGlobPattern();

$sourcesASTs = $this->getASTFromFilesLocator($input);

$additionalLocators = $checkDevSources ? [
$getPackageSourceFiles($composerData, dirname($composerJson), 'autoload-dev'),
(new LocateComposerPackageDirectDependenciesSourceFiles())->__invoke($composerJson, 'require-dev')
] : [];

$this->verbose("Collecting defined vendor symbols... ", $output);
$definedVendorSymbols = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs(
(new ComposeGenerators())->__invoke(
$getAdditionalSourceFiles($options->getScanFiles(), dirname($composerJson)),
$getPackageSourceFiles($composerData, dirname($composerJson)),
(new LocateComposerPackageDirectDependenciesSourceFiles())->__invoke($composerJson)
$getPackageSourceFiles($composerData, dirname($composerJson), 'autoload'),
(new LocateComposerPackageDirectDependenciesSourceFiles())->__invoke($composerJson, 'require'),
...$additionalLocators
)
));
$this->verbose("found " . count($definedVendorSymbols) . " symbols.", $output, true);
Expand All @@ -89,7 +103,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->verbose("Collecting used symbols... ", $output);
$usedSymbols = (new LocateUsedSymbolsFromASTRoots())->__invoke($sourcesASTs(
(new ComposeGenerators())->__invoke(
$getPackageSourceFiles($composerData, dirname($composerJson)),
$getPackageSourceFiles($composerData, dirname($composerJson), $checkDevSources ? 'autoload-dev' : 'autoload'),
$getAdditionalSourceFiles($options->getScanFiles(), dirname($composerJson))
)
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ private function filterFilesByExtension(Traversable $files, string $fileExtensio
{
$extensionMatcher = '/.*' . preg_quote($fileExtension) . '$/';

$blacklist = $this->prepareBlacklistPatterns($blacklist);
$blacklistMatcher = '{('.implode('|', $this->prepareBlacklistPatterns($blacklist)).')}';

/* @var $file \SplFileInfo */
foreach ($files as $file) {
if ($blacklist && preg_match('{('.implode('|', $blacklist).')}', $file->getPathname())) {
if ($blacklist && preg_match($blacklistMatcher, $file->getPathname())) {
continue;
}

Expand All @@ -40,16 +40,18 @@ private function filterFilesByExtension(Traversable $files, string $fileExtensio
}
}

private function prepareBlacklistPatterns(?array $blacklistPaths)
private function prepareBlacklistPatterns(?array $blacklistPaths): array
{
if ($blacklistPaths === null) {
return $blacklistPaths;
return [];
}

$dirSep = \preg_quote(DIRECTORY_SEPARATOR, '{}');

foreach ($blacklistPaths as &$path) {
$path = preg_replace('{/+}', '/', preg_quote(trim(strtr($path, '\\', '/'), '/')));
$path = preg_replace('{' . $dirSep . '+}', DIRECTORY_SEPARATOR, \preg_quote(trim(str_replace('/', DIRECTORY_SEPARATOR, $path), DIRECTORY_SEPARATOR), '{}'));
$path = str_replace('\\*\\*', '.+?', $path);
$path = str_replace('\\*', '[^/]+?', $path);
$path = str_replace('\\*', '[^' . $dirSep . ']+?', $path);
}

return $blacklistPaths;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,25 @@
use ComposerRequireChecker\Exception\NotReadableException;
use ComposerRequireChecker\JsonLoader;
use Generator;
use Webmozart\Assert\Assert;
use function array_key_exists;
use function file_get_contents;
use function json_decode;

final class LocateComposerPackageDirectDependenciesSourceFiles
{
public function __invoke(string $composerJsonPath): Generator
public function __invoke(string $composerJsonPath, string $requireKey): Generator
{
Assert::oneOf($requireKey, ['require', 'require-dev']);

$packageDir = dirname($composerJsonPath);

$composerJson = json_decode(file_get_contents($composerJsonPath), true);
$configVendorDir = $composerJson['config']['vendor-dir'] ?? 'vendor';
$vendorDirs = [];
foreach ($composerJson['require'] ?? [] as $vendorName => $vendorRequiredVersion) {
foreach ($composerJson[$requireKey] ?? [] as $vendorName => $vendorRequiredVersion) {
$vendorDirs[$vendorName] = $packageDir . '/' . $configVendorDir . '/' . $vendorName;
};
}

$installedPackages = $this->getInstalledPackages($packageDir . '/' . $configVendorDir);

Expand All @@ -30,7 +33,7 @@ public function __invoke(string $composerJsonPath): Generator
continue;
}

yield from (new LocateComposerPackageSourceFiles())->__invoke($installedPackages[$vendorName], $vendorDir);
yield from (new LocateComposerPackageSourceFiles())->__invoke($installedPackages[$vendorName], $vendorDir, 'autoload');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,39 @@
namespace ComposerRequireChecker\FileLocator;

use Generator;
use Webmozart\Assert\Assert;

final class LocateComposerPackageSourceFiles
{
/**
* @param mixed[] $composerData The contents of composer.json for a package
* @param string $packageDir The path to package
* @param string $autoloadKey The key of autoload section from composer.json
*
* @return Generator
*/
public function __invoke(array $composerData, string $packageDir): Generator
public function __invoke(array $composerData, string $packageDir, string $autoloadKey): Generator
{
$blacklist = $composerData['autoload']['exclude-from-classmap'] ?? null;
Assert::oneOf($autoloadKey, ['autoload', 'autoload-dev']);

$autoloadData = $composerData[$autoloadKey] ?? [];

$blacklist = $autoloadData['exclude-from-classmap'] ?? null;

yield from $this->locateFilesInClassmapDefinitions(
$this->getFilePaths($composerData['autoload']['classmap'] ?? [], $packageDir),
$this->getFilePaths($autoloadData['classmap'] ?? [], $packageDir),
$blacklist
);
yield from $this->locateFilesInFilesInFilesDefinitions(
$this->getFilePaths($composerData['autoload']['files'] ?? [], $packageDir),
$this->getFilePaths($autoloadData['files'] ?? [], $packageDir),
$blacklist
);
yield from $this->locateFilesInPsr0Definitions(
$this->getFilePaths($composerData['autoload']['psr-0'] ?? [], $packageDir),
$this->getFilePaths($autoloadData['psr-0'] ?? [], $packageDir),
$blacklist
);
yield from $this->locateFilesInPsr4Definitions(
$this->getFilePaths($composerData['autoload']['psr-4'] ?? [], $packageDir),
$this->getFilePaths($autoloadData['psr-4'] ?? [], $packageDir),
$blacklist
);
}
Expand Down
6 changes: 5 additions & 1 deletion test/ComposerRequireCheckerTest/BinaryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ final class BinaryTest extends TestCase

protected function setUp(): void
{
$this->bin = __DIR__ . "/../../bin/composer-require-checker";
if (strpos(\PHP_OS, "WIN") === 0) {
$this->bin = __DIR__ . "\\..\\..\\bin\\composer-require-checker.bat";
} else {
$this->bin = __DIR__ . "/../../bin/composer-require-checker";
}
}

public function testSuccess(): void
Expand Down
17 changes: 17 additions & 0 deletions test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ public function testSelfCheckShowsNoErrors(): void
$this->assertNotRegExp('/Collecting used symbols... found \d+ symbols./', $this->commandTester->getDisplay());
}

public function testSelfCheckIncludingDevShowsNoErrors(): void
{
$this->commandTester->execute([
// that's our own composer.json, lets be sure our self check does not throw errors
'composer-json' => dirname(__DIR__, 3) . '/composer.json'
], [
'include-dev' => true
]);

$this->assertSame(0, $this->commandTester->getStatusCode());
$this->assertStringContainsString('no unknown symbols found', $this->commandTester->getDisplay());

// verbose output should not be shown
$this->assertNotRegExp('/Collecting defined (vendor|extension) symbols... found \d+ symbols./', $this->commandTester->getDisplay());
$this->assertNotRegExp('/Collecting used symbols... found \d+ symbols./', $this->commandTester->getDisplay());
}

public function testVerboseSelfCheckShowsCounts(): void
{
$this->commandTester->execute([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,99 @@ public function testLocateFromASingleDirectory(): void
}
}

public function testLocateWithFilenameBlackList(): void
{
$dir = vfsStream::newDirectory('MyNamespaceA')->at($this->root);
for ($i = 0; $i < 10; $i++) {
vfsStream::newFile("MyClass$i.php")->at($dir);
}

$foundFiles = $this->locate([$dir->url()], '.php', ['MyClass6']);

$this->assertCount(9, $foundFiles);
$this->assertNotContains(vfsStream::url('MyClass6.php'), $foundFiles);
}

public function testLocateWithDirectoryBlackList(): void
{
$dir = vfsStream::newDirectory('MyNamespaceA')->at($this->root);
for ($i = 0; $i < 10; $i++) {
vfsStream::newFile("Directory$i/MyClass.php")->at($dir);
}

$foundFiles = $this->locate([$dir->url()], '.php', ['Directory5/']);

$this->assertCount(9, $foundFiles);
$this->assertNotContains(vfsStream::url('Directory5/MyClass.php'), $foundFiles);
}

/**
* @param array $blacklist
* @param array $expectedFiles
*
* @dataProvider provideBlacklists
*/
public function testLocateWithBlackList(array $blacklist, array $expectedFiles): void
{
$this->root = vfsStream::create([
'MyNamespaceA' => [
'MyClass.php' => '<?php class MyClass {}',
'Foo' => [
'FooClass.php' => '<?php class FooCalls {}',
'Bar' => [
'BarClass.php' => '<?php class BarClass {}',
]
],
'Bar' => [
'AnotherBarClass.php' => '<?php class AnotherBarClass {}',
]
]
]);

$foundFiles = $this->locate([$this->root->url()], '.php', $blacklist);

$this->assertCount(count($expectedFiles), $foundFiles);
foreach ($expectedFiles as $file) {
$this->assertContains($this->root->getChild($file)->url(), $foundFiles);
}
$this->assertContains($this->root->getChild('MyNamespaceA/Foo/FooClass.php')->url(), $foundFiles);
}

public function provideBlacklists(): array {
return [
'No blacklist' => [
[],
[
'MyNamespaceA/MyClass.php',
'MyNamespaceA/Foo/FooClass.php',
'MyNamespaceA/Foo/Bar/BarClass.php',
'MyNamespaceA/Bar/AnotherBarClass.php',
]
],
'* wildcard' => [
['Another*.php'],
[
'MyNamespaceA/MyClass.php',
'MyNamespaceA/Foo/FooClass.php',
'MyNamespaceA/Foo/Bar/BarClass.php',
]
],
'** wildcard' => [
['**/Bar'],
[
'MyNamespaceA/MyClass.php',
'MyNamespaceA/Foo/FooClass.php',
]
],
'Combined patterns' => [
['My*.php', 'Bar/'],
[
'MyNamespaceA/Foo/FooClass.php',
]
],
];
}

/**
* @param string[] $directories
* @param string $fileExtension
Expand All @@ -57,6 +150,10 @@ public function testLocateFromASingleDirectory(): void
*/
private function locate(array $directories, string $fileExtension, ?array $blacklist): array
{
return iterator_to_array(($this->locator)(new ArrayObject($directories), $fileExtension, $blacklist));
$files = [];
foreach (($this->locator)(new ArrayObject($directories), $fileExtension, $blacklist) as $file) {
$files[] = str_replace('\\', '/', $file);
}
return $files;
}
}
Loading