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

Fatal because of conflicting interfaces #21

Closed
Idrinth opened this Issue Aug 30, 2018 · 11 comments

Comments

Projects
None yet
3 participants
@Idrinth
Contributor

Idrinth commented Aug 30, 2018

Description

When symfony/console:2.8.45 is analysed, the dependency guard stops with a fatal error.

[30-Aug-2018 10:48:00 Europe/Berlin] PHP Fatal error:  Class Symfony\Component\Console\Style\SymfonyStyle contains 4 abstract methods and must therefore be declared abstract or implement the remaining methods (Symfony\Component\Console\Output\OutputInterface::isQuiet, Symfony\Component\Console\Output\OutputInterface::isVerbose, Symfony\Component\Console\Output\OutputInterface::isVeryVerbose, ...) in /builds/NotRelevant/vendor/symfony/console/Style/SymfonyStyle.php on line 33

These methods do not exist in the given interfaces in this version of symfony console.

Steps to Reproduce

  1. create a project requiring symfony/console:2.8.45
  2. composer install
  3. run dependency-guard in project root

Expected behavior: continues, since there is no issue

Actual behavior: dies with a fatal error

Reproduces how often: 100%

Versions

mediact/dependency-guard: 1.0.3 (seperate)
symfony/console: 2.8.45 (included in a project)

@willemstuursma

This comment has been minimized.

Show comment
Hide comment
@willemstuursma

willemstuursma Aug 30, 2018

Contributor

I guess it should be scoped and then shipped as a .phar file to work around this.

Contributor

willemstuursma commented Aug 30, 2018

I guess it should be scoped and then shipped as a .phar file to work around this.

@Idrinth

This comment has been minimized.

Show comment
Hide comment
@Idrinth

Idrinth Aug 30, 2018

Contributor

would be sufficient if all files are only analysed statically, I assume somewhere in the code they are still loaded in some way.
https://github.com/Roave/BetterReflection could be a solution to that if it's the case - haven't had time to dig through DG's code to check if that's possible.

Contributor

Idrinth commented Aug 30, 2018

would be sufficient if all files are only analysed statically, I assume somewhere in the code they are still loaded in some way.
https://github.com/Roave/BetterReflection could be a solution to that if it's the case - haven't had time to dig through DG's code to check if that's possible.

@johmanx10 johmanx10 added the bug label Aug 30, 2018

@johmanx10

This comment has been minimized.

Show comment
Hide comment
@johmanx10

johmanx10 Aug 30, 2018

Contributor

@Idrinth indeed, Dependency Guard uses ReflectionClass to determine if a class exists and whether the class lives in user land. Using the BetterReflection package would definitely solve this issue. I encourage you to introduce this package and solve the problem.

There are only 2 places where Reflection is used. The \Mediact\DependencyGuard\Candidate\CandidateExtractor, which uses it to determine the filename of the symbol and \Mediact\DependencyGuard\Php\Filter\UserDefinedSymbolFilter, which uses it to determine whether the symbol exists and whether it is an internal or user defined class. It seems that these features are present within BetterReflection.

Contributor

johmanx10 commented Aug 30, 2018

@Idrinth indeed, Dependency Guard uses ReflectionClass to determine if a class exists and whether the class lives in user land. Using the BetterReflection package would definitely solve this issue. I encourage you to introduce this package and solve the problem.

There are only 2 places where Reflection is used. The \Mediact\DependencyGuard\Candidate\CandidateExtractor, which uses it to determine the filename of the symbol and \Mediact\DependencyGuard\Php\Filter\UserDefinedSymbolFilter, which uses it to determine whether the symbol exists and whether it is an internal or user defined class. It seems that these features are present within BetterReflection.

@johmanx10 johmanx10 self-assigned this Aug 30, 2018

@Idrinth

This comment has been minimized.

Show comment
Hide comment
@Idrinth

Idrinth Aug 30, 2018

Contributor

will hopefully have time this weekend @johmanx10 and will have a look then.

