maglnet / ComposerRequireChecker

A CLI tool to check whether a specific composer package uses imported symbols that aren't part of its direct composer dependencies
MIT License
895 stars 73 forks source link

Prevent vendor directory from being parsed #110

Closed Levivb closed 5 years ago

Levivb commented 5 years ago

This could be a duplicate of https://github.com/maglnet/ComposerRequireChecker/issues/54 I'll open a separate issue though in order to avoid cluttering or take over that issue

When running composer-require-checker -vvv I'm greeted with the following output after a few seconds:

ComposerRequireChecker unknown-development
Collecting defined vendor symbols...
In ParserAbstract.php line 315:

  [PhpParser\Error]
  Syntax error, unexpected ';', expecting T_VARIABLE or '{' or '$' on line 4

Exception trace:
 () at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php:315
 PhpParser\ParserAbstract->doParse() at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php:158
 PhpParser\ParserAbstract->parse() at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php:51
 PhpParser\Parser\Multiple->tryParse() at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php:32
 PhpParser\Parser\Multiple->parse() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/src/ComposerRequireChecker/ASTLocator/LocateASTFromFiles.php:35
 ComposerRequireChecker\ASTLocator\LocateASTFromFiles->__invoke() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php:27
 ComposerRequireChecker\DefinedSymbolsLocator\LocateDefinedSymbolsFromASTRoots->__invoke() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/src/ComposerRequireChecker/Cli/CheckCommand.php:79
 ComposerRequireChecker\Cli\CheckCommand->execute() at /home/vagrant/.composer/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /home/vagrant/.composer/vendor/symfony/console/Application.php:908
 Symfony\Component\Console\Application->doRunCommand() at /home/vagrant/.composer/vendor/symfony/console/Application.php:269
 Symfony\Component\Console\Application->doRun() at /home/vagrant/.composer/vendor/symfony/console/Application.php:145
 Symfony\Component\Console\Application->run() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/bin/composer-require-checker.php:32
 include() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/bin/composer-require-checker:3

check [--config-file CONFIG-FILE] [--ignore-parse-errors] [--] [<composer-json>]

From the looks of it, it seems the error originates from the nikic/php-parser package. Unfortunately, the error doesn't tell which file is invalid. Given the fact that my code doesn't contain any syntax errors and the nikic/php-parser package due to it's wide adoption probably won't contain any obvious syntax errors as well, I figured the parser checks the vendor directory.

After running $ find . -name *.php -exec php -l {} \; in my vendor directory and ignoring all syntactically valid files, I'm left with the following output:

PHP Parse error:  syntax error, unexpected ';', expecting variable (T_VARIABLE) or '{' or '$' in ./vendor/jakub-onderka/php-parallel-lint/tests/examples/example-03/example.php on line 4

Parse error: syntax error, unexpected ';', expecting variable (T_VARIABLE) or '{' or '$' in ./vendor/jakub-onderka/php-parallel-lint/tests/examples/example-03/example.php on line 4
Errors parsing ./vendor/jakub-onderka/php-parallel-lint/tests/examples/example-03/example.php
PHP Parse error:  syntax error, unexpected 'echo' (T_ECHO) in ./vendor/jakub-onderka/php-parallel-lint/tests/examples/example-04/dir1/dir2/index.php on line 3

Parse error: syntax error, unexpected 'echo' (T_ECHO) in ./vendor/jakub-onderka/php-parallel-lint/tests/examples/example-04/dir1/dir2/index.php on line 3
Errors parsing ./vendor/jakub-onderka/php-parallel-lint/tests/examples/example-04/dir1/dir2/index.php
PHP Deprecated:  Methods with the same name as their class will not be constructors in a future version of PHP; MockeryTest_OldStyleConstructor has a deprecated constructor in ./vendor/mockery/mockery/tests/PHP56/MockingOldStyleConstructorTest.php on line 39

Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; MockeryTest_OldStyleConstructor has a deprecated constructor in ./vendor/mockery/mockery/tests/PHP56/MockingOldStyleConstructorTest.php on line 39
PHP Fatal error:  Cannot use Riak\Object as Object because 'Object' is a special class name in ./vendor/doctrine/cache/lib/Doctrine/Common/Cache/RiakCache.php on line 8

Fatal error: Cannot use Riak\Object as Object because 'Object' is a special class name in ./vendor/doctrine/cache/lib/Doctrine/Common/Cache/RiakCache.php on line 8
Errors parsing ./vendor/doctrine/cache/lib/Doctrine/Common/Cache/RiakCache.php
PHP Deprecated:  Methods with the same name as their class will not be constructors in a future version of PHP; Foo has a deprecated constructor in ./vendor/phpunit/php-token-stream/tests/_fixture/source.php on line 5

Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; Foo has a deprecated constructor in ./vendor/phpunit/php-token-stream/tests/_fixture/source.php on line 5
PHP Parse error:  syntax error, unexpected '{', expecting identifier (T_STRING) in ./vendor/phpunit/php-code-coverage/tests/_files/Crash.php on line 2

