From 36fe9b397834ff6c01696ae536b4b10523872faf Mon Sep 17 00:00:00 2001 From: Pablo Ferrandez Roca Date: Fri, 19 Jun 2026 10:52:49 +0200 Subject: [PATCH] corregidos problemas con comillas escapadas y parentesis anidados --- src/FileUpdater.php | 66 +++-- tests/FileUpdaterUpgradeTest.php | 234 ++++++++++++++++++ tests/SampleFiles/EscapedQuotes.txt | 20 ++ tests/SampleFiles/HttpFoundationBoth.txt | 21 ++ tests/SampleFiles/NestedParentheses.txt | 20 ++ .../SampleFiles/RedirectResponseWithFqcn.txt | 14 ++ 6 files changed, 359 insertions(+), 16 deletions(-) create mode 100644 tests/SampleFiles/EscapedQuotes.txt create mode 100644 tests/SampleFiles/HttpFoundationBoth.txt create mode 100644 tests/SampleFiles/NestedParentheses.txt create mode 100644 tests/SampleFiles/RedirectResponseWithFqcn.txt diff --git a/src/FileUpdater.php b/src/FileUpdater.php index 78a7b7a..99bd8dc 100644 --- a/src/FileUpdater.php +++ b/src/FileUpdater.php @@ -285,7 +285,7 @@ public static function upgradePhpFiles(): void $fileStr = str_replace('use Symfony\Component\HttpFoundation\Request;', 'use FacturaScripts\Core\Request;', $fileStr); $fileStr = str_replace('use Symfony\Component\HttpFoundation\Response;', 'use FacturaScripts\Core\Response;', $fileStr); - if (strpos($fileStr, 'HttpFoundation\RedirectResponse') !== false && strpos($fileStr, 'FacturaScripts\Core\Response') !== false) { + if (strpos($fileStr, 'HttpFoundation\RedirectResponse') !== false && strpos($fileStr, 'use FacturaScripts\Core\Response;') !== false) { $fileStr = str_replace('use Symfony\Component\HttpFoundation\RedirectResponse;', '', $fileStr); } else { $fileStr = str_replace('use Symfony\Component\HttpFoundation\RedirectResponse;', 'use FacturaScripts\Core\Response;', $fileStr); @@ -304,7 +304,12 @@ public static function upgradePhpFiles(): void } // reemplazamos loadFromCode('', $where) por loadWhere($where) - $fileStr = preg_replace('/->loadFromCode\(\s*[\'\"]\s*\'\s*,\s*([^)]+)\)/', '->loadWhere($1)', $fileStr); + $fileStr = self::replaceMethodCall($fileStr, '->loadFromCode(', function (array $args) { + if (count($args) >= 2 && in_array(trim($args[0]), ["''", '""'], true)) { + return '->loadWhere(' . implode(', ', array_slice($args, 1)) . ')'; + } + return '->loadFromCode(' . implode(', ', $args) . ')'; + }); // reemplazamos loadFromCode($code) por load($code) $fileStr = str_replace('->loadFromCode($', '->load($', $fileStr); @@ -359,12 +364,12 @@ public static function upgradePhpFiles(): void $fileStr = preg_replace('/\$this->previousData\[([^\]]+)\]/', '$this->getOriginal($1)', $fileStr); // reemplazamos llamadas al método all() con 3 parámetros añadiendo el 4º parámetro (50) - // añade ", 50" solo cuando hay exactamente 3 argumentos - $fileStr = preg_replace( - '/(->|::)all\(\s*([^,()]+)\s*,\s*([^,()]+)\s*,\s*([^,()]+)\s*\)/', - '$1all($2, $3, $4, 50)', - $fileStr - ); + foreach (['->all(', '::all('] as $allNeedle) { + $fileStr = self::replaceMethodCall($fileStr, $allNeedle, function (array $args) use ($allNeedle) { + $suffix = count($args) === 3 ? ', 50' : ''; + return substr($allNeedle, 0, 2) . 'all(' . implode(', ', $args) . $suffix . ')'; + }); + } // reemplazamos protected function onChange($field) por protected function onChange(string $field): bool $fileStr = str_replace('protected function onChange($field)', 'protected function onChange(string $field): bool', $fileStr); @@ -622,6 +627,23 @@ private static function convertDataBaseWhereCalls(string $fileStr): string return $result . substr($fileStr, $pos); } + private static function replaceMethodCall(string $fileStr, string $needle, callable $fn): string + { + $result = ''; + $pos = 0; + + while (($start = strpos($fileStr, $needle, $pos)) !== false) { + $result .= substr($fileStr, $pos, $start - $pos); + $openParen = $start + strlen($needle) - 1; + $closeParen = self::findClosingParen($fileStr, $openParen); + $argsStr = substr($fileStr, $openParen + 1, $closeParen - $openParen - 1); + $result .= $fn(self::parseCallArguments($argsStr)); + $pos = $closeParen + 1; + } + + return $result . substr($fileStr, $pos); + } + /** * Localiza el paréntesis de cierre balanceado a partir de la posición del paréntesis de apertura. * Ignora correctamente los paréntesis que aparecen dentro de cadenas de texto. @@ -633,12 +655,19 @@ private static function findClosingParen(string $code, int $openAt): int $inDouble = false; for ($i = $openAt, $len = strlen($code); $i < $len; $i++) { - $ch = $code[$i]; - $prev = $i > 0 ? $code[$i - 1] : ''; + $ch = $code[$i]; + + //contamos el numero de barras consecutivas + $count = 0; + for ($j = $i - 1; $j >= 0 && $code[$j] === '\\'; $j--) { + $count++; + } + // si el número de barras es impar, el carácter actual está escapado + $escaped = ($count % 2) === 1; - if ($ch === "'" && !$inDouble && $prev !== '\\') { + if ($ch === "'" && !$inDouble && !$escaped) { $inSingle = !$inSingle; - } elseif ($ch === '"' && !$inSingle && $prev !== '\\') { + } elseif ($ch === '"' && !$inSingle && !$escaped) { $inDouble = !$inDouble; } elseif (!$inSingle && !$inDouble) { if ($ch === '(' || $ch === '[') { @@ -719,14 +748,19 @@ private static function parseCallArguments(string $str): array $inDouble = false; // cursor dentro de una cadena con comillas dobles for ($i = 0, $len = strlen($str); $i < $len; $i++) { - $ch = $str[$i]; - $prev = $i > 0 ? $str[$i - 1] : ''; + $ch = $str[$i]; + + $bsCount = 0; + for ($j = $i - 1; $j >= 0 && $str[$j] === '\\'; $j--) { + $bsCount++; + } + $escaped = ($bsCount % 2) === 1; - if ($ch === "'" && !$inDouble && $prev !== '\\') { + if ($ch === "'" && !$inDouble && !$escaped) { // apertura/cierre de cadena simple; el carácter pertenece al argumento $inSingle = !$inSingle; $current .= $ch; - } elseif ($ch === '"' && !$inSingle && $prev !== '\\') { + } elseif ($ch === '"' && !$inSingle && !$escaped) { // apertura/cierre de cadena doble; el carácter pertenece al argumento $inDouble = !$inDouble; $current .= $ch; diff --git a/tests/FileUpdaterUpgradeTest.php b/tests/FileUpdaterUpgradeTest.php index 0095977..2343d50 100644 --- a/tests/FileUpdaterUpgradeTest.php +++ b/tests/FileUpdaterUpgradeTest.php @@ -283,4 +283,238 @@ public function testUpgradeSalesModSignature(): void } } } + + public function testUpgradeNestedCallArguments(): void + { + $originalDir = getcwd(); + $sandbox = sys_get_temp_dir() . '/fsmaker_upgrade_' . uniqid('', true); + + if (!mkdir($sandbox, 0755, true) && !is_dir($sandbox)) { + $this->fail('Unable to create temporary plugin directory.'); + } + + try { + chdir($sandbox); + Utils::setSilent(true); + Utils::setFolder($sandbox); + + file_put_contents('facturascripts.ini', "[plugin]\nname = TestPlugin\n"); + mkdir('Model', 0755, true); + + $fixturePath = __DIR__ . '/SampleFiles/NestedParentheses.txt'; + $targetPath = $sandbox . '/Model/TestModel.php'; + if (!copy($fixturePath, $targetPath)) { + $this->fail('Unable to copy sample file.'); + } + + FileUpdater::upgradePhpFiles(); + + $updated = file_get_contents($targetPath); + + // loadFromCode con 1 argumento se convierte en loadWhere con array de Where + $this->assertStringNotContainsString('->loadFromCode(', $updated); + $this->assertStringContainsString( + '$this->loadWhere([Where::eq(\'id\', $this->getParentId())])', + $updated + ); + + // all() con 3 args cuyo último contiene '()' → debe añadir el 4º parámetro (50) + $this->assertStringContainsString( + 'self::all([Where::eq(\'active\', 1)], null, $this->getOrder(), 50)', + $updated + ); + } finally { + chdir($originalDir); + Utils::setSilent(false); + + if (is_dir($sandbox)) { + $items = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($sandbox, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($items as $item) { + $path = $item->getPathname(); + $item->isDir() ? rmdir($path) : unlink($path); + } + rmdir($sandbox); + } + } + } + + public function testUpgradeDoubleBackslashBefore(): void + { + // número par de backslashes antes de comilla → la comilla cierra el string (no está escapada) + $originalDir = getcwd(); + $sandbox = sys_get_temp_dir() . '/fsmaker_upgrade_' . uniqid('', true); + + if (!mkdir($sandbox, 0755, true) && !is_dir($sandbox)) { + $this->fail('Unable to create temporary plugin directory.'); + } + + try { + chdir($sandbox); + Utils::setSilent(true); + Utils::setFolder($sandbox); + + file_put_contents('facturascripts.ini', "[plugin]\nname = TestPlugin\n"); + mkdir('Model', 0755, true); + + $fixturePath = __DIR__ . '/SampleFiles/EscapedQuotes.txt'; + $targetPath = $sandbox . '/Model/TestModel.php'; + if (!copy($fixturePath, $targetPath)) { + $this->fail('Unable to copy sample file.'); + } + + FileUpdater::upgradePhpFiles(); + + $updated = file_get_contents($targetPath); + + $this->assertStringNotContainsString('DataBaseWhere', $updated); + $this->assertSame(2, substr_count($updated, 'Where::eq(')); + + // 'C:\\' → dos backslashes + comilla que cierra: argumento único, no partido + $this->assertStringContainsString( + <<<'EOT' + Where::eq('ruta', 'C:\\') + EOT, + $updated + ); + + // "valor\\\\fin" → cuatro backslashes (dos literales) + comilla que cierra + $this->assertStringContainsString( + <<<'EOT' + Where::eq('desc', "valor\\\\fin") + EOT, + $updated + ); + } finally { + chdir($originalDir); + Utils::setSilent(false); + + if (is_dir($sandbox)) { + $items = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($sandbox, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($items as $item) { + $path = $item->getPathname(); + $item->isDir() ? rmdir($path) : unlink($path); + } + rmdir($sandbox); + } + } + } + + public function testUpgradeFilesIsIdempotent(): void + { + + $cases = [ + ['fixture' => 'DeprecatedDataBaseWhere.txt', 'dir' => 'Model', 'file' => 'TestModel.php'], + ['fixture' => 'HttpFoundationBoth.txt', 'dir' => 'Controller', 'file' => 'TestController.php'], + ]; + + foreach ($cases as $case) { + $originalDir = getcwd(); + $sandbox = sys_get_temp_dir() . '/fsmaker_upgrade_' . uniqid('', true); + + if (!mkdir($sandbox, 0755, true) && !is_dir($sandbox)) { + $this->fail('Unable to create temporary plugin directory.'); + } + + try { + chdir($sandbox); + Utils::setSilent(true); + Utils::setFolder($sandbox); + + file_put_contents('facturascripts.ini', "[plugin]\nname = TestPlugin\n"); + mkdir($sandbox . '/' . $case['dir'], 0755, true); + + $targetPath = $sandbox . '/' . $case['dir'] . '/' . $case['file']; + if (!copy(__DIR__ . '/SampleFiles/' . $case['fixture'], $targetPath)) { + $this->fail('Unable to copy sample file: ' . $case['fixture']); + } + + // primera pasada + FileUpdater::upgradePhpFiles(); + $afterFirstPass = file_get_contents($targetPath); + + // segunda pasada: debe ser un no-op + FileUpdater::upgradePhpFiles(); + $afterSecondPass = file_get_contents($targetPath); + + $this->assertSame( + $afterFirstPass, + $afterSecondPass, + 'Second upgrade pass must not change already-upgraded file: ' . $case['fixture'] + ); + } finally { + chdir($originalDir); + Utils::setSilent(false); + + if (is_dir($sandbox)) { + $items = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($sandbox, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($items as $item) { + $path = $item->getPathname(); + $item->isDir() ? rmdir($path) : unlink($path); + } + rmdir($sandbox); + } + } + } + } + + public function testUpgradeRedirectResponse(): void + { + // ResponseFactory contiene el prefijo "Response": sin búsqueda exacta confundiría el import + $originalDir = getcwd(); + $sandbox = sys_get_temp_dir() . '/fsmaker_upgrade_' . uniqid('', true); + + if (!mkdir($sandbox, 0755, true) && !is_dir($sandbox)) { + $this->fail('Unable to create temporary plugin directory.'); + } + + try { + chdir($sandbox); + Utils::setSilent(true); + Utils::setFolder($sandbox); + + file_put_contents('facturascripts.ini', "[plugin]\nname = TestPlugin\n"); + mkdir('Controller', 0755, true); + + $fixturePath = __DIR__ . '/SampleFiles/RedirectResponseWithFqcn.txt'; + $targetPath = $sandbox . '/Controller/TestController.php'; + if (!copy($fixturePath, $targetPath)) { + $this->fail('Unable to copy sample file.'); + } + + FileUpdater::upgradePhpFiles(); + + $updated = file_get_contents($targetPath); + + // el import de Symfony desaparece y se añade el de FacturaScripts + $this->assertStringNotContainsString('use Symfony\Component\HttpFoundation\RedirectResponse;', $updated); + $this->assertStringContainsString('use FacturaScripts\Core\Response;', $updated); + + // ResponseFactory no se toca + $this->assertStringContainsString('use FacturaScripts\Core\ResponseFactory;', $updated); + } finally { + chdir($originalDir); + Utils::setSilent(false); + + if (is_dir($sandbox)) { + $items = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($sandbox, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($items as $item) { + $path = $item->getPathname(); + $item->isDir() ? rmdir($path) : unlink($path); + } + rmdir($sandbox); + } + } + } } diff --git a/tests/SampleFiles/EscapedQuotes.txt b/tests/SampleFiles/EscapedQuotes.txt new file mode 100644 index 0000000..35b6093 --- /dev/null +++ b/tests/SampleFiles/EscapedQuotes.txt @@ -0,0 +1,20 @@ +loadFromCode('', [new DataBaseWhere('id', $this->getParentId())]); + } + + // all: tercer argumento contiene paréntesis anidados (3 args → debe añadir 50) + public function findActive(): array + { + return self::all([new DataBaseWhere('active', 1)], null, $this->getOrder()); + } +} \ No newline at end of file diff --git a/tests/SampleFiles/RedirectResponseWithFqcn.txt b/tests/SampleFiles/RedirectResponseWithFqcn.txt new file mode 100644 index 0000000..5a97595 --- /dev/null +++ b/tests/SampleFiles/RedirectResponseWithFqcn.txt @@ -0,0 +1,14 @@ +