[ticket/16955] Clean up code in db classes

PHPBB3-16955

phpBB/phpbb/db/doctrine/oci8/connection.php

@@ -68,7 +68,7 @@ class connection implements DriverConnection
 	/**
 	 * {@inheritDoc}
 	 */
-	public function lastInsertId($name = null): ?string
+	public function lastInsertId($name = null)
 	{
 		return $this->wrapped->lastInsertId($name);
 	}

phpBB/phpbb/db/doctrine/table_helper.php

@@ -21,7 +21,7 @@ class table_helper
 	 *
 	 * @param array $column_data Column data.
 	 *
-	 * @return array<string, array> A pair of type and array of column options.
+	 * @return array{string, array} A pair of type and array of column options.
 	 */
 	public static function convert_column_data(array $column_data, string $dbms_layer): array
 	{

phpBB/phpbb/db/doctrine/type_converter.php

@@ -54,7 +54,7 @@ class type_converter
 	 *
 	 * @param string $type Legacy type name
 	 *
-	 * @return array<string, array> Pair of type name and options.
+	 * @return array{string, array} Pair of type name and options.
 	 */
 	public static function convert(string $type, string $dbms): array
 	{
@@ -73,7 +73,7 @@ class type_converter
 	 * @param string	$type	Legacy type name.
 	 * @param int		$length	Type length.
 	 *
-	 * @return array<string, array> Pair of type name and options.
+	 * @return array{string, array} Pair of type name and options.
 	 */
 	private static function mapWithLength(string $type, int $length): array
 	{
@@ -107,7 +107,7 @@ class type_converter
 	 *
 	 * @param string $type Type name.
 	 *
-	 * @return array<string, array> Pair of type name and an array of options.
+	 * @return array{string, array} Pair of type name and an array of options.
 	 */
 	private static function mapType(string $type, string $dbms): array
 	{

phpBB/phpbb/db/driver/driver.php

@@ -31,6 +31,9 @@ abstract class driver implements driver_interface
 	var $html_hold = '';
 	var $sql_report = '';
 
+	/** @var string Last query text */
+	protected $last_query_text = '';
+
 	var $persistency = false;
 	var $user = '';
 	var $server = '';
@@ -279,6 +282,13 @@ abstract class driver implements driver_interface
 		return $result;
 	}
 
+	/**
+	 * Close sql connection
+	 *
+	 * @return	bool		False if failure
+	 */
+	abstract protected function _sql_close(): bool;
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -296,6 +306,18 @@ abstract class driver implements driver_interface
 		return $this->_sql_query_limit($query, $total, $offset, $cache_ttl);
 	}
 
+	/**
+	 * Build LIMIT query
+	 *
+	 * @param	string	$query		The SQL query to execute
+	 * @param	int		$total		The number of rows to select
+	 * @param	int		$offset
+	 * @param	int		$cache_ttl	Either 0 to avoid caching or
+	 *				the time in seconds which the result shall be kept in cache
+	 * @return	mixed	Buffered, seekable result handle, false on error
+	 */
+	abstract protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0);
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -404,6 +426,18 @@ abstract class driver implements driver_interface
 		return $this->_sql_like_expression('LIKE \'' . $this->sql_escape($expression) . '\'');
 	}
 
+	/**
+	 * Build LIKE expression
+	 *
+	 * @param string $expression Base expression
+	 *
+	 * @return string LIKE expression
+	 */
+	protected function _sql_like_expression(string $expression): string
+	{
+		return $expression;
+	}
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -415,6 +449,18 @@ abstract class driver implements driver_interface
 		return $this->_sql_not_like_expression('NOT LIKE \'' . $this->sql_escape($expression) . '\'');
 	}
 
+	/**
+	 * Build NOT LIKE expression
+	 *
+	 * @param string $expression Base expression
+	 *
+	 * @return string NOT LIKE expression
+	 */
+	protected function _sql_not_like_expression(string $expression): string
+	{
+		return $expression;
+	}
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -510,12 +556,22 @@ abstract class driver implements driver_interface
 		return $result;
 	}
 
+	/**
+	 * SQL Transaction
+	 *
+	 * @param string $status		Should be one of the following strings:
+	 *								begin, commit, rollback
+	 *
+	 * @return	bool	Success/failure of the transaction query
+	 */
+	abstract protected function _sql_transaction(string $status = 'begin'): bool;
+
 	/**
 	* {@inheritDoc}
 	*/
-	function sql_build_array($query, $assoc_ary = false)
+	function sql_build_array($query, $assoc_ary = [])
 	{
-		if (!is_array($assoc_ary))
+		if (!count($assoc_ary))
 		{
 			return false;
 		}
@@ -836,6 +892,18 @@ abstract class driver implements driver_interface
 		return $sql;
 	}
 
+	/**
+	 * Build db-specific query data
+	 *
+	 * @param string $stage Query stage, can be 'FROM' or 'WHERE'
+	 * @param string|array $data A string containing the CROSS JOIN query or an array of WHERE clauses
+	 *
+	 * @return string|array The db-specific query fragment
+	 */
+	protected function _sql_custom_build(string $stage, $data)
+	{
+		return $data;
+	}
 
 	protected function _process_boolean_tree_first($operations_ary)
 	{
@@ -1017,7 +1085,7 @@ abstract class driver implements driver_interface
 				global $msg_long_text;
 				$msg_long_text = $message;
 
-				trigger_error(false, E_USER_ERROR);
+				trigger_error('', E_USER_ERROR);
 			}
 
 			trigger_error($message, E_USER_ERROR);
