Convert test annotations to attributes in modules' Functional tests

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Appears to need a rebase

If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

This touches 1,304 files, so it could become stale easily. I would encourage at reviewing as is also if not mergeable (at the end of the day this is again and again the same conversion pattern repeating), and then re-run the patch generation last minute before commit, if necessary. Otherwise we spend time running the same steps again with no value.

The way one managed these bulk conversions is outline the explicit steps taken so that someone can repeat them.

I convert them to added script and post them to agitlab snippet.

I clearly call out that it should be reviewed as a bulk conversion even if there are conflicts but that it should not be rtbc unless there are no conflicts just a comment with process review.

Then a couple of times a week I run the ddev script in a new branch on the issue.

You can see the most recent here: 📌 Bulk convert the remaining hooks to OOP Active

It took me a few hours, maybe more, but I read through the whole raw diff of over 1300 changes. As you'd expect the most common change is the switch to the

Group

attribute. I did my best to check every class addition, cover statement, and provider reference. It all looked correct to me.

There were some other PHPCS changes too, mainly alphabetizing use statements, periods at the end of sentences, and extra line break removals. I'm assuming these were either necessary or impossible to filter out. The good news is that the diff is clean otherwise. There were no other totally unrelated changes.

It needs to be rebased again. I can review the update commit when it's ready later. But for now I'm going to mark this as RTBC.

Actually, maybe I should set it to NW instead for the rebase.

No manual changes, just rerun the script at 📌 [meta] Define a Rector rule to convert test annotations to attributes Active , , then reset this branch to 11.x with --hard option, applied the regenerated patch, and pushed to MR branch with the --force option.

Self-RTBCing given #10.

Thanks @dcam for the massive review effort here.

I trusted @dcam's line by line review here - had more closely reviewed previously automated issues for the change, and the patterns are all the same, good to know it's reliable. I think the small re-orderings etc. are a result of phpstan processing the file, and it's OK to not filter them out because it would make the MRs not reproducible from the script any more (or at least not without a lot more work to try to prevent them from happening).

Committed/pushed to 11.x, thanks!

catch → closed merge request !12652

Re #15, Rector adds use imports to the top of the list, it does not have an option to sort them. Since that was rather annoying to have the PHPUnit imports before Drupal ones, the sorting is done by PHPCBF with the command

vendor/bin/phpcbf --filter=GitModified --standard=Drupal,SlevomatCodingStandard --sniffs=SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses,PSR2.Namespaces.UseDeclaration,PSR2.Namespaces.NamespaceDeclaration,Drupal.Classes.UnusedUseStatement,Drupal.Commenting.DocComment,Drupal.WhiteSpace.ScopeIndent,Generic.PHP.UpperCaseConstant,Squiz.WhiteSpace.SuperfluousWhitespace,Squiz.WhiteSpace.FunctionSpacing,Drupal.Classes.FullyQualifiedNamespace,Drupal.Files.EndFileNewline --ignore=core/tests/Drupal/Tests/Component/Annotation/Doctrine/* core

that runs in 📌 [meta] Define a Rector rule to convert test annotations to attributes Active after the Rector processing. This also fixes some whitespace and punctuation issues - it seems to me the final outcome is better that the as-was, in general.