Skip to content

fix: phpstan error in v1.8.8 #6637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MGatner merged 3 commits into from
Oct 6, 2022
Merged

fix: phpstan error in v1.8.8 #6637

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8bd750f
fix: phpstan error in v1.8.8
ddevsr Oct 6, 2022
a40f7a5
fix: phpstan analyze error in array_helper
ddevsr Oct 6, 2022
0ad1683
fix: phpstan analyze error in array_helper
ddevsr Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions system/Helpers/array_helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ function dot_array_search(string $index, array $array)
function _array_search_dot(array $indexes, array $array)
{
// Grab the current index
$currentIndex = $indexes ? array_shift($indexes) : null;
$currentIndex = $indexes ? array_shift($indexes) : null;
$intCurrentIndex = (int) $currentIndex;

if ((empty($currentIndex) && (int) $currentIndex !== 0) || (! isset($array[$currentIndex]) && $currentIndex !== '*')) {
if (empty($currentIndex) && $intCurrentIndex !== 0 || (! isset($array[$currentIndex]) && $currentIndex !== '*')) {
Copy link
Copy Markdown
Member

@kenjis kenjis Oct 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

If $currentIndex is null, empty($currentIndex) is true and $intCurrentIndex !== 0 is false.
If $currentIndex is not null, empty($currentIndex) is false and $intCurrentIndex !== 0 is true.
So the left side is still always false.

Copy link
Copy Markdown
Member

@kenjis kenjis Oct 6, 2022

Choose a reason for hiding this comment

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

Does this seem better?

        // Grab the current index
        $currentIndex = $indexes ? array_shift($indexes) : null;

        if ($currentIndex === null || (! isset($array[$currentIndex]) && $currentIndex !== '*')) {
            return null;
        }

Copy link
Copy Markdown
Member

@MGatner MGatner Oct 7, 2022

Choose a reason for hiding this comment

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

I'm sorry, I didn't realize it changed between my review and the merge. This was originally just removing the extraneous part of the conditional.

Yours is better but this still has a lot of cognitive complexity and anyone working on it in the future will need to spend some time figuring it out. Maybe we should break it up into separate return statements? Or extract the second half of the conditional as a variable with a helpful name?

Copy link
Copy Markdown
Member

@kenjis kenjis Oct 7, 2022

Choose a reason for hiding this comment

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

Okay, I will fix this.

Copy link
Copy Markdown
Member

@kenjis kenjis Oct 7, 2022

Choose a reason for hiding this comment

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

I sent a PR #6645

return null;
}

Expand Down