Problem/Motivation

Metadata in doc-comments is deprecated and will no longer be supported in PHPUnit 12.

Proposed resolution

In this issue, replace @dataProvider annotations with #[DataProvider()] attributes.

Use the Rector\PHPUnit\AnnotationsToAttributes\Rector\ClassMethod\DataProviderAnnotationToAttributeRector Rector Rule as a starting point.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 rector-changes-1.patch866.73 KBmondrake

Issue fork drupal-3445106

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new rector-changes-1.patch866.73 KB

Out of curiosity I tried run Rector with this config

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\PHPUnit\AnnotationsToAttributes\Rector\ClassMethod\DataProviderAnnotationToAttributeRector;

return RectorConfig::configure()
    ->withPaths([
        __DIR__ . '/composer',
        __DIR__ . '/core',
    ])
    ->withSkipPath(
        __DIR__ . '/core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures',
    )
    ->withSkipPath(
        '*/ProxyClass/*',
    )
    ->withSkipPath(
        '*.api.php',
    )
    ->withRules([
        DataProviderAnnotationToAttributeRector::class,
    ])
    ->withImportNames(
        importDocBlockNames: false,
        importShortClasses: false,
        removeUnusedImports: false,
    );

and it produced the attached patch; it's not perfect but it should fix 95% of the cases. A bit annoying the added use import is not sorted alphabeticalyl, but I assume some PHPCS magic could happen to fix that as well.

mstrelan’s picture

mstrelan commented
Status: Postponed » Needs work

I think we can continue with this now that #3417066: Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency is in. The patch looks like a great start!

A bit annoying the added use import is not sorted alphabeticalyl, but I assume some PHPCS magic could happen to fix that as well

There is SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses, but see #1624564: Coding standards for "use" statements where it's being discussed as a coding standard.

mstrelan’s picture

mstrelan commented

I think we should also add @dataProvider to the SlevomatCodingStandard.Commenting.ForbiddenAnnotations configuration in phpcs.xml.dist

mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: mondrake » Unassigned
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

As spotted over in #3417066-120: Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency, it looks like we can't mix PHPUnit attributes and annotations, we will have to convert them all at once unfortunately:

        $metadata = $this->attributeReader->forMethod($className, $methodName);

        if (!$metadata->isEmpty()) {
            return $metadata;
        }

        return $this->annotationReader->forMethod($className, $methodName);
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Closed (won't fix)

#6 - ouch that's painful.. however we could keep annotations as well as attributes in the same test file, so for BC to have PHPUnit 9 compatiblity (using annotations) and PHPUnit 10/11 compatibility (using attributes) at the same time. See for example these tests in a library I maintain on GitHub: https://github.com/FileEye/MimeMap/blob/ba0b04e179976e7d6a487fdb166c5f1f...

However, we can't proceed by annotation. Do we need an issue to build a comprehensive Rector that would address all annotations and then use that to proceed by test suite/core-module?

solideogloria’s picture

solideogloria commented

I think having some issue open would be good. I just found this issue after reading about the availability of attributes

https://docs.phpunit.de/en/10.5/writing-tests-for-phpunit.html#data-prov...

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3446380: [no-commit] Define a Rector rule to convert test annotations to attributes

Filed #3446380: [no-commit] Define a Rector rule to convert test annotations to attributes. See you there!