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

PHPDoc tag have invalid value on PHPMD suppress warning #256

Closed
GErpeldinger opened this issue Nov 11, 2024 · 8 comments
Closed

PHPDoc tag have invalid value on PHPMD suppress warning #256

GErpeldinger opened this issue Nov 11, 2024 · 8 comments

Comments

@GErpeldinger
Copy link

GErpeldinger commented Nov 11, 2024

Bug report

Hello,

First, congratulations on the release of PHPStan 2.0! I was testing it on my project and noticed that PHPStan now detects an error with PHPMD-specific suppression rules.

As you can see in the example that I copied from the PHPMD documentation, there’s no issue with @SuppressWarnings(PHPMD). However, it seems PHPStan raises an error when there's a dot in the annotation, like @SuppressWarnings(PHPMD.UnusedLocalVariable).

Best regards,
G.Erpeldinger

Code snippet that reproduces the problem

https://phpstan.org/r/8ba21fe2-5c43-496e-9827-7f08c3366e33

Expected output

PhpStan doesn't recognize PHPMD specific rule suppression like an error.

Did PHPStan help you today? Did it make you happy in any way?

It always make me happy, maybe a little less when it finds 526 errors after updating to 2.0 but that's the price of a rock-solid code 😜

@sreichel
Copy link

sreichel commented Nov 11, 2024

Wanted to report same 👍

@ondrejmirtes ondrejmirtes transferred this issue from phpstan/phpstan Nov 12, 2024
@ondrejmirtes
Copy link
Member

It makes PHPStan think this annotation should be parsed as a Doctrine annotation.

@ariademur
Copy link

I have the same issue. Any ideas how to solve, or workarounds would be appreciated.

@ariademur
Copy link

Turns out that adding quotes makes the error go away, and PHPMD is also OK with it.

/**
 * @SuppressWarnings("PHPMD.UnusedLocalVariable")
 */

https://phpstan.org/r/d35797f2-9a97-4c0b-a301-d68316e7bed0

@ondrejmirtes
Copy link
Member

Awesome, let that be the solution 🎉

@sreichel
Copy link

Have to change dozen of DOCs that worked for years is not a good solution. :(

@GErpeldinger
Copy link
Author

GErpeldinger commented Nov 13, 2024

I agree that it's not the best solution too. in the long term, I think that PhpStan parser needs to be fixed. @ondrejmirtes I know it's not one of your priority right now, but maybe this issue can stay open for later ?

Otherwise temporarily, it will work fine, and the change can be made pretty fast.

I have found two solutions for this:

  • If you use PhpStorm and want a one-time change, use the Replace function with regex activated so that you replace @SuppressWarnings\(([^"\)]+)\) with @SuppressWarnings("$1").

  • Use a custom Rector rule (This is the first time I've tried a custom rule, but it seems to work fine on my end and I have quite a large code base):

<?php

declare(strict_types=1);

namespace App;

use Override;
use PhpParser\Comment\Doc;
use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassConst;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\Enum_;
use PhpParser\Node\Stmt\EnumCase;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

final class SuppressWarningsAddQuotesRector extends AbstractRector
{
    #[Override]
    public function getRuleDefinition(): RuleDefinition
    {
        return new RuleDefinition(
            'Add quotes around the content of @SuppressWarnings annotations.',
            [
                new CodeSample(
                    '/** @SuppressWarnings(someWarning) */',
                    '/** @SuppressWarnings("someWarning") */'
                ),
            ]
        );
    }

    #[Override]
    public function getNodeTypes(): array
    {
        return [
            Class_::class,
            ClassMethod::class,
            Property::class,
            ClassConst::class,
            Function_::class,
            Enum_::class,
            EnumCase::class,
        ];
    }

    #[Override]
    public function refactor(Node $node): ?Node
    {
        $docComment = $node->getDocComment();
        if ($docComment === null) {
            return null;
        }

        $originalDocText = $docComment->getText();

        $updatedDocText = preg_replace_callback(
            '/@SuppressWarnings\(\s*([^)"]+?)\s*\)/',
            static fn($matches) => sprintf('@SuppressWarnings("%s")', trim($matches[1])),
            $originalDocText
        );

        if ($updatedDocText !== $originalDocText) {
            $node->setDocComment(new Doc($updatedDocText));
            return $node;
        }

        return null;
    }
}

Warning: i have one weird case with the rule that i don't found a solution to fix, it's when an attribute is on top of the annotation block

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants