NamedArgumentForDataProviderRector only changing the first data provider

[gnutix]

Hey there,

I've ran NamedArgumentForDataProviderRector on my project and noticed it only refactors the first #[DataProvider] it encounters. On my project, it missed about a 1/3 of the deprecations.

Changing getDataProviderMethodName to return an array of method names :

    private function getDataProviderMethodNames(ClassMethod $classMethod) : array
    {
        $methodNames = [];
        $attributeClassName = DataProvider::class;
        foreach ($classMethod->attrGroups as $attributeGroup) {
            foreach ($attributeGroup->attrs as $attribute) {
                if (!$this->isName($attribute->name, $attributeClassName)) {
                    continue;
                }
                foreach ($attribute->args as $arg) {
                    if ($arg->value instanceof String_) {
                        $methodNames[] = $arg->value->value;
                    }
                }
            }
        }
        return $methodNames;
    }

and calling it like this :

            $dataProviderMethodNames = $this->getDataProviderMethodNames($classMethod);
            if ($dataProviderMethodNames === []) {
                continue;
            }
            foreach ($dataProviderMethodNames as $dataProviderMethodName) {
                // ... rest of the code untouched
            }

fixes the issue. /cc @marcelthole

gnutix

[TomasVotruba]

Great 👍 Could you send the PR with fix? :)

[gnutix]

Sorry, too much on my plate these days.

[TomasVotruba]

No worries 👍

Closing as duplicate of similar issue to keep focus: #346 (comment)

[gnutix]

Without this fix, the rule is only partially doing its job, and the "optional feature" triggers deprecations that are outputted by PHUnit. So please, don't close this issue, it's a separate problem from the other you've linked.

[TomasVotruba]

I missunderstood then. We'll need a failing demo link first.

Ref: #330

/cc @marcelthole

[marcelthole]

I added a failing testcase and fixed that case. We didn't had the case with multiple dataproviders on a single test method. But it should be fixed now :)