@@ -1031,6 +1099,13 @@ abstract class driver implements driver_interface
 		return $this->sql_error_returned;
 	}
 
+	/**
+	 * Return sql error array
+	 *
+	 * @return array{message: string, code: int|string} SQL error array with message and error code
+	 */
+	abstract protected function _sql_error(): array;
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -1216,6 +1291,16 @@ abstract class driver implements driver_interface
 		return true;
 	}
 
+	/**
+	 * Build db-specific report
+	 *
+	 * @param string $mode 'start' to add to report, 'fromcache' to output it
+	 * @param string $query Query to add to sql report
+	 *
+	 * @return void
+	 */
+	abstract protected function _sql_report(string $mode, string $query = ''): void;
+
 	/**
 	* {@inheritDoc}
 	*/

phpBB/phpbb/db/driver/driver_interface.php

@@ -180,7 +180,7 @@ interface driver_interface
 	* Return on error or display error message
 	*
 	* @param bool	$fail		Should we return on errors, or stop
-	* @return null
+	* @return void
 	*/
 	public function sql_return_on_error($fail = false);
 
@@ -190,9 +190,9 @@ interface driver_interface
 	* @param	string	$query		Should be on of the following strings:
 	*						INSERT, INSERT_SELECT, UPDATE, SELECT, DELETE
 	* @param	array	$assoc_ary	Array with "column => value" pairs
-	* @return	string		A SQL statement like "c1 = 'a' AND c2 = 'b'"
+	* @return	string|false		A SQL statement like "c1 = 'a' AND c2 = 'b'", false on invalid assoc_ary
 	*/
-	public function sql_build_array($query, $assoc_ary = array());
+	public function sql_build_array($query, $assoc_ary = []);
 
 	/**
 	* Fetch all rows
@@ -299,7 +299,7 @@ interface driver_interface
 	* Add to query count
 	*
 	* @param bool $cached	Is this query cached?
-	* @return null
+	* @return void
 	*/
 	public function sql_add_num_queries($cached = false);
 

phpBB/phpbb/db/driver/factory.php

@@ -49,7 +49,9 @@ class factory implements driver_interface
 	{
 		if ($this->driver === null)
 		{
-			$this->driver = $this->container->get('dbal.conn.driver');
+			/** @var driver_interface $driver */
+			$driver = $this->container->get('dbal.conn.driver');
+			$this->driver = $driver;
 		}
 
 		return $this->driver;
@@ -238,13 +240,13 @@ class factory implements driver_interface
 	*/
 	public function sql_return_on_error($fail = false)
 	{
-		return $this->get_driver()->sql_return_on_error($fail);
+		$this->get_driver()->sql_return_on_error($fail);
 	}
 
 	/**
 	* {@inheritdoc}
 	*/
-	public function sql_build_array($query, $assoc_ary = array())
+	public function sql_build_array($query, $assoc_ary = [])
 	{
 		return $this->get_driver()->sql_build_array($query, $assoc_ary);
 	}
@@ -326,7 +328,7 @@ class factory implements driver_interface
 	*/
 	public function sql_add_num_queries($cached = false)
 	{
-		return $this->get_driver()->sql_add_num_queries($cached);
+		$this->get_driver()->sql_add_num_queries($cached);
 	}
 
 	/**
@@ -374,7 +376,7 @@ class factory implements driver_interface
 	*/
 	public function sql_freeresult($query_id = false)
 	{
-		return $this->get_driver()->sql_freeresult($query_id);
+		$this->get_driver()->sql_freeresult($query_id);
 	}
 
 	/**

phpBB/phpbb/db/driver/mssql_base.php

@@ -43,19 +43,17 @@ abstract class mssql_base extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build LIKE expression
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_like_expression($expression)
+	protected function _sql_like_expression(string $expression): string
 	{
 		return $expression . " ESCAPE '\\'";
 	}
 
 	/**
-	* Build NOT LIKE expression
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_not_like_expression($expression)
+	protected function _sql_not_like_expression(string $expression): string
 	{
 		return $expression . " ESCAPE '\\'";
 	}
@@ -68,15 +66,6 @@ abstract class mssql_base extends \phpbb\db\driver\driver
 		return 'CONVERT(BIGINT, ' . $expression . ')';
 	}
 
-	/**
-	* Build db-specific query data
-	* @access private
-	*/
-	function _sql_custom_build($stage, $data)
-	{
-		return $data;
-	}
-
 	/**
 	* {@inheritDoc}
 	*/

phpBB/phpbb/db/driver/mssql_odbc.php

@@ -24,7 +24,6 @@ namespace phpbb\db\driver;
 */
 class mssql_odbc extends \phpbb\db\driver\mssql_base
 {
-	var $last_query_text = '';
 	var $connect_error = '';
 
 	/**
@@ -112,31 +111,27 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base
 
 		if ($raw)
 		{
-			return $this->sql_server_version;
+			return (string) $this->sql_server_version;
 		}
 
 		return ($this->sql_server_version) ? 'MSSQL (ODBC)<br />' . $this->sql_server_version : 'MSSQL (ODBC)';
 	}
 
 	/**
-	* SQL Transaction
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_transaction($status = 'begin')
+	protected function _sql_transaction(string $status = 'begin'): bool
 	{
 		switch ($status)
 		{
 			case 'begin':
-				return @odbc_exec($this->db_connect_id, 'BEGIN TRANSACTION');
+				return (bool) @odbc_exec($this->db_connect_id, 'BEGIN TRANSACTION');
-			break;
 
 			case 'commit':
-				return @odbc_exec($this->db_connect_id, 'COMMIT TRANSACTION');
+				return (bool) @odbc_exec($this->db_connect_id, 'COMMIT TRANSACTION');
-			break;
 
 			case 'rollback':
-				return @odbc_exec($this->db_connect_id, 'ROLLBACK TRANSACTION');
+				return (bool) @odbc_exec($this->db_connect_id, 'ROLLBACK TRANSACTION');
-			break;
 		}
 
 		return true;
@@ -209,9 +204,9 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base
 	}
 
 	/**
-	* Build LIMIT query
+	 * {@inheritDoc}
 	 */
-	function _sql_query_limit($query, $total, $offset = 0, $cache_ttl = 0)
+	protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0)
 	{
 		$this->query_result = false;
 
@@ -303,23 +298,19 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base
 
 		if ($cache && !is_object($query_id) && $cache->sql_exists($query_id))
 		{
-			return $cache->sql_freeresult($query_id);
+			$cache->sql_freeresult($query_id);
 		}
-
+		else if (isset($this->open_queries[(int) $query_id]))
-		if (isset($this->open_queries[(int) $query_id]))
 		{
 			unset($this->open_queries[(int) $query_id]);
-			return odbc_free_result($query_id);
+			odbc_free_result($query_id);
 		}
-
-		return false;
 	}
 
 	/**
-	* return sql error array
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_error()
+	protected function _sql_error(): array
 	{
 		if (function_exists('odbc_errormsg'))
 		{
@@ -340,19 +331,18 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base
 	}
 
 	/**
-	* Close sql connection
+	 * {@inheritDoc}
-	* @access private
 	 */
-	function _sql_close()
+	protected function _sql_close(): bool
 	{
-		return @odbc_close($this->db_connect_id);
+		@odbc_close($this->db_connect_id);
+		return true;
 	}
 
 	/**
-	* Build db-specific report
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_report($mode, $query = '')
+	protected function _sql_report(string $mode, string $query = ''): void
 	{
 		switch ($mode)
 		{

phpBB/phpbb/db/driver/mssqlnative.php

@@ -23,10 +23,12 @@ namespace phpbb\db\driver;
 class mssqlnative extends \phpbb\db\driver\mssql_base
 {
 	var $m_insert_id = null;
-	var $last_query_text = '';
 	var $query_options = array();
 	var $connect_error = '';
 
+	/** @var string|false Last error result or false if no last error set */
+	private $last_error_result = false;
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -92,24 +94,20 @@ class mssqlnative extends \phpbb\db\driver\mssql_base
 	}
 
 	/**
-	* SQL Transaction
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_transaction($status = 'begin')
+	protected function _sql_transaction(string $status = 'begin'): bool
 	{
 		switch ($status)
 		{
 			case 'begin':
 				return sqlsrv_begin_transaction($this->db_connect_id);
-			break;
 
 			case 'commit':
 				return sqlsrv_commit($this->db_connect_id);
-			break;
 
 			case 'rollback':
 				return sqlsrv_rollback($this->db_connect_id);
-			break;
 		}
 		return true;
 	}
@@ -182,9 +180,9 @@ class mssqlnative extends \phpbb\db\driver\mssql_base
 	}
 
 	/**
-	* Build LIMIT query
+	 * {@inheritDoc}
 	 */
