Merge pull request #31333 from nextcloud/fix/fix-archive-typing

Improve typing in OC\Archive classes

3 files changed, 123 insertions, 169 deletions

diff --git a/lib/private/Archive/Archive.php b/lib/private/Archive/Archive.php
index 141db11f419..cef306230fd 100644
--- a/lib/private/Archive/Archive.php
+++ b/lib/private/Archive/Archive.php
@@ -30,97 +30,84 @@
 namespace OC\Archive;
 
 abstract class Archive {
-	/**
-	 * @param $source
-	 */
-	abstract public function __construct($source);
+	abstract public function __construct(string $source);
+
 	/**
 	 * add an empty folder to the archive
-	 * @param string $path
-	 * @return bool
 	 */
-	abstract public function addFolder($path);
+	abstract public function addFolder(string $path): bool;
+
 	/**
 	 * add a file to the archive
-	 * @param string $path
 	 * @param string $source either a local file or string data
-	 * @return bool
 	 */
-	abstract public function addFile($path, $source = '');
+	abstract public function addFile(string $path, string $source = ''): bool;
+
 	/**
 	 * rename a file or folder in the archive
-	 * @param string $source
-	 * @param string $dest
-	 * @return bool
 	 */
-	abstract public function rename($source, $dest);
+	abstract public function rename(string $source, string $dest): bool;
+
 	/**
 	 * get the uncompressed size of a file in the archive
-	 * @param string $path
-	 * @return int
+	 * @return int|false
 	 */
-	abstract public function filesize($path);
+	abstract public function filesize(string $path);
+
 	/**
 	 * get the last modified time of a file in the archive
-	 * @param string $path
-	 * @return int
+	 * @return int|false
 	 */
-	abstract public function mtime($path);
+	abstract public function mtime(string $path);
+
 	/**
 	 * get the files in a folder
 	 * @param string $path
 	 * @return array
 	 */
-	abstract public function getFolder($path);
+	abstract public function getFolder(string $path): array;
+
 	/**
 	 * get all files in the archive
-	 * @return array
 	 */
-	abstract public function getFiles();
+	abstract public function getFiles(): array;
+
 	/**
 	 * get the content of a file
-	 * @param string $path
-	 * @return string
+	 * @return string|false
 	 */
-	abstract public function getFile($path);
+	abstract public function getFile(string $path);
+
 	/**
 	 * extract a single file from the archive
-	 * @param string $path
-	 * @param string $dest
-	 * @return bool
 	 */
-	abstract public function extractFile($path, $dest);
+	abstract public function extractFile(string $path, string $dest): bool;
+
 	/**
 	 * extract the archive
-	 * @param string $dest
-	 * @return bool
 	 */
-	abstract public function extract($dest);
+	abstract public function extract(string $dest): bool;
+
 	/**
 	 * check if a file or folder exists in the archive
-	 * @param string $path
-	 * @return bool
 	 */
-	abstract public function fileExists($path);
+	abstract public function fileExists(string $path): bool;
+
 	/**
 	 * remove a file or folder from the archive
-	 * @param string $path
-	 * @return bool
 	 */
-	abstract public function remove($path);
+	abstract public function remove(string $path): bool;
+
 	/**
 	 * get a file handler
-	 * @param string $path
-	 * @param string $mode
 	 * @return bool|resource
 	 */
-	abstract public function getStream($path, $mode);
+	abstract public function getStream(string $path, string $mode);
+
 	/**
 	 * add a folder and all its content
-	 * @param string $path
-	 * @param string $source
 	 */
-	public function addRecursive($path, $source) {
+	public function addRecursive(string $path, string $source): void {
 		$dh = opendir($source);
 		if (is_resource($dh)) {
 			$this->addFolder($path);
diff --git a/lib/private/Archive/TAR.php b/lib/private/Archive/TAR.php
index a024ce84b2f..79c09cbe9e2 100644
--- a/lib/private/Archive/TAR.php
+++ b/lib/private/Archive/TAR.php
@@ -39,19 +39,26 @@ class TAR extends Archive {
 	public const GZIP = 1;
 	public const BZIP = 2;
 
-	private $fileList;
-	private $cachedHeaders;
+	/**
+	 * @var string[]|false
+	 */
+	private $fileList = false;
+	/**
+	 * @var array|false
+	 */
+	private $cachedHeaders = false;
 
 	/**
-	 * @var \Archive_Tar tar
+	 * @var \Archive_Tar
 	 */
 	private $tar = null;
-	private $path;
 
 	/**
-	 * @param string $source
+	 * @var string
 	 */
-	public function __construct($source) {
+	private $path;
+
+	public function __construct(string $source) {
 		$types = [null, 'gz', 'bz2'];
 		$this->path = $source;
 		$this->tar = new \Archive_Tar($source, $types[self::getTarType($source)]);
@@ -59,11 +66,8 @@ class TAR extends Archive {
 
 	/**
 	 * try to detect the type of tar compression
-	 *
-	 * @param string $file
-	 * @return integer
 	 */
-	public static function getTarType($file) {
+	public static function getTarType(string $file): int {
 		if (strpos($file, '.')) {
 			$extension = substr($file, strrpos($file, '.'));
 			switch ($extension) {
@@ -85,11 +89,8 @@ class TAR extends Archive {
 
 	/**
 	 * add an empty folder to the archive
-	 *
-	 * @param string $path
-	 * @return bool
 	 */
-	public function addFolder($path) {
+	public function addFolder(string $path): bool {
 		$tmpBase = \OC::$server->getTempManager()->getTemporaryFolder();
 		$path = rtrim($path, '/') . '/';
 		if ($this->fileExists($path)) {
@@ -113,11 +114,9 @@ class TAR extends Archive {
 	/**
 	 * add a file to the archive
 	 *
-	 * @param string $path
 	 * @param string $source either a local file or string data
-	 * @return bool
 	 */
-	public function addFile($path, $source = '') {
+	public function addFile(string $path, string $source = ''): bool {
 		if ($this->fileExists($path)) {
 			$this->remove($path);
 		}
@@ -132,12 +131,8 @@ class TAR extends Archive {
 
 	/**
 	 * rename a file or folder in the archive
-	 *
-	 * @param string $source
-	 * @param string $dest
-	 * @return bool
 	 */
-	public function rename($source, $dest) {
+	public function rename(string $source, string $dest): bool {
 		//no proper way to delete, rename entire archive, rename file and remake archive
 		$tmp = \OC::$server->getTempManager()->getTemporaryFolder();
 		$this->tar->extract($tmp);
@@ -152,10 +147,7 @@ class TAR extends Archive {
 		return true;
 	}
 
-	/**
-	 * @param string $file
-	 */
-	private function getHeader($file) {
+	private function getHeader(string $file): ?array {
 		if (!$this->cachedHeaders) {
 			$this->cachedHeaders = $this->tar->listContent();
 		}
@@ -174,32 +166,27 @@ class TAR extends Archive {
 	/**
 	 * get the uncompressed size of a file in the archive
 	 *
-	 * @param string $path
-	 * @return int
+	 * @return int|false
 	 */
-	public function filesize($path) {
+	public function filesize(string $path) {
 		$stat = $this->getHeader($path);
-		return $stat['size'];
+		return $stat['size'] ?? false;
 	}
 
 	/**
 	 * get the last modified time of a file in the archive
 	 *
-	 * @param string $path
-	 * @return int
+	 * @return int|false
 	 */
-	public function mtime($path) {
+	public function mtime(string $path) {
 		$stat = $this->getHeader($path);
-		return $stat['mtime'];
+		return $stat['mtime'] ?? false;
 	}
 
 	/**
 	 * get the files in a folder
-	 *
-	 * @param string $path
-	 * @return array
 	 */
-	public function getFolder($path) {
+	public function getFolder(string $path): array {
 		$files = $this->getFiles();
 		$folderContent = [];
 		$pathLength = strlen($path);
@@ -222,10 +209,8 @@ class TAR extends Archive {
 
 	/**
 	 * get all files in the archive
-	 *
-	 * @return array
 	 */
-	public function getFiles() {
+	public function getFiles(): array {
 		if ($this->fileList) {
 			return $this->fileList;
 		}
@@ -243,21 +228,21 @@ class TAR extends Archive {
 	/**
 	 * get the content of a file
 	 *
-	 * @param string $path
-	 * @return string
+	 * @return string|false
 	 */
-	public function getFile($path) {
-		return $this->tar->extractInString($path);
+	public function getFile(string $path) {
+		$string = $this->tar->extractInString($path);
+		if (is_string($string)) {
+			return $string;
+		} else {
+			return false;
+		}
 	}
 
 	/**
 	 * extract a single file from the archive
-	 *
-	 * @param string $path
-	 * @param string $dest
-	 * @return bool
 	 */
-	public function extractFile($path, $dest) {
+	public function extractFile(string $path, string $dest): bool {
 		$tmp = \OC::$server->getTempManager()->getTemporaryFolder();
 		if (!$this->fileExists($path)) {
 			return false;
@@ -276,21 +261,15 @@ class TAR extends Archive {
 
 	/**
 	 * extract the archive
-	 *
-	 * @param string $dest
-	 * @return bool
 	 */
-	public function extract($dest) {
+	public function extract(string $dest): bool {
 		return $this->tar->extract($dest);
 	}
 
 	/**
 	 * check if a file or folder exists in the archive
-	 *
-	 * @param string $path
-	 * @return bool
 	 */
-	public function fileExists($path) {
+	public function fileExists(string $path): bool {
 		$files = $this->getFiles();
 		if ((array_search($path, $files) !== false) or (array_search($path . '/', $files) !== false)) {
 			return true;
@@ -312,11 +291,8 @@ class TAR extends Archive {
 
 	/**
 	 * remove a file or folder from the archive
-	 *
-	 * @param string $path
-	 * @return bool
 	 */
-	public function remove($path) {
+	public function remove(string $path): bool {
 		if (!$this->fileExists($path)) {
 			return false;
 		}
@@ -336,13 +312,12 @@ class TAR extends Archive {
 	/**
 	 * get a file handler
 	 *
-	 * @param string $path
-	 * @param string $mode
 	 * @return bool|resource
 	 */
-	public function getStream($path, $mode) {
-		if (strrpos($path, '.') !== false) {
-			$ext = substr($path, strrpos($path, '.'));
+	public function getStream(string $path, string $mode) {
+		$lastPoint = strrpos($path, '.');
+		if ($lastPoint !== false) {
+			$ext = substr($path, $lastPoint);
 		} else {
 			$ext = '';
 		}
@@ -365,7 +340,7 @@ class TAR extends Archive {
 	/**
 	 * write back temporary files
 	 */
-	public function writeBack($tmpFile, $path) {
+	public function writeBack(string $tmpFile, string $path): void {
 		$this->addFile($path, $tmpFile);
 		unlink($tmpFile);
 	}
@@ -373,7 +348,7 @@ class TAR extends Archive {
 	/**
 	 * reopen the archive to ensure everything is written
 	 */
-	private function reopen() {
+	private function reopen(): void {
 		if ($this->tar) {
 			$this->tar->_close();
 			$this->tar = null;
diff --git a/lib/private/Archive/ZIP.php b/lib/private/Archive/ZIP.php
index e058d476021..ca9a046ab83 100644
--- a/lib/private/Archive/ZIP.php
+++ b/lib/private/Archive/ZIP.php
@@ -32,41 +32,42 @@
 namespace OC\Archive;
 
 use Icewind\Streams\CallbackWrapper;
-use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 
 class ZIP extends Archive {
 	/**
 	 * @var \ZipArchive zip
 	 */
-	private $zip = null;
-	private $path;
+	private $zip;
 
 	/**
-	 * @param string $source
+	 * @var string
 	 */
-	public function __construct($source) {
+	private $path;
+
+	public function __construct(string $source) {
 		$this->path = $source;
 		$this->zip = new \ZipArchive();
 		if ($this->zip->open($source, \ZipArchive::CREATE)) {
 		} else {
-			\OCP\Util::writeLog('files_archive', 'Error while opening archive '.$source, ILogger::WARN);
+			\OC::$server->get(LoggerInterface::class)->warning('Error while opening archive '.$source, ['app' => 'files_archive']);
 		}
 	}
+
 	/**
 	 * add an empty folder to the archive
 	 * @param string $path
 	 * @return bool
 	 */
-	public function addFolder($path) {
+	public function addFolder(string $path): bool {
 		return $this->zip->addEmptyDir($path);
 	}
+
 	/**
 	 * add a file to the archive
-	 * @param string $path
 	 * @param string $source either a local file or string data
-	 * @return bool
 	 */
-	public function addFile($path, $source = '') {
+	public function addFile(string $path, string $source = ''): bool {
 		if ($source and $source[0] == '/' and file_exists($source)) {
 			$result = $this->zip->addFile($source, $path);
 		} else {
@@ -78,40 +79,37 @@ class ZIP extends Archive {
 		}
 		return $result;
 	}
+
 	/**
 	 * rename a file or folder in the archive
-	 * @param string $source
-	 * @param string $dest
-	 * @return boolean|null
 	 */
-	public function rename($source, $dest) {
+	public function rename(string $source, string $dest): bool {
 		$source = $this->stripPath($source);
 		$dest = $this->stripPath($dest);
-		$this->zip->renameName($source, $dest);
+		return $this->zip->renameName($source, $dest);
 	}
+
 	/**
 	 * get the uncompressed size of a file in the archive
-	 * @param string $path
-	 * @return int
+	 * @return int|false
 	 */
-	public function filesize($path) {
+	public function filesize(string $path) {
 		$stat = $this->zip->statName($path);
-		return $stat['size'];
+		return $stat['size'] ?? false;
 	}
+
 	/**
 	 * get the last modified time of a file in the archive
-	 * @param string $path
-	 * @return int
+	 * @return int|false
 	 */
-	public function mtime($path) {
+	public function mtime(string $path) {
 		return filemtime($this->path);
 	}
+
 	/**
 	 * get the files in a folder
-	 * @param string $path
-	 * @return array
 	 */
-	public function getFolder($path) {
+	public function getFolder(string $path): array {
 		$files = $this->getFiles();
 		$folderContent = [];
 		$pathLength = strlen($path);
@@ -124,11 +122,11 @@ class ZIP extends Archive {
 		}
 		return $folderContent;
 	}
+
 	/**
 	 * get all files in the archive
-	 * @return array
 	 */
-	public function getFiles() {
+	public function getFiles(): array {
 		$fileCount = $this->zip->numFiles;
 		$files = [];
 		for ($i = 0;$i < $fileCount;$i++) {
@@ -136,67 +134,65 @@ class ZIP extends Archive {
 		}
 		return $files;
 	}
+
 	/**
 	 * get the content of a file
-	 * @param string $path
-	 * @return string
+	 * @return string|false
 	 */
-	public function getFile($path) {
+	public function getFile(string $path) {
 		return $this->zip->getFromName($path);
 	}
+
 	/**
 	 * extract a single file from the archive
-	 * @param string $path
-	 * @param string $dest
-	 * @return boolean|null
 	 */
-	public function extractFile($path, $dest) {
+	public function extractFile(string $path, string $dest): bool {
 		$fp = $this->zip->getStream($path);
-		file_put_contents($dest, $fp);
+		if ($fp === false) {
+			return false;
+		}
+		return file_put_contents($dest, $fp) !== false;
 	}
+
 	/**
 	 * extract the archive
-	 * @param string $dest
-	 * @return bool
 	 */
-	public function extract($dest) {
+	public function extract(string $dest): bool {
 		return $this->zip->extractTo($dest);
 	}
+
 	/**
 	 * check if a file or folder exists in the archive
-	 * @param string $path
-	 * @return bool
 	 */
-	public function fileExists($path) {
+	public function fileExists(string $path): bool {
 		return ($this->zip->locateName($path) !== false) or ($this->zip->locateName($path.'/') !== false);
 	}
+
 	/**
 	 * remove a file or folder from the archive
-	 * @param string $path
-	 * @return bool
 	 */
-	public function remove($path) {
+	public function remove(string $path): bool {
 		if ($this->fileExists($path.'/')) {
 			return $this->zip->deleteName($path.'/');
 		} else {
 			return $this->zip->deleteName($path);
 		}
 	}
+
 	/**
 	 * get a file handler
-	 * @param string $path
-	 * @param string $mode
 	 * @return bool|resource
 	 */
-	public function getStream($path, $mode) {
+	public function getStream(string $path, string $mode) {
 		if ($mode == 'r' or $mode == 'rb') {
 			return $this->zip->getStream($path);
 		} else {
 			//since we can't directly get a writable stream,
 			//make a temp copy of the file and put it back
 			//in the archive when the stream is closed
-			if (strrpos($path, '.') !== false) {
-				$ext = substr($path, strrpos($path, '.'));
+			$lastPoint = strrpos($path, '.');
+			if ($lastPoint !== false) {
+				$ext = substr($path, $lastPoint);
 			} else {
 				$ext = '';
 			}
@@ -214,16 +210,12 @@ class ZIP extends Archive {
 	/**
 	 * write back temporary files
 	 */
-	public function writeBack($tmpFile, $path) {
+	public function writeBack(string $tmpFile, string $path): void {
 		$this->addFile($path, $tmpFile);
 		unlink($tmpFile);
 	}
 
-	/**
-	 * @param string $path
-	 * @return string
-	 */
-	private function stripPath($path) {
+	private function stripPath(string $path): string {
 		if (!$path || $path[0] == '/') {
 			return substr($path, 1);
 		} else {