Improve parenthesization for union types #1433
Conversation
src/Type/UnionType.php
Outdated
| @@ -206,6 +206,8 @@ public function describe(VerbosityLevel $level): string | |||
| foreach ($types as $type) { | |||
| if ($type instanceof ClosureType || $type instanceof CallableType || $type instanceof TemplateUnionType) { | |||
| $typeNames[] = sprintf('(%s)', $type->describe($level)); | |||
| } elseif ($type instanceof TemplateType && ($level->isTypeOnly() || $level->isValue())) { | |||
There was a problem hiding this comment.
I was doubting to do this regardless of level, but for precise/cache the template scope and variance is appended which already makes it unambiguous.
| $errors = $this->runAnalyse(__DIR__ . '/data/bug-7484.php'); | ||
| $this->assertCount(1, $errors); | ||
| $this->assertSame('Generator expects key type K of int|string, (K of int)|string given.', $errors[0]->getMessage()); | ||
| $this->assertSame(21, $errors[0]->getLine()); |
There was a problem hiding this comment.
FYI I usually keep AnalyserIntegrationTest for things that crashed PHPStan/caused infinite loop. It's better to put the test in a specific rule test case. (or a specific rule test case + NodeScopeResolverTest for type inference verification).
But I'm gonna merge it like this so that the fix isn't belated :)
There was a problem hiding this comment.
I'll keep it in mind :)
There was a problem hiding this comment.
Oh actually, it looks lit's adding parentheses in more cases where it isn't necessary, and breaking people's baselines: https://github.com/phpstan/phpstan-src/runs/6933410671?check_suite_focus=true, https://github.com/phpstan/phpstan-src/runs/6933409913?check_suite_focus=true, https://github.com/phpstan/phpstan-src/runs/6933409824?check_suite_focus=true
I'd say that if the template type is last, the parenthesis isn't necessary...
|
Even though there are failures, I'm happy with the results now! |
|
Thank you! |
Fixes phpstan/phpstan#7484