Skip to content

Report dead types even in multi-exception catch#2067

Closed
janedbal wants to merge 13 commits intophpstan:1.10.xfrom
janedbal:dead-type-in-multi-exception-catch
Closed

Report dead types even in multi-exception catch#2067
janedbal wants to merge 13 commits intophpstan:1.10.xfrom
janedbal:dead-type-in-multi-exception-catch

Conversation

@janedbal
Copy link
Contributor

@janedbal janedbal commented Dec 9, 2022

@janedbal janedbal force-pushed the dead-type-in-multi-exception-catch branch from 89adae8 to b42a687 Compare December 13, 2022 20:25
@janedbal janedbal marked this pull request as ready for review December 13, 2022 20:46
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@janedbal janedbal force-pushed the dead-type-in-multi-exception-catch branch from 02aea0c to 29e3358 Compare February 6, 2023 09:56
@janedbal janedbal changed the base branch from 1.9.x to 1.10.x February 6, 2023 09:56
closureDefaultParameterTypeRule: false
newRuleLevelHelper: false
instanceofType: false
detectDeadTypeInMultiCatch: true
Copy link
Member

Choose a reason for hiding this comment

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

Should be false here

{

public function __construct(private Catch_ $originalNode, private Type $caughtType, private Type $originalCaughtType)
public function __construct(private Catch_ $originalNode, private Type $unthrownType, private Type $originalCaughtType)
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to rename the parameter

{
return array_merge(
parent::getAdditionalConfigFiles(),
!self::$detectDeadMultiCatch ? [__DIR__ . '/disable-detect-multi-catch.neon'] : [],
Copy link
Member

Choose a reason for hiding this comment

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

Should be two separate test cases instead of switching the property. I haven't tested if this works, and the convention is to have two separate test cases... You can't be sure when getAdditionalConfigFiles will be called first and thus it can lead to unexpected behaviour.

@janedbal
Copy link
Contributor Author

Solved by #2399

@janedbal janedbal closed this May 15, 2023
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.

Dead catch rule vs union type catch

3 participants