Skip to content

Improve void or never inference - #458

Open
IanDelMar wants to merge 11 commits into
php-stubs:masterfrom
IanDelMar:void-or-never
Open

Improve void or never inference#458
IanDelMar wants to merge 11 commits into
php-stubs:masterfrom
IanDelMar:void-or-never

Conversation

@IanDelMar

Copy link
Copy Markdown
Contributor

This PR improves the automatic inference and addition of @phpstan-return void and @phpstan-return never.

Changes are split across several commits to make it easier to review the impact of each individual change.

  1. Refactored void/never detection logic into a dedicated VoidOrNeverAnalyzer helper class to reduce complexity in the Visitor class. This encapsulates the analysis logic into a focused, single-responsibility class.
  2. Improved never detection by analyzing top-level throw statements. Functions that throw or exit with messages containing "override" or "overridden" are skipped and receive no tags (neither void nor never) as they are meant to be overridden and it can not be inferred what the return type should be.
  3. Skipped adding tags for: constructors, functions with existing @return or @phpstan-return tags (auto-detected tags have the lowest priority and manually written annotations from core or from this package are never overwritten), @deprecated functions, and @abstract methods (meant to be implemented by subclasses eventually changing the return type).
  4. Added @phpstan-return void tags 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 individual void tag, 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 VoidOrNeverAnalyzer helper to encapsulate void/never detection.
  • Updates Visitor to delegate inference to the new analyzer and to avoid adding inferred @phpstan-return tags when one is already present from the function map.
  • Expands “never” inference with top-level throw detection 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.

Comment thread src/VoidOrNeverAnalyzer.php
Comment thread src/VoidOrNeverAnalyzer.php Outdated
@szepeviktor

Copy link
Copy Markdown
Member

Actually I just want a simple package with minimal/no code.
This is an inner struggle in me.

@IanDelMar

Copy link
Copy Markdown
Contributor Author

Actually I just want a simple package with minimal/no code. This is an inner struggle in me.

Before I address the throw handling issue, please let me know if you will consider merging this or not.

@szepeviktor

Copy link
Copy Markdown
Member

please let me know if you will consider merging this or not.

Yes, I would.
But I prefer PRs making everything smaller/simpler.

@IanDelMar IanDelMar mentioned this pull request Aug 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, so count($args) raises a TypeError on PHP 8 for common code such as throw new \Exception;. This makes the new throw analysis abort stub generation instead of inferring a type; use getArgs(), 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 permits throw $exception and throw makeException(), both of which always transfer control. With no Return_ nodes those functions fall through to the default void inference, producing an incorrect contract. Treat every Throw_ as never, and only inspect New_ arguments for the override-message exception.
        if (! ($stmt->expr instanceof Throw_) || ! ($stmt->expr->expr instanceof New_)) {
            return false;

src/VoidOrNeverAnalyzer.php:260

  • Known scalar $args values are all treated as non-exiting by this branch. However, the documented third parameter includes string (wordpress-stubs.php:131729), and functionMap.php:253 models every value other than array{exit:false} as never; for example, 'response=500' leaves the default exit=true. A wrapper containing that call is therefore inferred as void instead of never. Handle the string form, including its parsed exit value, 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 only return is inside an inner closure is treated as having a non-void return, so the outer function misses its void tag; 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 void fallback and several new never/skip branches, but the existing tests do not exercise these paths: VisitorTest only covers handwritten return-tag handling, and the return fixtures only assert wp_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_());

Comment thread src/VoidOrNeverAnalyzer.php Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants