Skip to content
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.

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

Narrow type with phpstan-assert even if we don't directly call the method for some Symfony class/interfaces. #325

Closed
VincentLanglet opened this issue Jan 18, 2023 · 6 comments

Comments

@VincentLanglet
Copy link
Contributor

I recently got an idea for the following issue #179

For context, Symfony expose code which can resume to this:

abstract class Voter
{
    public function vote(string $attribute, $subject, $token): bool {
			if ($this->supports($attribute, $subject) {
				return $this->voteOnAttribute($attribute, $subject, $token);
			}
    }

	abstract protected function supports(string $attribute, $subject): bool;
	abstract protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool;
}

So I'd like that, if I write

class MyVoter extends Voter
{
    /** 
	 * @phpstan-assert-if-true User $subject 
	 * @phpstan-assert-if-true 'a'|'b' $attribute 
	 */
    protected function supports(string $attribute, $subject): bool
    {
        return $subject instanceof User && in_array($attribute, ['a', 'b']);
    }

    protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
    {
		// I'd like phpstan to narrow $attribute, $subject like if I made the 
		// if ($this->supports($attribute, $subject) {
		// call by myself
    }
}

Do you think it's possible with some TypeSpecifier @ondrejmirtes ? I'm not familiar with this part of PHPStan.
If this logic is possible this could be later generalize to lot of other Symfony interfaces/classes.

@ondrejmirtes
Copy link
Member

  1. Why do you want to write a type-specifying extension if you're showing the example with @phpstan-assert-if-true that should also work?
  2. Sure, what's possible with @phpstan-assert* is also possible with type-specifying extension, and more, that's been true since the beginning :)

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jan 18, 2023

  1. Why do you want to write a type-specifying extension if you're showing the example with @phpstan-assert-if-true that should also work?

Because it doesn't work like this https://phpstan.org/r/fb87f7e8-5e3d-4026-98a6-338fe3162b67

@ondrejmirtes
Copy link
Member

That doesn't make sense to me - you have no guarantee someone will call supports before calling voteOnAttribute.

But if you want to enforce that, you can: https://phpstan.org/r/bc1e9de0-3dcb-466d-8b0d-e823a3c9f12f

@VincentLanglet
Copy link
Contributor Author

That doesn't make sense to me - you have no guarantee someone will call supports before calling voteOnAttribute.

That's why I opened the issue in the Symfony plugin, because this specific class have the following comment:
"It is safe to assume that $attribute and $subject already passed the "supports()" method check."

https://github.com/symfony/symfony/blob/1f7bc1098f0b03181095d35b11fa48523864e6f2/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php#L81-L85

Symfony provide a lot of classes with a supports method and I wanted to find a way to support this architecture.

But if you want to enforce that, you can: phpstan.org/r/bc1e9de0-3dcb-466d-8b0d-e823a3c9f12f

With this, you have a strict-rule error for contravariant issues https://phpstan.org/r/b72f4692-b3a2-4281-93de-a8f740e5ce75 I wanted to avoid.

@ondrejmirtes
Copy link
Member

There's no extension to change the current behaviour now.

With this, you have a strict-rule error for contravariant issues

Generics to the rescue: https://phpstan.org/r/51cce0fb-27ac-4fff-b9d4-214878850e54 (notice also there's no duplication of 'a'|'b' and User anymore)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants