accessCheck https://www.drupal.org/node/3201242 missed compared to 1.1.15

[bryanmanalo]

missing explicit accessCheck is not detected on 1.1.16 but this is detected on 1.1.15

$query = \Drupal::entityQuery('user')
      ->condition('status', 1)
      ->condition('uid', 1, '<>')
      ->condition('field_profile_visibility', 1)
      ->condition('field_account_type', '', '<>')
      ->condition('field_last_name.value', '', '<>');
  if( isset($qparam['expertise']) && !empty($qparam['expertise']) ) {
    $query->condition('field_field_expertise.entity.tid', $qparam['expertise']);
  }
  if( isset($qparam['origin']) && !empty($qparam['origin']) ) {
    $query->condition('field_place_origin.entity:taxonomy_term.field_country_tags', $qparam['origin']);
  }
  if( isset($qparam['place_residence']) && !empty($qparam['place_residence']) ) {
    $query->condition('field_place_residence.entity:taxonomy_term.field_country_tags', $qparam['place_residence']);
  }
  if( isset($qparam['city']) && !empty($qparam['city']) ) {
    $query->condition('field_city', $qparam['city'], 'CONTAINS');
  }
  if( isset($qparam['user_type']) && !empty($qparam['user_type']) ) {
    $query->condition('field_account_type.entity.tid', $qparam['user_type']);
  }
  $users = $query->execute();

[mglaman]

Thanks for providing an example! It was overaggressive before. This helps build the test cases with examples.

[Boegie]

An example of the reverse: (Source: https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/tests/Drupal/KernelTests/Core/Entity/FieldableEntityDefinitionUpdateTest.php#L296-L298)

    /** @var \Drupal\Core\Entity\TranslatableRevisionableStorageInterface|\Drupal\Core\Entity\EntityStorageInterface $storage */
    $storage = $this->entityTypeManager->getStorage($this->entityTypeId);
    $next_id = $storage->getQuery()->accessCheck(FALSE)->count()->execute() + 1;

1.1.16 reports

 ------ ------------------------------------------------
  Line   FieldableEntityDefinitionUpdateTest.php
 ------ ------------------------------------------------
  298    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
 ------ ------------------------------------------------

[Boegie]

Source https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php#L175-L183

    $query = $this->storage
      ->getQuery('OR')
      ->accessCheck(FALSE)
      ->exists($greetings, 'tr')
      ->condition("$figures.color", 'red')
      ->sort('id');
    $count_query = clone $query;
    $this->assertEquals(12, $count_query->count()->execute());

1.1.16 reports:

 ------ ------------------------------------------------
  Line   EntityQueryTest.php
 ------ ------------------------------------------------
  182    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242

[Boegie]

Source: https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php#L175-442

    $query = $this->storage
      ->getQuery()
      ->accessCheck(FALSE)
      ->sort("$figures.color")
      ->sort("$greetings.format")
      ->sort('id');
[snipped-for-sanity]
    $count_query = clone $query;
    $this->assertEquals(15, $count_query->count()->execute());

1.1.16 reports:

 ------ ------------------------------------------------
  Line   EntityQueryTest.php
 ------ ------------------------------------------------
  442    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242

[Boegie]

Source: https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php#L464-L471

    $query = $this->storage
      ->getQuery()
      ->accessCheck(FALSE)
      ->sort("$figures.color", 'DESC')
      ->sort("$greetings.format", 'DESC')
      ->sort('id', 'DESC');
    $count_query = clone $query;
    $this->assertEquals(15, $count_query->count()->execute());

1.1.16 reports:

 ------ ------------------------------------------------
  Line   EntityQueryTest.php
 ------ ------------------------------------------------
  471    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242

[Boegie]

Source: https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php#L560-L566

    $count = $this->container->get('entity_type.manager')
      ->getStorage('entity_test')
      ->getQuery()
      ->accessCheck(FALSE)
      ->exists("$field_name.color")
      ->count()
      ->execute();

1.1.16 reports:

 ------ ------------------------------------------------
  Line   EntityQueryTest.php
 ------ ------------------------------------------------
  560    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
 ------ ------------------------------------------------

[heddn]

I can mostly get it working, except for 1 situation where I clone the original query to build a count query.

    assert($query instanceof QueryInterface);
    $count_query = clone $query;

MutatingScope::getType() returns ObjectType in this case. I'm not sure how to trick phpstan into realizing I have accessCheck actually added.

The rest of my red flagged warnings I handled by adding \assert($storage instanceof ContentEntityStorageInterface). But in this case, I don't have anything to assert upon.

[heddn]

I got the last one working by:

assert($query instanceof QueryInterface);
$count_query = clone $query;
$count_query->count();
$total = $count_query->execute();

Splitting the calls up tricked MutatingScope::getType() into realizing the correct objects are returned and it passes green.

[mglaman]

oh boy, that's fun: so clone gets weird?

[heddn]

Another one that got a bit weird:

    $query = $this->userStorage->getQuery()
      ->accessCheck(FALSE)
      ->condition('status', 1);
    $query->condition('roles', ['silo_admin', 'developer'], 'IN');
    $ids = $query->execute();

Because the replacement for the constants did not map. Each constant was replaced by something else. But splitting up the query into multiple parts meant that $this->resolvedTypes[$key]; in MutatingScope keyed things correctly. Above works. Doing one long chained $this->storage->condition()->accessCheck()->execute() failed.

[mglaman]

Working on a PR to try and fix this: https://github.com/mglaman/phpstan-drupal/pull/454

[mglaman]

I am closing this as https://github.com/mglaman/phpstan-drupal/pull/454 was merged and there have been many more fixes around this area since then. If there is still a remaining case, please open a new issue.