Improve void or never inference - #458
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors and expands the logic that infers when WordPress stubs should receive an automatically-added @phpstan-return void or @phpstan-return never, aiming to improve PHPStan’s detection of invalid usage of return values from void/never functions.
Changes:
- Introduces a new
VoidOrNeverAnalyzerhelper to encapsulate void/never detection. - Updates
Visitorto delegate inference to the new analyzer and to avoid adding inferred@phpstan-returntags when one is already present from the function map. - Expands “never” inference with top-level
throwdetection and refines safeguards around when to skip adding tags.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/VoidOrNeverAnalyzer.php | New analyzer class for inferring and attaching void/never attributes used by the visitor. |
| src/Visitor.php | Integrates the analyzer and adjusts how inferred @phpstan-return tags are appended. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Actually I just want a simple package with minimal/no code. |
Before I address the throw handling issue, please let me know if you will consider merging this or not. |
Yes, I would. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
src/VoidOrNeverAnalyzer.php:190
New_::getRawArgs()is nullable when the constructor has no arguments, socount($args)raises aTypeErroron PHP 8 for common code such asthrow new \Exception;. This makes the new throw analysis abort stub generation instead of inferring a type; usegetArgs(), which normalizes missing arguments to an empty array.
$args = $stmt->expr->expr->getRawArgs();
src/VoidOrNeverAnalyzer.php:187
- This restricts never inference to
throw new ..., but PHP also permitsthrow $exceptionandthrow makeException(), both of which always transfer control. With noReturn_nodes those functions fall through to the defaultvoidinference, producing an incorrect contract. Treat everyThrow_as never, and only inspectNew_arguments for the override-message exception.
if (! ($stmt->expr instanceof Throw_) || ! ($stmt->expr->expr instanceof New_)) {
return false;
src/VoidOrNeverAnalyzer.php:260
- Known scalar
$argsvalues are all treated as non-exiting by this branch. However, the documented third parameter includesstring(wordpress-stubs.php:131729), andfunctionMap.php:253models every value other thanarray{exit:false}asnever; for example,'response=500'leaves the defaultexit=true. A wrapper containing that call is therefore inferred asvoidinstead ofnever. Handle the string form, including its parsedexitvalue, before falling through.
if (! is_array($arg)) {
return false;
src/VoidOrNeverAnalyzer.php:49
findInstanceOf($node, Return_::class)traverses nested function-like nodes. For example, a function whose onlyreturnis inside an inner closure is treated as having a non-void return, so the outer function misses itsvoidtag; the analogous yield search also skips outer analysis when a nested closure yields. Restrict these searches to the current function body without descending into nested functions or closures.
$returnStmts = $this->nodeFinder->findInstanceOf($node, Return_::class);
src/VoidOrNeverAnalyzer.php:64
- The new analyzer adds a bulk
voidfallback and several newnever/skip branches, but the existing tests do not exercise these paths:VisitorTestonly covers handwritten return-tag handling, and the return fixtures only assertwp_die()call behavior. Add generator/type-inference fixtures for no-return functions, direct throws and exits, override-message skips, constructors/deprecated/abstract methods, and nested closures before relying on the generated annotations.
// No return statements and no inferred never, default to void.
$node->setAttribute(self::ATTRIBUTE_NAME, new Void_());
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR improves the automatic inference and addition of
@phpstan-return voidand@phpstan-return never.Changes are split across several commits to make it easier to review the impact of each individual change.
VoidOrNeverAnalyzerhelper class to reduce complexity in theVisitorclass. This encapsulates the analysis logic into a focused, single-responsibility class.neverdetection by analyzing top-level throw statements. Functions that throw or exit with messages containing "override" or "overridden" are skipped and receive no tags (neithervoidnornever) as they are meant to be overridden and it can not be inferred what the return type should be.@returnor@phpstan-returntags (auto-detected tags have the lowest priority and manually written annotations from core or from this package are never overwritten),@deprecatedfunctions, and@abstractmethods (meant to be implemented by subclasses eventually changing the return type).@phpstan-return voidtags to all remaining functions without return statements (after applying the safeguards above concerning "abstract" methods). This is the primary change that affects a large number of functions. While the safeguards should catch most edge cases, I did not check and verify each individualvoidtag, so there might be functions that shouldn't have received a void tag. If needed, this change can be easily reverted by reverting the last commit.Why all those void tags? This allows PHPStan to flag the usage of the return value of a void function. See this example.