Summary of issues with `api:upgrade-resource`

[usu]

API Platform version(s) affected: 3.0.0

Thanks for writing the upgrade-resource console command. I notice a few issues when using it. Already corrected manually for my code, but fixing this might help others with the transition.

~~Removes ApiFilter if there is more than one~~

Edit: duplicate of #4966 Before:

#[ApiResource]
#[ApiFilter(SearchFilter::class, properties: ['category'])]
#[ApiFilter(CustomFilter::class)]
#[ORM\Entity]
class Books {
}

After:

#[ApiResource]
#[ApiFilter(filterClass: CustomFilter::class)]
#[ORM\Entity]
class Books {
}

Looses reference to self:: variables and inserts the actual content

Before:

#[ApiResource(
    itemOperations: [
        'get' => [
            'normalization_context' => self::ITEM_NORMALIZATION_CONTEXT,
        ],
    ],
    denormalizationContext: ['groups' => ['write']],
    normalizationContext: ['groups' => ['read']],
)]
#[ORM\Entity]
class Category {
    public const ITEM_NORMALIZATION_CONTEXT = [
        'groups' => [
            'read',
            'Category:EmbeddedData',
        ],
    ];
}

After:

#[ApiResource(
    operations: [
        new Get(
            normalizationContext: ['groups' => ['read', 'Category:PreferredContentTypes', 'Category:ContentNodes'], 'swagger_definition_name' => 'read'],
        ),
    ],
    denormalizationContext: ['groups' => ['write']],
    normalizationContext: ['groups' => ['read']]
)]
#[ORM\Entity]
class Category {
    public const ITEM_NORMALIZATION_CONTEXT = [
        'groups' => [
            'read',
            'Category:EmbeddedData',
        ],
    ];
}

General observations

Some other general peculiarities and improvement ideas. Might be difficult to implement, though:

• The complete ApiResource attribute is printed on a single line. Breaking the line for each operation or even for every property inside an operation would be nice.
• Order of property attributes is changed. By purpose?
• Some extra lines in code are removed (within entity logic). Is this coming from a formatter that could optionally be disabled?
• All extra lines between properties are removed. Is this coming from a formatter that could optionally be disabled?
• Some other changes that deviate from our code formatting, but they can easily be corrected/reverted by running cs-fixer

[soyuka]

The complete ApiResource attribute is printed on a single line. Breaking the line for each operation or even for every property inside an operation would be nice.

Could be done, we recommend to use php-cs-fixer on top of that.

Order of property attributes is changed. By purpose?

Nope we could fix this.

Some extra lines in code are removed (within entity logic). Is this coming from a formatter that could optionally be disabled?

Probably php-parser stuff. You need to see it as a helper it's hard to do better and as this tool was removed from 3.0 maintenance is quite time-consuming.

For the rest: use php-cs fixer on top of this tool.

[maxcreation]

I don't even have that command at all. In any of the versions cloned, 2.7 ... 3.0.2. What is going on? I see it in the commits list and diff in GitHub, but when I actually browse the file, it's gone. How is that even possible?

[lermontex]

@maxcreation,

https://github.com/api-platform/core/blob/2.7/src/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommand.php

[n-valverde]

Some extra lines in code are removed (within entity logic). Is this coming from a formatter that could optionally be disabled?

Probably php-parser stuff. You need to see it as a helper it's hard to do better and as this tool was removed from 3.0 maintenance is quite time-consuming.

For anyone ever coming through this, here's a tiny patch to fix the formatting issue. The command currently mess a lot with formatting, making it barely usable. I'm not opening a PR based on @soyuka's comment, but I'd be happy to do so if you ever change your mind

===================================================================
diff --git a/tests/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommandTest.php b/tests/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommandTest.php
--- a/tests/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommandTest.php	(revision 8257741fca0e6e5a9b71096b3965a0a5547e8570)
+++ b/tests/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommandTest.php	(date 1686072943179)
@@ -68,6 +68,8 @@
         $display = $commandTester->getDisplay();
         foreach ($expectedStrings as $expectedString) {
             $this->assertStringContainsString($expectedString, $display);
+            $this->assertStringNotContainsString('-declare(strict_types=1);', $display);
+            $this->assertStringNotContainsString('+declare (strict_types=1);', $display);
         }
     }
 
===================================================================
diff --git a/src/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommand.php b/src/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommand.php
--- a/src/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommand.php	(revision 8257741fca0e6e5a9b71096b3965a0a5547e8570)
+++ b/src/Core/Bridge/Symfony/Bundle/Command/UpgradeApiResourceCommand.php	(date 1686072677548)
@@ -28,6 +28,7 @@
 use Doctrine\Common\Annotations\AnnotationReader;
 use PhpParser\Lexer\Emulative;
 use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitor\CloningVisitor;
 use PhpParser\Parser\Php7;
 use PhpParser\PrettyPrinter\Standard;
 use SebastianBergmann\Diff\Differ;
@@ -141,7 +142,10 @@
             $oldStmts = $parser->parse($oldCode);
             $oldTokens = $lexer->getTokens();
 
-            $newStmts = $traverser->traverse($oldStmts);
+            $oldTraverser = new NodeTraverser();
+            $oldTraverser->addVisitor(new CloningVisitor()); // Required to preserve formatting
+
+            $newStmts = $traverser->traverse($oldTraverser->traverse($oldStmts));
             $newCode = $prettyPrinter->printFormatPreserving($newStmts, $oldStmts, $oldTokens);
 
             if (!$input->getOption('force') && $input->getOption('dry-run')) {