Check datetime instantiation by b1rdex · Pull Request #380

Check datetime instantiation by b1rdex · Pull Request #380 · phpstan/phpstan-src

Choose a reason for hiding this comment

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

And finally, please fix the build (Failures on PHP 8 are fine - they're on master already.)

Choose a reason for hiding this comment

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

This actually needs fixing:

Cannot access offset 'warnings' on array('warning_count' => int,     
         'warnings' => array<int, string>, 'error_count' => int, 'errors' =>  
         array<int, string>)|false.

Where that false comes from? I can’t reproduce returning false by that method. Отправлено с iPhone

…

24 нояб. 2020 г., в 01:23, Ondřej Mirtes ***@***.***> написал(а): @ondrejmirtes requested changes on this pull request. This actually needs fixing: Cannot access offset 'warnings' on array('warning_count' => int, 'warnings' => array<int, string>, 'error_count' => int, 'errors' => array<int, string>)|false. In src/Rules/DateTimeInstantiationRule.php: > + public function getNodeType(): string + { + return New_::class; + } + + /** + * @param New_ $node + */ + public function processNode(Node $node, Scope $scope): array + { + if ( + !($node->class instanceof \PhpParser\Node\Name) + || \count($node->args) === 0 + || ( + strtolower((string) $node->class) !== strtolower(DateTime::class) + && strtolower((string) $node->class) !== strtolower(DateTimeImmutable::class) This unnecessarily calls strtolower((string) $node->class) twice. You can hardcode datetime instead. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

It tells me that you haven't tested a case where the rule should not report anything :)

Actually there are tests with no errors. I haven’t tried calling it without constructing a date. Ok, will fix it tomorrow. Отправлено с iPhone

…

24 нояб. 2020 г., в 01:39, Anatoly Pashin ***@***.***> написал(а): Where that false comes from? I can’t reproduce returning false by that method. Отправлено с iPhone >> 24 нояб. 2020 г., в 01:23, Ondřej Mirtes ***@***.***> написал(а): >> > > @ondrejmirtes requested changes on this pull request. > > This actually needs fixing: > > Cannot access offset 'warnings' on array('warning_count' => int, > 'warnings' => array<int, string>, 'error_count' => int, 'errors' => > array<int, string>)|false. > In src/Rules/DateTimeInstantiationRule.php: > > > + public function getNodeType(): string > + { > + return New_::class; > + } > + > + /** > + * @param New_ $node > + */ > + public function processNode(Node $node, Scope $scope): array > + { > + if ( > + !($node->class instanceof \PhpParser\Node\Name) > + || \count($node->args) === 0 > + || ( > + strtolower((string) $node->class) !== strtolower(DateTime::class) > + && strtolower((string) $node->class) !== strtolower(DateTimeImmutable::class) > This unnecessarily calls strtolower((string) $node->class) twice. > You can hardcode datetime instead. > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub, or unsubscribe.

b1rdex deleted the datetime-instantiation branch

November 24, 2020 09:50