Parse error: syntax error, unexpected '{', expecting identifier (T_STRING) in ./vendor/phpunit/php-code-coverage/tests/_files/Crash.php on line 2
Errors parsing ./vendor/phpunit/php-code-coverage/tests/_files/Crash.php
PHP Parse error:  syntax error, unexpected '}', expecting ';' in ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithClosureFixture.php on line 60

Parse error: syntax error, unexpected '}', expecting ';' in ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithClosureFixture.php on line 60
Errors parsing ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithClosureFixture.php
PHP Parse error:  syntax error, unexpected '$foo' (T_VARIABLE) in ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedAfterUsedFixture.php on line 24

Parse error: syntax error, unexpected '$foo' (T_VARIABLE) in ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedAfterUsedFixture.php on line 24
Errors parsing ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedAfterUsedFixture.php
PHP Fatal error:  Cannot use "self" when no class scope is active in ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionsOutsideClassFixture.php on line 8

Fatal error: Cannot use "self" when no class scope is active in ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionsOutsideClassFixture.php on line 8
Errors parsing ./vendor/sirbrillig/phpcs-variable-analysis/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionsOutsideClassFixture.php

As I suspected, the vendor directory contains syntactically invalid files. Upon manually 'fixing' the first two errors in the vendor directory, a rerun of composer-require-checker -vvv got me the next error in line:

ComposerRequireChecker unknown-development
Collecting defined vendor symbols...
In ParserAbstract.php line 315:

  [PhpParser\Error]
  Syntax error, unexpected T_ECHO on line 3

Exception trace:
 () at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php:315
 PhpParser\ParserAbstract->doParse() at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php:158
 PhpParser\ParserAbstract->parse() at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php:51
 PhpParser\Parser\Multiple->tryParse() at /home/vagrant/.composer/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php:32
 PhpParser\Parser\Multiple->parse() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/src/ComposerRequireChecker/ASTLocator/LocateASTFromFiles.php:35
 ComposerRequireChecker\ASTLocator\LocateASTFromFiles->__invoke() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php:27
 ComposerRequireChecker\DefinedSymbolsLocator\LocateDefinedSymbolsFromASTRoots->__invoke() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/src/ComposerRequireChecker/Cli/CheckCommand.php:79
 ComposerRequireChecker\Cli\CheckCommand->execute() at /home/vagrant/.composer/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /home/vagrant/.composer/vendor/symfony/console/Application.php:908
 Symfony\Component\Console\Application->doRunCommand() at /home/vagrant/.composer/vendor/symfony/console/Application.php:269
 Symfony\Component\Console\Application->doRun() at /home/vagrant/.composer/vendor/symfony/console/Application.php:145
 Symfony\Component\Console\Application->run() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/bin/composer-require-checker.php:32
 include() at /home/vagrant/.composer/vendor/maglnet/composer-require-checker/bin/composer-require-checker:3

check [--config-file CONFIG-FILE] [--ignore-parse-errors] [--] [<composer-json>]

Which confirms my initial theory. Unfortunately... there's little for me to do here. The third party packages are there to stay and most of these syntax errors originate from test files of the packages themselves. Hence they should contain syntax errors.

I did run composer-require-checker -vvv --ignore-parse-errors as well Which got me (after 5 minute of waiting) an insane list of 'soft' dependencies of which 99.9% isn't referred to in my code. Given the fact that all the files in the vendor directory are parsed, I presume all files in there are checked against my first party composer.json

Now, given release note 2.0.0 states use installed.json instead of composer.json, the vendor directory should obviously exist. Removing it gives me The composer dependencies have not been installed, run composer install/update first.

A few questions:

  1. Is it possible to show the violating file. it would make debugging a whole lot easier (this might be more of a request for nikic/php-parser)
  2. Why is the full vendor directory being parsed?
  3. Is there a way to prevent files or whole directories from being parsed (vendor?)
maglnet commented 5 years ago

could you please provide your composer.json file?

  1. No, sadly not at the moment
  2. We need to scan all autoloaded files, but there may be a misconfigured autoload section, so I'd like to check the composer.json
  3. Afaik, not now, but files that are not required by your application or not in the autoload section should not be parsed. Test-Files, like listed in the output should be in autoload-dev and not be parsed.
Levivb commented 5 years ago

composer.json is as following:

