Skip to content

More array fixes#8943

Merged
orklah merged 22 commits intovimeo:masterfrom
nicelocal:fix_8940
Dec 19, 2022
Merged

More array fixes#8943
orklah merged 22 commits intovimeo:masterfrom
nicelocal:fix_8940

Conversation

@danog
Copy link
Copy Markdown
Collaborator

@danog danog commented Dec 19, 2022

We really should add support for disjoint arrays...

@danog danog marked this pull request as ready for review December 19, 2022 11:07
@danog danog added the release:fix The PR will be included in 'Fixes' section of the release notes label Dec 19, 2022
@danog
Copy link
Copy Markdown
Collaborator Author

danog commented Dec 19, 2022

Whoops, the logic is actually incorrect, hang on...

@danog danog changed the title Fix #8940 Fix #8940, #8942 Dec 19, 2022
@danog danog marked this pull request as draft December 19, 2022 13:07
@danog
Copy link
Copy Markdown
Collaborator Author

danog commented Dec 19, 2022

One very nasty remaining baseline issue is caused by #8949, the rest is caused by false positives

@danog danog changed the title Fix #8940, #8942 More array fixes Dec 19, 2022
@danog danog marked this pull request as ready for review December 19, 2022 20:52
@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 19, 2022

Thanks!

@orklah orklah merged commit 62db5d4 into vimeo:master Dec 19, 2022
@Ocramius
Copy link
Copy Markdown
Contributor

Not sure if worth investigating, but consider checking the baseline additions @ laminas/laminas-validator#166, @danog

I think what's happening here is that Psalm cannot determine the type of $decoded after this code block:

        $decoded   = [];
        $separator = strrpos($encoded, '-');
        if ($separator > 0) {
            for ($x = 0; $x < $separator; ++$x) {
                // prepare decoding matrix
                $decoded[] = ord($encoded[$x]);
            }
        }

        $lengthd = count($decoded);
        $lengthe = strlen($encoded);

        // decoding
        $init  = true;
        $base  = 72;
        $index = 0;
        $char  = 0x80;

        for ($indexe = $separator ? $separator + 1 : 0; $indexe < $lengthe; ++$lengthd) {
            for ($oldIndex = $index, $pos = 1, $key = 36; 1; $key += 36) {
                $hex   = ord($encoded[$indexe++]);
                $digit = $hex - 48 < 10 ? $hex - 22
                       : ($hex - 65 < 26 ? $hex - 65
                       : ($hex - 97 < 26 ? $hex - 97
                       : 36));

                $index += $digit * $pos;
                $tag    = $key <= $base ? 1 : ($key >= $base + 26 ? 26 : $key - $base);
                if ($digit < $tag) {
                    break;
                }

                $pos = (int) ($pos * (36 - $tag));
            }

            $delta  = intval($init ? ($index - $oldIndex) / 700 : ($index - $oldIndex) / 2);
            $delta += intval($delta / ($lengthd + 1));
            for ($key = 0; $delta > 910 / 2; $key += 36) {
                $delta = intval($delta / 35);
            }

            $base   = intval($key + 36 * $delta / ($delta + 38));
            $init   = false;
            $char  += (int) ($index / ($lengthd + 1));
            $index %= $lengthd + 1;
            if ($lengthd > 0) {
                for ($i = $lengthd; $i > $index; $i--) {
                    $decoded[$i] = $decoded[$i - 1];
                }
            }

            $decoded[$index++] = $char;
        }

For now, I'm OK with adding it to the baseline, but I think this patch may have led to this tiny regression :-)

@danog
Copy link
Copy Markdown
Collaborator Author

danog commented Dec 21, 2022

@Ocramius Hmm, mind opening a new issue for this one?
At a quick glance, the inferred type seems correct: https://psalm.dev/r/8adf377b9f

@psalm-github-bot
Copy link
Copy Markdown

I found these snippets:

https://psalm.dev/r/8adf377b9f
<?php
/** @var string */
$encoded = '';
$decoded   = [];
        $separator = strrpos($encoded, '-');
        if ($separator > 0) {
            for ($x = 0; $x < $separator; ++$x) {
                // prepare decoding matrix
                $decoded[] = ord($encoded[$x]);
            }
        }

        $lengthd = count($decoded);
        $lengthe = strlen($encoded);

        // decoding
        $init  = true;
        $base  = 72;
        $index = 0;
        $char  = 0x80;

        for ($indexe = $separator ? $separator + 1 : 0; $indexe < $lengthe; ++$lengthd) {
            for ($oldIndex = $index, $pos = 1, $key = 36; 1; $key += 36) {
                $hex   = ord($encoded[$indexe++]);
                $digit = $hex - 48 < 10 ? $hex - 22
                       : ($hex - 65 < 26 ? $hex - 65
                       : ($hex - 97 < 26 ? $hex - 97
                       : 36));

                $index += $digit * $pos;
                $tag    = $key <= $base ? 1 : ($key >= $base + 26 ? 26 : $key - $base);
                if ($digit < $tag) {
                    break;
                }

                $pos = (int) ($pos * (36 - $tag));
            }

            $delta  = intval($init ? ($index - $oldIndex) / 700 : ($index - $oldIndex) / 2);
            $delta += intval($delta / ($lengthd + 1));
            for ($key = 0; $delta > 910 / 2; $key += 36) {
                $delta = intval($delta / 35);
            }

            $base   = intval($key + 36 * $delta / ($delta + 38));
            $init   = false;
            $char  += (int) ($index / ($lengthd + 1));
            $index %= $lengthd + 1;
            if ($lengthd > 0) {
                for ($i = $lengthd; $i > $index; $i--) {
                    $decoded[$i] = $decoded[$i - 1];
                }
            }

            $decoded[$index++] = $char;
        }
/** @psalm-trace $decoded */;
Psalm output (using commit 59149e9):

INFO: Trace - 57:29 - $decoded: array<int, int>

INFO: UnusedVariable - 20:9 - $char is never referenced or the value is not used

INFO: UnusedVariable - 47:13 - $char is never referenced or the value is not used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:fix The PR will be included in 'Fixes' section of the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants