Skip to content

RenderCallbackRule ignores ::trustedCallbacks() #561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Boegie opened this issue May 12, 2023 · 5 comments · May be fixed by #562
Open

RenderCallbackRule ignores ::trustedCallbacks() #561

Boegie opened this issue May 12, 2023 · 5 comments · May be fixed by #562
Labels
bug Something isn't working enhancement New feature or request

Comments

@Boegie
Copy link
Contributor

Boegie commented May 12, 2023 •
edited
Loading

Bug report

(Now that all Entity Query problems seem to be solved, I looked a bit closer at the Trusted Callbacks, so expect some more issues on that)

Currently \mglaman\PHPStanDrupal\Rules\Drupal\RenderCallbackRule only checks if the class implements TrustedCallbackInterface.
However the callbacks should also be in the return array from trustedCallbacks() to be trusted.

Now I highly doubt that people "in the wild" go through the trouble of creating callbacks and making sure the containing class implements TrustedCallbackInterface and finally don't add the callback to trustedCallbacks(), so the impact seems low.

However what we're currently doing seems incorrect.

Code snippet that reproduces the problem

See testcases in (extended) tests/src/Rules/data/bug-543.php
@Boegie Boegie added the bug Something isn't working label May 12, 2023
@Boegie Boegie linked a pull request May 12, 2023 that will close this issue
Open
@mglaman
Copy link
Owner

mglaman commented May 12, 2023

Yep, this is a big gap. I thought I had documented it in the code with a @todo but I did not!

@Boegie
Copy link
Contributor Author

Boegie commented May 12, 2023

// @todo: Write @todo...

@mglaman
Copy link
Owner

mglaman commented May 17, 2023

Problem: we would need to call the method to get its values. I think. Even though the method is static, I don't believe it's return values can be parsed that way.

@mglaman mglaman added the enhancement New feature or request label May 17, 2023
@mglaman
Copy link
Owner

mglaman commented May 17, 2023

Ideas:

                preg_match('#^([a-zA-Z_\\x7f-\\xff\\\\][a-zA-Z0-9_\\x7f-\\xff\\\\]*)::([a-zA-Z_\\x7f-\\xff][a-zA-Z0-9_\\x7f-\\xff]*)\\z#', $constantStringType->getValue(), $matches);
$foo = (new ObjectType($matches[1]))->getClassReflection()
                        ->getNativeReflection()->getMethod($matches[2])

We can then do \ReflectionMethod::invoke with null for first param.

So:

$foo = (new ObjectType($matches[1]))->getClassReflection()
    ->getNativeReflection()
    ->getMethod($matches[2])
    ->invoke(null)

$foo should have the method names, then.

@mglaman
Copy link
Owner

mglaman commented Jun 28, 2023

The PR is almost ready, but it crashes on Drupal core due to the way a class fixture is defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

RenderCallbackRule: Check if method exists in trustedCallbacks Boegie/phpstan-drupal
2 participants
@mglaman @Boegie