Return of void method reported as used, when it is not

[gmazzap]

Summary of problem

The rule: "Result of method {xxx} (void) is used" should IMO check that the result of a void method is actually used, i.e. set to a variable or passed to a function/method...

However it does not seems to work like that.

Code snippet that reproduces the problem

class HelloWorld {

	public function test(): void {
		return;
	}
	
	public function testVoidResult(): void {
                // this does not use the return value of `test()`, still it is reported
		true or $this->test();
	}
}

https://phpstan.org/r/dea43fe8-b309-491b-bdde-b06bfcbaed88

Expected output

I would expect the rule to report an error only if the void return is actually used.

[iluuu1994]

true or $this->test(); is indeed an expression, you're just throwing away the result. Nonetheless, your code doesn't contain a bug and so PHPStan shouldn't complain. But from a readability standpoint you should probably just use an if statement.

[gmazzap]

true or $this->test(); is indeed an expression, you're just throwing away the result.

Yes it is, but it is not relevant IMO. The rule is about "usage of void return" not about "found expressions that throw away results".

But from a readability standpoint you should probably just use an if statement.

I disagree. First of all, readability is a subjective thing, so what's readable for you it is not necesarily for me. And for me, a thing like:

if (!$thing->isInitialized()) {
    $thing->initialize();
}

is far less readable then:

$thing->isInitialized() or $thing->initialize();

But that's totally unrelevant IMO. I don't think PHPStan is about finding unreadable things, I think it is about finding (possible) bugs, and the code above has no sign of being buggy.

I am not familiar with the internals of PHPStan and I'm not sure how hard it is to implement this, but IMO the error should be raised only if the return value of a void function is either assigned (to a variable, a constant, or a class property), passed to a function, or returned from a function. In any other case, there should be no error.

[iluuu1994]

is far less readable then:

Shortness does not automatically imply readability. You're using or as a side effect. if statements is how we conditionally run code in pretty much any language under the sun.

As for the rest, I never disagreed with you:

Nonetheless, your code doesn't contain a bug and so PHPStan shouldn't complain.

[RobDWaller]

Just to contribute, I believe I have roughly the same issue, unless I'm missing something obvious.

I'm getting the same error on this code...

private function error(string $message): void
{
    throw new ValidateException($message);
}

My travis output may also be of use:
https://travis-ci.org/RobDWaller/ReallySimpleJWT/jobs/470397891

[ondrejmirtes]

@RobDWaller That looks unrelated. Please open a separate issue with reproduction on phpstan.org.

[RobDWaller]

@ondrejmirtes you're right, I spoke too soon. Apologies for taking up your time. Keep up the great work. 👍

[Majkl578]

IMO true or $this->test() is an expression that uses $this->test() in read context (and whether you use result of this expression or not is not relevant), and reading void is a bug in your code.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.