phpstan / phpdoc-parser

Next-gen phpDoc parser with support for intersection types and generics
MIT License
1.35k stars 61 forks source link

PHPDoc tag have invalid value on PHPMD suppress warning #256

Closed GErpeldinger closed 1 week ago

GErpeldinger commented 1 week ago

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 commented 1 week ago

Wanted to report same :+1:

ondrejmirtes commented 1 week ago

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

ariademur commented 1 week ago

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

ariademur commented 1 week ago

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 commented 1 week ago

Awesome, let that be the solution 🎉

sreichel commented 1 week ago

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

GErpeldinger commented 1 week ago

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:

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