Repository navigation
AddSeeTestAnnotationRector adds a duplicate @see when the existing annotation is imported #9875
Description
Activity
Thank you for your report!
We'll need an isolated failing demo link from: http://getrector.com/demo,
that way we can reproduce the bug.The demo can't show this one. The rule bails out before it gets anywhere near the broken part:
matchExistingClassName()goes throughReflectionProvider::hasClass(), so the test class has to be autoloadable. The demo runs one snippet with no autoloader, so there is noFooTestto find and it returns null every time. Any demo link is going to say "no change" no matter what I put in it.Smallest thing I could get it to fail on — four files, 2.6.5:
composer.json
{ "name": "acme/see-repro", "autoload": { "psr-4": { "App\\": "src/" } } }rector.php
<?php declare(strict_types=1); use Rector\Config\RectorConfig; return RectorConfig::configure() ->withPaths([__DIR__ . '/src']) ->withPreparedSets(rectorPreset: true);
src/Tests/FooTest.php
<?php declare(strict_types=1); namespace App\Tests; final class FooTest {}
src/Foo.php
<?php declare(strict_types=1); namespace App; use App\Tests\FooTest; /** * @see FooTest */ final class Foo {}
composer dump-autoload && vendor/bin/rector process --dry-rungives:/** * @see FooTest + * @see \App\Tests\FooTest */ final class Foo {}The tag is already there, just imported.
hasAlreadySeeAnnotation()compares the raw docblock text to'\\' . $testCaseClassName, soFooTestnever equals\App\Tests\FooTestand it adds a second one.The fallback in
shouldSkipClass()misses it too, sincehasClass('FooTest')is false for the short name.I suspect that's what happened in #9795 as well — if the test class isn't autoloadable under one of the names the resolver generates, the rule quietly does nothing and looks fixed.
Worth noting it compounds: php-cs-fixer's
fully_qualified_strict_typesimports@seetargets by default, so on a project running both, every run leaves one more tag behind.- Reacted by Tomas Votruba
Sorry, i don't know what I did wrong, since it wasn't able to reproduce it in the dmeo myself. Thanks @samsonasik for helping out and providing the fix!
@Brainshaker95 that's fine. Please try latest
"rector/rector": "dev-main"in case there is still issue with it.
Bug Report
AddSeeTestAnnotationRectoradds a second@seeannotation when the first one is imported rather than fully qualified.The rule writes
@see \App\Tests\FooTest. php-cs-fixer'sfully_qualified_strict_types, which PER-CS enables, then shortens that to@see FooTestand adds ause. On the next run the rule no longer recognizes its own annotation and appends another, so the count grows on every run.hasAlreadySeeAnnotation()compares the raw docblock text against the fully qualified name, so the short form never matches:https://github.com/rectorphp/rector-phpunit/blob/c4e571ee6d61aa0bf68346bab12b85e2df3aead2/rules/CodeQuality/Rector/Class_/AddSeeTestAnnotationRector.php#L164-L169
This was reported once before in #9795 and closed because the reporter's test classes turned out to be in the wrong namespace, so the reproduction never held up. The one below does — the test class is found, which is what makes the rule add the duplicate.
Minimal PHP Code Causing Issue
With
App\Tests\FooTestpresent andrectorPreset: true:/** * @see FooTest + * @see \App\Tests\FooTest */Expected Behaviour
Skip the class — the annotation is already there, just imported. Resolving the name against the file's
usestatements before comparing would do it.