noverify icon indicating copy to clipboard operation
noverify copied to clipboard

Report is_T($x) where $x is always/never is T

Open quasilyte opened this issue 5 years ago • 2 comments

Code Example

$x = new Foo();
if (is_int($x)) { // <----
}
$y = 10;
if (is_int($y)) { // <----
}

Actual Behavior

NoVerify gives no warnings right now.

Expected Behavior

Warning about is_int($x) being always false. Warning about is_int($y) being always true.

See #356.

quasilyte avatar Feb 26 '20 10:02 quasilyte

Hello, I tried to develop kind of a solution for this issue, but was challenged by the type inferring for function arguments. Seems kind of strange that default values(like null) are assumed as a type of variable so as phpdoc declarations of parameters' types. E.g. following code:

function f($x=null) {
....
}

When analysing, $x has only null type. Also:

 /**
     * Mount filesystems.
     *
     * @param string              $prefix
     * @param FilesystemInterface $filesystem
     *
     * @throws InvalidArgumentException
     *
     * @return $this
*/
public function mountFilesystem($prefix, FilesystemInterface $filesystem) {
        if ( ! is_string($prefix)) {
            throw new InvalidArgumentException(__METHOD__ . ' expects argument #1 to be a string.');
        }
        ....
}

Here $prefix always has string type, even though code has checks for it. Of course typeMap is marked as imprecise, but is there any interest to analyse only precised types? How it is considered to deal with such situation? Personally, I believe that in first case it's better to mark parameter with mixed type, however, second case is harder, string type is needed for other checks and I don't have any ideas.

hazzus avatar Jun 15 '20 20:06 hazzus

I think the state of our types inference and type modeling system is not enough right now to implement this diagnostic reliably. For example, we can't express not<T>.

The second case would require smart-cast like support in linter. We only handle instanceof cases at this moment, but we can infer more types in the future.

is there any interest to analyse only precised types?

I would assume that this is the most reliable way to do it right now. False positives are worse than false negatives.

The best way to figure out whether this limited implementation is worth it is to run a new diagnostic over a big PHP corpus and see if it can find anything.

quasilyte avatar Jun 15 '20 21:06 quasilyte