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

Remove incorrect doc leftover from 1.x #3732

Merged
merged 1 commit into from
Dec 15, 2024
Merged

Conversation

AJenbo
Copy link
Contributor

@AJenbo AJenbo commented Dec 15, 2024

This appears to have come from a now removed function, or at least it doesn't really make sens to say to use accepts() instead of accepts() :)

@ondrejmirtes
Copy link
Member

Yeah, sure, thanks :)

@ondrejmirtes ondrejmirtes merged commit 5d07949 into phpstan:2.0.x Dec 15, 2024
426 of 427 checks passed
@AJenbo AJenbo deleted the patch-1 branch December 15, 2024 14:53
@AJenbo
Copy link
Contributor Author

AJenbo commented Dec 15, 2024

On a side note I'm trying to update Bladestan to PHPStan 2.0, but I can't figure out how to not make use of instanceof ObjectType,

if ($objectType instanceof ObjectType) {
    if ($objectType->isInstanceOf(Component::class)->yes()) return true;
    if ($objectType->isInstanceOf(Mailable::class)->yes()) return true;
    if ($objectType->isInstanceOf(MailMessage::class)->yes()) return true;
}

If I use isSuperTypeOf() it returns maybe that that just isn't as confidence raising.

If I instead create a new instance of ObjectType using the class name from $objectType calling isInstanceOf then also goes from yes to maybe :(

P.s.s. PHPStan 2.0 has solved the performance issues we where having with Bladestan, so I'm very excited to get this out for everyone to use :)

@ondrejmirtes
Copy link
Member

The proper way of asking about this is:

$component = new ObjectType(Component::class);
if ($component->isSuperTypeOf($objectType)->yes()) return true;

More about that here: https://phpstan.org/developing-extensions/type-system#querying-a-specific-type

@AJenbo
Copy link
Contributor Author

AJenbo commented Dec 15, 2024

@ondrejmirtes
Copy link
Member

There's a big difference between:

$component->isSuperTypeOf($objectType)

And

$objectType->isSuperTypeOf($component)

If you do the first approach and the class in $objectType extends/implements Component then the answer is yes() being true.

@herndlm
Copy link
Contributor

herndlm commented Dec 15, 2024

You might need to create a union of those 3 classes and do the super type check with that I assume.

Because, otherwise, PHPStan is correct, if you're only asking for Component, it could still be something else, which is the reason you end up with maybe.

@ondrejmirtes
Copy link
Member

If you're not sure we can schedule a call and figure it out together.

@AJenbo
Copy link
Contributor Author

AJenbo commented Dec 15, 2024

There's a big difference between...

I did try $component->isSuperTypeOf($objectType) as well as $objectType->isSuperTypeOf($component), neither worked and I am completely out of ideas.

You might need to create a union of those 3 classes and do the super type check with that I assume.

I don't think that's the issue here, but that could be an idea for simplifying the code.

If you're not sure we can schedule a call and figure it out together.

I would appreciate that a lot :)

In case someone want's to give it a go the offending code is covered by a unit test so you can test it by simply running phpunit for the project. PHPStan 2.0 support is found in the phpstan2 branch, it's fully working ... PHPStan is just less then happy with how things are implemented in a few places.

@ondrejmirtes
Copy link
Member

I just checked out the project. Some tests are failing for me locally, all of it is about assert($ruleError instanceof IdentifierRuleError).

You're doing:

            $ruleError = RuleErrorBuilder::message($error->getMessage())
                ->file($phpFilePath)
                ->line($phpFileLine)
                ->metadata([
                    'template_file_path' => array_key_first($fileNameAndTemplateLine),
                    'template_line' => current($fileNameAndTemplateLine),
                ])
                ->build();
            assert($ruleError instanceof IdentifierRuleError);

But that doesn't make sense. You're supposed to call ->identiifer() method on RuleErrorBuilder and set an identifier for the reported error.

Here's more about that https://phpstan.org/blog/using-rule-error-builder

@ondrejmirtes
Copy link
Member

You can get that from $error->getIdentifier().

@ondrejmirtes
Copy link
Member

I'm working on the repo right now and I will send a PR once the code looks like it should and tests pass.

@ondrejmirtes
Copy link
Member

Here you go: bladestan/bladestan#120

The main reason why some methods were returning maybe instead of yes was that those classes were unknown to PHPStan: bladestan/bladestan@975b9b5

@AJenbo
Copy link
Contributor Author

AJenbo commented Dec 16, 2024

Arh that makes a lot of sens I was feeling like it somehow just didn't recognize the classes when trying to rebuild the ObjectsType but only when it directly analyzed the code. Thank you so much for helping with this, I feel a lot better about releasing the next version now :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants