Problem/Motivation

Once #3523994: Add .gitlab-ci.yml and fix PHP CodeSniffer errors/warnings and #3431487: Add support for Drupal 11 are merged we will be running PHPStan checks via GitLab CI. They are currently failing with a few errors in #3431487: Add support for Drupal 11. This issue is for fixing them.

Proposed resolution

Fix PHPStan errors.

Remaining tasks

Fix PHPStan errors.

User interface changes

None.

API changes

None.

Data model changes

None.

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:

Comments

m.stenta created an issue. See original summary.

m.stenta’s picture

Status: Active » Needs review
Related issues: +#3456616: [Meta, Plan] Version 2.0

Opened a merge request with two commits (but this is built on top of #3431487: Add support for Drupal 11, which in turn is built on #3523994: Add .gitlab-ci.yml and fix PHP CodeSniffer errors/warnings, so all of those commits are included - they will need to merged before this).

The first commit simply adds drupal/jsonapi_hypermedia as a dev dependency of the module, because PHPStan couldn't find the classes that this module references.

The second commit replaces usage of \Drupal in classes with dependency injection. This change might be considered "breaking" because it changes the constructor arguments for all of the normalizers. Maybe there's another way to do this that is BC. If not, then perhaps we should wait and merge this ahead of a 2.0.0 release and document it as a breaking change (see #3456616: [Meta, Plan] Version 2.0).

Alternatively, we could remove this line entirely, which would avoid the need for all the changes: https://git.drupalcode.org/project/jsonapi_schema/-/blob/ed083e4274feec8...

m.stenta’s picture

For what it's worth, Drupal core does not consider constructor changes to be breaking: https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...

However, we may still want to, for the sake of any downstream code that might be extending DataDefinitionNormalizer.

m.stenta’s picture

Opened a second MR that adds a phpstan-baseline.neon file for ignoring the \Drupal calls. This allows us to acknowledge their existence, and instruct PHPStan to ignore them for the time being, while still flagging any future errors that new changes introduce.

I will open a follow-up issue with a MR for replacing the \Drupal calls with dependency injection, which can be considered for merge into 2.x as a breaking change.

m.stenta’s picture

m.stenta’s picture

Status: Needs review » Postponed

This is ready for merge, but must wait for #3431487: Add support for Drupal 11.

m.stenta changed the visibility of the branch 3524246-fix-phpstan-errors to hidden.

m.stenta’s picture

Status: Postponed » Fixed

Status: Fixed » Closed (fixed)

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