Drupal entity ids, even when defined as auto-increment id, notoriously get returned as string int like "123" instead of a real int like 123.
A simple fix is to at least have the ->id() methods return true integer.
This will help with phpstan, and also allows to use proper parameter types.
Similar attempts are happening in Drupal core, see #3224376: UserInterface::id(), AccountInterface::id() should return int to match typehints.
Create a trait and interface for entities where ->id() returns real integer (or NULL).
Mark as internal, so that we can later replace it when a solution is provided by Drupal core.
Show commandsStart 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:
changes
Comments
Comment #3
donquixote commentedShould be not disruptive at all.
Comment #5
teebeecoder commentedComment #7
teebeecoder commentedHi @donquixote,
Thanks for the work on this and for opening the MR.
At this stage, we’d prefer not to merge this change yet. Since similar efforts are already ongoing in Drupal core (see the related issue as you spotted it), we want to avoid duplicating the work and potentially having to refactor everything again once a core solution is available.
For the PHPStan concerns, we suggest handling them on purpose at the analysis level by ignoring the relevant cases where needed. In the code itself, we can add explicit checks for empty or non-existent IDs where it makes sense, to ensure safety without enforcing a custom interim API.
This approach should keep things stable for now while allowing us to align more cleanly with the core solution once it lands.
Thomas
Comment #8
teebeecoder commentedComment #9
donquixote commentedThe difference is that Drupal needs to worry about BC and about edge cases like AccountInterface::id().
In this module we do not have these concerns.
No matter how this will be solved in core, I don't see how it would conflict with the solution proposed here.
Comment #10
donquixote commentedComment #11
fmb commentedI am afraid I am not convinced we should remedy this kind of core typing issues at our level by adding this kind of layer (trait and so on). I agree with @TeeBeeCoder about «add[ing] explicit checks for empty or non-existent IDs where it makes sense».
Comment #12
donquixote commentedAlright let's close it by majority.
I would do it if it was my own module that I maintain myself, but it is ok.