Improved type hinting to pass some PHPStan / PHPCS rules better

[voxpelli]

I generally believe that auto-generated code should not be made to pass the coding style of the project that uses it – but this code was so close that I decided to give it a quick look to see if my PHPStan and PHPCS configs required required some adaptions to pass or if all could be auto-fixed.

This PR is a summary of all the PHPStan / PHPCS fixed I found and which would be feasible to do directly in this project, and which should not have any negative side consequences.

I have kept the commits atomic and descriptive.

---

Copilot summary

This pull request includes a variety of changes to improve code clarity, ensure proper type annotations, and enhance the handling of nullable and non-required parameters in JSON serialization. The changes span across multiple files, focusing on both the main codebase and test files.

Improvements to type annotations and interface clarity:

• Updated @implements annotations in src/Generator/ArrayObjectResolver.php to include fully qualified class names for ArrayAccess and IteratorAggregate.

Enhancements to JSON serialization:

• Added @return array<string, mixed> annotations to jsonSerialize methods in src/Serialization/SerializableResolver.php and various test files for better type clarity. [1] [2] [3] [4] [5] and others)
• Improved the handling of nullable and non-required parameters in jsonSerialize methods by refactoring lambda functions and simplifying conditions in src/Serialization/SerializableResolver.php and test files. [1] [2] [3] [4] [5]

Handling of enum values:

• Added a check to skip null values in the transformEnum method in src/Generator/ClassTransformer.php to ensure only valid enum values are processed.

Additional minor improvements:

• Added @return type annotations for jsonSerialize methods in src/Serialization/ArrayObjectSerialization.php to improve code documentation.

These changes collectively enhance code readability, maintainability, and correctness, particularly in scenarios involving JSON serialization and type annotations.

[reinfi]

That is not correct as it requires to check the key because if you have a nullable but required value it now would be removed

[voxpelli]

You are totally right, I embarrassingly failed to apply the outer negation when I removed the needless in_array() 🙈 Fixed it now!

[reinfi]

still not correct, you explicitly need to check for the key

[voxpelli]

My intent was to fix this:

Call to functionin_array() with arguments 'applicationId', ['applicationId'] and true will always evaluate to true.

Checking whether 'applicationId' is in the array ['applicationId'] is not meaningful, but maybe I did the "fix" wrong?

[reinfi]

    Test:
      type: object
      required:
        - requiredNotNull
        - requiredNullable
      properties:
        requiredNotNull:
          type: string
        requiredNullable:
          type: string
        notRequired:
          type: string

currently produces

public function __construct(
        public string $requiredNotNull,
        public string $requiredNullable,
        public ?string $notRequired = null,
    ) {
    }

    public function jsonSerialize(): array
    {
        return array_filter(
            [
                'requiredNotNull' => $this->requiredNotNull,
                'requiredNullable' => $this->requiredNullable,
                'notRequired' => $this->notRequired,
            ],
            static fn (mixed $value, string $key): bool => !(in_array($key, ['notRequired'], true) && $value === null),
            ARRAY_FILTER_USE_BOTH
        );
    }

should be to pass PHPStan inspection:

public function __construct(
        public string $requiredNotNull,
        public string $requiredNullable,
        public ?string $notRequired = null,
    ) {
    }

    public function jsonSerialize(): array
    {
        return array_filter(
            [
                'requiredNotNull' => $this->requiredNotNull,
                'requiredNullable' => $this->requiredNullable,
                'notRequired' => $this->notRequired,
            ],
            static fn (mixed $value, string $key): bool => !($key === 'notRequired' && $value === null),
            ARRAY_FILTER_USE_BOTH
        );
    }

hope that helps

[voxpelli]

@reinfi I finally took time to delve into this, I understand you now and I fixed the logic + added tests for all of the different cases 👍

[reinfi]

not really a fan of this because the classes are already imported.

[voxpelli]

Yeah, I agree in principle. In my project though there's a Drupal.Classes.UseGlobalClass.RedundantUseStatement ("Non-namespaced classes/interfaces/traits should not be referenced with use statements") phpcs rule that php code beautifier auto-fixes but without auto-fixing the comments, so I thought that maybe it could be okay to do the direct reference in the comments to avoid this 🙈

It should really be something to be filed for Drupal.Classes.UseGlobalClass.RedundantUseStatement, I will do that as well

[reinfi]

Great, thank you