Problem/Motivation

PHPStan baseline is currently skipping multiple Call to an undefined method errors, #3291519: Fix remaining method.notFound PHPStan L0 errors.

Proposed resolution

Fix the error about getExpectedUnauthorizedEntityAccessCacheability.

        'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\file\\\\Functional\\\\FileUploadJsonBasicAuthTest\\:\\:getExpectedUnauthorizedEntityAccessCacheability\\(\\)\\.$#',
        'count' => 1,
        'path' => __DIR__ . '/modules/file/tests/src/Functional/FileUploadJsonBasicAuthTest.php',
--
        'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\file\\\\Functional\\\\FileUploadJsonCookieTest\\:\\:getExpectedUnauthorizedEntityAccessCacheability\\(\\)\\.$#',
        'count' => 1,
        'path' => __DIR__ . '/modules/file/tests/src/Functional/FileUploadJsonCookieTest.php',

        'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\user\\\\Functional\\\\UserRegistrationRestTest\\:\\:getExpectedUnauthorizedEntityAccessCacheability\\(\\)\\.$#',
        'count' => 1,
        'path' => __DIR__ . '/modules/user/tests/src/Functional/UserRegistrationRestTest.php',

Issue fork drupal-3469843

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:

Comments

quietone created an issue. See original summary.

quietone’s picture

Title: Fix getExpectedUnauthorizedEntityAccessCacheability PHPStan-0 issues » Fix 2 getExpectedUnauthorizedEntityAccessCacheability PHPStan-0 issues
Issue summary: View changes
Status: Active » Needs review

Not sure the solution for the last one.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Seems straight forward and failure appears to be random block one.

mondrake’s picture

Component: database system » rest.module
catch’s picture

Status: Reviewed & tested by the community » Needs work

Two questions on the MR.

quietone’s picture

Status: Needs work » Needs review
quietone’s picture

Title: Fix 2 getExpectedUnauthorizedEntityAccessCacheability PHPStan-0 issues » Fix getExpectedUnauthorizedEntityAccessCacheability PHPStan-0 issues

If we can continue to use this solution then the remaining instance, in UserRegistrationRestTest, can be fixed.

quietone’s picture

Issue summary: View changes
smustgrave’s picture

Status: Needs review » Needs work

Actually since these aren't doing anything do we need the parameter to be based? Surprised now a phpstan rule that picks up unused parameter

quietone’s picture

Status: Needs work » Needs review

What is the work to do here? If there is work to be done from #10, can you be more specific? I do not understand the question in #10.

smustgrave’s picture

Status: Needs review » Needs work

I meant why add bool $is_authenticated?

NW for the rebase though.

rpayanm made their first commit to this issue’s fork.

quietone’s picture

Status: Needs work » Needs review

rebased and rebuilt the phpstan baseline.

smustgrave’s picture

Status: Needs review » Needs work

So seems now we are trading 1 phpstan error for another. Does it need to return something (even empty string)

quietone’s picture

Status: Needs work » Needs review

So, void no longer work here. How about adding a return of \Drupal\Core\Cache\CacheableMetadata which is what is expected by the parent method, \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::getExpectedUnauthorizedEntityAccessCacheability/

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Works for me!

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone’s picture

Status: Needs work » Needs review

Rebase with conflicts in the baseline, which I then rebuilt.

mstrelan’s picture

Status: Needs review » Needs work

I think this fix is just covering up a code smell. The method getExpectedUnauthorizedEntityAccessCacheability exists in \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase but not in \Drupal\Tests\rest\Functional\ResourceTestBase, yet the test trait that calls this method is added to classes that don't extend from EntityResourceTestBase. If we're saying this method needs to exist in all child classes then we might as well just add it to the base, but that makes it clear that it doesn't belong there. Ideally it should be refactored out of the shared trait, or failing that we should check if $this instanceof EntityResourceTestBase before calling that function.

longwave made their first commit to this issue’s fork.

longwave’s picture

Status: Needs work » Needs review

Agree with #20, either the trait should declare the method itself or check that it exists - the latter seems cleaner, so let's do that.

dcam’s picture

Status: Needs review » Reviewed & tested by the community

Tests are passing. The change was OKed and written by a framework manager. There are no more "undefined method" entries in the baseline for getExpectedUnauthorizedEntityAccessCacheability(). So I think we're good here.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

dcam changed the visibility of the branch 3469843-fix-getexpectedunauthorizedentityaccesscacheability-phpstan-0 to hidden.

dcam’s picture

Status: Needs work » Reviewed & tested by the community

The bot was complaining about the old MR.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

dcam’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: +no-needs-review-bot

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 06cdb3bb on 11.x
    task: #3469843 Fix getExpectedUnauthorizedEntityAccessCacheability...

  • catch committed 95aaf831 on main
    task: #3469843 Fix getExpectedUnauthorizedEntityAccessCacheability...

Status: Fixed » Closed (fixed)

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