Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
PHPStan baseline is currently skipping multiple Call to an undefined method
errors.
Proposed resolution
Fix DataReferenceBase errors, clean up the baseline.
Comment | File | Size | Author |
---|---|---|---|
#14 | interdiff_13-14.txt | 1.46 KB | mondrake |
#14 | 3317651-14.patch | 2.75 KB | mondrake |
|
Comments
Comment #2
mondrakeComment #3
mondrakeComment #4
longwaveCan we just change getString() now, given this is not actually implemented in core?
Comment #5
mondrakeAn external class calling getString() right now in HEAD would lead to fatal, no? So what should getString() return, really? It is public API but quite evidently not called by anything at the moment. Maybe we can just deprecate it?
Comment #6
longwaveWe could just delete the
getString()
method from DataReferenceBase, and fall back to the parent implementation?Comment #7
mondrakeGood idea!
Comment #8
longwaveWorks for me, given that the existing method would cause a fatal error.
Comment #9
alexpottSomething could extend DataReferenceBase in contrib and then implement getType(). This is not that farfetched because DataReferenceBase is abstract.
I think we should do something like:
We can add a test for this in \Drupal\KernelTests\Core\TypedData\TypedDataDefinitionTest::testDataReferences as we have \Drupal\Core\TypedData\Plugin\DataType\LanguageReference in core that extends but does not implement this method.
Comment #10
mondrakeI am 97% that #9 won’t work for PHPStan because it will still report the missing method. IMHO the best option is to go back to #2 here, then.
Comment #11
longwave@mondrake I am not so sure: https://github.com/phpstan/phpstan/issues/323 implies that
method_exists()
checks inside the same method will work.Comment #12
mondrakeLet’s try then! Thx
Comment #13
mondrakeComment #14
mondrakeAdding a test as suggested in #9.
Comment #15
longwaveLooks great!
Comment #16
alexpottCommitted 4695b21 and pushed to 10.1.x. Thanks!
Committed 5615ede and pushed to 10.0.x. Thanks!