Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: api:upgrade-resource output formatting #5624

Merged

Conversation

n-valverde
Copy link
Contributor

Q A
Branch? 2.7
Tickets #5620 & #4973
License MIT

Hello, fixed 2 issues affecting the output of bin/console api:upgrade-resource. I know the command is removed in v3, but I think it's an important part of the upgrade path, and it is really hard to use right now because almost the whole file appears in diff, see below.

  1. The PhpParser\PrettyPrinter\Standard::printFormatPreserving method has 3 requirements to preserve formatting, and one of them was not met.

The AST needs to be traversed by CloningVisitor before making changes to the AST. This is adding a property on the AST nodes, keeping a reference to the original node, and especially original tokens positions.
This is documented here and here

This is obviously still working on a best effort basis, but fixing this makes a huge difference in the output, but one issue remains with docblocks tags, which is point 2

  1. When removing an annotation, the RemoveAnnotationTrait::removeAnnotationByTag method is creating a diff for every line containing a tag in docblocks.

This occurs because we actually rebuild the whole docblock, and tags are outputed with an extra space, no matter the initial formatting

At the end, because we just need to delete a specific annotation in a string here, and annotation format is pretty predictible, I ended up using a good old regex.

I tested this patched version in my own application where the command needs to alter ~200 files, and I am super happy with the result (see example below), you still need to run cs-fixer to eventually cleanup a few remaining blank lines, but those blank lines are legitimate, i.e next to a deleted one, it's not random blank lines & spaces everywhere

Example diff after fix for a User entity

---------- begin diff ----------
@@ @@

 namespace App\Entity;

-use ApiPlatform\Core\Annotation\ApiFilter;
-use ApiPlatform\Core\Annotation\ApiProperty;
-use ApiPlatform\Core\Annotation\ApiResource;
+use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Metadata\Put;
+use ApiPlatform\Metadata\Get;
+use ApiPlatform\Metadata\ApiResource;
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiFilter;
 use ApiPlatform\Serializer\Filter\PropertyFilter;
 use App\Art\SavedArtInterface;
 use App\DBAL\Types\MTSWizardTypeType;
@@ @@
  *
  * @Assert\Callback({"App\Validator\UserCallback", "validate"})
  *
- * @ApiResource(
- *     attributes={"security"="is_granted('ROLE_USER')"},
- *     collectionOperations={
- *         "get"={
- *             "openapi_context"={
- *                 "description"="Lists all users the currently authenticated user has access to know about.",
- *             },
- *         }
- *     },
- *     itemOperations={
- *         "get"={
- *             "security"="is_granted('view', object)",
- *             "openapi_context"={
- *                 "description"="Loads a user by the given ID. You can also provide `""me""` as the ID to get the currently-authenticated user. The response for the current user will also contain a ""features"" array listing what features/areas of the system they have access to.",
- *             },
- *         },
- *         "put"={
- *             "security"="is_granted('edit', object)",
- *             "openapi_context"={
- *                 "description"="Updates the user profile matching the given ID. You can also provide `""me""` as the ID to get the currently-authenticated user.",
- *             },
- *         },
- *         "change_password"={
- *             "security"="is_granted('change_password', object)",
- *             "method"="PUT",
- *             "path"="/users/{id}/password.{_format}",
- *             "controller"="api_platform.action.put_item",
- *             "requirements"={"id": "me|\d+"},
- *             "denormalization_context"={"groups"={"user:write:change_password"}},
- *             "validation_groups"={"Default", "change_password"},
- *         }
- *     },
- *     normalizationContext={"groups"={"user:read", "business_unit:read:within_user", "sapuser:read:minimal", "product_catalog:read"}},
- *     denormalizationContext={"groups"={"user:write"}},
- * )
- * @ApiFilter(PropertyFilter::class)
  *
  * @UniqueEntity("email")
  */
