From 9187dc8ec2acff6256ed09545494d7b5d397cb43 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 15 Feb 2023 12:18:05 +0000 Subject: [PATCH] Export plugins refactoring (#18071) This is a simplification of the code to remove confusing ($row[$j] == '0' || $row[$j] != ''). To make the code simpler, I added a @var annotation and replace for with foreach. I realize this adds Psalm warnings, and they are valid warnings. But they are because of dead code related to $GLOBALS['what']. This just looks like another nonsense of its own that needs a little more work to be removed so I kept it out of this PR. For the time being, let's keep them in Psalm baseline. * Simplify ExportCsv * Simplify ExportHtmlword * Simplify ExportTexytext * Fix broken tests https://github.com/phpmyadmin/phpmyadmin/pull/18071 --------- Signed-off-by: Kamil Tekiela --- .../classes/Plugins/Export/ExportCsv.php | 59 +++++++++---------- .../classes/Plugins/Export/ExportHtmlword.php | 17 +++--- .../classes/Plugins/Export/ExportTexytext.php | 17 ++++-- phpstan-baseline.neon | 5 ++ psalm-baseline.xml | 12 ++-- test/classes/Plugins/Export/ExportCsvTest.php | 10 ++-- 6 files changed, 67 insertions(+), 53 deletions(-) diff --git a/libraries/classes/Plugins/Export/ExportCsv.php b/libraries/classes/Plugins/Export/ExportCsv.php index 1fba8c8cdc..af9066b73c 100644 --- a/libraries/classes/Plugins/Export/ExportCsv.php +++ b/libraries/classes/Plugins/Export/ExportCsv.php @@ -9,6 +9,7 @@ namespace PhpMyAdmin\Plugins\Export; use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Dbal\Connection; +use PhpMyAdmin\Dbal\ResultInterface; use PhpMyAdmin\Plugins\ExportPlugin; use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyMainGroup; use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyRootGroup; @@ -18,11 +19,10 @@ use PhpMyAdmin\Properties\Options\Items\TextPropertyItem; use PhpMyAdmin\Properties\Plugins\ExportPluginProperties; use function __; +use function implode; use function mb_strtolower; -use function mb_substr; use function preg_replace; use function str_replace; -use function trim; use const PHP_EOL; @@ -214,7 +214,7 @@ class ExportCsv extends ExportPlugin ): bool { $GLOBALS['what'] = $GLOBALS['what'] ?? null; $GLOBALS['csv_terminated'] = $GLOBALS['csv_terminated'] ?? null; - $GLOBALS['csv_separator'] = $GLOBALS['csv_separator'] ?? null; + $GLOBALS['csv_separator'] = $GLOBALS['csv_separator'] ?? ''; $GLOBALS['csv_enclosed'] = $GLOBALS['csv_enclosed'] ?? null; $GLOBALS['csv_escaped'] = $GLOBALS['csv_escaped'] ?? null; @@ -222,26 +222,30 @@ class ExportCsv extends ExportPlugin $table_alias = $table; $this->initAlias($aliases, $db_alias, $table_alias); - // Gets the data from the database + /** + * Gets the data from the database + * + * @var ResultInterface $result + * @psalm-ignore-var + */ $result = $GLOBALS['dbi']->query( $sqlQuery, Connection::TYPE_USER, DatabaseInterface::QUERY_UNBUFFERED ); - $fields_cnt = $result->numFields(); // If required, get fields name at the first line if (isset($GLOBALS['csv_columns']) && $GLOBALS['csv_columns']) { - $schema_insert = ''; + $insertFields = []; foreach ($result->getFieldNames() as $col_as) { if (! empty($aliases[$db]['tables'][$table]['columns'][$col_as])) { $col_as = $aliases[$db]['tables'][$table]['columns'][$col_as]; } if ($GLOBALS['csv_enclosed'] == '') { - $schema_insert .= $col_as; + $insertFields[] = $col_as; } else { - $schema_insert .= $GLOBALS['csv_enclosed'] + $insertFields[] = $GLOBALS['csv_enclosed'] . str_replace( $GLOBALS['csv_enclosed'], $GLOBALS['csv_escaped'] . $GLOBALS['csv_enclosed'], @@ -249,11 +253,9 @@ class ExportCsv extends ExportPlugin ) . $GLOBALS['csv_enclosed']; } - - $schema_insert .= $GLOBALS['csv_separator']; } - $schema_insert = trim(mb_substr($schema_insert, 0, -1)); + $schema_insert = implode($GLOBALS['csv_separator'], $insertFields); if (! $this->export->outputHandler($schema_insert . $GLOBALS['csv_terminated'])) { return false; } @@ -261,68 +263,63 @@ class ExportCsv extends ExportPlugin // Format the data while ($row = $result->fetchRow()) { - $schema_insert = ''; - for ($j = 0; $j < $fields_cnt; $j++) { - if (! isset($row[$j])) { - $schema_insert .= $GLOBALS[$GLOBALS['what'] . '_null']; - } elseif ($row[$j] == '0' || $row[$j] != '') { + $insertValues = []; + foreach ($row as $field) { + if ($field === null) { + $insertValues[] = $GLOBALS[$GLOBALS['what'] . '_null']; + } elseif ($field !== '') { // always enclose fields if ($GLOBALS['what'] === 'excel') { - $row[$j] = preg_replace("/\015(\012)?/", "\012", $row[$j]); + $field = preg_replace("/\015(\012)?/", "\012", $field); } // remove CRLF characters within field if ( isset($GLOBALS[$GLOBALS['what'] . '_removeCRLF']) && $GLOBALS[$GLOBALS['what'] . '_removeCRLF'] ) { - $row[$j] = str_replace( + $field = str_replace( [ "\r", "\n", ], '', - $row[$j] + $field ); } if ($GLOBALS['csv_enclosed'] == '') { - $schema_insert .= $row[$j]; + $insertValues[] = $field; } else { // also double the escape string if found in the data if ($GLOBALS['csv_escaped'] != $GLOBALS['csv_enclosed']) { - $schema_insert .= $GLOBALS['csv_enclosed'] + $insertValues[] = $GLOBALS['csv_enclosed'] . str_replace( $GLOBALS['csv_enclosed'], $GLOBALS['csv_escaped'] . $GLOBALS['csv_enclosed'], str_replace( $GLOBALS['csv_escaped'], $GLOBALS['csv_escaped'] . $GLOBALS['csv_escaped'], - $row[$j] + $field ) ) . $GLOBALS['csv_enclosed']; } else { // avoid a problem when escape string equals enclose - $schema_insert .= $GLOBALS['csv_enclosed'] + $insertValues[] = $GLOBALS['csv_enclosed'] . str_replace( $GLOBALS['csv_enclosed'], $GLOBALS['csv_escaped'] . $GLOBALS['csv_enclosed'], - $row[$j] + $field ) . $GLOBALS['csv_enclosed']; } } } else { - $schema_insert .= ''; + $insertValues[] = ''; } - - if ($j >= $fields_cnt - 1) { - continue; - } - - $schema_insert .= $GLOBALS['csv_separator']; } + $schema_insert = implode($GLOBALS['csv_separator'], $insertValues); if (! $this->export->outputHandler($schema_insert . $GLOBALS['csv_terminated'])) { return false; } diff --git a/libraries/classes/Plugins/Export/ExportHtmlword.php b/libraries/classes/Plugins/Export/ExportHtmlword.php index bf21b36ab2..184d99ccd3 100644 --- a/libraries/classes/Plugins/Export/ExportHtmlword.php +++ b/libraries/classes/Plugins/Export/ExportHtmlword.php @@ -10,6 +10,7 @@ namespace PhpMyAdmin\Plugins\Export; use PhpMyAdmin\Database\Triggers; use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Dbal\Connection; +use PhpMyAdmin\Dbal\ResultInterface; use PhpMyAdmin\Plugins\ExportPlugin; use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyMainGroup; use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyRootGroup; @@ -201,13 +202,17 @@ class ExportHtmlword extends ExportPlugin return false; } - // Gets the data from the database + /** + * Gets the data from the database + * + * @var ResultInterface $result + * @psalm-ignore-var + */ $result = $GLOBALS['dbi']->query( $sqlQuery, Connection::TYPE_USER, DatabaseInterface::QUERY_UNBUFFERED ); - $fields_cnt = $result->numFields(); // If required, get fields name at the first line if (isset($GLOBALS['htmlword_columns'])) { @@ -231,13 +236,11 @@ class ExportHtmlword extends ExportPlugin // Format the data while ($row = $result->fetchRow()) { $schema_insert = ''; - for ($j = 0; $j < $fields_cnt; $j++) { - if (! isset($row[$j])) { + foreach ($row as $field) { + if ($field === null) { $value = $GLOBALS[$GLOBALS['what'] . '_null']; - } elseif ($row[$j] == '0' || $row[$j] != '') { - $value = $row[$j]; } else { - $value = ''; + $value = $field; } $schema_insert .= '' diff --git a/libraries/classes/Plugins/Export/ExportTexytext.php b/libraries/classes/Plugins/Export/ExportTexytext.php index 19016b1dd9..3161ab60fc 100644 --- a/libraries/classes/Plugins/Export/ExportTexytext.php +++ b/libraries/classes/Plugins/Export/ExportTexytext.php @@ -10,6 +10,7 @@ namespace PhpMyAdmin\Plugins\Export; use PhpMyAdmin\Database\Triggers; use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Dbal\Connection; +use PhpMyAdmin\Dbal\ResultInterface; use PhpMyAdmin\Plugins\ExportPlugin; use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyMainGroup; use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyRootGroup; @@ -181,13 +182,17 @@ class ExportTexytext extends ExportPlugin return false; } - // Gets the data from the database + /** + * Gets the data from the database + * + * @var ResultInterface $result + * @psalm-ignore-var + */ $result = $GLOBALS['dbi']->query( $sqlQuery, Connection::TYPE_USER, DatabaseInterface::QUERY_UNBUFFERED ); - $fields_cnt = $result->numFields(); // If required, get fields name at the first line if (isset($GLOBALS[$GLOBALS['what'] . '_columns'])) { @@ -209,11 +214,11 @@ class ExportTexytext extends ExportPlugin // Format the data while ($row = $result->fetchRow()) { $text_output = ''; - for ($j = 0; $j < $fields_cnt; $j++) { - if (! isset($row[$j])) { + foreach ($row as $field) { + if ($field === null) { $value = $GLOBALS[$GLOBALS['what'] . '_null']; - } elseif ($row[$j] == '0' || $row[$j] != '') { - $value = $row[$j]; + } elseif ($field !== '') { + $value = $field; } else { $value = ' '; } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 41b4ffb59a..8a2240fc41 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6260,6 +6260,11 @@ parameters: count: 1 path: libraries/classes/Plugins/Export/ExportCsv.php + - + message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#" + count: 3 + path: libraries/classes/Plugins/Export/ExportCsv.php + - message: "#^Method PhpMyAdmin\\\\Plugins\\\\Export\\\\ExportHtmlword\\:\\:exportData\\(\\) has parameter \\$aliases with no value type specified in iterable type array\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 5415b11747..208c9b0cde 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -10899,6 +10899,9 @@ + + $insertValues + $GLOBALS['excel_columns'] $GLOBALS['what'] @@ -10907,17 +10910,20 @@ $col_as + + $insertFields + $GLOBALS['what'] $GLOBALS['what'] $col_as + $insertFields[] + $insertValues[] - $GLOBALS[$GLOBALS['what'] . '_null'] $GLOBALS['what'] $GLOBALS['what'] $GLOBALS['what'] - $col_as $GLOBALS['csv_enclosed'] @@ -10941,8 +10947,6 @@ $GLOBALS['csv_escaped'] $GLOBALS['csv_escaped'] $GLOBALS['csv_escaped'] - $GLOBALS['csv_separator'] - $GLOBALS['csv_separator'] $GLOBALS['csv_terminated'] $GLOBALS['csv_terminated'] diff --git a/test/classes/Plugins/Export/ExportCsvTest.php b/test/classes/Plugins/Export/ExportCsvTest.php index a9ddd72ee2..01ce36dd31 100644 --- a/test/classes/Plugins/Export/ExportCsvTest.php +++ b/test/classes/Plugins/Export/ExportCsvTest.php @@ -404,7 +404,7 @@ class ExportCsvTest extends AbstractTestCase $result = ob_get_clean(); $this->assertEquals( - 'idnamedatetimefiel;1abcd2011-01-20 02:00:02;2foo2010-01-20 02:00:02;3Abcd2012-01-20 02:00:02;', + 'idnamedatetimefield;1abcd2011-01-20 02:00:02;2foo2010-01-20 02:00:02;3Abcd2012-01-20 02:00:02;', $result ); @@ -422,7 +422,7 @@ class ExportCsvTest extends AbstractTestCase $result = ob_get_clean(); $this->assertEquals( - '"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";' + '"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";' . '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";', $result ); @@ -443,7 +443,7 @@ class ExportCsvTest extends AbstractTestCase $result = ob_get_clean(); $this->assertEquals( - '"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";' + '"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";' . '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";', $result ); @@ -463,7 +463,7 @@ class ExportCsvTest extends AbstractTestCase $result = ob_get_clean(); $this->assertEquals( - '"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";' + '"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";' . '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";', $result ); @@ -483,7 +483,7 @@ class ExportCsvTest extends AbstractTestCase $result = ob_get_clean(); $this->assertEquals( - '"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";' + '"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";' . '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";', $result );