From 538cdec2ee9d290f65faa975378abb83d1a33ebb Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 22 Apr 2024 11:29:01 +0300 Subject: [PATCH 1/9] Add a CS workflow & fix issues (#101) This will prevent coding-standards issues creep-in in future commits --- .github/workflows/cs.yml | 68 +++++++ composer.json | 21 ++- phpcs.xml.dist | 172 +++++++++++++++--- tests/WP_SQLite_Translator_Tests.php | 147 +++++++-------- .../sqlite/class-wp-sqlite-translator.php | 108 ++++++----- wp-includes/sqlite/install-functions.php | 2 +- 6 files changed, 362 insertions(+), 156 deletions(-) create mode 100644 .github/workflows/cs.yml diff --git a/.github/workflows/cs.yml b/.github/workflows/cs.yml new file mode 100644 index 00000000..3dc0cd15 --- /dev/null +++ b/.github/workflows/cs.yml @@ -0,0 +1,68 @@ +name: CS + +on: + # Run on all relevant pushes (except to main) and on all relevant pull requests. + push: + paths: + - '**.php' + - 'composer.json' + - 'composer.lock' + - '.phpcs.xml.dist' + - 'phpcs.xml.dist' + - '.github/workflows/cs.yml' + pull_request: + paths: + - '**.php' + - 'composer.json' + - 'composer.lock' + - '.phpcs.xml.dist' + - 'phpcs.xml.dist' + - '.github/workflows/cs.yml' + # Allow manually triggering the workflow. + workflow_dispatch: + +# Cancels all previous workflow runs for the same branch that have not yet completed. +concurrency: + # The concurrency group contains the workflow name and the branch name. + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + checkcs: + name: 'Check code style' + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@main + + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '7.4' + coverage: none + tools: cs2pr + + # Validate the composer.json file. + # @link https://getcomposer.org/doc/03-cli.md#validate + - name: Validate Composer installation + run: composer validate --no-check-all + + # Install dependencies and handle caching in one go. + # @link https://github.com/marketplace/actions/install-composer-dependencies + - name: Install Composer dependencies + uses: ramsey/composer-install@v2 + with: + # Bust the cache at least once a month - output format: YYYY-MM. + custom-cache-suffix: $(date -u "+%Y-%m") + + # Check the codestyle of the files. + # The results of the CS check will be shown inline in the PR via the CS2PR tool. + # @link https://github.com/staabm/annotate-pull-request-from-checkstyle/ + - name: Check PHP code style + id: phpcs + run: composer check-cs -- --no-cache --report-full --report-checkstyle=./phpcs-report.xml + + - name: Show PHPCS results in PR + if: ${{ always() && steps.phpcs.outcome == 'failure' }} + run: cs2pr ./phpcs-report.xml diff --git a/composer.json b/composer.json index 1c555a60..38d597bf 100644 --- a/composer.json +++ b/composer.json @@ -13,13 +13,26 @@ "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0", "squizlabs/php_codesniffer": "^3.7", - "wp-coding-standards/wpcs": "^3.0", - "phpcompatibility/phpcompatibility-wp": "^2.1", - "yoast/phpunit-polyfills": "^1.0.1" + "wp-coding-standards/wpcs": "^3.1", + "yoast/phpunit-polyfills": "^1.0.1", + "phpcompatibility/phpcompatibility-wp": "*", + "php-parallel-lint/php-parallel-lint": "^1.3", + "phpstan/phpstan": "^1.10", + "szepeviktor/phpstan-wordpress": "^1.3", + "phpstan/extension-installer": "^1.3" }, "config": { "allow-plugins": { - "dealerdirect/phpcodesniffer-composer-installer": true + "dealerdirect/phpcodesniffer-composer-installer": true, + "phpstan/extension-installer": true } + }, + "scripts": { + "check-cs": [ + "@php ./vendor/bin/phpcs" + ], + "fix-cs": [ + "@php ./vendor/bin/phpcbf" + ] } } diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 557065e5..7a3b78bc 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -5,11 +5,11 @@ - - + @@ -20,50 +20,172 @@ - . + - - + . /vendor/* - + /wp-includes/sqlite/class-wp-sqlite-crosscheck-db.php - - - /wp-includes/*.php + + + + + + + + + + warning + + /tests/phpunit/* - - /wp-includes/*.php + + warning + + + warning + + + warning + + + warning - - - 0 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - 0 + + + + + + + + + + + + + + + + + + + + + + - - 0 + + + + + + /wp-includes/sqlite/class-wp-sqlite-translator.php + + + + + * - - + + /src/wp-includes/sqlite/*\.php /tests/* - /wp-includes/sqlite/class-wp-sqlite-crosscheck-db.php - + + + /tests/* + + /wp-includes/sqlite/class-wp-sqlite-crosscheck-db.php - + + /tests/* + + /tests/* - /wp-includes/sqlite/class-wp-sqlite-crosscheck-db.php /tests/* - /wp-includes/sqlite/class-wp-sqlite-crosscheck-db.php /wp-includes/sqlite/class-wp-sqlite-crosscheck-db.php diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index 3b1abec1..f30c6f6b 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -33,17 +33,17 @@ public function setUp(): void { $this->engine = new WP_SQLite_Translator( $this->sqlite ); $this->engine->query( "CREATE TABLE _options ( - ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL, - option_name TEXT NOT NULL default '', - option_value TEXT NOT NULL default '' - );" + ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL, + option_name TEXT NOT NULL default '', + option_value TEXT NOT NULL default '' + );" ); $this->engine->query( "CREATE TABLE _dates ( - ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL, - option_name TEXT NOT NULL default '', - option_value DATE NOT NULL - );" + ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL, + option_name TEXT NOT NULL default '', + option_value DATE NOT NULL + );" ); } @@ -202,7 +202,7 @@ public function testUpdateWithoutWhereButWithSubSelect() { ); $this->assertQuery( "INSERT INTO _dates (option_name, option_value) VALUES ('first', '2003-05-27 10:08:48');" - ); + ); $this->assertQuery( "INSERT INTO _dates (option_name, option_value) VALUES ('second', '2003-05-27 10:08:48');" ); @@ -210,7 +210,7 @@ public function testUpdateWithoutWhereButWithSubSelect() { "UPDATE _dates SET option_value = (SELECT option_value from _options WHERE option_name = 'User 0000019')" ); $this->assertSame( 2, $return, 'UPDATE query did not return 2 when two row were changed' ); - + $result1 = $this->engine->query( "SELECT option_value FROM _dates WHERE option_name='first'" ); $result2 = $this->engine->query( "SELECT option_value FROM _dates WHERE option_name='second'" ); $this->assertEquals( 'second', $result1[0]->option_value ); @@ -220,7 +220,7 @@ public function testUpdateWithoutWhereButWithSubSelect() { public function testUpdateWithoutWhereButWithLimit() { $this->assertQuery( "INSERT INTO _dates (option_name, option_value) VALUES ('first', '2003-05-27 10:08:48');" - ); + ); $this->assertQuery( "INSERT INTO _dates (option_name, option_value) VALUES ('second', '2003-05-27 10:08:48');" ); @@ -228,7 +228,7 @@ public function testUpdateWithoutWhereButWithLimit() { "UPDATE _dates SET option_value = 'second' LIMIT 1" ); $this->assertSame( 1, $return, 'UPDATE query did not return 2 when two row were changed' ); - + $result1 = $this->engine->query( "SELECT option_value FROM _dates WHERE option_name='first'" ); $result2 = $this->engine->query( "SELECT option_value FROM _dates WHERE option_name='second'" ); $this->assertEquals( 'second', $result1[0]->option_value ); @@ -252,16 +252,15 @@ public function testSelectFromDual() { $this->assertEquals( 1, $result[0]->output ); } - public function testShowCreateTable1() - { + public function testShowCreateTable1() { $this->assertQuery( "CREATE TABLE _tmp_table ( - ID BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL, - option_name VARCHAR(255) default '', - option_value TEXT NOT NULL, - UNIQUE KEY option_name (option_name), - KEY composite (option_name, option_value) - );" + ID BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL, + option_name VARCHAR(255) default '', + option_value TEXT NOT NULL, + UNIQUE KEY option_name (option_name), + KEY composite (option_name, option_value) + );" ); $this->assertQuery( @@ -270,22 +269,21 @@ public function testShowCreateTable1() $results = $this->engine->get_query_results(); $this->assertEquals( "CREATE TABLE _tmp_table ( - `ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL, - `option_name` varchar(255) DEFAULT '', - `option_value` text NOT NULL, - KEY _tmp_table__composite (option_name, option_value), - UNIQUE KEY _tmp_table__option_name (option_name) -);", + `ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL, + `option_name` varchar(255) DEFAULT '', + `option_value` text NOT NULL, + KEY _tmp_table__composite (option_name, option_value), + UNIQUE KEY _tmp_table__option_name (option_name) + );", $results[0]->{'Create Table'} ); } - public function testShowCreateTableSimpleTable() - { + public function testShowCreateTableSimpleTable() { $this->assertQuery( - "CREATE TABLE _tmp_table ( + 'CREATE TABLE _tmp_table ( ID BIGINT NOT NULL - );" + );' ); $this->assertQuery( @@ -293,21 +291,20 @@ public function testShowCreateTableSimpleTable() ); $results = $this->engine->get_query_results(); $this->assertEquals( - "CREATE TABLE _tmp_table ( + 'CREATE TABLE _tmp_table ( `ID` bigint NOT NULL -);", +);', $results[0]->{'Create Table'} ); } - public function testShowCreateTableWithAlterAndCreateIndex() - { + public function testShowCreateTableWithAlterAndCreateIndex() { $this->assertQuery( "CREATE TABLE _tmp_table ( - ID BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL, - option_name VARCHAR(255) default '', - option_value TEXT NOT NULL - );" + ID BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL, + option_name VARCHAR(255) default '', + option_value TEXT NOT NULL + );" ); $this->assertQuery( @@ -323,14 +320,15 @@ public function testShowCreateTableWithAlterAndCreateIndex() ); $results = $this->engine->get_query_results(); $this->assertEquals( - "CREATE TABLE _tmp_table ( + 'CREATE TABLE _tmp_table ( `ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL, `option_name` smallint NOT NULL DEFAULT 14, `option_value` text NOT NULL, KEY _tmp_table__option_name (option_name) -);", +);', $results[0]->{'Create Table'} ); + } public function testSelectIndexHintForce() { $this->assertQuery( "INSERT INTO _options (option_name) VALUES ('first');" ); @@ -383,7 +381,6 @@ public function testInsertSelectFromDual() { $this->assertEquals( 1, $result ); } - public function testCreateTemporaryTable() { $this->assertQuery( "CREATE TEMPORARY TABLE _tmp_table ( @@ -426,11 +423,10 @@ public function testShowTablesLike() { ); } - public function testShowTableStatusFrom() - { + public function testShowTableStatusFrom() { // Created in setUp() function - $this->assertQuery("DROP TABLE _options"); - $this->assertQuery("DROP TABLE _dates"); + $this->assertQuery( 'DROP TABLE _options' ); + $this->assertQuery( 'DROP TABLE _dates' ); $this->assertQuery( "CREATE TABLE _tmp_table ( @@ -450,11 +446,10 @@ public function testShowTableStatusFrom() ); } - public function testShowTableStatusIn() - { + public function testShowTableStatusIn() { // Created in setUp() function - $this->assertQuery("DROP TABLE _options"); - $this->assertQuery("DROP TABLE _dates"); + $this->assertQuery( 'DROP TABLE _options' ); + $this->assertQuery( 'DROP TABLE _dates' ); $this->assertQuery( "CREATE TABLE _tmp_table ( @@ -474,11 +469,10 @@ public function testShowTableStatusIn() ); } - public function testShowTableStatusInTwoTables() - { + public function testShowTableStatusInTwoTables() { // Created in setUp() function - $this->assertQuery("DROP TABLE _options"); - $this->assertQuery("DROP TABLE _dates"); + $this->assertQuery( 'DROP TABLE _options' ); + $this->assertQuery( 'DROP TABLE _dates' ); $this->assertQuery( "CREATE TABLE _tmp_table ( @@ -507,8 +501,8 @@ public function testShowTableStatusInTwoTables() public function testShowTableStatusLike() { // Created in setUp() function - $this->assertQuery("DROP TABLE _options"); - $this->assertQuery("DROP TABLE _dates"); + $this->assertQuery( 'DROP TABLE _options' ); + $this->assertQuery( 'DROP TABLE _dates' ); $this->assertQuery( "CREATE TABLE _tmp_table1 ( @@ -849,7 +843,6 @@ public function testAlterTableAddFulltextIndex() { ); } - public function testAlterTableModifyColumn() { $this->assertQuery( "CREATE TABLE _tmp_table ( @@ -2140,8 +2133,7 @@ public function testTranslatesUtf8SELECT() { $this->assertQuery( 'DELETE FROM _options' ); } - public function testOnConflictReplace() - { + public function testOnConflictReplace() { $this->assertQuery( "CREATE TABLE _tmp_table ( ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, @@ -2156,15 +2148,15 @@ public function testOnConflictReplace() $this->assertQuery( "INSERT INTO _tmp_table VALUES (1, null, null, null, '');" ); - $result = $this->assertQuery("SELECT * FROM _tmp_table WHERE ID = 1"); + $result = $this->assertQuery( 'SELECT * FROM _tmp_table WHERE ID = 1' ); $this->assertEquals( array( (object) array( - 'ID' => '1', - 'name' => 'default-value', - 'unique_name' => 'unique-default-value', + 'ID' => '1', + 'name' => 'default-value', + 'unique_name' => 'unique-default-value', 'inline_unique_name' => 'inline-unique-default-value', - 'no_default' => '', + 'no_default' => '', ), ), $result @@ -2174,10 +2166,10 @@ public function testOnConflictReplace() "INSERT INTO _tmp_table VALUES (2, '1', '2', '3', '4');" ); $this->assertQuery( - "UPDATE _tmp_table SET name = null WHERE ID = 2;" + 'UPDATE _tmp_table SET name = null WHERE ID = 2;' ); - $result = $this->assertQuery("SELECT name FROM _tmp_table WHERE ID = 2"); + $result = $this->assertQuery( 'SELECT name FROM _tmp_table WHERE ID = 2' ); $this->assertEquals( array( (object) array( @@ -2189,48 +2181,47 @@ public function testOnConflictReplace() // This should fail because of the UNIQUE constraint $this->assertQuery( - "UPDATE _tmp_table SET unique_name = NULL WHERE ID = 2;", + 'UPDATE _tmp_table SET unique_name = NULL WHERE ID = 2;', 'UNIQUE constraint failed: _tmp_table.unique_name' ); // Inline unique constraint aren't supported currently, so this should pass $this->assertQuery( - "UPDATE _tmp_table SET inline_unique_name = NULL WHERE ID = 2;", + 'UPDATE _tmp_table SET inline_unique_name = NULL WHERE ID = 2;', '' ); // WPDB allows for NULL values in columns that don't have a default value and a NOT NULL constraint $this->assertQuery( - "UPDATE _tmp_table SET no_default = NULL WHERE ID = 2;", + 'UPDATE _tmp_table SET no_default = NULL WHERE ID = 2;', '' ); - $result = $this->assertQuery("SELECT * FROM _tmp_table WHERE ID = 2"); + $result = $this->assertQuery( 'SELECT * FROM _tmp_table WHERE ID = 2' ); $this->assertEquals( array( (object) array( - 'ID' => '2', - 'name' => 'default-value', - 'unique_name' => '2', + 'ID' => '2', + 'name' => 'default-value', + 'unique_name' => '2', 'inline_unique_name' => 'inline-unique-default-value', - 'no_default' => '', + 'no_default' => '', ), ), $result ); } - public function testDefaultNullValue() - { + public function testDefaultNullValue() { $this->assertQuery( - "CREATE TABLE _tmp_table ( + 'CREATE TABLE _tmp_table ( name varchar(20) NOT NULL default NULL, no_default varchar(20) NOT NULL - );" + );' ); $result = $this->assertQuery( - "DESCRIBE _tmp_table;" + 'DESCRIBE _tmp_table;' ); $this->assertEquals( array( diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index 17f2f812..6ef1eedb 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -1501,7 +1501,7 @@ private function execute_select() { * [FOR {JOIN|ORDER BY|GROUP BY}] ([index_list]) * | {IGNORE|FORCE} {INDEX|KEY} * [FOR {JOIN|ORDER BY|GROUP BY}] (index_list) - * + * * @see https://dev.mysql.com/doc/refman/8.3/en/index-hints.html * @return bool */ @@ -1524,8 +1524,8 @@ private function skip_index_hint() { return false; } - $this->rewriter->skip(); // USE, FORCE, IGNORE - $this->rewriter->skip(); // INDEX, KEY + $this->rewriter->skip(); // USE, FORCE, IGNORE. + $this->rewriter->skip(); // INDEX, KEY. $maybe_for = $this->rewriter->peek(); if ( $maybe_for && $maybe_for->matches( @@ -1533,7 +1533,7 @@ private function skip_index_hint() { WP_SQLite_Token::FLAG_KEYWORD_RESERVED, array( 'FOR' ) ) ) { - $this->rewriter->skip(); // FOR + $this->rewriter->skip(); // FOR. $token = $this->rewriter->peek(); if ( $token && $token->matches( @@ -1541,9 +1541,9 @@ private function skip_index_hint() { WP_SQLite_Token::FLAG_KEYWORD_RESERVED, array( 'JOIN', 'ORDER', 'GROUP' ) ) ) { - $this->rewriter->skip(); // JOIN, ORDER, GROUP + $this->rewriter->skip(); // JOIN, ORDER, GROUP. if ( 'BY' === strtoupper( $this->rewriter->peek()->value ) ) { - $this->rewriter->skip(); // BY + $this->rewriter->skip(); // BY. } } } @@ -1592,8 +1592,14 @@ private function execute_describe() { } } - private function describe($table_name) - { + /** + * Executes a SELECT statement. + * + * @param string $table_name The table name. + * + * @return array + */ + private function describe( $table_name ) { return $this->execute_sqlite_query( "SELECT `name` as `Field`, @@ -1647,9 +1653,9 @@ private function describe($table_name) */ private function execute_update() { $this->rewriter->consume(); // Consume the UPDATE keyword. - $has_where = false; + $has_where = false; $needs_closing_parenthesis = false; - $params = array(); + $params = array(); while ( true ) { $token = $this->rewriter->peek(); if ( ! $token ) { @@ -1664,20 +1670,20 @@ private function execute_update() { * will be rewritten to: * - UPDATE table SET column = value WHERE rowid IN (SELECT rowid FROM table WHERE condition LIMIT 1); */ - if ($this->rewriter->depth === 0) { - if (($token->value === 'LIMIT' || $token->value === 'ORDER') && !$has_where) { + if ( 0 === $this->rewriter->depth ) { + if ( ( 'LIMIT' === $token->value || 'ORDER' === $token->value ) && ! $has_where ) { $this->rewriter->add( - new WP_SQLite_Token('WHERE', WP_SQLite_Token::TYPE_KEYWORD) + new WP_SQLite_Token( 'WHERE', WP_SQLite_Token::TYPE_KEYWORD ) ); $needs_closing_parenthesis = true; - $this->preface_WHERE_clause_with_a_subquery(); - } else if ($token->value === 'WHERE') { - $has_where = true; + $this->preface_where_clause_with_a_subquery(); + } elseif ( 'WHERE' === $token->value ) { + $has_where = true; $needs_closing_parenthesis = true; $this->rewriter->consume(); - $this->preface_WHERE_clause_with_a_subquery(); + $this->preface_where_clause_with_a_subquery(); $this->rewriter->add( - new WP_SQLite_Token('WHERE', WP_SQLite_Token::TYPE_KEYWORD, WP_SQLite_Token::FLAG_KEYWORD_RESERVED) + new WP_SQLite_Token( 'WHERE', WP_SQLite_Token::TYPE_KEYWORD, WP_SQLite_Token::FLAG_KEYWORD_RESERVED ) ); } } @@ -1710,7 +1716,7 @@ private function execute_update() { $this->rewriter->consume(); } - // Wrap up the WHERE clause with the nested SELECT statement + // Wrap up the WHERE clause with the nested SELECT statement. if ( $needs_closing_parenthesis ) { $this->rewriter->add( new WP_SQLite_Token( ')', WP_SQLite_Token::TYPE_OPERATOR ) ); } @@ -1725,16 +1731,16 @@ private function execute_update() { /** * Injects `rowid IN (SELECT rowid FROM table WHERE ...` into the WHERE clause at the current * position in the query. - * + * * This is necessary to emulate the behavior of MySQL's UPDATE LIMIT and DELETE LIMIT statement * as SQLite does not support LIMIT in UPDATE and DELETE statements. - * + * * The WHERE clause is wrapped in a subquery that selects the rowid of the rows that match the original - * WHERE clause. - * + * WHERE clause. + * * @return void */ - private function preface_WHERE_clause_with_a_subquery() { + private function preface_where_clause_with_a_subquery() { $this->rewriter->add_many( array( new WP_SQLite_Token( ' ', WP_SQLite_Token::TYPE_WHITESPACE ), @@ -3253,7 +3259,7 @@ private function execute_show() { // Fall through. case 'COLUMNS FROM': $table_name = $this->rewriter->consume()->token; - + $this->set_results_from_fetched_data( $this->get_columns_from( $table_name ) ); return; @@ -3331,27 +3337,27 @@ private function execute_show() { case 'CREATE TABLE': $table_name = $this->rewriter->consume()->token; - $columns = $this->get_columns_from($table_name); - $keys = $this->get_keys($table_name); + $columns = $this->get_columns_from( $table_name ); + $keys = $this->get_keys( $table_name ); - foreach($columns as $column) { - $column = (array) $column; - $definition = ''; + foreach ( $columns as $column ) { + $column = (array) $column; + $definition = ''; $definition .= '`' . $column['Field'] . '` '; $definition .= $this->get_cached_mysql_data_type( $table_name, $column['Field'] ) ?? $column['Type']; - $definition .= $column['Key'] === 'PRI' ? ' PRIMARY KEY' : ''; - $definition .= $column['Key'] === 'PRI' && $column['Type'] === 'INTEGER' ? ' AUTO_INCREMENT' : ''; - $definition .= $column['Null'] === 'NO' ? ' NOT NULL' : ''; + $definition .= 'PRI' === $column['Key'] ? ' PRIMARY KEY' : ''; + $definition .= 'PRI' === $column['Key'] && 'INTEGER' === $column['Type'] ? ' AUTO_INCREMENT' : ''; + $definition .= 'NO' === $column['Null'] ? ' NOT NULL' : ''; $definition .= $column['Default'] ? ' DEFAULT ' . $column['Default'] : ''; - $entries[] = $definition; + $entries[] = $definition; } - foreach($keys as $key) { - $key = (array) $key; - $definition = ''; - $definition .= $key['index']['unique'] === '1' ? 'UNIQUE ' : ''; + foreach ( $keys as $key ) { + $key = (array) $key; + $definition = ''; + $definition .= '1' === $key['index']['unique'] ? 'UNIQUE ' : ''; $definition .= 'KEY '; $definition .= $key['index']['name']; $definition .= ' ('; @@ -3360,22 +3366,22 @@ private function execute_show() { array_column( $key['columns'], 'name' ) ); $definition .= ')'; - $entries[] = $definition; + $entries[] = $definition; } - $create_table = "CREATE TABLE $table_name (\n\t"; - $create_table .= implode(",\n\t", $entries); + $create_table = "CREATE TABLE $table_name (\n\t"; + $create_table .= implode( ",\n\t", $entries ); $create_table .= "\n);"; $this->set_results_from_fetched_data( array( (object) array( - "Create Table" => $create_table, + 'Create Table' => $create_table, ), ) ); return; case 'TABLE STATUS': // FROM `database`. - // Match the optional [{FROM | IN} db_name] + // Match the optional [{FROM | IN} db_name]. $database_expression = $this->rewriter->consume(); if ( 'FROM' === $database_expression->token || 'IN' === $database_expression->token ) { $this->rewriter->consume(); @@ -3471,9 +3477,15 @@ private function execute_show() { } } - private function get_columns_from($table_name) - { - $stmt = $this->execute_sqlite_query( + /** + * Gets the columns from a table. + * + * @param string $table_name The table name. + * + * @return array The columns. + */ + private function get_columns_from( $table_name ) { + $stmt = $this->execute_sqlite_query( "PRAGMA table_info(\"$table_name\");" ); /* @todo we may need to add the Extra column if anybdy needs it. 'auto_increment' is the value */ @@ -3486,8 +3498,8 @@ private function get_columns_from($table_name) 'pk' => null, ); $columns = $stmt->fetchAll( $this->pdo_fetch_mode ); - $columns = array_map( - function ($row) use ($name_map) { + $columns = array_map( + function ( $row ) use ( $name_map ) { $new = array(); $is_object = is_object( $row ); $row = $is_object ? (array) $row : $row; diff --git a/wp-includes/sqlite/install-functions.php b/wp-includes/sqlite/install-functions.php index 51451642..cfce3a2f 100644 --- a/wp-includes/sqlite/install-functions.php +++ b/wp-includes/sqlite/install-functions.php @@ -31,7 +31,7 @@ function sqlite_make_db_sqlite() { wp_die( $message, 'Database Error!' ); } - $translator = new WP_SQLite_Translator( $pdo, $GLOBALS['table_prefix'] ); + $translator = new WP_SQLite_Translator( $pdo ); $query = null; try { From 037f6efe3ac27e9cda668e6d13eb087ed5471bce Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Mon, 22 Apr 2024 11:37:15 +0300 Subject: [PATCH 2/9] Remove phpstan dependencies --- composer.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 38d597bf..d889d0db 100644 --- a/composer.json +++ b/composer.json @@ -16,10 +16,7 @@ "wp-coding-standards/wpcs": "^3.1", "yoast/phpunit-polyfills": "^1.0.1", "phpcompatibility/phpcompatibility-wp": "*", - "php-parallel-lint/php-parallel-lint": "^1.3", - "phpstan/phpstan": "^1.10", - "szepeviktor/phpstan-wordpress": "^1.3", - "phpstan/extension-installer": "^1.3" + "php-parallel-lint/php-parallel-lint": "^1.3" }, "config": { "allow-plugins": { From 23ed2215dd468a4d9d5f488ecefa66d0118efc50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Tue, 30 Apr 2024 03:55:08 +0200 Subject: [PATCH 3/9] CI: Run PHPUnit tests (#99) Co-authored-by: Ari Stathopoulos Co-authored-by: Brandon Payton --- .github/workflows/phpunit-tests-run.yml | 47 +++++++++++++++++++ .github/workflows/phpunit-tests.yml | 28 +++++++++++ composer.json | 10 ++-- tests/WP_SQLite_Metadata_Tests.php | 1 + ...QLite_PDO_User_Defined_Functions_Tests.php | 2 +- tests/WP_SQLite_Query_Tests.php | 1 + tests/WP_SQLite_Translator_Tests.php | 30 ++++++------ tests/bootstrap.php | 2 +- 8 files changed, 102 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/phpunit-tests-run.yml create mode 100644 .github/workflows/phpunit-tests.yml diff --git a/.github/workflows/phpunit-tests-run.yml b/.github/workflows/phpunit-tests-run.yml new file mode 100644 index 00000000..f460a7ba --- /dev/null +++ b/.github/workflows/phpunit-tests-run.yml @@ -0,0 +1,47 @@ +name: Run PHPUnit tests + +on: + workflow_call: + inputs: + os: + description: 'Operating system to run tests on' + required: false + type: 'string' + default: 'ubuntu-latest' + php: + description: 'The version of PHP to use, in the format of X.Y' + required: true + type: 'string' + phpunit-config: + description: 'The PHPUnit configuration file to use' + required: false + type: 'string' + default: 'phpunit.xml.dist' +env: + LOCAL_PHP: ${{ inputs.php }}-fpm + PHPUNIT_CONFIG: ${{ inputs.phpunit-config }} + +jobs: + phpunit-tests: + name: ${{ inputs.os }} + runs-on: ${{ inputs.os }} + timeout-minutes: 20 + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '${{ inputs.php }}' + tools: phpunit-polyfills + + - name: Install Composer dependencies + uses: ramsey/composer-install@v3 + with: + ignore-cache: "yes" + composer-options: "--optimize-autoloader" + + - name: Run PHPUnit tests + run: php ./vendor/bin/phpunit -c ./phpunit.xml.dist diff --git a/.github/workflows/phpunit-tests.yml b/.github/workflows/phpunit-tests.yml new file mode 100644 index 00000000..19e770a2 --- /dev/null +++ b/.github/workflows/phpunit-tests.yml @@ -0,0 +1,28 @@ +name: PHPUnit Tests + +on: + push: + branches: + - main + pull_request: + +jobs: + test: + name: PHP ${{ matrix.php }} + uses: ./.github/workflows/phpunit-tests-run.yml + permissions: + contents: read + secrets: inherit + strategy: + fail-fast: false + matrix: + os: [ ubuntu-latest ] + # NOTE: There does not appear to be a single phpunit version that supports all + # PHP versions tested here. For now, we are removing PHP 7.0. and 7.1 tests + # in order to run a single phpunit version for PHP 7.2 and up. + php: [ '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3' ] + + with: + os: ${{ matrix.os }} + php: ${{ matrix.php }} + phpunit-config: ${{ 'phpunit.xml.dist' }} diff --git a/composer.json b/composer.json index d889d0db..fb6b2899 100644 --- a/composer.json +++ b/composer.json @@ -8,15 +8,16 @@ "issues": "https://github.com/wordpress/sqlite-database-integration/issues" }, "require": { - "php": ">=5.6" + "php": ">=7.0" }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0", "squizlabs/php_codesniffer": "^3.7", "wp-coding-standards/wpcs": "^3.1", - "yoast/phpunit-polyfills": "^1.0.1", "phpcompatibility/phpcompatibility-wp": "*", - "php-parallel-lint/php-parallel-lint": "^1.3" + "php-parallel-lint/php-parallel-lint": "^1.3", + "yoast/phpunit-polyfills": "2.0.0", + "phpunit/phpunit": "8.5.38" }, "config": { "allow-plugins": { @@ -30,6 +31,9 @@ ], "fix-cs": [ "@php ./vendor/bin/phpcbf" + ], + "test": [ + "phpunit" ] } } diff --git a/tests/WP_SQLite_Metadata_Tests.php b/tests/WP_SQLite_Metadata_Tests.php index dd1d3f1a..535fd222 100644 --- a/tests/WP_SQLite_Metadata_Tests.php +++ b/tests/WP_SQLite_Metadata_Tests.php @@ -24,6 +24,7 @@ public static function setUpBeforeClass(): void { $GLOBALS['wpdb']->suppress_errors = false; $GLOBALS['wpdb']->show_errors = true; } + return; } // Before each test, we create a new database diff --git a/tests/WP_SQLite_PDO_User_Defined_Functions_Tests.php b/tests/WP_SQLite_PDO_User_Defined_Functions_Tests.php index d74715b5..9824148d 100644 --- a/tests/WP_SQLite_PDO_User_Defined_Functions_Tests.php +++ b/tests/WP_SQLite_PDO_User_Defined_Functions_Tests.php @@ -18,7 +18,7 @@ public function testFieldFunction( $expected, $args ) { ); } - public function dataProviderForTestFieldFunction() { + public static function dataProviderForTestFieldFunction() { return array( array( 1, array( 'a', 'a' ) ), array( 2, array( 'User 0000019', 'User 0000018', 'User 0000019', 'User 0000020' ) ), diff --git a/tests/WP_SQLite_Query_Tests.php b/tests/WP_SQLite_Query_Tests.php index 8da2e96e..059fed99 100644 --- a/tests/WP_SQLite_Query_Tests.php +++ b/tests/WP_SQLite_Query_Tests.php @@ -27,6 +27,7 @@ public static function setUpBeforeClass(): void { $GLOBALS['wpdb']->suppress_errors = false; $GLOBALS['wpdb']->show_errors = true; } + return; } /** diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index f30c6f6b..95b8db21 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -24,6 +24,7 @@ public static function setUpBeforeClass(): void { $GLOBALS['wpdb']->suppress_errors = false; $GLOBALS['wpdb']->show_errors = true; } + return; } // Before each test, we create a new database @@ -92,7 +93,7 @@ public function testRegexps( $operator, $regexp, $expected_result ) { ); } - public function regexpOperators() { + public static function regexpOperators() { $lowercase_rss = (object) array( 'ID' => '1', 'option_name' => 'rss_123', @@ -255,26 +256,27 @@ public function testSelectFromDual() { public function testShowCreateTable1() { $this->assertQuery( "CREATE TABLE _tmp_table ( - ID BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL, - option_name VARCHAR(255) default '', - option_value TEXT NOT NULL, - UNIQUE KEY option_name (option_name), - KEY composite (option_name, option_value) - );" + ID BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL, + option_name VARCHAR(255) default '', + option_value TEXT NOT NULL, + UNIQUE KEY option_name (option_name), + KEY composite (option_name, option_value) + );" ); $this->assertQuery( 'SHOW CREATE TABLE _tmp_table;' ); $results = $this->engine->get_query_results(); + # TODO: Should we fix mismatch with original `option_value` text NOT NULL,` without default? $this->assertEquals( "CREATE TABLE _tmp_table ( - `ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL, - `option_name` varchar(255) DEFAULT '', - `option_value` text NOT NULL, - KEY _tmp_table__composite (option_name, option_value), - UNIQUE KEY _tmp_table__option_name (option_name) - );", + `ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL, + `option_name` varchar(255) DEFAULT '', + `option_value` text NOT NULL DEFAULT '', + KEY _tmp_table__composite (option_name, option_value), + UNIQUE KEY _tmp_table__option_name (option_name) +);", $results[0]->{'Create Table'} ); } @@ -323,7 +325,7 @@ public function testShowCreateTableWithAlterAndCreateIndex() { 'CREATE TABLE _tmp_table ( `ID` bigint PRIMARY KEY AUTO_INCREMENT NOT NULL, `option_name` smallint NOT NULL DEFAULT 14, - `option_value` text NOT NULL, + `option_value` text NOT NULL DEFAULT \'\', KEY _tmp_table__option_name (option_name) );', $results[0]->{'Create Table'} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 6b8068bd..4aa40455 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -80,7 +80,7 @@ function str_contains( string $haystack, string $needle ) { * @return bool */ function str_ends_with( string $haystack, string $needle ) { - return empty( $needle ) || substr( $haystack, -strlen( $needle ) === $needle ); + return empty( $needle ) || substr( $haystack, -strlen( $needle ) ) === $needle; } } if ( extension_loaded( 'mbstring' ) ) { From f237134bf76d39f176cbac6629922ffd0e3ed025 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Wed, 1 May 2024 15:53:04 -0400 Subject: [PATCH 4/9] Tolerate selecting MySQL system variables (#109) The purpose of this PR is to avoid errors when selecting MySQL system variables. Fixes #104. Related to: https://github.com/WordPress/wordpress-playground/issues/1272 - "UpdraftPlus plugins doesn't fully work in Playground." --- tests/WP_SQLite_Translator_Tests.php | 29 +++++++++++++++++++ .../sqlite/class-wp-sqlite-translator.php | 5 ++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/WP_SQLite_Translator_Tests.php b/tests/WP_SQLite_Translator_Tests.php index 95b8db21..f0b7251e 100644 --- a/tests/WP_SQLite_Translator_Tests.php +++ b/tests/WP_SQLite_Translator_Tests.php @@ -2247,4 +2247,33 @@ public function testDefaultNullValue() { $result ); } + + /** + * @dataProvider mysqlVariablesToTest + */ + public function testSelectVariable( $variable_name ) { + // Make sure the query does not error + $this->assertQuery( "SELECT $variable_name;" ); + } + + public static function mysqlVariablesToTest() { + return array( + // NOTE: This list was derived from the variables used by the UpdraftPlus plugin. + // We will start here and plan to expand supported variables over time. + array( '@@character_set_client' ), + array( '@@character_set_results' ), + array( '@@collation_connection' ), + array( '@@GLOBAL.gtid_purged' ), + array( '@@GLOBAL.log_bin' ), + array( '@@GLOBAL.log_bin_trust_function_creators' ), + array( '@@GLOBAL.sql_mode' ), + array( '@@SESSION.max_allowed_packet' ), + array( '@@SESSION.sql_mode' ), + + // Intentionally mix letter casing to help demonstrate case-insensitivity + array( '@@cHarActer_Set_cLient' ), + array( '@@gLoBAL.gTiD_purGed' ), + array( '@@sEssIOn.sqL_moDe' ), + ); + } } diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index 6ef1eedb..542ee6cc 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -1453,8 +1453,9 @@ private function execute_select() { $updated_query = $this->get_information_schema_query( $updated_query ); $params = array(); } elseif ( - strpos( $updated_query, '@@SESSION.sql_mode' ) !== false - || strpos( $updated_query, 'CONVERT( ' ) !== false + // Examples: @@SESSION.sql_mode, @@GLOBAL.max_allowed_packet, @@character_set_client + preg_match( '/@@((SESSION|GLOBAL)\s*\.\s*)?\w+\b/i', $updated_query ) === 1 || + strpos( $updated_query, 'CONVERT( ' ) !== false ) { /* * If the query contains a function that is not supported by SQLite, From 915df6fea09cb4cb3ca0525f404a540d0acc4892 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 10 May 2024 10:21:14 +0300 Subject: [PATCH 5/9] indentation fix --- admin-page.php | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/admin-page.php b/admin-page.php index f0528e88..6a44c0e7 100644 --- a/admin-page.php +++ b/admin-page.php @@ -58,25 +58,25 @@ function sqlite_integration_admin_screen() {
-

+

+ ' . esc_html( basename( WP_CONTENT_DIR ) ) . '/db.php' + ); + ?> +

+
+ ' . esc_html( basename( WP_CONTENT_DIR ) ) . '/db.php' ); ?> -

- - - ' . esc_html( basename( WP_CONTENT_DIR ) ) . '/db.php' - ); - ?> - +

From 4b0dab50d559bb3c43e1b90310193691430e66b9 Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Fri, 10 May 2024 10:21:56 +0300 Subject: [PATCH 6/9] Add PHP polyfills --- load.php | 1 + php-polyfills.php | 54 +++++++++++++++++++++++++++++++++++++++++++++ tests/bootstrap.php | 2 +- 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 php-polyfills.php diff --git a/load.php b/load.php index a83daf06..785dbffa 100644 --- a/load.php +++ b/load.php @@ -14,6 +14,7 @@ define( 'SQLITE_MAIN_FILE', __FILE__ ); +require_once __DIR__ . '/php-polyfills.php'; require_once __DIR__ . '/admin-page.php'; require_once __DIR__ . '/activate.php'; require_once __DIR__ . '/deactivate.php'; diff --git a/php-polyfills.php b/php-polyfills.php new file mode 100644 index 00000000..89d6d1a7 --- /dev/null +++ b/php-polyfills.php @@ -0,0 +1,54 @@ + Date: Fri, 10 May 2024 10:24:58 +0300 Subject: [PATCH 7/9] v2.1.10 --- load.php | 2 +- readme.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/load.php b/load.php index 785dbffa..bd6d875b 100644 --- a/load.php +++ b/load.php @@ -3,7 +3,7 @@ * Plugin Name: SQLite Database Integration * Description: SQLite database driver drop-in. * Author: The WordPress Team - * Version: 2.1.9 + * Version: 2.1.10 * Requires PHP: 7.0 * Textdomain: sqlite-database-integration * diff --git a/readme.txt b/readme.txt index 282f0187..f94cfbc6 100644 --- a/readme.txt +++ b/readme.txt @@ -4,7 +4,7 @@ Contributors: wordpressdotorg, aristath Requires at least: 6.0 Tested up to: 6.4 Requires PHP: 5.6 -Stable tag: 2.1.9 +Stable tag: 2.1.10 License: GPLv2 or later License URI: https://www.gnu.org/licenses/gpl-2.0.html Tags: performance, database From 58f55b6390471e1445df77adcbd403b384888870 Mon Sep 17 00:00:00 2001 From: Bero Date: Fri, 17 May 2024 16:46:22 +0200 Subject: [PATCH 8/9] ON DUPLICATE KEY: Prevent adding a comma if key isn't in the first position (#113) [WordPress Playground observed an issu](https://github.com/WordPress/wordpress-playground/issues/731)e with insert queries that use `ON DUPLICATE KEY` and don't have the KEY as the first value of the insert. After investigating it it turns out that the condition for adding commas didn't work well in case the KEY value is in a "random" place. This PR ensures the comma is added for all items except for the last one. ## Testing instructions - Ensure tests pass --- tests/WP_SQLite_Query_Tests.php | 15 +++++++++++++++ wp-includes/sqlite/class-wp-sqlite-translator.php | 5 +++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/WP_SQLite_Query_Tests.php b/tests/WP_SQLite_Query_Tests.php index 059fed99..119f1f2f 100644 --- a/tests/WP_SQLite_Query_Tests.php +++ b/tests/WP_SQLite_Query_Tests.php @@ -505,6 +505,21 @@ public function testRecoverSerialized() { $this->assertEquals( $obj, $unserialized ); } + public function testOnDuplicateKey() { + $this->assertQuery( + 'CREATE TABLE `test` ( + `id` INT PRIMARY KEY, + `text` VARCHAR(255), + );' + ); + // The order is deliberate to test that the query works with the keys in any order. + $this->assertQuery( + 'INSERT INTO test (`text`, `id`) + VALUES ("test", 1) + ON DUPLICATE KEY UPDATE `text` = "test1"' + ); + } + public function testShowColumns() { $query = 'SHOW COLUMNS FROM wp_posts'; diff --git a/wp-includes/sqlite/class-wp-sqlite-translator.php b/wp-includes/sqlite/class-wp-sqlite-translator.php index 542ee6cc..05588427 100644 --- a/wp-includes/sqlite/class-wp-sqlite-translator.php +++ b/wp-includes/sqlite/class-wp-sqlite-translator.php @@ -2803,9 +2803,10 @@ private function translate_on_duplicate_key( $table_name ) { $this->rewriter->add( new WP_SQLite_Token( '(', WP_SQLite_Token::TYPE_OPERATOR ) ); $max = count( $conflict_columns ); - foreach ( $conflict_columns as $i => $conflict_column ) { + $i = 0; + foreach ( $conflict_columns as $conflict_column ) { $this->rewriter->add( new WP_SQLite_Token( '"' . $conflict_column . '"', WP_SQLite_Token::TYPE_KEYWORD, WP_SQLite_Token::FLAG_KEYWORD_KEY ) ); - if ( $i !== $max - 1 ) { + if ( ++$i < $max ) { $this->rewriter->add( new WP_SQLite_Token( ',', WP_SQLite_Token::TYPE_OPERATOR ) ); $this->rewriter->add( new WP_SQLite_Token( ' ', WP_SQLite_Token::TYPE_WHITESPACE ) ); } From e47669844e2c0f5d255e82b503d80526a7f8a9eb Mon Sep 17 00:00:00 2001 From: Ari Stathopoulos Date: Sat, 18 May 2024 11:56:03 +0300 Subject: [PATCH 9/9] v2.1.11 --- load.php | 2 +- readme.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/load.php b/load.php index bd6d875b..9402d91c 100644 --- a/load.php +++ b/load.php @@ -3,7 +3,7 @@ * Plugin Name: SQLite Database Integration * Description: SQLite database driver drop-in. * Author: The WordPress Team - * Version: 2.1.10 + * Version: 2.1.11 * Requires PHP: 7.0 * Textdomain: sqlite-database-integration * diff --git a/readme.txt b/readme.txt index f94cfbc6..72950e57 100644 --- a/readme.txt +++ b/readme.txt @@ -4,7 +4,7 @@ Contributors: wordpressdotorg, aristath Requires at least: 6.0 Tested up to: 6.4 Requires PHP: 5.6 -Stable tag: 2.1.10 +Stable tag: 2.1.11 License: GPLv2 or later License URI: https://www.gnu.org/licenses/gpl-2.0.html Tags: performance, database