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

False positive PhanTypeMismatchArgumentNullable on recursive call with bool argument #4550

Closed
umherirrender opened this issue Sep 25, 2021 · 1 comment · Fixed by #4564
Closed

Comments

@umherirrender
Copy link

Getting a false positive PhanTypeMismatchArgumentNulllable on a scalar type, which is never null in a recursive call.
Not sure, but it looks a bit like #4412, but this is not about PassByRef.

The example code is:

<?php

class TestClass {
	public function getNewId() {
		$this->generateNewId( new \stdClass );
	}

	/**
	 * @param stdClass $database
	 * @param bool $retry
	 */
	public function generateNewId( $database, $retry = true ) {
		if ( $retry ) {
			$id = $this->generateNewId( $database, false );
		}

		if ( mt_rand( 0, 1 ) ) {
			$id = $this->generateNewId( $database, $retry );  # PhanTypeMismatchArgumentNullable Argument 2 ($retry) is $retry of type ?''|?'0'|?0|?0.0|?array{}|?bool but \TestClass::generateNewId() takes bool|true (expected type to be non-nullable)
		}
	}

}

Using phan 5.2.1 with php8.
Only visible with read_type_annotations = true
If the first argument is removed, the issue is gone.
If the call in getNewId is removed, the issue is gone.
If the $retry gets a real type hint of bool, the issue is gone.
If the if with $retry is removed/replaced, the issue is gone.

@TysonAndre
Copy link
Member

Only visible with read_type_annotations = true
If the first argument is removed, the issue is gone.
If the call in getNewId is removed, the issue is gone.
If the $retry gets a real type hint of bool, the issue is gone.
If the if with $retry is removed/replaced, the issue is gone.

This is known - Phan recursively analyzes calls. Among other things, it treats the parameter default differently, e.g. to catch passing $retry to a function expecting string|false as being definitely invalid.

It has a phpdoc type of true and a real type of empty, so when it infers an else branch exists, it infers all possible types for the negation, and the condition (and following statements) gets combined with the negation.

Probably, that specifically could be changed for boolean assertions when there were no real types, but for conditions such as if ($var !== SOME_POSSIBLE_LITERAL) { } and if (!$var instanceof SomeClass) we'd want to infer SOME_POSSIBLE_LITERAL in the negation if there was no real type.

@TysonAndre TysonAndre changed the title False positive PhanTypeMismatchArgumentNulllable on recursive call with bool argument False positive PhanTypeMismatchArgumentNullable on recursive call with bool argument Oct 3, 2021
TysonAndre added a commit to TysonAndre/phan that referenced this issue Oct 3, 2021
TysonAndre added a commit to TysonAndre/phan that referenced this issue Oct 3, 2021
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 a pull request may close this issue.

2 participants