Contributor

Idrinth commented Aug 30, 2018

will hopefully have time this weekend @johmanx10 and will have a look then.

@johmanx10

This comment has been minimized.

Show comment
Hide comment
@johmanx10

johmanx10 Aug 31, 2018

Contributor

@Idrinth I have provided a fix in the branch issue/21. Could you verify that this solves your problem?

Contributor

johmanx10 commented Aug 31, 2018

@Idrinth I have provided a fix in the branch issue/21. Could you verify that this solves your problem?

@Idrinth

This comment has been minimized.

Show comment
Hide comment
@Idrinth

Idrinth Aug 31, 2018

Contributor

working on it @johmanx10 - give me 15mins

Contributor

Idrinth commented Aug 31, 2018

working on it @johmanx10 - give me 15mins

@Idrinth

This comment has been minimized.

Show comment
Hide comment
@Idrinth

Idrinth Aug 31, 2018

Contributor

The issue does not seem to be fixed yet, seems the newer interface is still in use.

Did shorten the paths, dependency-guard is a freshly cloned one from issue/21, folder contains a composer package with symfony/console as dependency.

$ php ../dependency-guard/bin/dependency-guard
PHP Fatal error:  Class Symfony\Component\Console\Style\SymfonyStyle contains 4 abstract methods and must therefore be declared abstract or implement the remaining methods (Symfony\Component\Console\Output\OutputInterface::isQuiet, Symfo
ny\Component\Console\Output\OutputInterface::isVerbose, Symfony\Component\Console\Output\OutputInterface::isVeryVerbose, ...) in folder\vendor\symfony\console\Style\SymfonyStyle.php on line 33
PHP Stack trace:
PHP   1. {main}() dependency-guard\bin\dependency-guard:0
PHP   2. Composer\Console\Application->run() dependency-guard\bin\dependency-guard:20
PHP   3. Composer\Console\Application->run() dependency-guard\vendor\composer\composer\src\Composer\Console\Application.php:103
PHP   4. Composer\Console\Application->doRun() depePHP   5. Composer\Console\Application->doRun() dependency-guard\vendor\composer\composer\src\Composer\Console\Appl
ication.php:254
PHP   6. Composer\Console\Application->doRunCommand() dependency-guard\vendor\symfony\console\Application.php:262
PHP   7. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->run() dependency-guard\vendor\symfony\console\Application.php:886
PHP   8. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->execute() dependency-guard\vendor\symfony\console\Command\Command.php:251
PHP   9. Mediact\DependencyGuard\Composer\Command\Exporter\ViolationExporterFactory->create() dependency-guard\src\Composer\Command\DependencyGuardCommand.php:94
PHP  10. spl_autoload_call() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
PHP  11. Composer\Autoload\ClassLoader->loadClass() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
PHP  12. Composer\Autoload\includeFile() dependency-guard\vendor\composer\ClassLoader.php:322
PHP  13. include() dependency-guard\vendor\composer\ClassLoader.php:444

Fatal error: Class Symfony\Component\Console\Style\SymfonyStyle contains 4 abstract methods and must therefore be declared abstract or implement the remaining methods (Symfony\Component\Console\Output\OutputInterface::isQuiet, Symfony\Co
mponent\Console\Output\OutputInterface::isVerbose, Symfony\Component\Console\Output\OutputInterface::isVeryVerbose, ...) in folder\vendor\symfony\console\Style\SymfonyStyle.php on line 33