-	function _sql_query_limit($query, $total, $offset = 0, $cache_ttl = 0)
+	protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0)
 	{
 		$this->query_result = false;
 
@@ -280,7 +278,7 @@ class mssqlnative extends \phpbb\db\driver\mssql_base
 		if ($result_id)
 		{
 			$row = sqlsrv_fetch_array($result_id);
-			$id = $row[0];
+			$id = isset($row[0]) ? (int) $row[0] : false;
 			sqlsrv_free_stmt($result_id);
 			return $id;
 		}
@@ -304,23 +302,19 @@ class mssqlnative extends \phpbb\db\driver\mssql_base
 
 		if ($cache && !is_object($query_id) && $cache->sql_exists($query_id))
 		{
-			return $cache->sql_freeresult($query_id);
+			$cache->sql_freeresult($query_id);
 		}
-
+		else if (isset($this->open_queries[(int) $query_id]))
-		if (isset($this->open_queries[(int) $query_id]))
 		{
 			unset($this->open_queries[(int) $query_id]);
-			return sqlsrv_free_stmt($query_id);
+			sqlsrv_free_stmt($query_id);
 		}
-
-		return false;
 	}
 
 	/**
-	* return sql error array
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_error()
+	protected function _sql_error(): array
 	{
 		if (function_exists('sqlsrv_errors'))
 		{
@@ -342,7 +336,7 @@ class mssqlnative extends \phpbb\db\driver\mssql_base
 			}
 			else
 			{
-				$error = (isset($this->last_error_result) && $this->last_error_result) ? $this->last_error_result : array();
+				$error = $this->last_error_result ?: '';
 			}
 
 			$error = array(
@@ -362,19 +356,17 @@ class mssqlnative extends \phpbb\db\driver\mssql_base
 	}
 
 	/**
-	* Close sql connection
+	 * {@inheritDoc}
-	* @access private
 	 */
-	function _sql_close()
+	protected function _sql_close(): bool
 	{
 		return @sqlsrv_close($this->db_connect_id);
 	}
 
 	/**
-	* Build db-specific report
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_report($mode, $query = '')
+	protected function _sql_report(string $mode, string $query = ''): void
 	{
 		switch ($mode)
 		{

phpBB/phpbb/db/driver/mysql_base.php

@@ -27,9 +27,9 @@ abstract class mysql_base extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build LIMIT query
+	 * {@inheritDoc}
 	 */
