Cleanup plsam issues in DB/ContactsManager and Console

Signed-off-by: Carl Schwan <[email protected]>

Author: CarlSchwan

build/psalm-baseline.xml

@@ -1806,32 +1806,6 @@
       <code>dispatch</code>
     </TooManyArguments>
   </file>
-  <file src="lib/private/ContactsManager.php">
-    <InvalidNullableReturnType occurrences="3">
-      <code>IAddressBook</code>
-      <code>array</code>
-      <code>bool</code>
-    </InvalidNullableReturnType>
-    <NullableReturnStatement occurrences="5">
-      <code>null</code>
-      <code>null</code>
-      <code>null</code>
-      <code>null</code>
-      <code>null</code>
-    </NullableReturnStatement>
-    <ParamNameMismatch occurrences="2">
-      <code>$addressBook</code>
-      <code>$addressBook</code>
-    </ParamNameMismatch>
-  </file>
-  <file src="lib/private/DB/Adapter.php">
-    <InvalidReturnStatement occurrences="1">
-      <code>$builder-&gt;execute()</code>
-    </InvalidReturnStatement>
-    <InvalidReturnType occurrences="1">
-      <code>int</code>
-    </InvalidReturnType>
-  </file>
   <file src="lib/private/DB/AdapterMySQL.php">
     <InternalMethod occurrences="1">
       <code>getParams</code>

lib/private/Console/Application.php