{
  "name": "laravel-modules/boilerplate",
  "description": "",
  "license": "proprietary",
  "require": {
    "php": ">=7.2.0",
    "laravel-modules/core": "^4.0"
  },
  "require-dev": {
    "codedungeon/phpunit-result-printer": "^0.24.1",
    "internations/http-mock": "^0.12.1",
    "jakub-onderka/php-console-highlighter": "^0.3.2",
    "jakub-onderka/php-parallel-lint": "^1.0",
    "localheinz/composer-normalize": "^1.1",
    "maximum/code-sniffer-rules": "^2.0",
    "mockery/mockery": "^1.2",
    "phpunit/phpunit": "^7.5",
    "sensiolabs/security-checker": "^5.0"
  },
  "config": {
    "optimize-autoloader": true,
    "preferred-install": "dist",
    "sort-packages": true
  },
  "extra": {
    "laravel": {
      "providers": [
        ...
      ]
    }
  },
  "autoload": {
    "psr-4": {
      "Modules\\Boilerplate\\": ""
    }
  },
  "repositories": [
    {
      "type": "composer",
      "url": "[FILTERED]",
      "options": {
        "http": {
          "header": [
            "X-TOKEN: [FILTERED]"
          ]
        }
      }
    }
  ],
  "scripts": {
    "fix-style": "vendor/bin/phpcbf --standard=phpcs.xml",
    "lint": "vendor/bin/parallel-lint --exclude vendor . --colors",
    "security-checker": "vendor/bin/security-checker security:check composer.lock",
    "style": "vendor/bin/phpcs --standard=phpcs.xml"
  }
}

laravel-modules/core is a company package with the following composer.json:

{
  "name": "laravel-modules/core",
  "description": "",
  "license": "proprietary",
  "require": {
    "php": ">=7.2.0",
    "ext-json": "*",
    "ext-simplexml": "*",
    "dimsav/laravel-translatable": "^9.2",
    "doctrine/dbal": "^2.7",
    "guidocella/eloquent-insert-on-duplicate-key": "^2.2",
    "guzzlehttp/guzzle": "^6.3",
    "laracasts/utilities": "^3.0",
    "laravel/framework": "5.7.*",
    "laravel/horizon": "^3.0",
    "laravel/telescope": "^1.0",
    "maatwebsite/laravel-sidebar": "~2.1",
    "maximum/laravel-modules": "^3.3",
    "nanigans/single-table-inheritance": "^0.8.7",
    "owen-it/laravel-auditing": "^8.0",
    "predis/predis": "^1.1",
    "renatomarinho/laravel-page-speed": "^1.8",
    "rtconner/laravel-tagging": "^3.0",
    "sentry/sentry-laravel": "^0.11",
    "sofa/eloquence": "^5.6",
    "spatie/laravel-analytics": "^3.6",
    "stevenmaguire/laravel-middleware-csp": "~0.1",
    "tymon/jwt-auth": "0.5.*",
    "zizaco/entrust": "^1.9"
  },
  "require-dev": {
    "codedungeon/phpunit-result-printer": "^0.24.1",
    "internations/http-mock": "^0.12.1",
    "jakub-onderka/php-console-highlighter": "^0.3.2",
    "jakub-onderka/php-parallel-lint": "^1.0",
    "localheinz/composer-normalize": "^1.1",
    "maximum/code-sniffer-rules": "^2.0",
    "mockery/mockery": "^1.2",
    "phpunit/phpunit": "^7.5",
    "sensiolabs/security-checker": "^5.0"
  },
  "config": {
    "optimize-autoloader": true,
    "preferred-install": "dist",
    "sort-packages": true
  },
  "extra": {
    "laravel": {
      "providers": [
        ...
      ],
      "aliases": {
        ...
      }
    }
  },
  "autoload": {
    "psr-4": {
      "Modules\\Core\\": ""
    },
    "files": [
      "Helpers/global_functions.php"
    ]
  },
  "repositories": [
    {
      "type": "composer",
      "url": "[FILTERED]",
      "options": {
        "http": {
          "header": [
            "X-TOKEN: [FILTERED]"
          ]
        }
      }
    }
  ],
  "scripts": {
    "fix-style": "vendor/bin/phpcbf --standard=phpcs.xml",
    "lint": "vendor/bin/parallel-lint --exclude vendor . --colors",
    "security-checker": "vendor/bin/security-checker security:check composer.lock",
    "style": "vendor/bin/phpcs --standard=phpcs.xml"
  }
}

the package with the syntax error seems to be jakub-onderka/php-parallel-lint

maglnet commented 5 years ago

The "Problem" in this case are the autoload sections. They cause composer-require-checker to parse ALL files within the root directory.

There should be a subfolger with your php files, like "src" or whatever you like to call it, and your autoload-sections should only point to this directory.

  "autoload": {
    "psr-4": {
      "Modules\\Boilerplate\\": ""
    }
  },

and

  "autoload": {
    "psr-4": {
      "Modules\\Core\\": ""
    },
    "files": [
      "Helpers/global_functions.php"
    ]
  },
Levivb commented 5 years ago

Thanks for getting back to me. Due to a holiday I wasn't able to respond any sooner.

It seems you'r right, putting files in a src directory does fix the issue.

packages mentioned here are Modules loaded by nwidart/laravel-modules It's gonna be a bit of a challenge to get those packages working in a sub-directory, since they contain some legacy code with hardcoded paths (I know, I know) and I'm also not sure how the laravel-modules package will respond to those changes. I'll put it on my backlog for now since it's quite a major overhaul

Thanks for helping me understand this one anywaay!