From 0114c4d25f325784298c964c141a12cd6d60560d Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 18 Jan 2024 16:48:42 +0900 Subject: [PATCH 01/22] refactor: rename variable names --- system/Database/Forge.php | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 57e43bf199fe..c23ddfad1127 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -380,15 +380,15 @@ public function addField($field) } if (is_array($field)) { - foreach ($field as $idx => $f) { - if (is_string($f)) { - $this->addField($f); + foreach ($field as $name => $attributes) { + if (is_string($attributes)) { + $this->addField($attributes); continue; } - if (is_array($f)) { - $this->fields = array_merge($this->fields, [$idx => $f]); + if (is_array($attributes)) { + $this->fields = array_merge($this->fields, [$name => $attributes]); } } } @@ -727,8 +727,8 @@ public function addColumn(string $table, $field): bool $field = [$field]; } - foreach (array_keys($field) as $k) { - $this->addField([$k => $field[$k]]); + foreach (array_keys($field) as $name) { + $this->addField([$name => $field[$name]]); } $sqls = $this->_alterTable('ADD', $this->db->DBPrefix . $table, $this->_processFields()); @@ -783,8 +783,8 @@ public function modifyColumn(string $table, $field): bool $field = [$field]; } - foreach (array_keys($field) as $k) { - $this->addField([$k => $field[$k]]); + foreach (array_keys($field) as $name) { + $this->addField([$name => $field[$name]]); } if ($this->fields === []) { @@ -852,7 +852,7 @@ protected function _processFields(bool $createTable = false): array { $fields = []; - foreach ($this->fields as $key => $attributes) { + foreach ($this->fields as $name => $attributes) { if (! is_array($attributes)) { $fields[] = ['_literal' => $attributes]; @@ -870,7 +870,7 @@ protected function _processFields(bool $createTable = false): array } $field = [ - 'name' => $key, + 'name' => $name, 'new_name' => $attributes['NAME'] ?? null, 'type' => $attributes['TYPE'] ?? null, 'length' => '', @@ -1163,10 +1163,10 @@ protected function _processForeignKeys(string $table, bool $asQuery = false): ar { $errorNames = []; - foreach ($this->foreignKeys as $name) { - foreach ($name['field'] as $f) { - if (! isset($this->fields[$f])) { - $errorNames[] = $f; + foreach ($this->foreignKeys as $fkeyInfo) { + foreach ($fkeyInfo['field'] as $fieldName) { + if (! isset($this->fields[$fieldName])) { + $errorNames[] = $fieldName; } } } From 6c18480a7c8607eb5aef497b7e25c4c42250001e Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 18 Jan 2024 16:57:04 +0900 Subject: [PATCH 02/22] docs: make @var more detailed --- system/Database/Forge.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index c23ddfad1127..acb6b963bc46 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -32,7 +32,7 @@ class Forge /** * List of fields. * - * @var array + * @var array [name => attributes] */ protected $fields = []; @@ -351,7 +351,7 @@ public function addUniqueKey($key, string $keyName = '') /** * Add Field * - * @param array|string $field + * @param array|string $field * * @return Forge */ @@ -716,7 +716,7 @@ public function renameTable(string $tableName, string $newTableName) } /** - * @param array|string $field + * @param array|string $field * * @throws DatabaseException */ @@ -772,7 +772,7 @@ public function dropColumn(string $table, $columnName) } /** - * @param array|string $field + * @param array|string $field * * @throws DatabaseException */ From c452ad017bb3ffd369712608d9e60d84656c9b76 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 15:05:35 +0900 Subject: [PATCH 03/22] docs: add PHPDocs --- system/Database/Forge.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index acb6b963bc46..a8d9efc89c30 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -518,7 +518,9 @@ public function dropForeignKey(string $table, string $foreignName) } /** - * @return mixed + * @param array $attributes Table attributes + * + * @return bool * * @throws DatabaseException */ @@ -562,6 +564,8 @@ public function createTable(string $table, bool $ifNotExists = false, array $att } /** + * @param array $attributes Table attributes + * * @return string SQL string * * @deprecated $ifNotExists is no longer used, and will be removed. From add3e072e7b5376e1117956d36048b5c0a60949d Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 15:06:02 +0900 Subject: [PATCH 04/22] style: break long line --- system/Database/SQLite3/Forge.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index b1dcb1dd599b..3a004492244c 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -183,8 +183,11 @@ protected function _attributeType(array &$attributes) */ protected function _attributeAutoIncrement(array &$attributes, array &$field) { - if (! empty($attributes['AUTO_INCREMENT']) && $attributes['AUTO_INCREMENT'] === true - && stripos($field['type'], 'int') !== false) { + if ( + ! empty($attributes['AUTO_INCREMENT']) + && $attributes['AUTO_INCREMENT'] === true + && stripos($field['type'], 'int') !== false + ) { $field['type'] = 'INTEGER PRIMARY KEY'; $field['default'] = ''; $field['null'] = ''; From 4ac6ca513af97de726a31912f8c201e176841a4c Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 15:39:36 +0900 Subject: [PATCH 05/22] refactor: rename variable names Variables with the same meaning have the same variable name. Variables with different meanings are given different names. --- system/Database/Forge.php | 61 ++++++++++++++++--------------- system/Database/MySQLi/Forge.php | 24 ++++++------ system/Database/OCI8/Forge.php | 28 +++++++------- system/Database/Postgre/Forge.php | 14 +++---- system/Database/SQLSRV/Forge.php | 16 ++++---- system/Database/SQLite3/Forge.php | 20 +++++----- 6 files changed, 82 insertions(+), 81 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index a8d9efc89c30..74bb5c8fde93 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -572,22 +572,22 @@ public function createTable(string $table, bool $ifNotExists = false, array $att */ protected function _createTable(string $table, bool $ifNotExists, array $attributes) { - $columns = $this->_processFields(true); + $processedFields = $this->_processFields(true); - for ($i = 0, $c = count($columns); $i < $c; $i++) { - $columns[$i] = ($columns[$i]['_literal'] !== false) ? "\n\t" . $columns[$i]['_literal'] - : "\n\t" . $this->_processColumn($columns[$i]); + for ($i = 0, $c = count($processedFields); $i < $c; $i++) { + $processedFields[$i] = ($processedFields[$i]['_literal'] !== false) ? "\n\t" . $processedFields[$i]['_literal'] + : "\n\t" . $this->_processColumn($processedFields[$i]); } - $columns = implode(',', $columns); + $processedFields = implode(',', $processedFields); - $columns .= $this->_processPrimaryKeys($table); - $columns .= current($this->_processForeignKeys($table)); + $processedFields .= $this->_processPrimaryKeys($table); + $processedFields .= current($this->_processForeignKeys($table)); if ($this->createTableKeys === true) { $indexes = current($this->_processIndexes($table)); if (is_string($indexes)) { - $columns .= $indexes; + $processedFields .= $indexes; } } @@ -595,7 +595,7 @@ protected function _createTable(string $table, bool $ifNotExists, array $attribu $this->createTableStr . '%s', 'CREATE TABLE', $this->db->escapeIdentifiers($table), - $columns, + $processedFields, $this->_createTableAttributes($attributes) ); } @@ -817,30 +817,31 @@ public function modifyColumn(string $table, $field): bool } /** - * @param array|string $fields + * @param 'ADD'|'CHANGE'|'DROP' $alterType + * @param array|string $processedFields Processed column definitions * * @return false|string|string[] */ - protected function _alterTable(string $alterType, string $table, $fields) + protected function _alterTable(string $alterType, string $table, $processedFields) { $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table) . ' '; // DROP has everything it needs now. if ($alterType === 'DROP') { - if (is_string($fields)) { - $fields = explode(',', $fields); + if (is_string($processedFields)) { + $processedFields = explode(',', $processedFields); } - $fields = array_map(fn ($field) => 'DROP COLUMN ' . $this->db->escapeIdentifiers(trim($field)), $fields); + $processedFields = array_map(fn ($field) => 'DROP COLUMN ' . $this->db->escapeIdentifiers(trim($field)), $processedFields); - return $sql . implode(', ', $fields); + return $sql . implode(', ', $processedFields); } $sql .= ($alterType === 'ADD') ? 'ADD ' : $alterType . ' COLUMN '; $sqls = []; - foreach ($fields as $data) { + foreach ($processedFields as $data) { $sqls[] = $sql . ($data['_literal'] !== false ? $data['_literal'] : $this->_processColumn($data)); @@ -850,15 +851,15 @@ protected function _alterTable(string $alterType, string $table, $fields) } /** - * Process fields + * Returns $processedFields array from $this->fields data. */ protected function _processFields(bool $createTable = false): array { - $fields = []; + $processedFields = []; foreach ($this->fields as $name => $attributes) { if (! is_array($attributes)) { - $fields[] = ['_literal' => $attributes]; + $processedFields[] = ['_literal' => $attributes]; continue; } @@ -932,24 +933,24 @@ protected function _processFields(bool $createTable = false): array $field['length'] = '(' . $attributes['CONSTRAINT'] . ')'; } - $fields[] = $field; + $processedFields[] = $field; } - return $fields; + return $processedFields; } /** - * Process column + * Converts $processedField array to field definition string. */ - protected function _processColumn(array $field): string + protected function _processColumn(array $processedField): string { - return $this->db->escapeIdentifiers($field['name']) - . ' ' . $field['type'] . $field['length'] - . $field['unsigned'] - . $field['default'] - . $field['null'] - . $field['auto_increment'] - . $field['unique']; + return $this->db->escapeIdentifiers($processedField['name']) + . ' ' . $processedField['type'] . $processedField['length'] + . $processedField['unsigned'] + . $processedField['default'] + . $processedField['null'] + . $processedField['auto_increment'] + . $processedField['unique']; } /** diff --git a/system/Database/MySQLi/Forge.php b/system/Database/MySQLi/Forge.php index 0d294c2d09a2..1b4f2f6b6130 100644 --- a/system/Database/MySQLi/Forge.php +++ b/system/Database/MySQLi/Forge.php @@ -162,23 +162,23 @@ protected function _alterTable(string $alterType, string $table, $field) /** * Process column */ - protected function _processColumn(array $field): string + protected function _processColumn(array $processedField): string { - $extraClause = isset($field['after']) ? ' AFTER ' . $this->db->escapeIdentifiers($field['after']) : ''; + $extraClause = isset($processedField['after']) ? ' AFTER ' . $this->db->escapeIdentifiers($processedField['after']) : ''; - if (empty($extraClause) && isset($field['first']) && $field['first'] === true) { + if (empty($extraClause) && isset($processedField['first']) && $processedField['first'] === true) { $extraClause = ' FIRST'; } - return $this->db->escapeIdentifiers($field['name']) - . (empty($field['new_name']) ? '' : ' ' . $this->db->escapeIdentifiers($field['new_name'])) - . ' ' . $field['type'] . $field['length'] - . $field['unsigned'] - . $field['null'] - . $field['default'] - . $field['auto_increment'] - . $field['unique'] - . (empty($field['comment']) ? '' : ' COMMENT ' . $field['comment']) + return $this->db->escapeIdentifiers($processedField['name']) + . (empty($processedField['new_name']) ? '' : ' ' . $this->db->escapeIdentifiers($processedField['new_name'])) + . ' ' . $processedField['type'] . $processedField['length'] + . $processedField['unsigned'] + . $processedField['null'] + . $processedField['default'] + . $processedField['auto_increment'] + . $processedField['unique'] + . (empty($processedField['comment']) ? '' : ' COMMENT ' . $processedField['comment']) . $extraClause; } diff --git a/system/Database/OCI8/Forge.php b/system/Database/OCI8/Forge.php index 6e1e85f666a1..33f43ebf3bf3 100644 --- a/system/Database/OCI8/Forge.php +++ b/system/Database/OCI8/Forge.php @@ -184,26 +184,26 @@ protected function _attributeAutoIncrement(array &$attributes, array &$field) /** * Process column */ - protected function _processColumn(array $field): string + protected function _processColumn(array $processedField): string { $constraint = ''; // @todo: can't cover multi pattern when set type. - if ($field['type'] === 'VARCHAR2' && strpos($field['length'], "('") === 0) { - $constraint = ' CHECK(' . $this->db->escapeIdentifiers($field['name']) - . ' IN ' . $field['length'] . ')'; + if ($processedField['type'] === 'VARCHAR2' && strpos($processedField['length'], "('") === 0) { + $constraint = ' CHECK(' . $this->db->escapeIdentifiers($processedField['name']) + . ' IN ' . $processedField['length'] . ')'; - $field['length'] = '(' . max(array_map('mb_strlen', explode("','", mb_substr($field['length'], 2, -2)))) . ')' . $constraint; - } elseif (isset($this->primaryKeys['fields']) && count($this->primaryKeys['fields']) === 1 && $field['name'] === $this->primaryKeys['fields'][0]) { - $field['unique'] = ''; + $processedField['length'] = '(' . max(array_map('mb_strlen', explode("','", mb_substr($processedField['length'], 2, -2)))) . ')' . $constraint; + } elseif (isset($this->primaryKeys['fields']) && count($this->primaryKeys['fields']) === 1 && $processedField['name'] === $this->primaryKeys['fields'][0]) { + $processedField['unique'] = ''; } - return $this->db->escapeIdentifiers($field['name']) - . ' ' . $field['type'] . $field['length'] - . $field['unsigned'] - . $field['default'] - . $field['auto_increment'] - . $field['null'] - . $field['unique']; + return $this->db->escapeIdentifiers($processedField['name']) + . ' ' . $processedField['type'] . $processedField['length'] + . $processedField['unsigned'] + . $processedField['default'] + . $processedField['auto_increment'] + . $processedField['null'] + . $processedField['unique']; } /** diff --git a/system/Database/Postgre/Forge.php b/system/Database/Postgre/Forge.php index 47be1b063c8d..60eb7aba0586 100644 --- a/system/Database/Postgre/Forge.php +++ b/system/Database/Postgre/Forge.php @@ -134,14 +134,14 @@ protected function _alterTable(string $alterType, string $table, $field) /** * Process column */ - protected function _processColumn(array $field): string + protected function _processColumn(array $processedField): string { - return $this->db->escapeIdentifiers($field['name']) - . ' ' . $field['type'] . ($field['type'] === 'text' ? '' : $field['length']) - . $field['default'] - . $field['null'] - . $field['auto_increment'] - . $field['unique']; + return $this->db->escapeIdentifiers($processedField['name']) + . ' ' . $processedField['type'] . ($processedField['type'] === 'text' ? '' : $processedField['length']) + . $processedField['default'] + . $processedField['null'] + . $processedField['auto_increment'] + . $processedField['unique']; } /** diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index ff1ce2e59bb9..9d7ab764e17a 100755 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -287,16 +287,16 @@ protected function _processIndexes(string $table, bool $asQuery = false): array /** * Process column */ - protected function _processColumn(array $field): string + protected function _processColumn(array $processedField): string { - return $this->db->escapeIdentifiers($field['name']) - . (empty($field['new_name']) ? '' : ' ' . $this->db->escapeIdentifiers($field['new_name'])) - . ' ' . $field['type'] . ($field['type'] === 'text' ? '' : $field['length']) - . $field['default'] - . $field['null'] - . $field['auto_increment'] + return $this->db->escapeIdentifiers($processedField['name']) + . (empty($processedField['new_name']) ? '' : ' ' . $this->db->escapeIdentifiers($processedField['new_name'])) + . ' ' . $processedField['type'] . ($processedField['type'] === 'text' ? '' : $processedField['length']) + . $processedField['default'] + . $processedField['null'] + . $processedField['auto_increment'] . '' - . $field['unique']; + . $processedField['unique']; } /** diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index 3a004492244c..eaa6643a85ab 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -141,19 +141,19 @@ protected function _alterTable(string $alterType, string $table, $field) /** * Process column */ - protected function _processColumn(array $field): string + protected function _processColumn(array $processedField): string { - if ($field['type'] === 'TEXT' && strpos($field['length'], "('") === 0) { - $field['type'] .= ' CHECK(' . $this->db->escapeIdentifiers($field['name']) - . ' IN ' . $field['length'] . ')'; + if ($processedField['type'] === 'TEXT' && strpos($processedField['length'], "('") === 0) { + $processedField['type'] .= ' CHECK(' . $this->db->escapeIdentifiers($processedField['name']) + . ' IN ' . $processedField['length'] . ')'; } - return $this->db->escapeIdentifiers($field['name']) - . ' ' . $field['type'] - . $field['auto_increment'] - . $field['null'] - . $field['unique'] - . $field['default']; + return $this->db->escapeIdentifiers($processedField['name']) + . ' ' . $processedField['type'] + . $processedField['auto_increment'] + . $processedField['null'] + . $processedField['unique'] + . $processedField['default']; } /** From f34fc584d94a5608d7ae9d3e46ff65e41883ff4e Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:00:17 +0900 Subject: [PATCH 06/22] refactor: rename variable names when DROP column --- system/Database/Forge.php | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 74bb5c8fde93..3028e0e571cb 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -755,15 +755,16 @@ public function addColumn(string $table, $field): bool } /** - * @param array|string $columnName + * @param array|string $columnNames column names to DROP * - * @return mixed + * @return bool * * @throws DatabaseException */ - public function dropColumn(string $table, $columnName) + public function dropColumn(string $table, $columnNames) { - $sql = $this->_alterTable('DROP', $this->db->DBPrefix . $table, $columnName); + $sql = $this->_alterTable('DROP', $this->db->DBPrefix . $table, $columnNames); + if ($sql === false) { if ($this->db->DBDebug) { throw new DatabaseException('This feature is not available for the database you are using.'); @@ -819,8 +820,10 @@ public function modifyColumn(string $table, $field): bool /** * @param 'ADD'|'CHANGE'|'DROP' $alterType * @param array|string $processedFields Processed column definitions + * or column names to DROP * - * @return false|string|string[] + * @return list|string SQL string + * @phpstan-return ($alterType is 'DROP' ? string : list) */ protected function _alterTable(string $alterType, string $table, $processedFields) { @@ -828,13 +831,15 @@ protected function _alterTable(string $alterType, string $table, $processedField // DROP has everything it needs now. if ($alterType === 'DROP') { - if (is_string($processedFields)) { - $processedFields = explode(',', $processedFields); + $columnNamesToDrop = $processedFields; + + if (is_string($columnNamesToDrop)) { + $columnNamesToDrop = explode(',', $columnNamesToDrop); } - $processedFields = array_map(fn ($field) => 'DROP COLUMN ' . $this->db->escapeIdentifiers(trim($field)), $processedFields); + $columnNamesToDrop = array_map(fn ($field) => 'DROP COLUMN ' . $this->db->escapeIdentifiers(trim($field)), $columnNamesToDrop); - return $sql . implode(', ', $processedFields); + return $sql . implode(', ', $columnNamesToDrop); } $sql .= ($alterType === 'ADD') ? 'ADD ' : $alterType . ' COLUMN '; From 8924cb0a10f15995907837416c81a44676b4c6dd Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:16:03 +0900 Subject: [PATCH 07/22] refactor: rename parameter name in _alterTable() --- system/Database/MySQLi/Forge.php | 26 ++++++++------- system/Database/OCI8/Forge.php | 55 ++++++++++++++++--------------- system/Database/Postgre/Forge.php | 12 ++++--- system/Database/SQLSRV/Forge.php | 22 +++++++------ system/Database/SQLite3/Forge.php | 16 +++++---- 5 files changed, 71 insertions(+), 60 deletions(-) diff --git a/system/Database/MySQLi/Forge.php b/system/Database/MySQLi/Forge.php index 1b4f2f6b6130..1b0ad79e7607 100644 --- a/system/Database/MySQLi/Forge.php +++ b/system/Database/MySQLi/Forge.php @@ -128,35 +128,37 @@ protected function _createTableAttributes(array $attributes): string /** * ALTER TABLE * - * @param string $alterType ALTER type - * @param string $table Table name - * @param array|string $field Column definition + * @param string $alterType ALTER type + * @param string $table Table name + * @param array|string $processedFields Processed column definitions + * or column names to DROP * - * @return string|string[] + * @return list|string SQL string + * @phpstan-return ($alterType is 'DROP' ? string : list) */ - protected function _alterTable(string $alterType, string $table, $field) + protected function _alterTable(string $alterType, string $table, $processedFields) { if ($alterType === 'DROP') { - return parent::_alterTable($alterType, $table, $field); + return parent::_alterTable($alterType, $table, $processedFields); } $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); - foreach ($field as $i => $data) { + foreach ($processedFields as $i => $data) { if ($data['_literal'] !== false) { - $field[$i] = ($alterType === 'ADD') ? "\n\tADD " . $data['_literal'] : "\n\tMODIFY " . $data['_literal']; + $processedFields[$i] = ($alterType === 'ADD') ? "\n\tADD " . $data['_literal'] : "\n\tMODIFY " . $data['_literal']; } else { if ($alterType === 'ADD') { - $field[$i]['_literal'] = "\n\tADD "; + $processedFields[$i]['_literal'] = "\n\tADD "; } else { - $field[$i]['_literal'] = empty($data['new_name']) ? "\n\tMODIFY " : "\n\tCHANGE "; + $processedFields[$i]['_literal'] = empty($data['new_name']) ? "\n\tMODIFY " : "\n\tCHANGE "; } - $field[$i] = $field[$i]['_literal'] . $this->_processColumn($field[$i]); + $processedFields[$i] = $processedFields[$i]['_literal'] . $this->_processColumn($processedFields[$i]); } } - return [$sql . implode(',', $field)]; + return [$sql . implode(',', $processedFields)]; } /** diff --git a/system/Database/OCI8/Forge.php b/system/Database/OCI8/Forge.php index 33f43ebf3bf3..e7688ff22f47 100644 --- a/system/Database/OCI8/Forge.php +++ b/system/Database/OCI8/Forge.php @@ -93,21 +93,24 @@ class Forge extends BaseForge /** * ALTER TABLE * - * @param string $alterType ALTER type - * @param string $table Table name - * @param array|string $field Column definition + * @param string $alterType ALTER type + * @param string $table Table name + * @param array|string $processedFields Processed column definitions + * or column names to DROP * - * @return string|string[] + * @return list|string SQL string + * @phpstan-return ($alterType is 'DROP' ? string : list) */ - protected function _alterTable(string $alterType, string $table, $field) + protected function _alterTable(string $alterType, string $table, $processedFields) { $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); if ($alterType === 'DROP') { - $fields = array_map(fn ($field) => $this->db->escapeIdentifiers(trim($field)), is_string($field) ? explode(',', $field) : $field); + $fields = array_map(fn ($field) => $this->db->escapeIdentifiers(trim($processedFields)), is_string($processedFields) ? explode(',', $processedFields) : $processedFields); return $sql . ' DROP (' . implode(',', $fields) . ') CASCADE CONSTRAINT INVALIDATE'; } + if ($alterType === 'CHANGE') { $alterType = 'MODIFY'; } @@ -115,50 +118,50 @@ protected function _alterTable(string $alterType, string $table, $field) $nullableMap = array_column($this->db->getFieldData($table), 'nullable', 'name'); $sqls = []; - for ($i = 0, $c = count($field); $i < $c; $i++) { + for ($i = 0, $c = count($processedFields); $i < $c; $i++) { if ($alterType === 'MODIFY') { // If a null constraint is added to a column with a null constraint, // ORA-01451 will occur, // so add null constraint is used only when it is different from the current null constraint. // If a not null constraint is added to a column with a not null constraint, // ORA-01442 will occur. - $wantToAddNull = strpos($field[$i]['null'], ' NOT') === false; - $currentNullable = $nullableMap[$field[$i]['name']]; + $wantToAddNull = strpos($processedFields[$i]['null'], ' NOT') === false; + $currentNullable = $nullableMap[$processedFields[$i]['name']]; if ($wantToAddNull === true && $currentNullable === true) { - $field[$i]['null'] = ''; - } elseif ($field[$i]['null'] === '' && $currentNullable === false) { + $processedFields[$i]['null'] = ''; + } elseif ($processedFields[$i]['null'] === '' && $currentNullable === false) { // Nullable by default - $field[$i]['null'] = ' NULL'; + $processedFields[$i]['null'] = ' NULL'; } elseif ($wantToAddNull === false && $currentNullable === false) { - $field[$i]['null'] = ''; + $processedFields[$i]['null'] = ''; } } - if ($field[$i]['_literal'] !== false) { - $field[$i] = "\n\t" . $field[$i]['_literal']; + if ($processedFields[$i]['_literal'] !== false) { + $processedFields[$i] = "\n\t" . $processedFields[$i]['_literal']; } else { - $field[$i]['_literal'] = "\n\t" . $this->_processColumn($field[$i]); + $processedFields[$i]['_literal'] = "\n\t" . $this->_processColumn($processedFields[$i]); - if (! empty($field[$i]['comment'])) { + if (! empty($processedFields[$i]['comment'])) { $sqls[] = 'COMMENT ON COLUMN ' - . $this->db->escapeIdentifiers($table) . '.' . $this->db->escapeIdentifiers($field[$i]['name']) - . ' IS ' . $field[$i]['comment']; + . $this->db->escapeIdentifiers($table) . '.' . $this->db->escapeIdentifiers($processedFields[$i]['name']) + . ' IS ' . $processedFields[$i]['comment']; } - if ($alterType === 'MODIFY' && ! empty($field[$i]['new_name'])) { - $sqls[] = $sql . ' RENAME COLUMN ' . $this->db->escapeIdentifiers($field[$i]['name']) - . ' TO ' . $this->db->escapeIdentifiers($field[$i]['new_name']); + if ($alterType === 'MODIFY' && ! empty($processedFields[$i]['new_name'])) { + $sqls[] = $sql . ' RENAME COLUMN ' . $this->db->escapeIdentifiers($processedFields[$i]['name']) + . ' TO ' . $this->db->escapeIdentifiers($processedFields[$i]['new_name']); } - $field[$i] = "\n\t" . $field[$i]['_literal']; + $processedFields[$i] = "\n\t" . $processedFields[$i]['_literal']; } } $sql .= ' ' . $alterType . ' '; - $sql .= count($field) === 1 - ? $field[0] - : '(' . implode(',', $field) . ')'; + $sql .= count($processedFields) === 1 + ? $processedFields[0] + : '(' . implode(',', $processedFields) . ')'; // RENAME COLUMN must be executed after MODIFY array_unshift($sqls, $sql); diff --git a/system/Database/Postgre/Forge.php b/system/Database/Postgre/Forge.php index 60eb7aba0586..e3cf2ee63fab 100644 --- a/system/Database/Postgre/Forge.php +++ b/system/Database/Postgre/Forge.php @@ -81,20 +81,22 @@ protected function _createTableAttributes(array $attributes): string } /** - * @param array|string $field + * @param array|string $processedFields Processed column definitions + * or column names to DROP * - * @return array|bool|string + * @return false|list|string SQL string or false + * @phpstan-return ($alterType is 'DROP' ? string : list|false) */ - protected function _alterTable(string $alterType, string $table, $field) + protected function _alterTable(string $alterType, string $table, $processedFields) { if (in_array($alterType, ['DROP', 'ADD'], true)) { - return parent::_alterTable($alterType, $table, $field); + return parent::_alterTable($alterType, $table, $processedFields); } $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); $sqls = []; - foreach ($field as $data) { + foreach ($processedFields as $data) { if ($data['_literal'] !== false) { return false; } diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index 9d7ab764e17a..38740b57e640 100755 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -126,11 +126,13 @@ protected function _createTableAttributes(array $attributes): string } /** - * @param array|string $field + * @param array|string $processedFields Processed column definitions + * or column names to DROP * - * @return false|string|string[] + * @return list|string SQL string + * @phpstan-return ($alterType is 'DROP' ? string : list) */ - protected function _alterTable(string $alterType, string $table, $field) + protected function _alterTable(string $alterType, string $table, $processedFields) { // Handle DROP here if ($alterType === 'DROP') { @@ -138,11 +140,11 @@ protected function _alterTable(string $alterType, string $table, $field) $indexData = $this->db->getIndexData($table); foreach ($indexData as $index) { - if (is_string($field)) { - $field = explode(',', $field); + if (is_string($processedFields)) { + $processedFields = explode(',', $processedFields); } - $fld = array_intersect($field, $index->fields); + $fld = array_intersect($processedFields, $index->fields); // Drop index if field is part of an index if ($fld !== []) { @@ -153,7 +155,7 @@ protected function _alterTable(string $alterType, string $table, $field) $fullTable = $this->db->escapeIdentifiers($this->db->schema) . '.' . $this->db->escapeIdentifiers($table); // Drop default constraints - $fields = implode(',', $this->db->escape((array) $field)); + $fields = implode(',', $this->db->escape((array) $processedFields)); $sql = << 'COLUMN [' . trim($item) . ']', (array) $field); + $fields = array_map(static fn ($item) => 'COLUMN [' . trim($item) . ']', (array) $processedFields); return $sql . implode(',', $fields); } @@ -181,14 +183,14 @@ protected function _alterTable(string $alterType, string $table, $field) $sqls = []; if ($alterType === 'ADD') { - foreach ($field as $data) { + foreach ($processedFields as $data) { $sqls[] = $sql . ($data['_literal'] !== false ? $data['_literal'] : $this->_processColumn($data)); } return $sqls; } - foreach ($field as $data) { + foreach ($processedFields as $data) { if ($data['_literal'] !== false) { return false; } diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index eaa6643a85ab..e6d14440108b 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -109,32 +109,34 @@ public function dropDatabase(string $dbName): bool } /** - * @param array|string $field + * @param array|string $processedFields Processed column definitions + * or column names to DROP * * @return array|string|null + * @return list|string|null SQL string or null */ - protected function _alterTable(string $alterType, string $table, $field) + protected function _alterTable(string $alterType, string $table, $processedFields) { switch ($alterType) { case 'DROP': $sqlTable = new Table($this->db, $this); $sqlTable->fromTable($table) - ->dropColumn($field) + ->dropColumn($processedFields) ->run(); - return ''; + return ''; // Why empty string? case 'CHANGE': (new Table($this->db, $this)) ->fromTable($table) - ->modifyColumn($field) + ->modifyColumn($processedFields) ->run(); - return null; + return null; // Why null? default: - return parent::_alterTable($alterType, $table, $field); + return parent::_alterTable($alterType, $table, $processedFields); } } From 145e89a828f89581ff629e6bfbc52b9c13b62054 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:24:01 +0900 Subject: [PATCH 08/22] docs: add @TODO --- system/Database/SQLite3/Forge.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index e6d14440108b..abe13d0035ec 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -130,7 +130,7 @@ protected function _alterTable(string $alterType, string $table, $processedField case 'CHANGE': (new Table($this->db, $this)) ->fromTable($table) - ->modifyColumn($processedFields) + ->modifyColumn($processedFields) // @TODO Bug: should be NOT processed fields ->run(); return null; // Why null? From a33ff9c17b30e0bcb94ac16193d2ef6ba2b28d56 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:24:19 +0900 Subject: [PATCH 09/22] docs: add PHPDoc types --- system/Database/SQLite3/Table.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/system/Database/SQLite3/Table.php b/system/Database/SQLite3/Table.php index 77fd8699459f..1b1bf4f01c51 100644 --- a/system/Database/SQLite3/Table.php +++ b/system/Database/SQLite3/Table.php @@ -28,7 +28,7 @@ class Table /** * All of the fields this table represents. * - * @var array> + * @var array> [name => attributes] */ protected $fields = []; @@ -156,7 +156,7 @@ public function run(): bool /** * Drops columns from the table. * - * @param array|string $columns + * @param list|string $columns Column names to drop. * * @return Table */ @@ -180,6 +180,8 @@ public function dropColumn($columns) * Modifies a field, including changing data type, * renaming, etc. * + * @param array> $fields [name => attributes] + * * @return Table */ public function modifyColumn(array $fields) From 3f56cc55c45afe082e232a19ff32d7dedb12977c Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:29:52 +0900 Subject: [PATCH 10/22] refactor: replace $data with $field --- system/Database/Forge.php | 8 +++---- system/Database/MySQLi/Forge.php | 8 +++---- system/Database/Postgre/Forge.php | 32 +++++++++++++-------------- system/Database/SQLSRV/Forge.php | 36 +++++++++++++++---------------- 4 files changed, 42 insertions(+), 42 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 3028e0e571cb..4d5ef643c34b 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -846,10 +846,10 @@ protected function _alterTable(string $alterType, string $table, $processedField $sqls = []; - foreach ($processedFields as $data) { - $sqls[] = $sql . ($data['_literal'] !== false - ? $data['_literal'] - : $this->_processColumn($data)); + foreach ($processedFields as $field) { + $sqls[] = $sql . ($field['_literal'] !== false + ? $field['_literal'] + : $this->_processColumn($field)); } return $sqls; diff --git a/system/Database/MySQLi/Forge.php b/system/Database/MySQLi/Forge.php index 1b0ad79e7607..b1beba2a9dd2 100644 --- a/system/Database/MySQLi/Forge.php +++ b/system/Database/MySQLi/Forge.php @@ -144,14 +144,14 @@ protected function _alterTable(string $alterType, string $table, $processedField $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); - foreach ($processedFields as $i => $data) { - if ($data['_literal'] !== false) { - $processedFields[$i] = ($alterType === 'ADD') ? "\n\tADD " . $data['_literal'] : "\n\tMODIFY " . $data['_literal']; + foreach ($processedFields as $i => $field) { + if ($field['_literal'] !== false) { + $processedFields[$i] = ($alterType === 'ADD') ? "\n\tADD " . $field['_literal'] : "\n\tMODIFY " . $field['_literal']; } else { if ($alterType === 'ADD') { $processedFields[$i]['_literal'] = "\n\tADD "; } else { - $processedFields[$i]['_literal'] = empty($data['new_name']) ? "\n\tMODIFY " : "\n\tCHANGE "; + $processedFields[$i]['_literal'] = empty($field['new_name']) ? "\n\tMODIFY " : "\n\tCHANGE "; } $processedFields[$i] = $processedFields[$i]['_literal'] . $this->_processColumn($processedFields[$i]); diff --git a/system/Database/Postgre/Forge.php b/system/Database/Postgre/Forge.php index e3cf2ee63fab..ee7583642684 100644 --- a/system/Database/Postgre/Forge.php +++ b/system/Database/Postgre/Forge.php @@ -96,37 +96,37 @@ protected function _alterTable(string $alterType, string $table, $processedField $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); $sqls = []; - foreach ($processedFields as $data) { - if ($data['_literal'] !== false) { + foreach ($processedFields as $field) { + if ($field['_literal'] !== false) { return false; } - if (version_compare($this->db->getVersion(), '8', '>=') && isset($data['type'])) { - $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($data['name']) - . " TYPE {$data['type']}{$data['length']}"; + if (version_compare($this->db->getVersion(), '8', '>=') && isset($field['type'])) { + $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($field['name']) + . " TYPE {$field['type']}{$field['length']}"; } - if (! empty($data['default'])) { - $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($data['name']) - . " SET DEFAULT {$data['default']}"; + if (! empty($field['default'])) { + $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($field['name']) + . " SET DEFAULT {$field['default']}"; } $nullable = true; // Nullable by default. - if (isset($data['null']) && ($data['null'] === false || $data['null'] === ' NOT ' . $this->null)) { + if (isset($field['null']) && ($field['null'] === false || $field['null'] === ' NOT ' . $this->null)) { $nullable = false; } - $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($data['name']) + $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($field['name']) . ($nullable === true ? ' DROP' : ' SET') . ' NOT NULL'; - if (! empty($data['new_name'])) { - $sqls[] = $sql . ' RENAME COLUMN ' . $this->db->escapeIdentifiers($data['name']) - . ' TO ' . $this->db->escapeIdentifiers($data['new_name']); + if (! empty($field['new_name'])) { + $sqls[] = $sql . ' RENAME COLUMN ' . $this->db->escapeIdentifiers($field['name']) + . ' TO ' . $this->db->escapeIdentifiers($field['new_name']); } - if (! empty($data['comment'])) { + if (! empty($field['comment'])) { $sqls[] = 'COMMENT ON COLUMN' . $this->db->escapeIdentifiers($table) - . '.' . $this->db->escapeIdentifiers($data['name']) - . " IS {$data['comment']}"; + . '.' . $this->db->escapeIdentifiers($field['name']) + . " IS {$field['comment']}"; } } diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index 38740b57e640..1e6db4b3c081 100755 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -183,45 +183,45 @@ protected function _alterTable(string $alterType, string $table, $processedField $sqls = []; if ($alterType === 'ADD') { - foreach ($processedFields as $data) { - $sqls[] = $sql . ($data['_literal'] !== false ? $data['_literal'] : $this->_processColumn($data)); + foreach ($processedFields as $field) { + $sqls[] = $sql . ($field['_literal'] !== false ? $field['_literal'] : $this->_processColumn($field)); } return $sqls; } - foreach ($processedFields as $data) { - if ($data['_literal'] !== false) { + foreach ($processedFields as $field) { + if ($field['_literal'] !== false) { return false; } - if (isset($data['type'])) { - $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($data['name']) - . " {$data['type']}{$data['length']}"; + if (isset($field['type'])) { + $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($field['name']) + . " {$field['type']}{$field['length']}"; } - if (! empty($data['default'])) { - $sqls[] = $sql . ' ALTER COLUMN ADD CONSTRAINT ' . $this->db->escapeIdentifiers($data['name']) . '_def' - . " DEFAULT {$data['default']} FOR " . $this->db->escapeIdentifiers($data['name']); + if (! empty($field['default'])) { + $sqls[] = $sql . ' ALTER COLUMN ADD CONSTRAINT ' . $this->db->escapeIdentifiers($field['name']) . '_def' + . " DEFAULT {$field['default']} FOR " . $this->db->escapeIdentifiers($field['name']); } $nullable = true; // Nullable by default. - if (isset($data['null']) && ($data['null'] === false || $data['null'] === ' NOT ' . $this->null)) { + if (isset($field['null']) && ($field['null'] === false || $field['null'] === ' NOT ' . $this->null)) { $nullable = false; } - $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($data['name']) - . " {$data['type']}{$data['length']} " . ($nullable === true ? '' : 'NOT') . ' NULL'; + $sqls[] = $sql . ' ALTER COLUMN ' . $this->db->escapeIdentifiers($field['name']) + . " {$field['type']}{$field['length']} " . ($nullable === true ? '' : 'NOT') . ' NULL'; - if (! empty($data['comment'])) { + if (! empty($field['comment'])) { $sqls[] = 'EXEC sys.sp_addextendedproperty ' - . "@name=N'Caption', @value=N'" . $data['comment'] . "' , " + . "@name=N'Caption', @value=N'" . $field['comment'] . "' , " . "@level0type=N'SCHEMA',@level0name=N'" . $this->db->schema . "', " . "@level1type=N'TABLE',@level1name=N'" . $this->db->escapeIdentifiers($table) . "', " - . "@level2type=N'COLUMN',@level2name=N'" . $this->db->escapeIdentifiers($data['name']) . "'"; + . "@level2type=N'COLUMN',@level2name=N'" . $this->db->escapeIdentifiers($field['name']) . "'"; } - if (! empty($data['new_name'])) { - $sqls[] = "EXEC sp_rename '[" . $this->db->schema . '].[' . $table . '].[' . $data['name'] . "]' , '" . $data['new_name'] . "', 'COLUMN';"; + if (! empty($field['new_name'])) { + $sqls[] = "EXEC sp_rename '[" . $this->db->schema . '].[' . $table . '].[' . $field['name'] . "]' , '" . $field['new_name'] . "', 'COLUMN';"; } } From 601ec5895148b2d8d501eed2f31df8cce19d027d Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:39:25 +0900 Subject: [PATCH 11/22] docs: fix @phpstan-return --- phpstan-baseline.php | 10 ---------- system/Database/Forge.php | 4 ++-- system/Database/SQLSRV/Forge.php | 4 ++-- system/Database/SQLite3/Forge.php | 1 + 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 9aa75c0b204b..48585d6fd969 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -1451,11 +1451,6 @@ 'count' => 1, 'path' => __DIR__ . '/system/Database/Postgre/Forge.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Return type \\(array\\|bool\\|string\\) of method CodeIgniter\\\\Database\\\\Postgre\\\\Forge\\:\\:_alterTable\\(\\) should be covariant with return type \\(array\\\\|string\\|false\\) of method CodeIgniter\\\\Database\\\\Forge\\:\\:_alterTable\\(\\)$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Database/Postgre/Forge.php', -]; $ignoreErrors[] = [ 'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#', 'count' => 1, @@ -1656,11 +1651,6 @@ 'count' => 1, 'path' => __DIR__ . '/system/Database/SQLite3/Forge.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Return type \\(array\\|string\\|null\\) of method CodeIgniter\\\\Database\\\\SQLite3\\\\Forge\\:\\:_alterTable\\(\\) should be covariant with return type \\(array\\\\|string\\|false\\) of method CodeIgniter\\\\Database\\\\Forge\\:\\:_alterTable\\(\\)$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Database/SQLite3/Forge.php', -]; $ignoreErrors[] = [ 'message' => '#^Return type \\(SQLite3Result\\|false\\) of method CodeIgniter\\\\Database\\\\SQLite3\\\\PreparedQuery\\:\\:_getResult\\(\\) should be covariant with return type \\(object\\|resource\\|null\\) of method CodeIgniter\\\\Database\\\\BasePreparedQuery\\\\:\\:_getResult\\(\\)$#', 'count' => 1, diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 4d5ef643c34b..497185aba6ec 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -822,8 +822,8 @@ public function modifyColumn(string $table, $field): bool * @param array|string $processedFields Processed column definitions * or column names to DROP * - * @return list|string SQL string - * @phpstan-return ($alterType is 'DROP' ? string : list) + * @return false|list|string|null SQL string + * @phpstan-return ($alterType is 'DROP' ? string : list|false|null) */ protected function _alterTable(string $alterType, string $table, $processedFields) { diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index 1e6db4b3c081..08e7952e7a0b 100755 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -129,8 +129,8 @@ protected function _createTableAttributes(array $attributes): string * @param array|string $processedFields Processed column definitions * or column names to DROP * - * @return list|string SQL string - * @phpstan-return ($alterType is 'DROP' ? string : list) + * @return false|list|string SQL string or false + * @phpstan-return ($alterType is 'DROP' ? string : list|false) */ protected function _alterTable(string $alterType, string $table, $processedFields) { diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index abe13d0035ec..3e34fa535f85 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -114,6 +114,7 @@ public function dropDatabase(string $dbName): bool * * @return array|string|null * @return list|string|null SQL string or null + * @phpstan-return ($alterType is 'DROP' ? string : list|false|null) */ protected function _alterTable(string $alterType, string $table, $processedFields) { From e14ca8be383a89022a94bda3bf48c6367058c8b8 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 16:58:04 +0900 Subject: [PATCH 12/22] style: break long line --- system/Database/OCI8/Forge.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/system/Database/OCI8/Forge.php b/system/Database/OCI8/Forge.php index e7688ff22f47..efef8245bb67 100644 --- a/system/Database/OCI8/Forge.php +++ b/system/Database/OCI8/Forge.php @@ -106,7 +106,10 @@ protected function _alterTable(string $alterType, string $table, $processedField $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); if ($alterType === 'DROP') { - $fields = array_map(fn ($field) => $this->db->escapeIdentifiers(trim($processedFields)), is_string($processedFields) ? explode(',', $processedFields) : $processedFields); + $fields = array_map( + fn ($field) => $this->db->escapeIdentifiers(trim($processedFields)), + is_string($processedFields) ? explode(',', $processedFields) : $processedFields + ); return $sql . ' DROP (' . implode(',', $fields) . ') CASCADE CONSTRAINT INVALIDATE'; } From bf30645d2f6d058dbed61312495d376d3fcced4f Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 17:01:08 +0900 Subject: [PATCH 13/22] fix: incorrect refactoring --- system/Database/OCI8/Forge.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/system/Database/OCI8/Forge.php b/system/Database/OCI8/Forge.php index efef8245bb67..232bdcf2721e 100644 --- a/system/Database/OCI8/Forge.php +++ b/system/Database/OCI8/Forge.php @@ -106,9 +106,11 @@ protected function _alterTable(string $alterType, string $table, $processedField $sql = 'ALTER TABLE ' . $this->db->escapeIdentifiers($table); if ($alterType === 'DROP') { + $columnNamesToDrop = $processedFields; + $fields = array_map( - fn ($field) => $this->db->escapeIdentifiers(trim($processedFields)), - is_string($processedFields) ? explode(',', $processedFields) : $processedFields + fn ($field) => $this->db->escapeIdentifiers(trim($field)), + is_string($columnNamesToDrop) ? explode(',', $columnNamesToDrop) : $columnNamesToDrop ); return $sql . ' DROP (' . implode(',', $fields) . ') CASCADE CONSTRAINT INVALIDATE'; From 4d930fbc441a9a3cc19df40266ee4f45a719b017 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 17:09:23 +0900 Subject: [PATCH 14/22] docs: fix @phpstan-return --- system/Database/SQLite3/Forge.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index 3e34fa535f85..f9b6b8af23c6 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -114,7 +114,7 @@ public function dropDatabase(string $dbName): bool * * @return array|string|null * @return list|string|null SQL string or null - * @phpstan-return ($alterType is 'DROP' ? string : list|false|null) + * @phpstan-return ($alterType is 'DROP' ? string : list|null) */ protected function _alterTable(string $alterType, string $table, $processedFields) { From 1dd2e965c33e8daf74b4f0a61d0389003eaa31d3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 18:40:01 +0900 Subject: [PATCH 15/22] refactor: rename parameter name in Table::modifyColumn() --- system/Database/SQLite3/Table.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/system/Database/SQLite3/Table.php b/system/Database/SQLite3/Table.php index 1b1bf4f01c51..620d28464738 100644 --- a/system/Database/SQLite3/Table.php +++ b/system/Database/SQLite3/Table.php @@ -177,16 +177,15 @@ public function dropColumn($columns) } /** - * Modifies a field, including changing data type, - * renaming, etc. + * Modifies a field, including changing data type, renaming, etc. * - * @param array> $fields [name => attributes] + * @param list> $fieldsToModify * * @return Table */ - public function modifyColumn(array $fields) + public function modifyColumn(array $fieldsToModify) { - foreach ($fields as $field) { + foreach ($fieldsToModify as $field) { $oldName = $field['name']; unset($field['name']); From f097aeb10faff92b0fca066d393e3cff329f3387 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 20:29:57 +0900 Subject: [PATCH 16/22] refactor: replace $field with $fields --- system/Database/Forge.php | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 497185aba6ec..5a3600e3388f 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -351,14 +351,14 @@ public function addUniqueKey($key, string $keyName = '') /** * Add Field * - * @param array|string $field + * @param array|string $fields Field array or Field string * * @return Forge */ - public function addField($field) + public function addField($fields) { - if (is_string($field)) { - if ($field === 'id') { + if (is_string($fields)) { + if ($fields === 'id') { $this->addField([ 'id' => [ 'type' => 'INT', @@ -368,19 +368,19 @@ public function addField($field) ]); $this->addKey('id', true); } else { - if (strpos($field, ' ') === false) { + if (strpos($fields, ' ') === false) { throw new InvalidArgumentException('Field information is required for that operation.'); } - $fieldName = explode(' ', $field, 2)[0]; + $fieldName = explode(' ', $fields, 2)[0]; $fieldName = trim($fieldName, '`\'"'); - $this->fields[$fieldName] = $field; + $this->fields[$fieldName] = $fields; } } - if (is_array($field)) { - foreach ($field as $name => $attributes) { + if (is_array($fields)) { + foreach ($fields as $name => $attributes) { if (is_string($attributes)) { $this->addField($attributes); @@ -720,19 +720,19 @@ public function renameTable(string $tableName, string $newTableName) } /** - * @param array|string $field + * @param array|string $fields Field array or Field string * * @throws DatabaseException */ - public function addColumn(string $table, $field): bool + public function addColumn(string $table, $fields): bool { // Work-around for literal column definitions if (! is_array($field)) { - $field = [$field]; + $fields = [$fields]; } - foreach (array_keys($field) as $name) { - $this->addField([$name => $field[$name]]); + foreach (array_keys($fields) as $name) { + $this->addField([$name => $fields[$name]]); } $sqls = $this->_alterTable('ADD', $this->db->DBPrefix . $table, $this->_processFields()); @@ -777,19 +777,19 @@ public function dropColumn(string $table, $columnNames) } /** - * @param array|string $field + * @param array|string $fields Field array or Field string * * @throws DatabaseException */ - public function modifyColumn(string $table, $field): bool + public function modifyColumn(string $table, $fields): bool { // Work-around for literal column definitions if (! is_array($field)) { - $field = [$field]; + $fields = [$fields]; } - foreach (array_keys($field) as $name) { - $this->addField([$name => $field[$name]]); + foreach (array_keys($fields) as $name) { + $this->addField([$name => $fields[$name]]); } if ($this->fields === []) { From b9c063f593c5217407d8952c1cdf0240daebb5bb Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 20:30:31 +0900 Subject: [PATCH 17/22] style: break long line --- system/Database/Forge.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 5a3600e3388f..b42995f90a05 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -404,8 +404,14 @@ public function addField($fields) * * @throws DatabaseException */ - public function addForeignKey($fieldName = '', string $tableName = '', $tableField = '', string $onUpdate = '', string $onDelete = '', string $fkName = ''): Forge - { + public function addForeignKey( + $fieldName = '', + string $tableName = '', + $tableField = '', + string $onUpdate = '', + string $onDelete = '', + string $fkName = '' + ): Forge { $fieldName = (array) $fieldName; $tableField = (array) $tableField; From 3d4e12b6b6e979fe5ccb634fd50acd038426d707 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 20:31:01 +0900 Subject: [PATCH 18/22] style: add empty lines --- system/Database/Forge.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index b42995f90a05..bf19980240ef 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -434,8 +434,9 @@ public function addForeignKey( */ public function dropKey(string $table, string $keyName, bool $prefixKeyName = true): bool { - $keyName = $this->db->escapeIdentifiers(($prefixKeyName === true ? $this->db->DBPrefix : '') . $keyName); - $table = $this->db->escapeIdentifiers($this->db->DBPrefix . $table); + $keyName = $this->db->escapeIdentifiers(($prefixKeyName === true ? $this->db->DBPrefix : '') . $keyName); + $table = $this->db->escapeIdentifiers($this->db->DBPrefix . $table); + $dropKeyAsConstraint = $this->dropKeyAsConstraint($table, $keyName); if ($dropKeyAsConstraint === true) { @@ -743,6 +744,7 @@ public function addColumn(string $table, $fields): bool $sqls = $this->_alterTable('ADD', $this->db->DBPrefix . $table, $this->_processFields()); $this->reset(); + if ($sqls === false) { if ($this->db->DBDebug) { throw new DatabaseException('This feature is not available for the database you are using.'); @@ -804,6 +806,7 @@ public function modifyColumn(string $table, $fields): bool $sqls = $this->_alterTable('CHANGE', $this->db->DBPrefix . $table, $this->_processFields()); $this->reset(); + if ($sqls === false) { if ($this->db->DBDebug) { throw new DatabaseException('This feature is not available for the database you are using.'); From b8db96a7ca0bc2bc1b0d83ab7b7612881f845335 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 20:31:17 +0900 Subject: [PATCH 19/22] docs: remove duplicated if --- system/Database/Forge.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index bf19980240ef..85135388c0f7 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -465,7 +465,7 @@ public function dropKey(string $table, string $keyName, bool $prefixKeyName = tr } /** - * Checks if if key needs to be dropped as a constraint. + * Checks if key needs to be dropped as a constraint. */ protected function dropKeyAsConstraint(string $table, string $constraintName): bool { From ee1076639abb24d190a9181daf6fa97f47b237ac Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 20:32:00 +0900 Subject: [PATCH 20/22] refactor: replace `! is_array()` with `is_string()` --- system/Database/Forge.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 85135388c0f7..79bf94e2c9f7 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -734,7 +734,7 @@ public function renameTable(string $tableName, string $newTableName) public function addColumn(string $table, $fields): bool { // Work-around for literal column definitions - if (! is_array($field)) { + if (is_string($fields)) { $fields = [$fields]; } @@ -792,7 +792,7 @@ public function dropColumn(string $table, $columnNames) public function modifyColumn(string $table, $fields): bool { // Work-around for literal column definitions - if (! is_array($field)) { + if (is_string($fields)) { $fields = [$fields]; } From 04b66435e7ad5222089c10b7bee786a2086aaf3e Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 19 Jan 2024 20:40:32 +0900 Subject: [PATCH 21/22] docs: fix @return types --- system/Database/Forge.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index 79bf94e2c9f7..adbdc208fc47 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -501,7 +501,7 @@ public function dropPrimaryKey(string $table, string $keyName = ''): bool } /** - * @return BaseResult|bool|false|mixed|Query + * @return bool * * @throws DatabaseException */ @@ -621,7 +621,7 @@ protected function _createTableAttributes(array $attributes): string } /** - * @return mixed + * @return bool * * @throws DatabaseException */ @@ -687,7 +687,7 @@ protected function _dropTable(string $table, bool $ifExists, bool $cascade) } /** - * @return mixed + * @return bool * * @throws DatabaseException */ From 213f0b94047fe58766e7008674c6a7569593a3d9 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 20 Jan 2024 09:33:41 +0900 Subject: [PATCH 22/22] refactor: add $columnNamesToDrop --- system/Database/SQLSRV/Forge.php | 12 +++++++----- system/Database/SQLite3/Forge.php | 4 +++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index 08e7952e7a0b..522d5fa02f54 100755 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -136,15 +136,17 @@ protected function _alterTable(string $alterType, string $table, $processedField { // Handle DROP here if ($alterType === 'DROP') { + $columnNamesToDrop = $processedFields; + // check if fields are part of any indexes $indexData = $this->db->getIndexData($table); foreach ($indexData as $index) { - if (is_string($processedFields)) { - $processedFields = explode(',', $processedFields); + if (is_string($columnNamesToDrop)) { + $columnNamesToDrop = explode(',', $columnNamesToDrop); } - $fld = array_intersect($processedFields, $index->fields); + $fld = array_intersect($columnNamesToDrop, $index->fields); // Drop index if field is part of an index if ($fld !== []) { @@ -155,7 +157,7 @@ protected function _alterTable(string $alterType, string $table, $processedField $fullTable = $this->db->escapeIdentifiers($this->db->schema) . '.' . $this->db->escapeIdentifiers($table); // Drop default constraints - $fields = implode(',', $this->db->escape((array) $processedFields)); + $fields = implode(',', $this->db->escape((array) $columnNamesToDrop)); $sql = << 'COLUMN [' . trim($item) . ']', (array) $processedFields); + $fields = array_map(static fn ($item) => 'COLUMN [' . trim($item) . ']', (array) $columnNamesToDrop); return $sql . implode(',', $fields); } diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index f9b6b8af23c6..d7112c61389f 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -120,10 +120,12 @@ protected function _alterTable(string $alterType, string $table, $processedField { switch ($alterType) { case 'DROP': + $columnNamesToDrop = $processedFields; + $sqlTable = new Table($this->db, $this); $sqlTable->fromTable($table) - ->dropColumn($processedFields) + ->dropColumn($columnNamesToDrop) ->run(); return ''; // Why empty string?