Call Stack:
    0.1923     384512   1. {main}() dependency-guard\bin\dependency-guard:0
    0.3669    3263376   2. Composer\Console\Application->run() dependency-guard\bin\dependency-guard:20
    0.3669    3263376   3. Composer\Console\Application->run() dependency-guard\vendor\composer\composer\src\Composer\Console\Application.php:103
    0.4319    3346432   4. Composer\Console\Application->doRun() dependency-guard\vendor\symfony\console\Application.php:145
    2.5720    7107960   5. Composer\Console\Application->doRun() dependency-guard\vendor\composer\composer\src\Composer\Console\Application.php:254
    2.5724    7107960   6. Composer\Console\Application->doRunCommand() dependency-guard\vendor\symfony\console\Application.php:262
    2.5726    7107960   7. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->run() dependency-guard\vendor\symfony\console\Application.php:886
    2.5828    7115176   8. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->execute() dependency-guard\vendor\symfony\console\Command\Command.php:251
    2.6603   10466688   9. Mediact\DependencyGuard\Composer\Command\Exporter\ViolationExporterFactory->create() dependency-guard\src\Composer\Command\DependencyGuardCommand.php:94
    2.6625   10494096  10. spl_autoload_call() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
    2.6625   10494160  11. Composer\Autoload\ClassLoader->loadClass() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
    2.6631   10494288  12. Composer\Autoload\includeFile() dependency-guard\vendor\composer\ClassLoader.php:322
    2.6638   10553280  13. include('folder\vendor\symfony\console\Style\SymfonyStyle.php') dependency-guard\vendor\composer\ClassLoader.php:444
Contributor

Idrinth commented Aug 31, 2018

The issue does not seem to be fixed yet, seems the newer interface is still in use.

Did shorten the paths, dependency-guard is a freshly cloned one from issue/21, folder contains a composer package with symfony/console as dependency.

$ php ../dependency-guard/bin/dependency-guard
PHP Fatal error:  Class Symfony\Component\Console\Style\SymfonyStyle contains 4 abstract methods and must therefore be declared abstract or implement the remaining methods (Symfony\Component\Console\Output\OutputInterface::isQuiet, Symfo
ny\Component\Console\Output\OutputInterface::isVerbose, Symfony\Component\Console\Output\OutputInterface::isVeryVerbose, ...) in folder\vendor\symfony\console\Style\SymfonyStyle.php on line 33
PHP Stack trace:
PHP   1. {main}() dependency-guard\bin\dependency-guard:0
PHP   2. Composer\Console\Application->run() dependency-guard\bin\dependency-guard:20
PHP   3. Composer\Console\Application->run() dependency-guard\vendor\composer\composer\src\Composer\Console\Application.php:103
PHP   4. Composer\Console\Application->doRun() depePHP   5. Composer\Console\Application->doRun() dependency-guard\vendor\composer\composer\src\Composer\Console\Appl
ication.php:254
PHP   6. Composer\Console\Application->doRunCommand() dependency-guard\vendor\symfony\console\Application.php:262
PHP   7. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->run() dependency-guard\vendor\symfony\console\Application.php:886
PHP   8. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->execute() dependency-guard\vendor\symfony\console\Command\Command.php:251
PHP   9. Mediact\DependencyGuard\Composer\Command\Exporter\ViolationExporterFactory->create() dependency-guard\src\Composer\Command\DependencyGuardCommand.php:94
PHP  10. spl_autoload_call() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
PHP  11. Composer\Autoload\ClassLoader->loadClass() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
PHP  12. Composer\Autoload\includeFile() dependency-guard\vendor\composer\ClassLoader.php:322
PHP  13. include() dependency-guard\vendor\composer\ClassLoader.php:444

Fatal error: Class Symfony\Component\Console\Style\SymfonyStyle contains 4 abstract methods and must therefore be declared abstract or implement the remaining methods (Symfony\Component\Console\Output\OutputInterface::isQuiet, Symfony\Co
mponent\Console\Output\OutputInterface::isVerbose, Symfony\Component\Console\Output\OutputInterface::isVeryVerbose, ...) in folder\vendor\symfony\console\Style\SymfonyStyle.php on line 33