@@ -47,17 +47,12 @@
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 class Application {
-	/** @var IConfig */
-	private $config;
+	private IConfig $config;
 	private SymfonyApplication $application;
-	/** @var EventDispatcherInterface */
-	private $dispatcher;
-	/** @var IRequest */
-	private $request;
-	/** @var LoggerInterface */
-	private $logger;
-	/** @var MemoryInfo */
-	private $memoryInfo;
+	private EventDispatcherInterface $dispatcher;
+	private IRequest $request;
+	private LoggerInterface $logger;
+	private MemoryInfo $memoryInfo;
 
 	public function __construct(IConfig $config,
 								EventDispatcherInterface $dispatcher,
@@ -74,8 +69,6 @@ public function __construct(IConfig $config,
 	}
 
 	/**
-	 * @param InputInterface $input
-	 * @param ConsoleOutputInterface $output
 	 * @throws \Exception
 	 */
 	public function loadCommands(

lib/private/ContactsManager.php

@@ -88,32 +88,32 @@ public function search($pattern, $searchProperties = [], $options = []) {
 	 * This function can be used to delete the contact identified by the given id
 	 *
 	 * @param int $id the unique identifier to a contact
-	 * @param string $address_book_key identifier of the address book in which the contact shall be deleted
+	 * @param string $addressBookKey identifier of the address book in which the contact shall be deleted
 	 * @return bool successful or not
 	 */
-	public function delete($id, $address_book_key) {
-		$addressBook = $this->getAddressBook($address_book_key);
+	public function delete($id, $addressBookKey) {
+		$addressBook = $this->getAddressBook($addressBookKey);
 		if (!$addressBook) {
-			return null;
+			return false;
 		}
 
 		if ($addressBook->getPermissions() & Constants::PERMISSION_DELETE) {
 			return $addressBook->delete($id);
 		}
 
-		return null;
+		return false;
 	}
 
 	/**
 	 * This function is used to create a new contact if 'id' is not given or not present.
 	 * Otherwise the contact will be updated by replacing the entire data set.
 	 *
 	 * @param array $properties this array if key-value-pairs defines a contact
-	 * @param string $address_book_key identifier of the address book in which the contact shall be created or updated
-	 * @return array representing the contact just created or updated
+	 * @param string $addressBookKey identifier of the address book in which the contact shall be created or updated
+	 * @return ?array representing the contact just created or updated
 	 */
-	public function createOrUpdate($properties, $address_book_key) {
-		$addressBook = $this->getAddressBook($address_book_key);
+	public function createOrUpdate($properties, $addressBookKey) {
+		$addressBook = $this->getAddressBook($addressBookKey);
 		if (!$addressBook) {
 			return null;
 		}
@@ -130,7 +130,7 @@ public function createOrUpdate($properties, $address_book_key) {
 	 *
 	 * @return bool true if enabled, false if not
 	 */
-	public function isEnabled() {
+	public function isEnabled(): bool {
 		return !empty($this->addressBooks) || !empty($this->addressBookLoaders);
 	}
 
@@ -189,11 +189,8 @@ public function register(\Closure $callable) {
 
 	/**
 	 * Get (and load when needed) the address book for $key
-	 *
-	 * @param string $addressBookKey
-	 * @return IAddressBook
 	 */
-	protected function getAddressBook($addressBookKey) {
+	protected function getAddressBook(string $addressBookKey): ?IAddressBook {
 		$this->loadAddressBooks();
 		if (!array_key_exists($addressBookKey, $this->addressBooks)) {
 			return null;

lib/private/DB/Adapter.php

@@ -142,7 +142,7 @@ public function insertIgnoreConflict(string $table, array $values) : int {
 			foreach ($values as $key => $value) {
 				$builder->setValue($key, $builder->createNamedParameter($value));
 			}
-			return $builder->execute();
+			return $builder->executeStatement();
 		} catch (UniqueConstraintViolationException $e) {
 			return 0;
 		}

lib/private/DB/Connection.php

@@ -137,7 +137,7 @@ public function connect() {
 			return $status;
 		} catch (Exception $e) {
 			// throw a new exception to prevent leaking info from the stacktrace
-			throw new Exception('Failed to connect to the database: ' . $e->getMessage(), $e->getCode());
+			throw new Exception('Failed to connect to the database: ' . $e->getMessage(), (int)$e->getCode());
 		}
 	}
 
@@ -220,7 +220,7 @@ public function getPrefix() {
 	 * @return Statement The prepared statement.
 	 * @throws Exception
 	 */
-	public function prepare($statement, $limit = null, $offset = null): Statement {
+	public function prepare($sql, $limit = null, $offset = null): Statement {
 		if ($limit === -1 || $limit === null) {
 			$limit = null;
 		} else {
@@ -231,9 +231,9 @@ public function prepare($statement, $limit = null, $offset = null): Statement {
 		}
 		if (!is_null($limit)) {
 			$platform = $this->getDatabasePlatform();
-			$statement = $platform->modifyLimitQuery($statement, $limit, $offset);
+			$sql = $platform->modifyLimitQuery($sql, $limit, $offset);
 		}
-		$statement = $this->replaceTablePrefix($statement);
+		$statement = $this->replaceTablePrefix($sql);
 		$statement = $this->adapter->fixupStatement($statement);
 
 		return parent::prepare($statement);
@@ -321,14 +321,14 @@ protected function logQueryToFile(string $sql): void {
 	 *
 	 * @param string $seqName Name of the sequence object from which the ID should be returned.
 	 *
-	 * @return string the last inserted ID.
+	 * @return int the last inserted ID.
 	 * @throws Exception
 	 */
-	public function lastInsertId($seqName = null) {
-		if ($seqName) {
-			$seqName = $this->replaceTablePrefix($seqName);
+	public function lastInsertId($name = null): int {
+		if ($name) {
+			$name = $this->replaceTablePrefix($name);
 		}
-		return $this->adapter->lastInsertId($seqName);
+		return $this->adapter->lastInsertId($name);
 	}
 
 	/**

lib/private/DB/ConnectionAdapter.php

@@ -87,7 +87,7 @@ public function executeStatement($sql, array $params = [], array $types = []): i
 
 	public function lastInsertId(string $table): int {
 		try {
-			return (int)$this->inner->lastInsertId($table);
+			return $this->inner->lastInsertId($table);
 		} catch (Exception $e) {
 			throw DbalException::wrap($e);
 		}

lib/private/DB/MigrationService.php

@@ -58,7 +58,7 @@ class MigrationService {
 	/**
 	 * @throws \Exception
 	 */
-	public function __construct($appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
+	public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
 		$this->appName = $appName;
 		$this->connection = $connection;
 		if ($output === null) {
@@ -100,18 +100,15 @@ public function __construct($appName, Connection $connection, ?IOutput $output =
 
 	/**
 	 * Returns the name of the app for which this migration is executed
-	 *
-	 * @return string
 	 */
-	public function getApp() {
+	public function getApp(): string {
 		return $this->appName;
 	}
 
 	/**
-	 * @return bool
 	 * @codeCoverageIgnore - this will implicitly tested on installation
 	 */
-	private function createMigrationTable() {
+	private function createMigrationTable(): bool {
 		if ($this->migrationTableCreated) {
 			return false;
 		}
@@ -176,7 +173,7 @@ private function createMigrationTable() {
 	/**
 	 * Returns all versions which have already been applied
 	 *
-	 * @return string[]
+	 * @return list<string>
 	 * @codeCoverageIgnore - no need to test this
 	 */
 	public function getMigratedVersions() {
@@ -188,7 +185,7 @@ public function getMigratedVersions() {
 			->where($qb->expr()->eq('app', $qb->createNamedParameter($this->getApp())))
 			->orderBy('version');
 
-		$result = $qb->execute();
+		$result = $qb->executeQuery();
 		$rows = $result->fetchAll(\PDO::FETCH_COLUMN);
 		$result->closeCursor();
 
@@ -197,15 +194,17 @@ public function getMigratedVersions() {
 
 	/**
 	 * Returns all versions which are available in the migration folder
-	 *
-	 * @return array
+	 * @return list<string>
 	 */
-	public function getAvailableVersions() {
+	public function getAvailableVersions(): array {
 		$this->ensureMigrationsAreLoaded();
 		return array_map('strval', array_keys($this->migrations));
 	}
 
-	protected function findMigrations() {
+	/**
+	 * @return array<string, string>
+	 */
+	protected function findMigrations(): array {
 		$directory = realpath($this->migrationsPath);
 		if ($directory === false || !file_exists($directory) || !is_dir($directory)) {
 			return [];
@@ -322,10 +321,9 @@ public function getMigrationsDirectory() {
 	/**
 	 * Return the explicit version for the aliases; current, next, prev, latest
 	 *
-	 * @param string $alias
 	 * @return mixed|null|string
 	 */
-	public function getMigration($alias) {
+	public function getMigration(string $alias) {
 		switch ($alias) {
 			case 'current':
 				return $this->getCurrentVersion();
@@ -342,29 +340,22 @@ public function getMigration($alias) {
 		return '0';
 	}
 
-	/**
-	 * @param string $version
-	 * @param int $delta
-	 * @return null|string
-	 */
-	private function getRelativeVersion($version, $delta) {
+	private function getRelativeVersion(string $version, int $delta): ?string {
 		$this->ensureMigrationsAreLoaded();
 
 		$versions = $this->getAvailableVersions();
-		array_unshift($versions, 0);
+		array_unshift($versions, '0');
+		/** @var int $offset */
 		$offset = array_search($version, $versions, true);
 		if ($offset === false || !isset($versions[$offset + $delta])) {
 			// Unknown version or delta out of bounds.
 			return null;
 		}
 
-		return (string) $versions[$offset + $delta];
+		return (string)$versions[$offset + $delta];
 	}
 
-	/**
-	 * @return string
-	 */
-	private function getCurrentVersion() {
+	private function getCurrentVersion(): string {
 		$m = $this->getMigratedVersions();
 		if (count($m) === 0) {
 			return '0';
@@ -374,11 +365,9 @@ private function getCurrentVersion() {
 	}
 
 	/**
-	 * @param string $version
-	 * @return string
 	 * @throws \InvalidArgumentException
 	 */
-	private function getClass($version) {
+	private function getClass(string $version): string {
 		$this->ensureMigrationsAreLoaded();
 
 		if (isset($this->migrations[$version])) {
@@ -390,21 +379,16 @@ private function getClass($version) {
 
 	/**
 	 * Allows to set an IOutput implementation which is used for logging progress and messages
-	 *
-	 * @param IOutput $output
 	 */
-	public function setOutput(IOutput $output) {
+	public function setOutput(IOutput $output): void {
 		$this->output = $output;
 	}
 
 	/**
 	 * Applies all not yet applied versions up to $to
-	 *
-	 * @param string $to
-	 * @param bool $schemaOnly
 	 * @throws \InvalidArgumentException
 	 */
-	public function migrate($to = 'latest', $schemaOnly = false) {
+	public function migrate(string $to = 'latest', bool $schemaOnly = false): void {
 		if ($schemaOnly) {
 			$this->migrateSchemaOnly($to);
 			return;
@@ -425,11 +409,9 @@ public function migrate($to = 'latest', $schemaOnly = false) {
 
 	/**
 	 * Applies all not yet applied versions up to $to
-	 *
-	 * @param string $to
 	 * @throws \InvalidArgumentException
 	 */
-	public function migrateSchemaOnly($to = 'latest') {
+	public function migrateSchemaOnly(string $to = 'latest'): void {
 		// read known migrations
 		$toBeExecuted = $this->getMigrationsToExecute($to);
 

lib/private/DB/OracleConnection.php

@@ -29,6 +29,8 @@
 class OracleConnection extends Connection {
 	/**
 	 * Quote the keys of the array
+	 * @param array<string, string> $data
+	 * @return array<string, string>
 	 */
 	private function quoteKeys(array $data) {
 		$return = [];