d13r / laravel-breadcrumbs

Please see the Diglactic fork.
https://github.com/diglactic/laravel-breadcrumbs
2.34k stars 416 forks source link

Fix PHPStan error with FQCN #187

Closed Omranic closed 6 years ago

Omranic commented 6 years ago

This fixes the following PHPStan error

 ------ --------------------------------------------------------------
  Line   app/path/to/breadcrumbs.php
 ------ --------------------------------------------------------------
         Internal error: Class BreadcrumbsManager does not exist
         Run PHPStan with --debug option and post the stack trace to:
         https://github.com/phpstan/phpstan/issues/new
 ------ --------------------------------------------------------------
d13r commented 6 years ago

Please can you update the unit test too, to prevent regressions? Thanks.

Omranic commented 6 years ago

Which test exactly? I just ran the test suite and it's all green..

d13r commented 6 years ago

I mean can you add a new test for this? See FacadePhpDocTest for how I did it for the Manager class.

Omranic commented 6 years ago

Ah, ok. Let me check that 👍

Omranic commented 6 years ago

Well, couldn't figure out why and how to make a useful test for this PR, sorry! 😅

d13r commented 6 years ago

OK, I'll take a look next week. Thanks 😎

d13r commented 6 years ago

Hi @Omranic,

On closer inspection, I don't think this is a bug. It may be a bug in PHPStan.

When you first posted it, I thought it was related to IDE Helper, like in #162, but when I tested it I see IDE Helper ignores this particular comment.

The code your commit referred to is:

use DaveJamesMiller\Breadcrumbs\BreadcrumbsManager;

/**
 * @see BreadcrumbsManager
 */

This is valid PHPDoc syntax, supported by both phpDocumentor and PhpStorm - they read the use statements and find the correct class. Perhaps PHPStan doesn't parse the use statements?

However, I can't reproduce this myself - I installed PHPStan and ran it on both the application and the package with --level max and I didn't get this error.