add Type::isError method by canvural · Pull Request #4530 · phpstan/phpstan-src

canvural · 2025-11-06T15:51:48Z

This one does not add too much value, but I just didn't want to do instanceof ErrorType in my extensions 😄

ondrejmirtes

ApiInstanceofTypeRule should be made aware of this 😊

ondrejmirtes · 2025-11-07T12:41:23Z

Do you also feel like doing isNever() + isExplicitNever()? :)

canvural · 2025-11-07T13:47:09Z

Do you also feel like doing isNever() + isExplicitNever()? :)

Didn't ever need that I guess, but sure I can. In a separate PR?

canvural · 2025-11-10T10:12:31Z

The mutations look valid, but I don't know how I can change them in bulk.

Edit: I guess $type instanceof ErrorType could be !$type->isError()->no() and !$type instanceof ErrorType can be $type->isError()->no() ?

ondrejmirtes · 2025-11-11T07:58:54Z

I'm not sure how to cover the mutations with tests here. i'm more concerned about the things the issue bot found https://github.com/phpstan/phpstan-src/actions/runs/19215786400, I'm not sure why that's happening.

staabm · 2025-11-11T08:50:47Z

src/Type/Generic/TemplateTypeTrait.php

 			if ($this->default !== null) {
 				$recursionGuard = RecursionGuard::runOnObjectIdentity($this->default, fn () => $this->default->describe($level));
-				if (!$recursionGuard instanceof ErrorType) {
+				if (is_string($recursionGuard)) {


this looks fishy

Yeah, definitely weird

Why? It can only be string or ErrorType here. So I thought is_string is ok instead of instanceof

staabm · 2025-11-11T08:52:51Z

src/Type/ObjectType.php


 		$classHasProperty = RecursionGuard::run($this, static fn (): bool => $classReflection->hasProperty($propertyName));
-		if ($classHasProperty === true || $classHasProperty instanceof ErrorType) {
+		if ($classHasProperty !== false) {


are we sure this does not change existing logic?

Yeah, definitely weird

Yeah, doesn't change it. It can only be bool or ErrorType.

staabm · 2025-11-11T08:58:49Z

just a gut feeling in case the 2 above comments regarding behaviour changes are not valid:

another source of problems could be

class ErrorType extends MixedType
class TemplateMixedType extends MixedType

because MixedType - because of substraction logic - might lead to a Maybe.

canvural · 2025-11-11T13:04:42Z

src/Rules/PhpDoc/UnresolvableTypeHelper.php

 		$containsUnresolvable = false;
 		TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$containsUnresolvable): Type {
-			if ($type instanceof ErrorType) {
+			if ($type->isError()->yes()) {


@staabm @ondrejmirtes

I debugged a bit and I think the issue bot errors might be valid. It's always an issue with ValueOfType and it comes down to this line. When we do $type->isError() here ValueOfType resolves the underlying value and it resolves to ErrorType. We do not get the issue with instanceof check because that's always ValueOfType vs ErrorType

Does it make sense?

#4548 Might help on this.

VincentLanglet

In #4547

$expressionType instanceof NeverType

by

!$expressionType->isNever()->no()

But here you're replacing

$type instanceof ErrorType

by

$type->isError()->yes()

How did you choose ? I feel like we should have consistency...

VincentLanglet · 2025-11-11T13:49:06Z

src/Type/MixedType.php

 		return TrinaryLogic::createMaybe();
 	}

+	public function isError(): TrinaryLogic


Isn't it just simpler to always return no ?

Cause currently MixedType instanceof ErrorType is just false.

And anyway, I don't think there is a isError()->maybe() used somewhere...

ErrorType extends MixedType for simplicity, but a "real" MixedType is never an error.
And Mixed~Error is not a Type I often saw...

Yeah makes sense. Did it with 6370bbc Thank you

VincentLanglet · 2025-11-11T13:50:06Z

src/Type/ObjectType.php


 		$classHasProperty = RecursionGuard::run($this, static fn (): bool => $classReflection->hasProperty($propertyName));
-		if ($classHasProperty === true || $classHasProperty instanceof ErrorType) {
+		if ($classHasProperty !== false) {


Might be easier to understand

if ($classHasProperty === true || $classHasProperty->isError()->yes())

VincentLanglet · 2025-11-11T14:51:13Z

src/Rules/PhpDoc/UnresolvableTypeHelper.php

 		$containsUnresolvable = false;
 		TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$containsUnresolvable): Type {
-			if ($type instanceof ErrorType) {
+			if ($type->isError()->yes()) {


#4548 Might help on this.

canvural · 2025-11-12T23:39:07Z

In #4547
$expressionType instanceof NeverType
by
!$expressionType->isNever()->no()
But here you're replacing
$type instanceof ErrorType
by
$type->isError()->yes()
How did you choose ? I feel like we should have consistency...

Yeah we should have consistency. In the isNever PR I did it that way to see how the mutation testing will behave. But honestly I'd rather have $type->isError()->yes() everywhere, it's much easier to understand. So I'll change the other PR later.

ondrejmirtes requested changes Nov 6, 2025

View reviewed changes

canvural force-pushed the is-error branch 2 times, most recently from b940dc5 to e8922d3 Compare November 6, 2025 16:23

ondrejmirtes force-pushed the is-error branch from e8922d3 to 2cd5ddf Compare November 7, 2025 12:40

canvural force-pushed the is-error branch from 2cd5ddf to 22f2043 Compare November 9, 2025 23:04

staabm reviewed Nov 11, 2025

View reviewed changes

canvural commented Nov 11, 2025

View reviewed changes

canvural mentioned this pull request Nov 11, 2025

add Type::isNever and Type::isExplicitNever methods #4547

Open

VincentLanglet reviewed Nov 11, 2025

View reviewed changes

canvural force-pushed the is-error branch from 22f2043 to 6370bbc Compare November 12, 2025 23:36

canvural force-pushed the is-error branch from 6370bbc to 08307bd Compare December 6, 2025 14:04

canvural added 6 commits December 8, 2025 19:00

add Type::isError method

123a656

replace instanceof ErrorType usages

3e07929

update ApiInstanceofTypeRule

9c147f3

fix sa errors

306ea00

simplify MixedType::isError

4d30cad

refactor new usages

f46acaf

canvural force-pushed the is-error branch from 08307bd to f46acaf Compare December 8, 2025 18:14

Conversation

canvural commented Nov 6, 2025

Uh oh!

ondrejmirtes left a comment

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes commented Nov 7, 2025

Uh oh!

canvural commented Nov 7, 2025

Uh oh!

canvural commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ondrejmirtes commented Nov 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staabm commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

canvural Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

canvural commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

canvural commented Nov 10, 2025 •

edited

Loading

staabm commented Nov 11, 2025 •

edited

Loading

canvural Nov 11, 2025 •

edited

Loading