+#[ApiResource(operations: [new Get(security: 'is_granted(\'view\', object)', openapiContext: ['description' => 'Loads a user by the given ID. You can also provide `"me"` as the ID to get the currently-authenticated user. The response for the current user will also contain a "features" array listing what features/areas of the system they have access to.']), new Put(security: 'is_granted(\'edit\', object)', openapiContext: ['description' => 'Updates the user profile matching the 
given ID. You can also provide `"me"` as the ID to get the currently-authenticated user.']), new Put(security: 'is_granted(\'change_password\', object)', uriTemplate: '/users/{id}/password.{_format}', controller: 'api_platform.action.put_item', requirements: ['id' => 'me|\\d+'], denormalizationContext: ['groups' => ['user:write:change_password']], validationContext: ['groups' => ['Default', 'change_password']]), new GetCollection(openapiContext: ['description' => 'Lists all users the currently authenticated user has access to know about.'])], security: 'is_granted(\'ROLE_USER\')', normalizationContext: ['groups' => ['user:read', 'business_unit:read:within_user', 'sapuser:read:minimal', 'product_catalog:read']], denormalizationContext: ['groups' => ['user:write']])]
+#[ApiFilter(filterClass: PropertyFilter::class)]
 class User implements UserInterface, PasswordAuthenticatedUserInterface, \Stringable
 {
     use TimestampableTrait;
@@ @@
      * @Assert\NotBlank()
      * @Assert\Length(max=255)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Full Name"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Full Name'])]
     private string $fullName = '';

     /**
@@ @@
      * @Assert\NotBlank()
      * @Assert\Length(max=100)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Short Name"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Short Name'])]
     private string $shortName = '';

     /**
@@ @@
      *
      * @Assert\Length(max=50)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Phone"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Phone'])]
     private ?string $phone = null;

     /**
@@ @@
      *
      * @Assert\Length(max=50)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Mobile Phone"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Mobile Phone'])]
     private ?string $mobile = null;

     /**
@@ @@
      *
      * @Assert\Length(max=255)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Address Line 1"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Address Line 1'])]
     private ?string $address1 = null;

     /**
@@ @@
      *
      * @Assert\Length(max=255)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Address Line 2"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Address Line 2'])]
     private ?string $address2 = null;

     /**
@@ @@
      *   groups={"create", "change_password"}
      * )
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Current Password",
-     *             "format"="password"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Current Password', 'format' => 'password'])]
     private ?string $currentPassword = null;

     /**
@@ @@
      *   groups={"create", "change_password"}
      * )
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="New Password",
-     *             "format"="password",
-     *             "minLength"=8,
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'New Password', 'format' => 'password', 'minLength' => 8])]
     private ?string $plainPassword = null;

     /**
@@ @@
      *
      * @Assert\Length(max=100)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="City"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'City'])]
     private ?string $localityName = null;

     /**
@@ @@
      *
      * @Assert\Length(max=100)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="State"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'State'])]
     private ?string $stateName = null;

     /**
@@ @@
      *
      * @Assert\Length(max=30)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Postal Code"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Postal Code'])]
     private ?string $postalCode = null;

     /**
@@ @@
      *
      * @Assert\Length(max=100)
      *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Country"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Country'])]
     private ?string $countryName = null;

     /**
    ----------- end diff -----------

Example diff before fix (same User entity)

---------- begin diff ----------
@@ @@
 <?php

-declare(strict_types=1);
+declare (strict_types=1);

 namespace App\Entity;

-use ApiPlatform\Core\Annotation\ApiFilter;
-use ApiPlatform\Core\Annotation\ApiProperty;
-use ApiPlatform\Core\Annotation\ApiResource;
+use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Metadata\Put;
+use ApiPlatform\Metadata\Get;
+use ApiPlatform\Metadata\ApiResource;
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiFilter;
 use ApiPlatform\Serializer\Filter\PropertyFilter;
 use App\Art\SavedArtInterface;
 use App\DBAL\Types\MTSWizardTypeType;
@@ @@
 use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
 use Symfony\Component\Security\Core\Validator\Constraints\UserPassword;
 use Symfony\Component\Validator\Constraints as Assert;
-
 /**
- * @ORM\Entity(repositoryClass=UserRepository::class)
- * @ORM\Table(
+ * @ORM\Entity (repositoryClass=UserRepository::class)
+ * @ORM\Table (
  *   uniqueConstraints={
  *     @ORM\UniqueConstraint(name="user_email_unique", columns={"email"}),
  *   }
  * )
- *
- * @Assert\Callback({"App\Validator\UserCallback", "validate"})
- *
- * @ApiResource(
- *     attributes={"security"="is_granted('ROLE_USER')"},
- *     collectionOperations={
- *         "get"={
- *             "openapi_context"={
- *                 "description"="Lists all users the currently authenticated user has access to know about.",
- *             },
- *         }
- *     },
- *     itemOperations={
- *         "get"={
- *             "security"="is_granted('view', object)",
- *             "openapi_context"={
- *                 "description"="Loads a user by the given ID. You can also provide `""me""` as the ID to get the currently-authenticated user. The response for the current user will also contain a ""features"" array listing what features/areas of the system they have access to.",
- *             },
- *         },
- *         "put"={
- *             "security"="is_granted('edit', object)",
- *             "openapi_context"={
- *                 "description"="Updates the user profile matching the given ID. You can also provide `""me""` as the ID to get the currently-authenticated user.",
- *             },
- *         },
- *         "change_password"={
- *             "security"="is_granted('change_password', object)",
- *             "method"="PUT",
- *             "path"="/users/{id}/password.{_format}",
- *             "controller"="api_platform.action.put_item",
- *             "requirements"={"id": "me|\d+"},
- *             "denormalization_context"={"groups"={"user:write:change_password"}},
- *             "validation_groups"={"Default", "change_password"},
- *         }
- *     },
- *     normalizationContext={"groups"={"user:read", "business_unit:read:within_user", "sapuser:read:minimal", "product_catalog:read"}},
- *     denormalizationContext={"groups"={"user:write"}},
- * )
- * @ApiFilter(PropertyFilter::class)
- *
- * @UniqueEntity("email")
+ * @Assert\Callback ({"App\Validator\UserCallback", "validate"})
+ * @UniqueEntity ("email")
  */
+#[ApiResource(operations: [new Get(security: 'is_granted(\'view\', object)', openapiContext: ['description' => 'Loads a user by the given ID. You can also provide `"me"` as the ID to get the currently-authenticated user. The response for the current user will also contain a "features" array listing what features/areas of the system they have access to.']), new Put(security: 'is_granted(\'edit\', object)', openapiContext: ['description' => 'Updates the user profile matching the 
given ID. You can also provide `"me"` as the ID to get the currently-authenticated user.']), new Put(security: 'is_granted(\'change_password\', object)', uriTemplate: '/users/{id}/password.{_format}', controller: 'api_platform.action.put_item', requirements: ['id' => 'me|\\d+'], denormalizationContext: ['groups' => ['user:write:change_password']], validationContext: ['groups' => ['Default', 'change_password']]), new GetCollection(openapiContext: ['description' => 'Lists all users the currently authenticated user has access to know about.'])], security: 'is_granted(\'ROLE_USER\')', normalizationContext: ['groups' => ['user:read', 'business_unit:read:within_user', 'sapuser:read:minimal', 'product_catalog:read']], denormalizationContext: ['groups' => ['user:write']])]
+#[ApiFilter(filterClass: PropertyFilter::class)]
 class User implements UserInterface, PasswordAuthenticatedUserInterface, \Stringable
 {
     use TimestampableTrait;
     use ToggleableTrait;
     use EntityIdentityTrait;
-
     /**
      * @ORM\Column
-     *
-     * @Assert\NotBlank()
-     * @Assert\Length(max=255)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Full Name"
-     *         }
-     *     }
-     * )
+     * @Assert\NotBlank ()
+     * @Assert\Length (max=255)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Full Name'])]
     private string $fullName = '';
-
     /**
-     * @ORM\Column(length=100)
-     *
-     * @Assert\NotBlank()
-     * @Assert\Length(max=100)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Short Name"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=100)
+     * @Assert\NotBlank ()
+     * @Assert\Length (max=100)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Short Name'])]
     private string $shortName = '';
-
     /**
      * @ORM\Column(length=100)
      *
@@ @@
      * @Assert\Length(max=100)
      */
     private string $email = '';
-
     /**
-     * @ORM\Column(length=50, nullable=true)
-     *
-     * @Assert\Length(max=50)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Phone"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=50, nullable=true)
+     * @Assert\Length (max=50)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Phone'])]
     private ?string $phone = null;
-
     /**
-     * @ORM\Column(length=50, nullable=true)
-     *
-     * @Assert\Length(max=50)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Mobile Phone"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=50, nullable=true)
+     * @Assert\Length (max=50)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Mobile Phone'])]
     private ?string $mobile = null;
-
     /**
-     * @ORM\Column(nullable=true)
-     *
-     * @Assert\Length(max=255)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Address Line 1"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (nullable=true)
+     * @Assert\Length (max=255)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Address Line 1'])]
     private ?string $address1 = null;
-
     /**
-     * @ORM\Column(nullable=true)
-     *
-     * @Assert\Length(max=255)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Address Line 2"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (nullable=true)
+     * @Assert\Length (max=255)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Address Line 2'])]
     private ?string $address2 = null;
-
     /**
      * @ORM\ManyToOne(targetEntity=SapUser::class, inversedBy="users")
      * @ORM\JoinColumn(name="sap_user_id", referencedColumnName="zemp")
      */
     private ?SAPUserInterface $sapUser = null;
-
     /**
      * @var array<int, string>
      *
@@ @@
      * @Assert\Count(min = 1)
      */
     private array $roles = [];
-
     /** @ORM\Column(nullable=true) */
     private ?string $password = null;
-
     /** @ORM\Column(type="boolean", options={"default": 0}) */
     private bool $isLocalAccount = true;
-
     /**
      * @var Collection<int, FavoriteAccount>
      *
@@ @@
      * )
      */
     private Collection $favoriteAccounts;
-
     /**
      * @var Collection<int, AccountInterface>
      *
@@ @@
      * @ORM\ManyToMany(targetEntity=Account::class, inversedBy="users")
      */
     private Collection $accounts;
-
     /**
      * @var Collection<int, SavedArtInterface>
      *
@@ @@
      * @ORM\OneToMany(targetEntity=SavedArt::class, mappedBy="owner")
      */
     private Collection $savedArts;
-
     /**
      * @var Collection<int, BusinessUnitInterface>
      *
@@ @@
      * @Assert\Count(min = 1)
      */
     private Collection $businessUnits;
-
     /** @ORM\Column(type="datetime_immutable", nullable=true) */
     private ?\DateTimeImmutable $lastLogin;
-
     /**
      * The current user's password to be encoded and checked against the actual encoded password stored in the system.
+     *
      * For safety reasons, this field is required when the user wants to change his\her password.
      *
-     * @Assert\NotBlank(groups={"change_password"})
-     *
-     * @UserPassword(
+     * @Assert\NotBlank (groups={"change_password"})
+     * @UserPassword (
      *   message="This password does not match the current user's password.",
      *   groups={"create", "change_password"}
      * )
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Current Password",
-     *             "format"="password"
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'Current Password', 'format' => 'password'])]
     private ?string $currentPassword = null;
-
     /**
      * The plain password that will be automatically encoded by the system in order to be stored safely.
      *
-     * @Assert\NotBlank(groups={"create", "change_password"})
-     * @Assert\Length(
+     * @Assert\NotBlank (groups={"create", "change_password"})
+     * @Assert\Length (
      *   min=8,
      *   minMessage="Password must be at least 8 characters long.",
      *   groups={"create", "change_password"}
      * )
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="New Password",
-     *             "format"="password",
-     *             "minLength"=8,
-     *         }
-     *     }
-     * )
      */
+    #[ApiProperty(openapiContext: ['title' => 'New Password', 'format' => 'password', 'minLength' => 8])]
     private ?string $plainPassword = null;
-
     /**
-     * @ORM\Column(length=100, nullable=true)
-     *
-     * @Assert\Length(max=100)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="City"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=100, nullable=true)
+     * @Assert\Length (max=100)
      */
+    #[ApiProperty(openapiContext: ['title' => 'City'])]
     private ?string $localityName = null;
-
     /**
-     * @ORM\Column(length=100, nullable=true)
-     *
-     * @Assert\Length(max=100)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="State"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=100, nullable=true)
+     * @Assert\Length (max=100)
      */
+    #[ApiProperty(openapiContext: ['title' => 'State'])]
     private ?string $stateName = null;
-
     /**
-     * @ORM\Column(length=30, nullable=true)
-     *
-     * @Assert\Length(max=30)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Postal Code"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=30, nullable=true)
+     * @Assert\Length (max=30)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Postal Code'])]
     private ?string $postalCode = null;
-
     /**
-     * @ORM\Column(length=100, nullable=true)
-     *
-     * @Assert\Length(max=100)
-     *
-     * @ApiProperty(
-     *     attributes={
-     *         "openapi_context"={
-     *             "title"="Country"
-     *         }
-     *     }
-     * )
+     * @ORM\Column (length=100, nullable=true)
+     * @Assert\Length (max=100)
      */
+    #[ApiProperty(openapiContext: ['title' => 'Country'])]
     private ?string $countryName = null;
-
     /**
      * @var Collection<int, ArtRequest>
      *
@@ @@
      * @ORM\OneToMany(targetEntity=ArtRequest::class, mappedBy="user")
      */
     private Collection $artRequests;
-
     /**
      * @param array<string, mixed> $data
      */
-    public static function create(array $data): self
+    public static function create(array $data) : self
     {
-        $user     = new self();
+        $user = new self();
         $user->id = $data['id'] ?? false ? (int) $data['id'] : null;
-
         // Support both first/last name and full/short name
-        $firstName       = $data['firstName'] ?? false ? (string) $data['firstName'] : ' ';
-        $lastName        = $data['lastName'] ?? false ? (string) $data['lastName'] : ' ';
-        $user->fullName  = $data['fullName'] ?? \trim($firstName . ' ' . $lastName);
+        $firstName = $data['firstName'] ?? false ? (string) $data['firstName'] : ' ';
+        $lastName = $data['lastName'] ?? false ? (string) $data['lastName'] : ' ';
+        $user->fullName = $data['fullName'] ?? \trim($firstName . ' ' . $lastName);
         $user->shortName = $data['shortName'] ?? \trim($firstName !== ' ' ? $firstName : $lastName);
-
-        $user->email          = (string) ($data['email'] ?? '');
-        $user->password       = $data['password'] ?? false ? (string) $data['password'] : null;
-        $user->roles          = \is_array($data['roles'] ?? false) ? (array) $data['roles'] : [];
-        $user->address1       = $data['address1'] ?? false ? (string) $data['address1'] : null;
-        $user->address2       = $data['address2'] ?? false ? (string) $data['address2'] : null;
-        $user->localityName   = $data['localityName'] ?? false ? (string) $data['localityName'] : null;
-        $user->stateName      = $data['stateName'] ?? false ? (string) $data['stateName'] : null;
-        $user->countryName    = $data['countryName'] ?? false ? (string) $data['countryName'] : null;
-        $user->postalCode     = $data['postalCode'] ?? false ? (string) $data['postalCode'] : null;
-        $user->phone          = $data['phone'] ?? false ? (string) $data['phone'] : null;
-        $user->mobile         = $data['mobile'] ?? false ? (string) $data['mobile'] : null;
-        $user->isEnabled      = (bool) ($data['isEnabled'] ?? false);
+        $user->email = (string) ($data['email'] ?? '');
+        $user->password = $data['password'] ?? false ? (string) $data['password'] : null;
+        $user->roles = \is_array($data['roles'] ?? false) ? (array) $data['roles'] : [];
+        $user->address1 = $data['address1'] ?? false ? (string) $data['address1'] : null;
+        $user->address2 = $data['address2'] ?? false ? (string) $data['address2'] : null;
+        $user->localityName = $data['localityName'] ?? false ? (string) $data['localityName'] : null;
+        $user->stateName = $data['stateName'] ?? false ? (string) $data['stateName'] : null;
+        $user->countryName = $data['countryName'] ?? false ? (string) $data['countryName'] : null;
+        $user->postalCode = $data['postalCode'] ?? false ? (string) $data['postalCode'] : null;
+        $user->phone = $data['phone'] ?? false ? (string) $data['phone'] : null;
+        $user->mobile = $data['mobile'] ?? false ? (string) $data['mobile'] : null;
+        $user->isEnabled = (bool) ($data['isEnabled'] ?? false);
         $user->isLocalAccount = (bool) ($data['isLocalAccount'] ?? false);
-        $user->createdAt      = new \DateTimeImmutable($data['createdAt'] ?? 'now');
-        $user->updatedAt      = new \DateTimeImmutable($data['updatedAt'] ?? 'now');
-
-        if (($data['sapUser'] ?? false) && ($data['sapUser'] instanceof SapUser)) {
+        $user->createdAt = new \DateTimeImmutable($data['createdAt'] ?? 'now');
+        $user->updatedAt = new \DateTimeImmutable($data['updatedAt'] ?? 'now');
+        if (($data['sapUser'] ?? false) && $data['sapUser'] instanceof SapUser) {
             $user->setSapUser($data['sapUser']);
         }
-
         if (($data['favoriteAccounts'] ?? false) && \is_iterable($data['favoriteAccounts'])) {
             foreach ($data['favoriteAccounts'] as $account) {
                 $user->addFavoriteAccount($account);
             }
         }
-
         if (($data['accounts'] ?? false) && \is_iterable($data['accounts'])) {
             foreach ($data['accounts'] as $account) {
                 $user->addAccount($account);
             }
         }
-
         if (($data['businessUnits'] ?? false) && \is_iterable($data['businessUnits'])) {
             foreach ($data['businessUnits'] as $businessUnit) {
                 $user->addBusinessUnit($businessUnit);
             }
         }
-
         return $user;
     }
-
-    public static function fromInvitation(UserInvitation $invite): self
+    public static function fromInvitation(UserInvitation $invite) : self
     {
         $inviteeEmail = $invite->getInviteeEmail();
         \assert(\is_string($inviteeEmail));
-
         $businessUnit = $invite->getBusinessUnit();
         \assert($businessUnit instanceof BusinessUnit);
-
         $account = $invite->getAccount();
         \assert($account instanceof Account);
-
         $user = new self();
         $user->setEmail($inviteeEmail);
         $user->setFullName($invite->getInviteeFullName() ?? '');
@@ @@
         $user->setRoles([UserRoles::ROLE_FRONT_CUSTOMER]);
         $user->getBusinessUnits()->add($businessUnit);
         $user->addAccount($account);
-
         return $user;
     }
-
     public function __construct()
     {
-        $this->businessUnits    = new ArrayCollection();
+        $this->businessUnits = new ArrayCollection();
         $this->favoriteAccounts = new ArrayCollection();
-        $this->accounts         = new ArrayCollection();
-        $this->savedArts        = new ArrayCollection();
-        $this->artRequests      = new ArrayCollection();
-        $this->isEnabled        = true;
+        $this->accounts = new ArrayCollection();
+        $this->savedArts = new ArrayCollection();
+        $this->artRequests = new ArrayCollection();
+        $this->isEnabled = true;
     }
-
-    public function __toString(): string
+    public function __toString() : string
     {
         return $this->getFullName();
     }
-
     /**
      * @return array<string, mixed>
      */
-    public function __serialize(): array
+    public function __serialize() : array
     {
         $properties = \get_object_vars($this);
         foreach ($properties as $key => $property) {
@@ @@
                 unset($properties[$key]);
             }
         }
-
         return $properties;
     }
-
-    public function getFullName(): string
+    public function getFullName() : string
     {
         return $this->fullName;
     }
-
-    public function setFullName(string $fullName): void
+    public function setFullName(string $fullName) : void
     {
         $this->fullName = $fullName;
-
-        if (! $this->shortName) {
+        if (!$this->shortName) {
             $this->shortName = $fullName;
         }
     }
-
-    public function getShortName(): string
+    public function getShortName() : string
     {
         return $this->shortName;
     }
-
-    public function setShortName(string $shortName): void
+    public function setShortName(string $shortName) : void
     {
         $this->shortName = $shortName;
     }
-
-    public function getEmail(): string
+    public function getEmail() : string
     {
         return $this->email;
     }
-
-    public function setEmail(string $email): void
+    public function setEmail(string $email) : void
     {
         $this->email = $email;
     }
-
-    public function getPhone(): ?string
+    public function getPhone() : ?string
     {
         return $this->phone;
     }
-
-    public function setPhone(?string $phone): void
+    public function setPhone(?string $phone) : void
     {
         $this->phone = $phone;
     }
-
-    public function getMobile(): ?string
+    public function getMobile() : ?string
     {
         return $this->mobile;
     }
-
-    public function setMobile(?string $mobile): void
+    public function setMobile(?string $mobile) : void
     {
         $this->mobile = $mobile;
     }
-
-    public function getAddress1(): ?string
+    public function getAddress1() : ?string
     {
         return $this->address1;
     }
-
-    public function setAddress1(?string $address1): void
+    public function setAddress1(?string $address1) : void
     {
         $this->address1 = $address1;
     }
-
-    public function getAddress2(): ?string
+    public function getAddress2() : ?string
     {
         return $this->address2;
     }
-
-    public function setAddress2(?string $address2): void
+    public function setAddress2(?string $address2) : void
     {
         $this->address2 = $address2;
     }
-
-    public function getPostalCode(): ?string
+    public function getPostalCode() : ?string
     {
         return $this->postalCode;
     }
-
-    public function setPostalCode(?string $postalCode): void
+    public function setPostalCode(?string $postalCode) : void
     {
         $this->postalCode = $postalCode;
     }
-
-    public function getSapUser(): ?SAPUserInterface
+    public function getSapUser() : ?SAPUserInterface
     {
         return $this->sapUser;
     }
-
-    public function setSapUser(?SapUser $sapUser): void
+    public function setSapUser(?SapUser $sapUser) : void
     {
         $this->sapUser = $sapUser;
     }
-
     /**
      * @return array<int, string>
      */
-    public function getRoles(): array
+    public function getRoles() : array
     {
         $roles = $this->roles;
-
         if ($this->isEnabled) {
             $roles[] = UserRoles::ROLE_USER;
         }
-
         return \array_unique($roles);
     }
-
     /**
      * @param array<int, string> $roles
      */
-    public function setRoles(array $roles): void
+    public function setRoles(array $roles) : void
     {
         $this->roles = $roles;
     }
-
-    public function getPassword(): ?string
+    public function getPassword() : ?string
     {
         return $this->password;
     }
-
-    public function setPassword(?string $password): void
+    public function setPassword(?string $password) : void
     {
         $this->password = $password;
     }
-
     /**
      * @deprecated since Symfony 5.3, this method is deprecated when salt is always null
      *
      * @todo Remove this method after having upgraded to Symfony 6
      */
-    public function getSalt(): ?string
+    public function getSalt() : ?string
     {
         return null;
     }
-
     /**
      * @deprecated since Symfony 5.3, use getUserIdentifier() instead
      *
      * @todo Remove this method after having upgraded to Symfony 6
      */
-    public function getUsername(): ?string
+    public function getUsername() : ?string
     {
         return $this->getUserIdentifier();
     }
-
-    public function getUserIdentifier(): ?string
+    public function getUserIdentifier() : ?string
     {
         return $this->email;
     }
-
-    public function isLocalAccount(): bool
+    public function isLocalAccount() : bool
     {
         return $this->isLocalAccount;
     }
-
-    public function setIsLocalAccount(bool $isLocalAccount): void
+    public function setIsLocalAccount(bool $isLocalAccount) : void
     {
         $this->isLocalAccount = $isLocalAccount;
     }
-
     /**
      * @return int[]
      */
-    public function getAccountIds(): array
+    public function getAccountIds() : array
     {
         $accountIds = [];
         foreach ($this->getAccounts() as $account) {
@@ @@
             \assert($account instanceof Account);
             $accountIds[] = (int) $account->getId();
         }
-
         return $accountIds;
     }
-
     /**
      * @return Collection<int, AccountInterface>
      */
-    public function getAccounts(): Collection
+    public function getAccounts() : Collection
     {
         return $this->accounts;
     }
-
     /**
      * @param Collection<int, AccountInterface> $accounts
      */
-    public function setAccounts(Collection $accounts): void
+    public function setAccounts(Collection $accounts) : void
     {
         $this->accounts = $accounts;
     }
-
-    public function addAccount(AccountInterface $account): void
+    public function addAccount(AccountInterface $account) : void
     {
         if ($this->accounts->contains($account)) {
             return;
         }
-
         $this->accounts->add($account);
     }
-
-    public function removeAccount(AccountInterface $account): void
+    public function removeAccount(AccountInterface $account) : void
     {
-        if (! $this->accounts->contains($account)) {
+        if (!$this->accounts->contains($account)) {
             return;
         }
-
         $this->accounts->removeElement($account);
-
         $this->removeFavoriteAccount($account);
     }
-
-    public function hasAccountRestrictions(): bool
+    public function hasAccountRestrictions() : bool
     {
         return $this->accounts->count() > 0;
     }
-
     /**
      * @return Collection<int, AccountInterface>
      */
-    public function getFavoriteAccounts(): Collection
+    public function getFavoriteAccounts() : Collection
     {
-        return $this->favoriteAccounts->map(static fn (FavoriteAccount $fav) => $fav->getAccount());
+        return $this->favoriteAccounts->map(static fn(FavoriteAccount $fav) => $fav->getAccount());
     }
-
-    public function addFavoriteAccount(AccountInterface $account): void
+    public function addFavoriteAccount(AccountInterface $account) : void
     {
-        if ($this->favoriteAccounts->exists(static fn (int $i, FavoriteAccount $fav) => $fav->getAccount() === $account)) {
+        if ($this->favoriteAccounts->exists(static fn(int $i, FavoriteAccount $fav) => $fav->getAccount() === $account)) {
             return;
         }
-
         $this->favoriteAccounts->add(new FavoriteAccount($this, $account));
     }
-
-    public function removeFavoriteAccount(AccountInterface $account): void
+    public function removeFavoriteAccount(AccountInterface $account) : void
     {
         foreach ($this->favoriteAccounts as $favorite) {
             \assert($favorite instanceof FavoriteAccount);
@@ @@
             }
         }
     }
-
     /**
      * @return Collection<int, SavedArtInterface>
      */
-    public function getSavedArts(): Collection
+    public function getSavedArts() : Collection
     {
         return $this->savedArts;
     }
-
-    public function addSavedArt(SavedArt $savedArt): void
+    public function addSavedArt(SavedArt $savedArt) : void
     {
         if ($this->savedArts->contains($savedArt)) {
             return;
         }
-
         $this->savedArts->add($savedArt);
         $savedArt->setOwner($this);
     }
-
-    public function removeSavedArt(SavedArt $savedArt): void
+    public function removeSavedArt(SavedArt $savedArt) : void
     {
-        if (! $this->savedArts->contains($savedArt)) {
+        if (!$this->savedArts->contains($savedArt)) {
             return;
         }
-
         $this->savedArts->removeElement($savedArt);
     }
-
     /**
      * @return Collection<int, BusinessUnitInterface>
      */
-    public function getBusinessUnits(): Collection
+    public function getBusinessUnits() : Collection
     {
         return $this->businessUnits;
     }
-
     /**
      * @param Collection<int, BusinessUnitInterface> $businessUnits
      */
-    public function setBusinessUnits(Collection $businessUnits): void
+    public function setBusinessUnits(Collection $businessUnits) : void
     {
         $this->businessUnits = $businessUnits;
     }
-
-    public function addBusinessUnit(BusinessUnit $businessUnit): void
+    public function addBusinessUnit(BusinessUnit $businessUnit) : void
     {
         if ($this->businessUnits->contains($businessUnit)) {
             return;
         }
-
         $this->businessUnits->add($businessUnit);
     }
-
-    public function removeBusinessUnit(BusinessUnit $businessUnit): void
+    public function removeBusinessUnit(BusinessUnit $businessUnit) : void
     {
-        if (! $this->businessUnits->contains($businessUnit)) {
+        if (!$this->businessUnits->contains($businessUnit)) {
             return;
         }
-
         $this->businessUnits->removeElement($businessUnit);
     }
-
-    public function clearBusinessUnits(): void
+    public function clearBusinessUnits() : void
     {
         foreach ($this->businessUnits as $businessUnit) {
             \assert($businessUnit instanceof BusinessUnit);
@@ @@
             $this->removeBusinessUnit($businessUnit);
         }
     }
-
     /**
      * @return string[]
      */
-    public function getAllowedMTSWCartTypes(): array
+    public function getAllowedMTSWCartTypes() : array
     {
         $types = [];
         foreach ($this->businessUnits as $businessUnit) {
@@ @@
                 $types[] = $type;
             }
         }
-
         return $types;
     }
-
-    public function getLastLogin(): ?\DateTimeImmutable
+    public function getLastLogin() : ?\DateTimeImmutable
     {
         return $this->lastLogin;
     }
-
-    public function setLastLogin(?\DateTimeImmutable $lastLogin): void
+    public function setLastLogin(?\DateTimeImmutable $lastLogin) : void
     {
         $this->lastLogin = $lastLogin;
     }
-
-    public function getPlainPassword(): ?string
+    public function getPlainPassword() : ?string
     {
         return $this->plainPassword;
     }
-
-    public function hasPlainPassword(): bool
+    public function hasPlainPassword() : bool
     {
         return (string) $this->plainPassword !== '';
     }
-
-    public function setPlainPassword(?string $plainPassword): void
+    public function setPlainPassword(?string $plainPassword) : void
     {
         $this->plainPassword = $plainPassword;
     }
-
-    public function eraseCredentials(): void
+    public function eraseCredentials() : void
     {
         $this->plainPassword = null;
     }
-
-    public function getLocalityName(): ?string
+    public function getLocalityName() : ?string
     {
         return $this->localityName;
     }
-
-    public function setLocalityName(?string $localityName): void
+    public function setLocalityName(?string $localityName) : void
     {
         $this->localityName = $localityName;
     }
-
-    public function getStateName(): ?string
+    public function getStateName() : ?string
     {
         return $this->stateName;
     }
-
-    public function setStateName(?string $stateName): void
+    public function setStateName(?string $stateName) : void
     {
         $this->stateName = $stateName;
     }
-
-    public function getCountryName(): ?string
+    public function getCountryName() : ?string
     {
         return $this->countryName;
     }
-
-    public function setCountryName(?string $countryName): void
+    public function setCountryName(?string $countryName) : void
     {
         $this->countryName = $countryName;
     }
-
     /**
      * @param array<string, string> $attributes
      */
-    public function setSamlAttributes(array $attributes): void
+    public function setSamlAttributes(array $attributes) : void
     {
         // no-op
     }
-
-    public function getCurrentPassword(): ?string
+    public function getCurrentPassword() : ?string
     {
         return $this->currentPassword;
     }
-
-    public function setCurrentPassword(?string $currentPassword): void
+    public function setCurrentPassword(?string $currentPassword) : void
     {
         $this->currentPassword = $currentPassword;
     }
-
     /**
      * @return Collection<int, ArtRequest>
      */
-    public function getArtRequests(): Collection
+    public function getArtRequests() : Collection
     {
         return $this->artRequests;
     }
-
-    public function addArtRequest(ArtRequest $artRequest): void
+    public function addArtRequest(ArtRequest $artRequest) : void
     {
         if ($this->artRequests->contains($artRequest)) {
             return;
         }
-
         $this->artRequests->add($artRequest);
         $artRequest->setUser($this);
     }
-
-    public function removeArtRequest(ArtRequest $artRequest): void
+    public function removeArtRequest(ArtRequest $artRequest) : void
     {
-        if (! $this->artRequests->contains($artRequest)) {
+        if (!$this->artRequests->contains($artRequest)) {
             return;
         }
-
         $this->artRequests->removeElement($artRequest);
         $artRequest->setUser(null);
     }
-
-    public function equals(mixed $other): bool
+    public function equals(mixed $other) : bool
     {
-        if (! $other instanceof self) {
+        if (!$other instanceof self) {
             return false;
         }
-
         return $this->id === $other->id;
     }
 }
    ----------- end diff -----------

@soyuka
Copy link
Member

soyuka commented Jun 11, 2023

Nice improvement. The maker errors are unrelated but this test is: https://github.com/api-platform/core/actions/runs/5232471488/jobs/9447198605?pr=5624

We have a test suite that runs the script on our fixtures and we run the behat tests again. This ensures the script is working.

@n-valverde n-valverde force-pushed the fix/api-upgrade-resource-formatting branch from acd2f33 to ea35399 Compare June 11, 2023 09:05
@n-valverde
Copy link
Contributor Author

Hello @soyuka , thanks for taking a look and for the hint, that's a nice test suite 👍 , my regex was halting on the first @, this was a mistake since there is potentially stuff like @id in the annotation body, should be fixed now 🙂 🤞

@n-valverde n-valverde marked this pull request as ready for review June 11, 2023 09:13
@soyuka soyuka merged commit c5f709d into api-platform:2.7 Jun 13, 2023
@soyuka
Copy link
Member

soyuka commented Jun 13, 2023

thanks @n-valverde

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants