Skip to content

Add non regression tests #5110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VincentLanglet merged 1 commit into from
Mar 1, 2026
+47 −0
Merged

Add non regression tests #5110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,21 @@ public function testBug9146(): void
]);
}

#[RequiresPhp('>= 8.3')]
public function testPr5105(): void
{
$this->analyse([__DIR__ . '/data/pr-5105.php'], [
[
'Dead catch - RuntimeException is never thrown in the try block.',
15,
],
[
'Dead catch - Error is never thrown in the try block.',
28,
],
]);
}

public function testBug9146NonStrict(): void
{
$this->analyse([__DIR__ . '/data/bug-9146-non-strict.php'], [
Expand Down
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Exceptions/data/pr-5105.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php // lint >= 8.3

declare(strict_types = 1);

namespace Pr5105;

enum ReactionType: string
{
case EMOJI_HEART = '❤️';

public static function tryFromName1(string $name): ?self
{
try {
return ReactionType::{$name};
} catch (\RuntimeException) {
Copy link
Contributor

@staabm staabm Mar 1, 2026

Choose a reason for hiding this comment

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

Suggested change
} catch (\RuntimeException) {
} catch (\RuntimeException) { // will throw ERROR on invalid $name

return null;
}
}

public static function tryFromName2(string $name): ?self
{
if ($name !== 'EMOJI_HEART') {
return null;
}

try {
return ReactionType::{$name};
} catch (\Error) {
return null;
}
Comment on lines +26 to +30
Copy link
Contributor

@staabm staabm Mar 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

\Error can still be thrown, see

Copy link
Contributor Author

@VincentLanglet VincentLanglet Mar 1, 2026

Choose a reason for hiding this comment

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

Not after the condition line 22 ?

Copy link
Contributor

@staabm staabm Mar 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

you are right.

maybe we should add a test for

	public static function tryFromInvalid(string $name): ?self
	{
		try {
			return ReactionType::XYZ;
		} catch (\Error $e) {
			return null;
		}
	}

Copy link
Contributor Author

@VincentLanglet VincentLanglet Mar 1, 2026

Choose a reason for hiding this comment

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

Currently it's not supported
https://phpstan.org/r/2b645c07-0bae-4dec-b0f1-1ce3012b0e8e

And it's like any method call

try {
			return self::acceptsString(true);
		} catch (\Error $e) {
			return null;
		}

we're currently reporting the method call as an error but also reporting the dead catch (if implictThrow = false)

}
}
Loading