Skip to content

Incorrect behavior of ChangeAndIfToEarlyReturnRector #5703

@staabm

Description

@staabm

Bug Report

Subject Details
Rector version last dev-master
Installed as composer dependency

Minimal PHP Code Causing Issue

See https://getrector.org/demo/01ae2e5e-fc96-4ff1-9b14-e73afc3360f1

<?php

class util {
    /**
     * Removes comment lines and splits up large sql files into individual queries.
     *
     * Last revision: September 23, 2001 - gandon
     *
     * @param array  $queries the splitted sql commands
     * @param string $sql     the sql commands
     * @param int    $release the MySQL release number (because certains php3 versions
     *                        can't get the value of a constant from within a function)
     *
     * @return bool always true
     */
    public static function splitSqlFile(&$queries, $sql, $release)
    {
        // do not trim, see bug #1030644
        //$sql          = trim($sql);
        $sql = rtrim($sql, "\n\r");
        $sqlLen = strlen($sql);
        $stringStart = '';
        $inString = false;
        $nothing = true;
        $time0 = time();

        for ($i = 0; $i < $sqlLen; ++$i) {
            $char = $sql[$i];

            // We are in a string, check for not escaped end of strings except for
            // backquotes that can't be escaped
            if ($inString) {
                for (;;) {
                    /** @psalm-suppress LoopInvalidation */
                    $i = strpos($sql, $stringStart, $i);
                    // No end of string found -> add the current substring to the
                    // returned array
                    if (!$i) {
                        $queries[] = $sql;
                        return true;
                    }
                    // Backquotes or no backslashes before quotes: it's indeed the
                    // end of the string -> exit the loop
                    if ('`' == $stringStart || '\\' != $sql[$i - 1]) {
                        $stringStart = '';
                        $inString = false;
                        break;
                    }
                    // one or more Backslashes before the presumed end of string...

                    // ... first checks for escaped backslashes
                    $j = 2;
                    $escapedBackslash = false;
                    while ($i - $j > 0 && '\\' == $sql[$i - $j]) {
                        $escapedBackslash = !$escapedBackslash;
                        ++$j;
                    }
                    // ... if escaped backslashes: it's really the end of the
                    // string -> exit the loop
                    if ($escapedBackslash) {
                        $stringStart = '';
                        $inString = false;
                        break;
                    }
                    // ... else loop

                    ++$i;

                    // end if...elseif...else
                } // end for
            } // end if (in string)

            // lets skip comments (/*, -- and #)
            elseif (('-' == $char && $sqlLen > $i + 2 && '-' == $sql[$i + 1] && $sql[$i + 2] <= ' ') || '#' == $char || ('/' == $char && $sqlLen > $i + 1 && '*' == $sql[$i + 1])) {
                /** @psalm-suppress LoopInvalidation */
                $i = strpos($sql, '/' == $char ? '*/' : "\n", $i);
                // didn't we hit end of string?
                if (false === $i) {
                    break;
                }
                if ('/' == $char) {
                    ++$i;
                }
            }

            // We are not in a string, first check for delimiter...
            elseif (';' == $char) {
                // if delimiter found, add the parsed part to the returned array
                $queries[] = ['query' => substr($sql, 0, $i), 'empty' => $nothing];
                $nothing = true;
                $sql = ltrim(substr($sql, min($i + 1, $sqlLen)));
                $sqlLen = strlen($sql);
                if ($sqlLen) {
                    /** @psalm-suppress LoopInvalidation */
                    $i = -1;
                } else {
                    // The submited statement(s) end(s) here
                    return true;
                }
            } // end else if (is delimiter)

            // ... then check for start of a string,...
            elseif (('"' == $char) || ('\'' == $char) || ('`' == $char)) {
                $inString = true;
                $nothing = false;
                $stringStart = $char;
            } // end else if (is start of string)

            elseif ($nothing) {
                $nothing = false;
            }

            // loic1: send a fake header each 30 sec. to bypass browser timeout
            $time1 = time();
            if ($time1 >= $time0 + 30) {
                $time0 = $time1;
                header('X-pmaPing: Pong');
            } // end if
        } // end for

        // add any rest to the returned array
        if (!empty($sql) && preg_match('@[^[:space:]]+@', $sql)) {
            $queries[] = ['query' => $sql, 'empty' => $nothing];
        }

        return true;
    }
}

Responsible rules

  • ChangeAndIfToEarlyReturnRector

Expected Behavior

rector leaves code behind, which misses a return case (which existed before).

after rectorizing the code, phpstan reports

Method rex_sql_util::splitSqlFile() should return bool but return statement is missing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions