Skip to content

Explicit never early terminates#366

Merged
ondrejmirtes merged 1 commit intophpstan:masterfrom
b1rdex:explicit-never-early-terminates
Nov 4, 2020
Merged

Explicit never early terminates#366
ondrejmirtes merged 1 commit intophpstan:masterfrom
b1rdex:explicit-never-early-terminates

Conversation

@b1rdex
Copy link
Copy Markdown
Contributor

@b1rdex b1rdex commented Nov 2, 2020

@b1rdex b1rdex force-pushed the explicit-never-early-terminates branch from 6f9fd27 to c22522d Compare November 2, 2020 09:57
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

This is almost perfect PR :) Please test it in a more modern way - see NodeScopeResolverTest::testFileAsserts - add a new dataProvider in it. You can use assertVariableCertainty function inside the test file. Thanks.

@b1rdex b1rdex force-pushed the explicit-never-early-terminates branch from c22522d to fd6f35c Compare November 3, 2020 02:58
@b1rdex
Copy link
Copy Markdown
Contributor Author

b1rdex commented Nov 3, 2020

@ondrejmirtes I updated the test

@b1rdex b1rdex force-pushed the explicit-never-early-terminates branch from fd6f35c to f0df73c Compare November 3, 2020 05:49
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.

Needs to start with data as any other data provider here.

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'm not even sure if these are executed. They might not be. Have you seen them fail?

The typical test here should be this instead:

if (rand(0, 1)) {
    $a = 1;
} else {
    Foo::doBarPhpDoc();
}

assertVariableCertainty(TrinaryLogic::createYes(), $a);

@ondrejmirtes
Copy link
Copy Markdown
Member

Yeah, I just verified that the asserts are not verified because they're "dead code". I'll bring it over the finish line myselfů

@ondrejmirtes ondrejmirtes force-pushed the explicit-never-early-terminates branch from f0df73c to 410c3ef Compare November 4, 2020 07:49
@ondrejmirtes ondrejmirtes merged commit 5f71943 into phpstan:master Nov 4, 2020
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you. There needs to be a second part - verifying that a function with @return never actually never returns.

@b1rdex b1rdex deleted the explicit-never-early-terminates branch November 4, 2020 12:00
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.

Type never is not detected as stopper

2 participants