Skip to content

Add throw point for UnhandledMatchError #1056

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

Jump to bottom
Merged
ondrejmirtes merged 9 commits into from
Mar 21, 2022
Merged

Add throw point for UnhandledMatchError #1056

ondrejmirtes merged 9 commits into from
Mar 21, 2022

Conversation

@rajyan
Copy link
Contributor

@rajyan rajyan commented Mar 7, 2022

@rajyan rajyan changed the title Fix/issue 6115 Add throw point for UnhandledMatchError Mar 7, 2022
ondrejmirtes
ondrejmirtes requested changes Mar 9, 2022
View reviewed changes
src/Analyser/NodeScopeResolver.php Outdated
Copy link
Member

@ondrejmirtes ondrejmirtes Mar 9, 2022

Choose a reason for hiding this comment

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

This isn't sufficient. It's possible that the match handles all the cases and doesn't throw the exception, even without the default cause.

See the rule on how I figure that out:

phpstan-src/src/Rules/Comparison/MatchExpressionRule.php

Lines 82 to 91 in d0ab72a

if (!$hasDefault && !$nextArmIsDead) {
$remainingType = $node->getEndScope()->getType($matchCondition);
if (!$remainingType instanceof NeverType) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Match expression does not handle remaining %s: %s',
$remainingType instanceof UnionType ? 'values' : 'value',
$remainingType->describe(VerbosityLevel::value()),
))->build();
}
}

Additionally, I'd like the linked error to not be reported if the match is surrounded by a try-catch for UnhandledMatchError.

Copy link
Contributor Author

@rajyan rajyan Mar 9, 2022

Choose a reason for hiding this comment

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

Thanks! I wasn't aware of this rule. I'll try to fix it sometime this week.

@rajyan rajyan marked this pull request as draft March 9, 2022 12:05
@rajyan rajyan marked this pull request as ready for review March 10, 2022 01:59
@rajyan rajyan requested a review from ondrejmirtes March 10, 2022 01:59
@rajyan
Copy link
Contributor Author

rajyan commented Mar 10, 2022 •
edited
Loading

Additionally, I'd like the linked error to not be reported if the match is surrounded by a try-catch for UnhandledMatchError.

Traversed parents to find try-catch, but not sure this is a good way. Also, maybe this error should be suppressed if there is an explicit @throws tag with UnhandledMatchError?

@ondrejmirtes ondrejmirtes force-pushed the 1.5.x branch 3 times, most recently from ddd20b4 to 95d480b Compare March 18, 2022 19:53
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Mar 21, 2022

Thank you! The @throws tag would be a nice addition in a next PR. You can check $scope->getFunction()->getThrowType() for that.

@ondrejmirtes ondrejmirtes merged commit b401ffa into phpstan:1.5.x Mar 21, 2022
@rajyan rajyan deleted the fix/issue-6115 branch March 21, 2022 09:38
@rajyan rajyan mentioned this pull request Mar 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dead catch - UnhandledMatchError is never thrown in the try block.

2 participants

@rajyan @ondrejmirtes