Annotations are ignored when scanning for used/unused imports

[Augustin82]

In the following code, Route is imported and used, but is being reported as unused. Conversely, Method is used without being imported, but PHPCS does not mind.

<?php

namespace AppBundle\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\JsonResponse;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;

class TestController extends Controller
{
    /**
     * @Route("/test")
     * @Method({"GET"})
     */
    public function postOrderAction()
    {
        return new JsonResponse("", 500);
    }
}

[Hywan]

Annotations are framework-specific (in this case, Symfony), and the goal of this project is to analyse PHP code, not framework specific features. The feature you are asking for should take the form of a PHPCS plugin/extension specifically tailored for Symfony.

[sirbrillig]

We could create an option to have this sniff scan comments for symbols; it would also help in other places where symbols are used in comments (like phpunit). However, this gets tricky fast because comments contain all kinds of words which are not going to be recognized, so how do you determine which ones should be treated as symbols? Maybe just capitalized words? That's still tricky for sentences which start with a capital letter.

I suppose another option would be to add an option like scanSymfonyAnnotations which enables logic that specifically knows how to recognize things from that framework. I'm definitely open to having a feature like that, but I don't have the cycles right now to add such a feature. PRs are welcome!

[Augustin82]

Thank you both for your comments, they're fair =) Indeed, my request might be out of scope of the project.

I'm trying to add solid linting/formatting to our Symfony project, and this is definitely going to be a problem if we have false positives/negatives when dealing with annotations (the same thing happens with Doctrine, obviously).

I have no idea how to contribute, but would be willing to try! Any pointers on where I should look for examples on adding parser logic?

Cheers!

[Augustin82]

Just a note: it also gives false "unused" warnings when using an import only within a block of PHPDoc, like this:

    use AppBundle\CustomException;

    /**
     * @return string
     *
     * @throws CustomException
     */
    public function testFunction() { ... }

[sirbrillig]

Just a note: it also gives false "unused" warnings when using an import only within a block of PHPDoc, like this:

That's expected. PHPDoc are really just comments. There will be no problems with running a script if the classes listed in PHPDoc annotations are not imported, so there's no need to scan them or report them. If an IDE integration shows a warning, that's an issue specific to that IDE and not to PHP itself.

I have no idea how to contribute, but would be willing to try! Any pointers on where I should look for examples on adding parser logic?

Thanks for your enthusiasm!

Options in PHPCS are pretty simple; you can see how this plugin uses the ignoreUnimportedSymbols option here. Basically you just set a public variable on the sniff class and it will be set automatically if it's used in the config.

The first thing I'd do is to add a fixture and a test. For example, take the code snippet you pasted above and put it in a file, then create a test to make sure there are no errors. Basically something like this test except that $expectedLines would be an empty array. Make sure that when you run the test, it fails.

We'd then need to include comments in the register function (beware, there are different kinds of comment token types) to include them in the scan. Next, the real challenge, we'd need to add a new function inside process() that does the following:

• Scans comments for symbols which match Symfony annotations (words starting with @? but will that also find phpdoc like @return?).
• Ignore any parts of comments which are not annotations.
• Treat any found annotations as symbols. Mostly this will happen automatically if you can take care of the above steps. See the existing process() function for examples.

In general, you can see my article on writing phpcs sniffs if you've never worked with one before.

[Augustin82]

Thanks, will look at it as soon as I have the time and will report back =)

[zanona]

I don't know how commonly PHPDoc is used, but if it's pretty common, it could be worth allowing an option to disable the warning if present on PHPDoc declarations. I'm using Psalm and declaring types through PHPDoc blocks. :)

Anyway, just a suggestion.

[mundschenk-at]

I agree with @zanona, having the sniff detect PHPDoc would be beneficial to everyone using static analysis tools (Psalm, PHPStan, etc.).