Problem/Motivation

Fix 

$ignoreErrors[] = [
	'message' => '#^Method Drupal\\\\jsonapi\\\\Normalizer\\\\DataNormalizer\\:\\:normalize\\(\\) return type with generic class ArrayObject does not specify its types\\: TKey, TValue$#',
	'identifier' => 'missingType.generics',
	'count' => 1,
	'path' => __DIR__ . '/modules/jsonapi/src/Normalizer/DataNormalizer.php',
];

and similar.

Proposed resolution

Add PHPDoc to document \ArrayDoc, and narrow types where possible.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3563533-nr-bot_akt9_fe8.txt547 bytesneeds-review-queue-bot

Issue fork drupal-3563533

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 opened merge request !14118

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
Status: Active » Needs review
smustgrave’s picture

smustgrave commented

should instances like 

 @return array|string|int|float|bool|\ArrayObject<mixed, mixed>|null
   *   \ArrayObject is used to make sure an empty object is encoded as an
   *   object not an array.

Be updated now that the comments are being taken over by core? I mean should the return for each instance only show what it can actually return with a comment that reflects that.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Re. #4: these are all normalization methods that more or less inherit from upstream https://github.com/symfony/symfony/blob/8.1/src/Symfony/Component/Serial...

"more or less" because not all of them inherit directly, some are just related and the inheritdoc does not really pick from the class hierarchy. That's why I added explicit comments.

That said, the interface definition is

public function normalize(mixed $data, ?string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null;

I already narrowed the return type when I was certain of the returned value, and left the original interface return type in the rest of the instances. Please note that type narrowing may have BC concerns.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Version: 11.x-dev » main
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Based on #5 believe this is probably good. Don't want to get into the weeds.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3563533-nr-bot_akt9_fe8.txt547 bytes

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.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Reviewed & tested by the community

local blip

  • amateescu committed 3659d1b9 on main 
    task: #3563533 Fix PHPStan missingType.generics for \ArrayObject not...
amateescu’s picture

amateescu commented
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 3659d1b and pushed to main. Thanks!

We'll need a 11.x MR for backporting.

amateescu closed merge request !14118

mondrake’s picture

mondrake
Location 🇮🇹
commented
Version: main » 11.x-dev

mondrake opened merge request !15486

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Patch (to be ported) » Needs review

11.x port

amateescu’s picture

amateescu commented
Status: Needs review » Fixed

Committed ca9f9be and pushed to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

amateescu closed merge request !15486

  • amateescu committed ca9f9beb on 11.x 
    task: #3563533 Fix PHPStan missingType.generics for \ArrayObject not...
idebr’s picture

idebr commented

This broke HAL's \Drupal\hal\Normalizer\FieldNormalizer and subsequently webform tests: https://git.drupalcode.org/issue/webform-3550117/-/pipelines/797824/test...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

godotislate’s picture

godotislate
he/him
commented

Consulted with other release managers @catch and @xjm and noting here that the return type changes are allowed for Drupal 11.4.0, because normalizers are tagged services and not considered public API per Drupal core's backwards compatibility policy.

For visibility, I have published a CR documenting the changes: https://www.drupal.org/node/3588047

tobiasb’s picture

tobiasb
Primary language German
Location Berlin
commented

@godotislate

There is also another CR.

https://www.drupal.org/node/3588046

The CR is now a draft again.