bug: fix CI on master branch #6663
Conversation
| @@ -261,7 +261,7 @@ protected function doTest(IntegrationCase $case): void | |||
| "Expected changes do not match result for \"%s\" in \"%s\".\nFixers applied:\n%s.", | |||
| $case->getTitle(), | |||
| $case->getFileName(), | |||
| null === $changed ? '[None]' : implode(',', $changed['appliedFixers']) | |||
There was a problem hiding this comment.
why phpstan complains here but not in https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/6663/files#diff-ad61fe78582daafae4a9b0efef0855f40970fddb72fb06e4031eba3add0cff44R247 ?
There was a problem hiding this comment.
because we unlock dev-tools on master branch: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/v3.12.0/.github/workflows/sca.yml#L63-L67
There was a problem hiding this comment.
@kubawerlos , I meant why phpstan, on master branch, complains about line 264 but not about line 247?
There was a problem hiding this comment.
That's because of line 255 - since we passed assertNotEmpty the value cannot be null.
There was a problem hiding this comment.
oh my. not sure if I like that. like, implementation of assertNotEmpty ext library will change, and if we don't run stan anymore, we are potentially in troubles.
but no-blocking here
There was a problem hiding this comment.
What kind of trouble do you have in mind?
I see it as worst-case scenario we have a type error in sprintf in the assertion message.
There was a problem hiding this comment.
What kind ...
calling null['index']
basically I have mixed feeling on assuming foo: Bar|Baz to be only Baz because of some hidden checks in external methods we are calling. but if that's the flow to go I'm not blocking
There was a problem hiding this comment.
In general yes, but I cannot imagine that such a popular tool as PHPUnit changes assertNotEmpty to allow null.
No description provided.