Call Stack:
    0.1923     384512   1. {main}() dependency-guard\bin\dependency-guard:0
    0.3669    3263376   2. Composer\Console\Application->run() dependency-guard\bin\dependency-guard:20
    0.3669    3263376   3. Composer\Console\Application->run() dependency-guard\vendor\composer\composer\src\Composer\Console\Application.php:103
    0.4319    3346432   4. Composer\Console\Application->doRun() dependency-guard\vendor\symfony\console\Application.php:145
    2.5720    7107960   5. Composer\Console\Application->doRun() dependency-guard\vendor\composer\composer\src\Composer\Console\Application.php:254
    2.5724    7107960   6. Composer\Console\Application->doRunCommand() dependency-guard\vendor\symfony\console\Application.php:262
    2.5726    7107960   7. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->run() dependency-guard\vendor\symfony\console\Application.php:886
    2.5828    7115176   8. Mediact\DependencyGuard\Composer\Command\DependencyGuardCommand->execute() dependency-guard\vendor\symfony\console\Command\Command.php:251
    2.6603   10466688   9. Mediact\DependencyGuard\Composer\Command\Exporter\ViolationExporterFactory->create() dependency-guard\src\Composer\Command\DependencyGuardCommand.php:94
    2.6625   10494096  10. spl_autoload_call() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
    2.6625   10494160  11. Composer\Autoload\ClassLoader->loadClass() dependency-guard\src\Composer\Command\Exporter\ViolationExporterFactory.php:44
    2.6631   10494288  12. Composer\Autoload\includeFile() dependency-guard\vendor\composer\ClassLoader.php:322
    2.6638   10553280  13. include('folder\vendor\symfony\console\Style\SymfonyStyle.php') dependency-guard\vendor\composer\ClassLoader.php:444
@johmanx10

This comment has been minimized.

Show comment
Hide comment
@johmanx10

johmanx10 Sep 1, 2018

Contributor

I had hoped this would not be required, but it seems we have to configure a different source locator for the reflector, in order to achieve what is desired. Either that or there is something we're overlooking. When next I work on this, I'll reproduce the environment you pointed out, instead of using what I already have, locally. Please allow a number of days to pass for this to be picked up again.

Contributor

johmanx10 commented Sep 1, 2018

I had hoped this would not be required, but it seems we have to configure a different source locator for the reflector, in order to achieve what is desired. Either that or there is something we're overlooking. When next I work on this, I'll reproduce the environment you pointed out, instead of using what I already have, locally. Please allow a number of days to pass for this to be picked up again.

@Idrinth

This comment has been minimized.

Show comment
Hide comment
@Idrinth

Idrinth Sep 1, 2018

Contributor

this is not a service I'm paying for @johmanx10 - I'm positively surprised at how fast you did get startet. On top of that it currently breaks only a single library's ci for us, it's not too hard to manually check that one.

Contributor

Idrinth commented Sep 1, 2018

this is not a service I'm paying for @johmanx10 - I'm positively surprised at how fast you did get startet. On top of that it currently breaks only a single library's ci for us, it's not too hard to manually check that one.

johmanx10 added a commit that referenced this issue Sep 3, 2018

Merge pull request #23 from Idrinth/issue/21
Regression test for issue #21
@johmanx10

This comment has been minimized.

Show comment
Hide comment
@johmanx10

johmanx10 Sep 3, 2018

Contributor

@Idrinth I have strengthened the patch with your regression test and am now pretty confident that this should fix your issue. Could you verify once more?

A pull request is open for review by my co-workers. Once the pull request is accepted, a new patch version, 1.0.4, will follow.

Contributor

johmanx10 commented Sep 3, 2018

@Idrinth I have strengthened the patch with your regression test and am now pretty confident that this should fix your issue. Could you verify once more?

A pull request is open for review by my co-workers. Once the pull request is accepted, a new patch version, 1.0.4, will follow.

@Idrinth

This comment has been minimized.

Show comment
Hide comment
@Idrinth

Idrinth Sep 3, 2018

Contributor

I can confirm it no longer produces a fatal with the current patch. Thank you for your quick work.

Contributor

Idrinth commented Sep 3, 2018

I can confirm it no longer produces a fatal with the current patch. Thank you for your quick work.

@sjokki sjokki closed this in #24 Sep 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment