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

Phan doesn't narrow property types after explicit tests #805

Closed
jdwx opened this issue Jun 1, 2017 · 3 comments
Closed

Phan doesn't narrow property types after explicit tests #805

jdwx opened this issue Jun 1, 2017 · 3 comments
Labels
enhancement This improves the quality of Phan's analysis of a codebase

Comments

@jdwx
Copy link

jdwx commented Jun 1, 2017

Given the following example:

<?php

function DoubleInt( int $i ) : int {
	return $i * 2;
}

function DoubleIntOrNull( ?int $i ) : ?int {
	if ( is_int( $i ) )
		return DoubleInt( $i );
	return null;
}

class Example {

	/** @var ?int */
	protected $i;

	function __construct( ?int $i ) {
		$this->i = $i;
	}

	function doubleMe() : ?int {
		if ( is_int( $this->i ) )
			return DoubleInt( $this->i );
		return null;
	}

}

$ex = new Example( 5 );
echo $ex->doubleMe(), "\n";

Phan will produce a warning for the Example::doubleMe() method, but not for the DoubleIntOrNull() function:

$ phan types.php
types.php:24 PhanTypeMismatchArgument Argument 1 (i) is ?int but \DoubleInt() takes int defined at types.php:3

It looks like Phan doesn't allow the type union of $this->i to be updated within the scope of the if statement the way it does for local variables and parameters. (Or after it, when an else clause appears or simplify_ast is used.)

Workarounds in the code are to copy the property to a local variable, or to explicitly cast it at the point of use when its type has been checked.

Thanks!

@TysonAndre
Copy link
Member

This issue is a consequence of pre-existing issues: #204 and #538

@TysonAndre TysonAndre added the enhancement This improves the quality of Phan's analysis of a codebase label Jun 4, 2017
@jdwx
Copy link
Author

jdwx commented Jul 17, 2017

Updating this with a shorter self-contained example that has come up today:

<?php

class Foo {
	/** @var ?string $bar */
	public $bar;
}

$foo = new Foo;
if ( is_string( $foo->bar ) )
	echo "strlen = ", strlen( $foo->bar ), "\n";

@TysonAndre
Copy link
Member

Early (broken) work is included in TysonAndre#113 . This needs to be rebased, and tests are still failing.

Additionally, inferences in the global scope may require more work.

wmfgerrit pushed a commit to wikimedia/mediawiki-services-parsoid that referenced this issue Apr 1, 2019
Phan doesn't propagate the result of the test back to the value of
the property, which is phan/phan#805.
Workarounds suggested include adding a null-coalescing ?? operator
or copying the property to a local variable, but I've chosen just
to suppress the warning instead.

Change-Id: I8c0e413d34ce6b34681657720995fadb25e79807
TysonAndre added a commit to TysonAndre/phan that referenced this issue Apr 22, 2019
This supports only one level of nesting of property/field accesses.

This deliberately supports only `$this`, because `$this` cannot be
reassigned.

A future release may make method calls of $this invalidate the inferred
property types.

This does not track fields of $this.

Fixes phan#805
Fixes #204
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This improves the quality of Phan's analysis of a codebase
Projects
None yet
Development

No branches or pull requests

2 participants