Skip to content

Mark return.type, return.empty, return.void, and return.never errors as non-ignorable when native return type is violated#5628

Open
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-iuxn6bm
Open

Mark return.type, return.empty, return.void, and return.never errors as non-ignorable when native return type is violated#5628
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-iuxn6bm

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a function/method has a native return type declaration (e.g., function foo(): array) and the code returns a value that violates that native type (e.g., return null), this causes a runtime TypeError. Such errors should not be ignorable via baseline or @phpstan-ignore comments, because suppressing them masks guaranteed runtime fatal errors.

This PR makes return type violation errors (return.type, return.empty, return.void, return.never) non-ignorable when the violation is against the native return type specifically. Violations that only affect PHPDoc types (but not the native type) remain ignorable.

Changes

  • src/Rules/FunctionReturnTypeCheck.php: Added ?Type $nativeReturnType parameter and isNativeTypeViolated() helper method. When native type is violated, errors are marked non-ignorable. In non-strict mode, scalar-to-scalar coercion is allowed to remain ignorable since PHP coerces those at runtime without a TypeError.
  • src/Rules/Methods/ReturnTypeRule.php: Pass $method->getNativeReturnType() to checkReturnType()
  • src/Rules/Functions/ReturnTypeRule.php: Pass $function->getNativeReturnType() to checkReturnType()
  • src/Rules/Functions/ClosureReturnTypeRule.php: Resolve native return type from closure AST node via ParserNodeTypeToPHPStanType and pass to checkReturnType()
  • src/Rules/Functions/ArrowFunctionReturnTypeRule.php: Same approach as closures
  • src/PhpDoc/ResolvedPhpDocBlock.php: Fix pre-existing native type violation in getNameScope() by adding assert($this->nameScope !== null)
  • phpstan-baseline.neon: Remove baseline entry for the now-fixed ResolvedPhpDocBlock::getNameScope() violation

Root cause

Return type errors were unconditionally ignorable regardless of whether they represented a native type violation (guaranteed runtime TypeError) or just a PHPDoc-level violation. The FunctionReturnTypeCheck::checkReturnType() method had no knowledge of the native return type, only the combined (native + PHPDoc) type.

Test

  • tests/PHPStan/Rules/Methods/data/bug-9833.php + ReturnTypeRuleTest::testBug9833() and testBug9833NonIgnorable(): Tests method return type violations, verifying native type violations are non-ignorable while PHPDoc-only violations remain ignorable.
  • tests/PHPStan/Rules/Functions/data/bug-9833.php + ReturnTypeRuleTest::testBug9833() and testBug9833NonIgnorable(): Same for standalone functions.
  • tests/PHPStan/Rules/Functions/data/bug-9833-closure.php + ClosureReturnTypeRuleTest::testBug9833() and testBug9833NonIgnorable(): Same for closures.
  • tests/PHPStan/Rules/Functions/data/bug-9833-arrow.php + ArrowFunctionReturnTypeRuleTest::testBug9833() and testBug9833NonIgnorable(): Same for arrow functions.

Fixes phpstan/phpstan#9833

… errors as non-ignorable when native return type is violated

- Add optional `?Type $nativeReturnType` parameter to `FunctionReturnTypeCheck::checkReturnType()`
- When the native return type is provided and the return value violates it, call `->nonIgnorable()` on the error builder
- For non-strict mode, allow scalar-to-scalar coercion to remain ignorable (int→string etc. don't cause TypeErrors at runtime)
- Pass native return type from `Methods\ReturnTypeRule`, `Functions\ReturnTypeRule`, `Functions\ClosureReturnTypeRule`, and `Functions\ArrowFunctionReturnTypeRule`
- Fix pre-existing native type violation in `ResolvedPhpDocBlock::getNameScope()` (now correctly caught as non-ignorable)
- Remove corresponding baseline entry that was suppressing the violation
@staabm staabm force-pushed the create-pull-request/patch-iuxn6bm branch from aaa96d4 to 67cfcda Compare May 10, 2026 09:39
$returnValueType->describe($verbosityLevel),
))
->line($returnNode->getStartLine())
->identifier('return.type')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I disagree about making return.type non-ignorable, it's too squishy and dependent on surrounding code. I still want people to be able to ignore this (for now).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. Removed the non-ignorable marking for return.type errors per the reviewer's request. The other three identifiers (return.empty, return.void, return.never) still become non-ignorable when the native return type is violated. All tests pass and make phpstan reports no errors.

…iolated

Only `return.empty`, `return.void`, and `return.never` are marked
non-ignorable when the native return type is violated. `return.type`
remains ignorable as requested in review.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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