Incorrect behavior of TestWithAnnotationToAttributeRector with integers

[Seldaek]

Bug Report

Subject	Details
Rector version	last dev-main
Installed as	composer dependency

Minimal PHP Code Causing Issue

See https://getrector.com/demo/322b336f-f232-4614-a264-6077e0a8f206

<?php

class UserControllerTest extends \PHPUnit\Framework\TestCase
{
    /**
     * @testWith ["2"]
     *           ["[email protected]"]
     */
    public function testSomething(string $userId): void
    {
    }
}

Responsible rules

• TestWithAnnotationToAttributeRector

Expected Behavior

I'd expect #[TestWith('2')] as the annotation was using a string, but somehow it thinks it is smart to convert that to an int because I guess the string is numeric. Anyway this breaks the test as the $userId param requires a string.

[samsonasik]

Re-open per rectorphp/rector-src#6608 (comment)

the fix for numeric string, "true", "false" should be in a single PR to ease rollback.

[staabm]

First time I see someone using this super ugly annotation based data-provider stuff instead of a proper php coded one :-).

Recently @sebastianbergmann and me were discussing whether anyone on the planet needs this phpunit feature 😅

[Seldaek]

I believe @heiglandreas was the author of this particular @testWith instance, which is indeed the only one I've got in my codebase here, maybe he can give you more info, maybe there is a secret society of testWith users out there 😅

But I think it's kinda nice if you have just 2-3 variations of something, then the "overhead" of a test provider method can feel overkill.

[staabm]

But I think it's kinda nice if you have just 2-3 variations of something, then the "overhead" of a test provider method can feel overkill.

IMO with this phpdoc based version you are loosing static analysis and other tooling support, which is a footgun.

maybe there is a secret society of testWith users out there 😅

I guess its a dark net thing ;-)

[heiglandreas]

I was working on several code-bases that heavily rely upon this 😂

And the attribute-version might actually be better in terms of types.

But yes: It does come with a gun pointing at your foot out of the box!

OTOH: Which data-provider is providing more type-safety...

/**
 * @return array{string, int, bool}[]
 */

or

#[TestWith(['foo', 1, true])]

Yes, the annotation one is about the same level of type-safety.

But for small datasets the TestWith really is a nice way to have everything together.

[staabm]

I don't like both. I would use a dataprovider method 😅
(just personal preference of course. use what you like)

[heiglandreas]

The first one is from a data-provider method 😉

[staabm]

OTOH: Which data-provider is providing more type-safety...

when time allows I will revive a phpstan-phpunit PR, which will verify types of dataprovider vs. tests without the need for the manual phpdoc return type

phpstan/phpstan-phpunit#152 (comment)

[Seldaek]

It could also inspect TestWith attribute types ;)