Closed (fixed)
Project:
JSON:API Schema
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
13 May 2025 at 18:29 UTC
Updated:
3 Jun 2025 at 18:49 UTC
Jump to comment: Most recent
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.
Fix PHPStan errors.
Fix PHPStan errors.
None.
None.
None.
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
Comment #3
m.stentaOpened 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_hypermediaas a dev dependency of the module, because PHPStan couldn't find the classes that this module references.The second commit replaces usage of
\Drupalin 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...
Comment #4
m.stentaFor 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.Comment #6
m.stentaOpened a second MR that adds a
phpstan-baseline.neonfile for ignoring the\Drupalcalls. 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
\Drupalcalls with dependency injection, which can be considered for merge into 2.x as a breaking change.Comment #7
m.stentaComment #8
m.stentaThis is ready for merge, but must wait for #3431487: Add support for Drupal 11.
Comment #11
m.stenta