scrutinizer-ci / scrutinizer

Legacy repository - archives past feature requests/bug reports
https://scrutinizer-ci.com/docs
140 stars 36 forks source link

Improve type checker to understand returning different types based on number of parameters #139

Open denis-sokolov opened 10 years ago

denis-sokolov commented 10 years ago

Id call_checks.maybe_mismatching_type_passed works like a good type checker, which is great. It catches potential problems.

Of course, when a function can return different types depending on the inputs, the type checker will inevitably be confused. However, generally it's not a good practice to return different types.

But here's one such pattern that's not generally considered bad practice, is popular and convenient. It's the chaining getter-setter:

/**
 * @param type|null $newValue
 * @return type|$this
 */
public function foo($newValue = null) {
    if (func_num_args() === 0) {
        return $this->foo;
    }
    $this->foo = $newValue;
    return $this;
}

If ->foo() is now used in a context requiring non-null type, the type checker flags it as a warning. But given that func_num_args can be resolved statically, perhaps this could be improved to not flag?

See the example from our codebase (Id: call_checks.maybe_mismatching_type_passed).

schmittjoh commented 10 years ago

Thanks for opening this issue.

As I have not had such code myself, could you show some examples how this method is used? I assume, it's a setter/getter in a single method?

$x->foo(); // getter
$x->foo($v); // setter
denis-sokolov commented 10 years ago

Yes, that's exactly how it is used.