Skip to content

added offset-value regression test#1596

Merged
ondrejmirtes merged 7 commits intophpstan:1.8.xfrom
staabm:regress-offsets
Aug 6, 2022
Merged

added offset-value regression test#1596
ondrejmirtes merged 7 commits intophpstan:1.8.xfrom
staabm:regress-offsets

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Aug 6, 2022

];

$data = array_combine($keys, $line);
$data['languages'] = explode(',', $data['languages']);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the reported error on this line is interessting:

1) PHPStan\Rules\Arrays\NonexistentOffsetInArrayDimFetchRuleTest::testBug7469
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'28: Cannot access offset 'languages' on array<'address'|'bankAccount'|'birthDate'|'email'|'firstName'|'ic'|'invoicing'|'invoicingAddress'|'languages'|'lastName'|'note'|'phone'|'radio'|'videoOnline'|'videoTvc'|'voiceExample', mixed>|false.
+28: Cannot access offset 'languages' on array<'address'|'bankAccount'|'birthDate'|'email'|'firstName'|'ic'|'invoicing'|'invoicingAddress'|'languages'|'lastName'|'note'|'phone'|'radio'|'videoOnline'|'videoTvc'|'voiceExample', mixed>|false.
 '

https://github.com/phpstan/phpstan-src/runs/7703414883?check_suite_focus=true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 4b54905

the error appeared only in the test-suit, not on the reported snippet, because of the phpstan level.
added a IF-case to compensate the possible false return, which happens on php7 only

Copy link
Copy Markdown
Contributor Author

@staabm staabm Aug 6, 2022

Choose a reason for hiding this comment

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

made the test compatible with php7 and php8.

there is still a double reported error on line 29 (php7 only), which I guess is a leftover bug?

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.

Why would it be a bug? The offset is accessed two times on the same line.

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.

Would be nice to sprinkle some assertType calls about the fixed type inference throughout the test files, and reference them from NodeScopeResolverTest too. Thanks!

@ondrejmirtes ondrejmirtes merged commit 9515d4a into phpstan:1.8.x Aug 6, 2022
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you. Please add NodeScopeResolverTest separately.

@staabm staabm deleted the regress-offsets branch August 6, 2022 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants