Problem/Motivation

From issue #3178534: Start running PHPStan on Drupal core (level 0): Start running PHPStan on Drupal core (level 0)

     Internal error: Internal error: Cannot assign an empty string to a string offset in file /app/core/modules/link/tests/src/Kernel/LinkItemTest.php                                                                
     Run PHPStan with --debug option and post the stack trace to:                                                                                                                                                     
     https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md

Steps to reproduce

Proposed resolution

Probably this should be checked explicitly that this is not a string.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3254966-7.patch1.71 KBtaran2l

Issue fork drupal-3254966

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

mallezie created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +PHPStan-0
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

This needs some debugging when analyzing. Here is the phpstan-drupal issue for the error: https://github.com/mglaman/phpstan-drupal/issues/228

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

I am working on a fix, now. I was able to reproduce the bug: https://github.com/mglaman/phpstan-drupal/pull/279

The problem is due the way PHPStan has decided the field_test property will act. It gets set to ConstantStringType. But still this shouldn't cause a crash in PHPStan. I don't think. Hopefully I can make a fix in phpstan-drupal until PHPStan has a fix, if we can determine why.

Error : Cannot assign an empty string to a string offset
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Type/Constant/ConstantStringType.php:236
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:2652
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1371
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:455
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:253
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:418
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:253
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:502
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:253
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:468
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:221
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php:164
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/Analyser.php:52
 phar:///Users/mglaman/Projects/php/phpstan-drupal/vendor/phpstan/phpstan/phpstan.phar/src/Testing/RuleTestCase.php:56
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

I can't figure this out. Opened an issue in PHPStan https://github.com/phpstan/phpstan/issues/6231

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Parent issue: » #3178534: Start running PHPStan on Drupal core (level 0)
taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Active » Needs review
StatusFileSize
new 3254966-7.patch1.71 KB

Ok, #3178534: Start running PHPStan on Drupal core (level 0) went in, so attaching patch from #3178534-153: Start running PHPStan on Drupal core (level 0)

Clearly, this is a big in PHPStan, but we can adjust a test little bit to make it pass.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
  1. +++ b/core/modules/link/tests/src/Kernel/LinkItemTest.php
@@ -150,14 +150,14 @@ public function testLinkItem() {
-    $entity->field_test = 'internal:/node/add';
+    $entity->field_test = ['internal:/node/add'];

    This is changing the test though - we want to check that setting the property directly, instead of setting a list of properties, does what is intended. 

  2. +++ b/core/modules/link/tests/src/Kernel/LinkItemTest.php
@@ -150,14 +150,14 @@ public function testLinkItem() {
-    $entity->field_test->options = NULL;
+    $entity->field_test[0]->options = NULL;

    Same here, we want to check that setting a property on the field sets the value on the first item.

To me this is a bug in either PHPStan or phpstan-drupal and should be fixed there instead.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented

@longwave, your points are valid, but from the comments from the code I see that test is checking:

// Check that if we set the direct value of link field it correctly set the
// uri and the default values of the field.
// Check that setting options to NULL does not trigger an error when
// calling getUrl();

to me, it seems like it tests LinkItem specifics rather than Typed data API or Field item API

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Needs work

to me, it seems like it tests LinkItem specifics rather than Typed data API or Field item API

Yeah, the test code is a bit blurred. But it shows the specifics of the properties for this field. It also proves there is a bug in PHPStan – albeit hard to track down. I'm not for changing the tests but rather trying to make a fix for PHPStan.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Postponed

Fix upstream: https://github.com/phpstan/phpstan/issues/6231#issuecomment-1017524310

We need to wait for a new release and upgrade phpstan/phpstan.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

I don't think the solution is going to do what we expect. It just prevents PHPStan from crashing and returns an error type, when we don't actually have an error type.

mallezie opened merge request !2354

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Status: Postponed » Needs review

This was fixed in one of the updates of phpstan / phpstan-drupal.

Removing this, runs a full scan, no new issues added to baseline.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Cool.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed ad5ff3f and pushed to 10.0.x. Thanks!

  • alexpott committed ad5ff3f on 10.0.x 
    Issue #3254966 by mallezie, Taran2L, mglaman, mondrake, longwave:...

Status: Fixed » Closed (fixed)

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