Skip to content

Improve support switch on ::class #4011

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

Open
wants to merge 1 commit into
base: 2.1.x
Choose a base branch
from

Conversation

VincentLanglet
Copy link
Contributor

@@ -1656,13 +1656,11 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
if (
$leftType instanceof ConstantScalarType
&& !$rightExpr instanceof ConstFetch
&& !$rightExpr instanceof ClassConstFetch
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The findTypeExpressionFromBinaryOperation is used two times:

  • Inside resolveIdentical
  • Inside resolveEqual

Inside resolveIdentical the ClassConstFetch scenario is not needed. But removing the check does not have impact since then the following code

$expressions = $this->findTypeExpressionsFromBinaryOperation($scope, $expr);
		if ($expressions !== null) {
			$exprNode = $expressions[0];
			$constantType = $expressions[1];

			$unwrappedExprNode = $exprNode;
			if ($exprNode instanceof AlwaysRememberedExpr) {
				$unwrappedExprNode = $exprNode->getExpr();
			}

			$specifiedType = $this->specifyTypesForConstantBinaryExpression($unwrappedExprNode, $constantType, $context, $scope, $expr);

gives a NULL $specifiedType.

Inside resolveEqual the ClassConstFetch is needed for an easy check

$context->true()
				&& $exprNode instanceof ClassConstFetch
				&& $exprNode->name instanceof Node\Identifier
				&& strtolower($exprNode->name->toString()) === 'class'
				&& $constantType->isString()->yes()

without checking both the right side and the left side.

Not sure if you're ok with such change.
I could had a boolean parameter to remove or not the ClassConstFetch check but I'm not sure it's worth it.

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.

Accessing ::class or calling get_class() on nullable types does not lead to a warning
1 participant