-	function _sql_query_limit($query, $total, $offset = 0, $cache_ttl = 0)
+	protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0)
 	{
 		$this->query_result = false;
 
@@ -103,34 +103,13 @@ abstract class mysql_base extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build LIKE expression
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_like_expression($expression)
+	protected function _sql_custom_build(string $stage, $data)
 	{
-		return $expression;
+		if ($stage === 'FROM' && is_string($data))
-	}
-
-	/**
-	* Build NOT LIKE expression
-	* @access private
-	*/
-	function _sql_not_like_expression($expression)
 		{
-		return $expression;
-	}
-
-	/**
-	* Build db-specific query data
-	* @access private
-	*/
-	function _sql_custom_build($stage, $data)
-	{
-		switch ($stage)
-		{
-			case 'FROM':
 			$data = '(' . $data . ')';
-			break;
 		}
 
 		return $data;

phpBB/phpbb/db/driver/mysqli.php

@@ -69,7 +69,7 @@ class mysqli extends \phpbb\db\driver\mysql_base
 		if ($this->db_connect_id && $this->dbname != '')
 		{
 			// Disable loading local files on client side
-			@mysqli_options($this->db_connect_id, MYSQLI_OPT_LOCAL_INFILE, false);
+			@mysqli_options($this->db_connect_id, MYSQLI_OPT_LOCAL_INFILE, 0);
 
 			/*
 			 * As of PHP 8.1 MySQLi default error mode is set to MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT
@@ -143,32 +143,28 @@ class mysqli extends \phpbb\db\driver\mysql_base
 			}
 		}
 
-		return ($raw) ? $this->sql_server_version : 'MySQL(i) ' . $this->sql_server_version;
+		return ($raw) ? (string) $this->sql_server_version : 'MySQL(i) ' . $this->sql_server_version;
 	}
 
 	/**
-	* SQL Transaction
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_transaction($status = 'begin')
+	protected function _sql_transaction(string $status = 'begin'): bool
 	{
 		switch ($status)
 		{
 			case 'begin':
 				return @mysqli_autocommit($this->db_connect_id, false);
-			break;
 
 			case 'commit':
 				$result = @mysqli_commit($this->db_connect_id);
 				@mysqli_autocommit($this->db_connect_id, true);
 				return $result;
-			break;
 
 			case 'rollback':
 				$result = @mysqli_rollback($this->db_connect_id);
 				@mysqli_autocommit($this->db_connect_id, true);
 				return $result;
-			break;
 		}
 
 		return true;
@@ -293,7 +289,7 @@ class mysqli extends \phpbb\db\driver\mysql_base
 	*/
 	function sql_nextid()
 	{
-		return ($this->db_connect_id) ? @mysqli_insert_id($this->db_connect_id) : false;
+		return ($this->db_connect_id) ? (int) @mysqli_insert_id($this->db_connect_id) : false;
 	}
 
 	/**
@@ -310,20 +306,12 @@ class mysqli extends \phpbb\db\driver\mysql_base
 
 		if ($cache && !is_object($query_id) && $cache->sql_exists($query_id))
 		{
-			return $cache->sql_freeresult($query_id);
+			$cache->sql_freeresult($query_id);
 		}
-
+		else if ($query_id && $query_id !== true)
-		if (!$query_id)
 		{
-			return false;
+			mysqli_free_result($query_id);
 		}
-
-		if ($query_id === true)
-		{
-			return true;
-		}
-
-		return mysqli_free_result($query_id);
 	}
 
 	/**
@@ -335,10 +323,9 @@ class mysqli extends \phpbb\db\driver\mysql_base
 	}
 
 	/**
-	* return sql error array
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_error()
+	protected function _sql_error(): array
 	{
 		if ($this->db_connect_id)
 		{
@@ -366,19 +353,17 @@ class mysqli extends \phpbb\db\driver\mysql_base
 	}
 
 	/**
-	* Close sql connection
+	 * {@inheritDoc}
-	* @access private
 	 */
-	function _sql_close()
+	protected function _sql_close(): bool
 	{
 		return @mysqli_close($this->db_connect_id);
 	}
 
 	/**
-	* Build db-specific report
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_report($mode, $query = '')
+	protected function _sql_report(string $mode, string $query = ''): void
 	{
 		static $test_prof;
 

phpBB/phpbb/db/driver/oracle.php

@@ -18,9 +18,11 @@ namespace phpbb\db\driver;
 */
 class oracle extends \phpbb\db\driver\driver
 {
-	var $last_query_text = '';
 	var $connect_error = '';
 
+	/** @var array|false Last error result or false if no last error set */
+	private $last_error_result = false;
+
 	/**
 	* {@inheritDoc}
 	*/
@@ -107,24 +109,20 @@ class oracle extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* SQL Transaction
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_transaction($status = 'begin')
+	protected function _sql_transaction(string $status = 'begin'): bool
 	{
 		switch ($status)
 		{
 			case 'begin':
 				return true;
-			break;
 
 			case 'commit':
 				return @oci_commit($this->db_connect_id);
-			break;
 
 			case 'rollback':
 				return @oci_rollback($this->db_connect_id);
-			break;
 		}
 
 		return true;
@@ -465,9 +463,9 @@ class oracle extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build LIMIT query
+	* {@inheritDoc}
 	*/
-	function _sql_query_limit($query, $total, $offset = 0, $cache_ttl = 0)
+	protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0)
 	{
 		$this->query_result = false;
 
@@ -621,16 +619,13 @@ class oracle extends \phpbb\db\driver\driver
 
 		if ($cache && !is_object($query_id) && $cache->sql_exists($query_id))
 		{
-			return $cache->sql_freeresult($query_id);
+			 $cache->sql_freeresult($query_id);
 		}
-
+		else if (isset($this->open_queries[(int) $query_id]))
-		if (isset($this->open_queries[(int) $query_id]))
 		{
 			unset($this->open_queries[(int) $query_id]);
-			return oci_free_statement($query_id);
+			oci_free_statement($query_id);
 		}
-
-		return false;
 	}
 
 	/**
@@ -642,28 +637,21 @@ class oracle extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build LIKE expression
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_like_expression($expression)
+	protected function _sql_like_expression(string $expression): string
 	{
 		return $expression . " ESCAPE '\\'";
 	}
 
 	/**
-	* Build NOT LIKE expression
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_not_like_expression($expression)
+	protected function _sql_not_like_expression(string $expression): string
 	{
 		return $expression . " ESCAPE '\\'";
 	}
 
-	function _sql_custom_build($stage, $data)
-	{
-		return $data;
-	}
-
 	function _sql_bit_and($column_name, $bit, $compare = '')
 	{
 		return 'BITAND(' . $column_name . ', ' . (1 << $bit) . ')' . (($compare) ? ' ' . $compare : '');
@@ -675,10 +663,9 @@ class oracle extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* return sql error array
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_error()
+	protected function _sql_error(): array
 	{
 		if (function_exists('oci_error'))
 		{
@@ -692,7 +679,7 @@ class oracle extends \phpbb\db\driver\driver
 			}
 			else
 			{
-				$error = (isset($this->last_error_result) && $this->last_error_result) ? $this->last_error_result : array();
+				$error = $this->last_error_result ?: ['message' => '', 'code' => ''];
 			}
 		}
 		else
@@ -707,19 +694,17 @@ class oracle extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Close sql connection
+	 * {@inheritDoc}
-	* @access private
 	 */
-	function _sql_close()
+	protected function _sql_close(): bool
 	{
 		return @oci_close($this->db_connect_id);
 	}
 
 	/**
-	* Build db-specific report
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_report($mode, $query = '')
+	protected function _sql_report(string $mode, string $query = ''): void
 	{
 		switch ($mode)
 		{

phpBB/phpbb/db/driver/postgres.php

@@ -20,7 +20,6 @@ namespace phpbb\db\driver;
 class postgres extends \phpbb\db\driver\driver
 {
 	var $multi_insert = true;
-	var $last_query_text = '';
 	var $connect_error = '';
 
 	/**
@@ -137,28 +136,24 @@ class postgres extends \phpbb\db\driver\driver
 			}
 		}
 
-		return ($raw) ? $this->sql_server_version : 'PostgreSQL ' . $this->sql_server_version;
+		return ($raw) ? (string) $this->sql_server_version : 'PostgreSQL ' . $this->sql_server_version;
 	}
 
 	/**
-	* SQL Transaction
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_transaction($status = 'begin')
+	protected function _sql_transaction(string $status = 'begin'): bool
 	{
 		switch ($status)
 		{
 			case 'begin':
-				return @pg_query($this->db_connect_id, 'BEGIN');
+				return @pg_query($this->db_connect_id, 'BEGIN') !== false;
-			break;
 
 			case 'commit':
-				return @pg_query($this->db_connect_id, 'COMMIT');
+				return @pg_query($this->db_connect_id, 'COMMIT') !== false;
-			break;
 
 			case 'rollback':
-				return @pg_query($this->db_connect_id, 'ROLLBACK');
+				return @pg_query($this->db_connect_id, 'ROLLBACK') !== false;
-			break;
 		}
 
 		return true;
@@ -233,18 +228,9 @@ class postgres extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build db-specific query data
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_custom_build($stage, $data)
+	protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0)
-	{
-		return $data;
-	}
-
-	/**
-	* Build LIMIT query
-	*/
-	function _sql_query_limit($query, $total, $offset = 0, $cache_ttl = 0)
 	{
 		$this->query_result = false;
 
@@ -385,16 +371,13 @@ class postgres extends \phpbb\db\driver\driver
 		$safe_query_id = $this->clean_query_id($query_id);
 		if ($cache && !is_object($query_id) && $cache->sql_exists($safe_query_id))
 		{
-			return $cache->sql_freeresult($safe_query_id);
+			$cache->sql_freeresult($safe_query_id);
 		}
-
+		else if (isset($this->open_queries[$safe_query_id]))
-		if (isset($this->open_queries[$safe_query_id]))
 		{
 			unset($this->open_queries[$safe_query_id]);
-			return pg_free_result($query_id);
+			pg_free_result($query_id);
 		}
-
-		return false;
 	}
 
 	/**
@@ -405,24 +388,6 @@ class postgres extends \phpbb\db\driver\driver
 		return @pg_escape_string($msg);
 	}
 
-	/**
-	* Build LIKE expression
-	* @access private
-	*/
-	function _sql_like_expression($expression)
-	{
-		return $expression;
-	}
-
-	/**
-	* Build NOT LIKE expression
-	* @access private
-	*/
-	function _sql_not_like_expression($expression)
-	{
-		return $expression;
-	}
-
 	/**
 	* {@inheritDoc}
 	*/
@@ -440,10 +405,9 @@ class postgres extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* return sql error array
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_error()
+	protected function _sql_error(): array
 	{
 		// pg_last_error only works when there is an established connection.
 		// Connection errors have to be tracked by us manually.
@@ -463,10 +427,9 @@ class postgres extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Close sql connection
+	 * {@inheritDoc}
-	* @access private
 	 */
-	function _sql_close()
+	protected function _sql_close(): bool
 	{
 		// Released resources are already closed, return true in this case
 		if (!is_resource($this->db_connect_id))
@@ -477,10 +440,9 @@ class postgres extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build db-specific report
+	* {@inheritDoc}
-	* @access private
 	*/
-	function _sql_report($mode, $query = '')
+	protected function _sql_report(string $mode, string $query = ''): void
 	{
 		switch ($mode)
 		{

phpBB/phpbb/db/driver/sqlite3.php

@@ -83,27 +83,20 @@ class sqlite3 extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* SQL Transaction
+	* {@inheritDoc}
-	*
-	* @param	string	$status		Should be one of the following strings:
-	*								begin, commit, rollback
-	* @return	bool	Success/failure of the transaction query
 	*/
-	protected function _sql_transaction($status = 'begin')
+	protected function _sql_transaction(string $status = 'begin'): bool
 	{
 		switch ($status)
 		{
 			case 'begin':
 				return $this->dbo->exec('BEGIN IMMEDIATE');
-			break;
 
 			case 'commit':
 				return $this->dbo->exec('COMMIT');
-			break;
 
 			case 'rollback':
 				return @$this->dbo->exec('ROLLBACK');
-			break;
 		}
 
 		return true;
@@ -188,16 +181,9 @@ class sqlite3 extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build LIMIT query
+	* {@inheritDoc}
-	*
-	* @param	string	$query		The SQL query to execute
-	* @param	int		$total		The number of rows to select
-	* @param	int		$offset
-	* @param	int		$cache_ttl	Either 0 to avoid caching or
-	*				the time in seconds which the result shall be kept in cache
-	* @return	mixed	Buffered, seekable result handle, false on error
 	*/
-	protected function _sql_query_limit($query, $total, $offset = 0, $cache_ttl = 0)
+	protected function _sql_query_limit(string $query, int $total, int $offset = 0, int $cache_ttl = 0)
 	{
 		$this->query_result = false;
 
@@ -263,12 +249,13 @@ class sqlite3 extends \phpbb\db\driver\driver
 
 		if ($cache && !is_object($query_id) && $cache->sql_exists($query_id))
 		{
-			return $cache->sql_freeresult($query_id);
+			$cache->sql_freeresult($query_id);
+			return;
 		}
 
 		if ($query_id)
 		{
-			return @$query_id->finalize();
+			@$query_id->finalize();
 		}
 	}
 
@@ -315,11 +302,9 @@ class sqlite3 extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* return sql error array
+	* {@inheritDoc}
-	*
-	* @return array
 	*/
-	protected function _sql_error()
+	protected function _sql_error(): array
 	{
 		if (class_exists('SQLite3', false) && isset($this->dbo))
 		{
@@ -340,24 +325,9 @@ class sqlite3 extends \phpbb\db\driver\driver
 	}
 
 	/**
-	* Build db-specific query data
+	* {@inheritDoc}
-	*
-	* @param	string	$stage		Available stages: FROM, WHERE
-	* @param	mixed	$data		A string containing the CROSS JOIN query or an array of WHERE clauses
-	*
-	* @return	string	The db-specific query fragment
 	*/
-	protected function _sql_custom_build($stage, $data)
+	protected function _sql_close(): bool
-	{
-		return $data;
-	}
-
-	/**
-	* Close sql connection
-	*
-	* @return	bool		False if failure
-	*/
-	protected function _sql_close()
 	{
 		return $this->dbo->close();
 	}
@@ -368,9 +338,10 @@ class sqlite3 extends \phpbb\db\driver\driver
 	* @param string $mode Available modes: display, start, stop,
 	*								add_select_row, fromcache, record_fromcache
 	* @param string $query		The Query that should be explained
-	* @return	mixed		Either a full HTML page, boolean or null
+	*
+	* @return    void		Either writes HTML to html_hold or outputs a full HTML page
 	*/
-	protected function _sql_report($mode, $query = '')
+	protected function _sql_report(string $mode, string $query = ''): void
 	{
 		switch ($mode)
 		{

phpBB/phpbb/db/extractor/factory.php

@@ -51,25 +51,30 @@ class factory
 		// Return the appropriate DB extractor
 		if ($this->db instanceof \phpbb\db\driver\mssql_base)
 		{
-			return $this->container->get('dbal.extractor.extractors.mssql_extractor');
+			$extractor = $this->container->get('dbal.extractor.extractors.mssql_extractor');
 		}
 		else if ($this->db instanceof \phpbb\db\driver\mysql_base)
 		{
-			return $this->container->get('dbal.extractor.extractors.mysql_extractor');
+			$extractor = $this->container->get('dbal.extractor.extractors.mysql_extractor');
 		}
 		else if ($this->db instanceof \phpbb\db\driver\oracle)
 		{
-			return $this->container->get('dbal.extractor.extractors.oracle_extractor');
+			$extractor = $this->container->get('dbal.extractor.extractors.oracle_extractor');
 		}
 		else if ($this->db instanceof \phpbb\db\driver\postgres)
 		{
-			return $this->container->get('dbal.extractor.extractors.postgres_extractor');
+			$extractor = $this->container->get('dbal.extractor.extractors.postgres_extractor');
 		}
 		else if ($this->db instanceof \phpbb\db\driver\sqlite3)
 		{
-			return $this->container->get('dbal.extractor.extractors.sqlite3_extractor');
+			$extractor = $this->container->get('dbal.extractor.extractors.sqlite3_extractor');
 		}
-
+		else
+		{
 			throw new \InvalidArgumentException('Invalid database driver given');
 		}
+
+		/** @var \phpbb\db\extractor\extractor_interface $extractor */
+		return $extractor;
+	}
 }

phpBB/phpbb/db/extractor/mssql_extractor.php

@@ -194,12 +194,12 @@ class mssql_extractor extends base_extractor
 	* Extracts data from database table (for MSSQL Native driver)
 	*
 	* @param	string	$table_name	name of the database table
-	* @return null
+	* @return void
 	* @throws extractor_not_initialized_exception when calling this function before init_extractor()
 	*/
 	protected function write_data_mssqlnative($table_name)
 	{
-		if (!$this->is_initialized)
+		if (!$this->is_initialized || !$this->db instanceof \phpbb\db\driver\mssqlnative)
 		{
 			throw new extractor_not_initialized_exception();
 		}

phpBB/phpbb/db/extractor/oracle_extractor.php

@@ -184,12 +184,12 @@ class oracle_extractor extends base_extractor
 			FROM $table_name";
 		$result = $this->db->sql_query($sql);
 
-		$i_num_fields = ocinumcols($result);
+		$i_num_fields = oci_num_fields($result);
 
 		for ($i = 0; $i < $i_num_fields; $i++)
 		{
-			$ary_type[$i] = ocicolumntype($result, $i + 1);
+			$ary_type[$i] = oci_field_type($result, $i + 1);
-			$ary_name[$i] = ocicolumnname($result, $i + 1);
+			$ary_name[$i] = oci_field_name($result, $i + 1);
 		}
 
 		while ($row = $this->db->sql_fetchrow($result))

phpBB/phpbb/db/migration/data/v310/soft_delete_mod_convert.php

@@ -122,6 +122,8 @@ class soft_delete_mod_convert extends container_aware_migration
 	 */
 	protected function get_content_visibility()
 	{
-		return $this->container->get('content.visibility');
+		/** @var \phpbb\content_visibility $content_visibility */
+		$content_visibility = $this->container->get('content.visibility');
+		return $content_visibility;
 	}
 }

phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php

@@ -49,17 +49,21 @@ class remove_duplicate_migrations extends \phpbb\db\migration\migration
 
 		$this->db->sql_freeresult($result);
 
+		/**
+		 * @var string $name
+		 * @var array $migration
+		 */
 		foreach ($migration_state as $name => $migration)
 		{
 			$prepended_name = ($name[0] == '\\' ? '' : '\\') . $name;
 			$prefixless_name = $name[0] == '\\' ? substr($name, 1) : $name;
 
-			if ($prepended_name != $name && isset($migration_state[$prepended_name]) && $migration_state[$prepended_name]['migration_depends_on'] == $migration_state[$name]['migration_depends_on'])
+			if ($prepended_name != $name && isset($migration_state[$prepended_name]) && $migration_state[$prepended_name]['migration_depends_on'] == $migration['migration_depends_on'])
 			{
 				$duplicate_migrations[] = $name;
 				unset($migration_state[$prepended_name]);
 			}
-			else if ($prefixless_name != $name && isset($migration_state[$prefixless_name]) && $migration_state[$prefixless_name]['migration_depends_on'] == $migration_state[$name]['migration_depends_on'])
+			else if ($prefixless_name != $name && isset($migration_state[$prefixless_name]) && $migration_state[$prefixless_name]['migration_depends_on'] == $migration['migration_depends_on'])
 			{
 				$duplicate_migrations[] = $prefixless_name;
 				unset($migration_state[$prefixless_name]);

phpBB/phpbb/db/migration/tool/config.php

@@ -47,7 +47,7 @@ class config implements \phpbb\db\migration\tool\tool_interface
 	* @param mixed $config_value The value of the config setting
 	* @param bool $is_dynamic True if it is dynamic (changes very often)
 	* 	and should not be stored in the cache, false if not.
-	* @return null
+	* @return void
 	*/
 	public function add($config_name, $config_value, $is_dynamic = false)
 	{
@@ -65,7 +65,7 @@ class config implements \phpbb\db\migration\tool\tool_interface
 	* @param string $config_name The name of the config setting you would
 	* 	like to update
 	* @param mixed $config_value The value of the config setting
-	* @return null
+	* @return void
 	* @throws \phpbb\db\migration\exception
 	*/
 	public function update($config_name, $config_value)
@@ -87,7 +87,7 @@ class config implements \phpbb\db\migration\tool\tool_interface
 	* @param string $config_name The name of the config setting you would
 	* 	like to update
 	* @param mixed $config_value The value of the config setting
-	* @return null
+	* @return void
 	* @throws \phpbb\db\migration\exception
 	*/
 	public function update_if_equals($compare, $config_name, $config_value)
@@ -105,7 +105,7 @@ class config implements \phpbb\db\migration\tool\tool_interface
 	*
 	* @param string $config_name The name of the config setting you would
 	* 	like to remove
-	* @return null
+	* @return void
 	*/
 	public function remove($config_name)
 	{

phpBB/phpbb/db/migration/tool/config_text.php

@@ -45,7 +45,7 @@ class config_text implements \phpbb\db\migration\tool\tool_interface
 	* @param string $config_name The name of the config_text setting
 	* 	you would like to add
 	* @param mixed $config_value The value of the config_text setting
-	* @return null
+	* @return void
 	*/
 	public function add($config_name, $config_value)
 	{
@@ -81,7 +81,7 @@ class config_text implements \phpbb\db\migration\tool\tool_interface
 	*
 	* @param string $config_name The name of the config_text setting you would
 	* 	like to remove
-	* @return null
+	* @return void
 	*/
 	public function remove($config_name)
 	{

phpBB/phpbb/db/migration/tool/module.php

@@ -162,7 +162,7 @@ class module implements \phpbb\db\migration\tool\tool_interface
 	* 		Optionally you may not send 'modes' and it will insert all of the
 	* 			modules in that info file.
 	* 	path, specify that here
-	* @return null
+	* @return void
 	* @throws \phpbb\db\migration\exception
 	*/
 	public function add($class, $parent = 0, $data = array())

phpBB/phpbb/db/migration/tool/permission.php

@@ -21,7 +21,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 	/** @var \phpbb\auth\auth */
 	protected $auth;
 
-	/** @var \includes\acp\auth\auth_admin */
+	/** @var \auth_admin */
 	protected $auth_admin;
 
 	/** @var \phpbb\cache\service */
@@ -115,7 +115,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 	* @param bool $global True for checking a global permission setting,
 	* 	False for a local permission setting
 	* @param int|false $copy_from If set, contains the id of the permission from which to copy the new one.
-	* @return null
+	* @return void
 	*/
 	public function add($auth_option, $global = true, $copy_from = false)
 	{
@@ -189,7 +189,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 	* @param string $auth_option The name of the permission (auth) option
 	* @param bool $global True for checking a global permission setting,
 	* 	False for a local permission setting
-	* @return null
+	* @return void
 	*/
 	public function remove($auth_option, $global = true)
 	{
@@ -266,7 +266,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 	* @param string $role_type The type (u_, m_, a_)
 	* @param string $role_description Description of the new role
 	*
-	* @return null
+	* @return int|null Inserted SQL id or false if role already exists
 	*/
 	public function role_add($role_name, $role_type, $role_description = '')
 	{
@@ -292,7 +292,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 		$sql = 'INSERT INTO ' . ACL_ROLES_TABLE . ' ' . $this->db->sql_build_array('INSERT', $sql_ary);
 		$this->db->sql_query($sql);
 
-		return $this->db->sql_nextid();
+		return (int) $this->db->sql_nextid();
 	}
 
 	/**
@@ -300,7 +300,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 	*
 	* @param string $old_role_name The old role name
 	* @param string $new_role_name The new role name
-	* @return null
+	* @return void
 	* @throws \phpbb\db\migration\exception
 	*/
 	public function role_update($old_role_name, $new_role_name)
@@ -320,7 +320,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 	* Remove a permission role
 	*
 	* @param string $role_name The role name to remove
-	* @return null
+	* @return void
 	*/
 	public function role_remove($role_name)
 	{
@@ -704,9 +704,6 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 			break;
 		}
 
-		if ($call)
+		return $call ? call_user_func_array(array(&$this, $call), $arguments) : null;
-		{
-			return call_user_func_array(array(&$this, $call), $arguments);
-		}
 	}
 }

phpBB/phpbb/db/migration/tool/tool_interface.php

@@ -31,7 +31,7 @@ interface tool_interface
 	* First argument is the original call to the class (e.g. add, remove)
 	* After the first argument, send the original arguments to the function in the original call
 	*
-	* @return null
+	* @return mixed|null Return of function call or null if no valid function call found
 	*/
 	public function reverse();
 }

phpBB/phpbb/db/migrator.php

@@ -90,7 +90,7 @@ class migrator
 	*
 	* 'effectively_installed' set and set to true if the migration was effectively_installed
 	*
-	* @var array|bool
+	* @var array|false
 	*/
 	protected $last_run_migration = false;
 
@@ -192,7 +192,7 @@ class migrator
 	 * The array contains 'name', 'class' and 'state'. 'effectively_installed' is set
 	 * and set to true if the last migration was effectively_installed.
 	 *
-	 * @return array
+	 * @return array|false Last run migration information or false if no migration has been run yet
 	 */
 	public function get_last_run_migration()
 	{
@@ -296,7 +296,7 @@ class migrator
 	/**
 	 * Effectively runs a single update step from the next migration to be applied.
 	 *
-	 * @return null
+	 * @return void
 	 */
 	protected function update_do()
 	{
@@ -517,7 +517,7 @@ class migrator
 	 * Effectively runs a single revert step from the last migration installed
 	 *
 	 * @param string $migration String migration name to revert (including any that depend on this migration)
-	 * @return null
+	 * @return void
 	 */
 	protected function revert_do($migration)
 	{
@@ -651,7 +651,7 @@ class migrator
 	* @param array $steps The steps to run
 	* @param bool|string $state Current state of the migration
 	* @param bool $revert true to revert a data step
-	* @return bool|string migration state. True if completed, serialized array if not finished
+	* @return bool|array{result: mixed, step: int}  migration state. True if completed, serialized array if not finished
 	* @throws \phpbb\db\migration\exception
 	*/
 	protected function process_data_step($steps, $state, $revert = false)
@@ -744,7 +744,8 @@ class migrator
 	* @param array $step Data step from migration
 	* @param mixed $last_result Result to pass to the callable (only for 'custom' method)
 	* @param bool $reverse False to install, True to attempt uninstallation by reversing the call
-	* @return array Array with parameters for call_user_func_array(), 0 is the callable, 1 is parameters
+	* @return array|false Array with parameters for call_user_func_array(), 0 is the callable, 1 is parameters;
+	*					false if no callable can be created from data setp
 	* @throws \phpbb\db\migration\exception
 	*/
 	protected function get_callable_from_step(array $step, $last_result = 0, $reverse = false)

phpBB/phpbb/db/tools/doctrine.php

@@ -462,7 +462,8 @@ class doctrine implements tools_interface
 		}
 		catch (Exception $e)
 		{
-			return $e->getMessage();
+			// @todo: check if it makes sense to properly handle the exception
+			return false;
 		}
 	}