Use ConstantScalarTypeTrait in NullType

[VincentLanglet]

Hi, I wanted to finish (#3484) and therefor following your advice on #3485 (comment).

The best option I see would be to introduce

public function compareToScalar(
    ConstantScalarType,
    Phpversion,
    callable
)

in the ConstantScalarTypeTrait ; but the NullType would require the exact same logic.

So it would be helpful to use the ConstantScalarTypeTrait in the NullType.
Also, there is multiple method duplicated between both trait/class.

The only issue I found is that ConstantScalarTypeTrait sometimes rely on parent method. This can be avoided with

get_parent_class($this) !== false

checks ; but PHPStan doesn't understand such things.

Should I just ignore the static analysis error or you don't want such PR ?
Also, I didn't know if I should have targeted the 2.0.x branch for such refacto to avoid conflict on merge ; or if it's ok on 1.12.x.

[staabm]

but PHPStan doesn't understand such things.

Maybe method_exists() works?
Or is_callable()

[VincentLanglet]

but PHPStan doesn't understand such things.

Maybe method_exists() works? Or is_callable()

The error comes from

phpstan-src/src/Rules/Methods/StaticMethodCallCheck.php

Lines 82 to 116 in 30d20e6

	} elseif ($lowercasedClassName === 'parent') {
	if (!$scope->isInClass()) {
	return [
	[
	RuleErrorBuilder::message(sprintf(
	'Calling %s::%s() outside of class scope.',
	$className,
	$methodName,
	))->identifier(sprintf('outOfClass.parent'))->build(),
	],
	null,
	];
	}
	$currentClassReflection = $scope->getClassReflection();
	if ($currentClassReflection->getParentClass() === null) {
	return [
	[
	RuleErrorBuilder::message(sprintf(
	'%s::%s() calls parent::%s() but %s does not extend any class.',
	$scope->getClassReflection()->getDisplayName(),
	$scope->getFunctionName(),
	$methodName,
	$scope->getClassReflection()->getDisplayName(),
	))->identifier('class.noParent')->build(),
	],
	null,
	];
	}
	if ($scope->getFunctionName() === null) {
	throw new ShouldNotHappenException();
	}
	$classType = $scope->resolveTypeByName($class);
	} else {

so I get an error because

$currentClassReflection->getParentClass() === null

in the context of NullType. PHPStan doesn't understand get_parent_class($this) !== false is a safeGuard.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

The method shouldn't just accept ConstantScalarType. It should always be asking a Type about any other Type.

I don't love traits and I'd like to see them be used less, not more.

[VincentLanglet]

The method shouldn't just accept ConstantScalarType. It should always be asking a Type about any other Type.

I really don't understand what you're asking.

Those methods already exists, they are

• looseCompare
• isSmallerThan
• isSmallerOrEqualThan

But there is a special logic needed for ConstantScalar which is that for constantScalar we need to know if we check one of the many following condition based on PHP version and type of constants.

(string) $a === $b;
$a === (int) $b;
(int) $a === $b;
...

This is a special logic needed only for ConstantScalarType when compared to another ConstantScalarType ; so I don't see how and why I would work with a new method to Type about any other Type. This is similar to the existing

if ($type instanceof ConstantScalarType) {
     return LooseComparisonHelper::compareConstantScalars($this, $type, $phpVersion);
}

I provided a refacto in #3485 which seems the simpler way (in my eyes) to have a reusable method in order to

• cast A
• cast B
• then compare A == B or A <= B or A < B.

Some guidelines would be helpful 🙏