Deprecate TypeUtils::getDirectClassNames()

[herndlm]

Just a simple migration over to Type::, as requested in #1922 (comment)

Are you fine with that name? E.g. we also have Type::getReferencedClasses() which is somehow related, right?

I'd try to get rid of more instanceof TypeWithClassName in a follow-up if that's fine

[herndlm]

Looks likes a deprecated method rule in the deprecation rules extension is causing a deprecation now that needs fixing then 🙃

[herndlm]

The new failing tests here are from the deprecation extension which needs fixing afterwards. But it would be ready

[ondrejmirtes]

Yeah, the name is OK :) Additionally, we could get rid of instanceof TypeWithClassName in this PR. Thank you.

[herndlm]

getting rid of all instanceof TypeWithClassName here can get ugly really fast. currently there are 40 of those and with many I'm not sure if we should really use the new method, introduce something else, or just leave it as it is.

e.g. some examples

calls where we explicitly want to know the class for whatever reasons

$classType = TypeTraverser::map($classType, static function (Type $type, callable $traverse) use (&$uncertainty): Type {
	if ($type instanceof UnionType || $type instanceof IntersectionType) {
		return $traverse($type);
	}
	if ($type instanceof TypeWithClassName) {
		$uncertainty = true;
		return $type;
	}
...

if (!$returnType instanceof TypeWithClassName) {
	return new MixedType();
}

calls where we want to know explicitly the one className

$thisType instanceof TypeWithClassName ? $thisType->getClassName() : null,

$isThrowable = $originalCatchType instanceof TypeWithClassName && strtolower($originalCatchType->getClassName()) === 'throwable';

 elseif (
	$argValueType instanceof GenericClassStringType
	&& $argValueType->getGenericType() instanceof TypeWithClassName
) {
	$scopeClass = $argValueType->getGenericType()->getClassName();
	$thisType = $argValueType->getGenericType();
}

I'm not entirely sure how to get rid of them yet, e.g. in some cases I'd need to check that getObjectClassNames() is not empty, in some I'd need to check that one entry is returned, in some I can maybe traverse them and adapt the surrounding logic (but many places seem to be using GenericTypeVariableResolver::getType() which expects a TypeWithClassName) and in some maybe I want to check that only one entry is returned and use that. any ideas? or should we do this in more PRs maybe? I want to avoiding messing this one up too much :)

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

It shouldn't be hard to come up with examples where those instanceof TypeWithClassName are wrong - for example having an intersection with another type (but the condition where we're asking we'd still want it to be true).

Also - UnionType::getObjectClassNames() doesn't match how it should behave. If we for example have Foo|null, the returned array should be empty. Look at getArrays() for an example how to do it.

[ondrejmirtes]

You can now just call pickTypes in UnionType :) 480626e

[herndlm]

You can now just call pickTypes in UnionType :) 480626e

that's great! I just realised that you type-hinted T of Type but here T would be string :/
can I just remove it or should I make it a T of Type|string or do you have some other idea?

[ondrejmirtes]

@herndlm Nothing might break if we remove the of Type part?

[herndlm]

@herndlm Nothing might break if we remove the of Type part?

no, but it starts to get weird naming-wise. pickTypes(), $getTypes, $innerTypes and $types are all kind of wrong then

[ondrejmirtes]

Yeah, you can rename that to something else :) Also maybe pickFromTypes instead of pickTypes.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Thank you!

[staabm]

Great job @herndlm 🙏

[ondrejmirtes]

An idea - Type::getStringClassNames() for these cases:

• ConstantStringType that's also a class string
• GenericClassStringType - a shortcut to $type->getGenericType()->getObjectClassNames()

There are some cases of instanceof GenericClassStringType that we'd be able to get rid of.