Roundup of issues reported by PHPStorm

Mostly redundant checks, logic errors, dead code, etc.

6 files changed, 31 insertions, 40 deletions

diff --git a/modules-available/sysconfig/inc/configmodule.inc.php b/modules-available/sysconfig/inc/configmodule.inc.php
index 3b877d10..e414a6cf 100644
--- a/modules-available/sysconfig/inc/configmodule.inc.php
+++ b/modules-available/sysconfig/inc/configmodule.inc.php
@@ -470,11 +470,11 @@ abstract class ConfigModule
 			$status = 'MISSING';
 		else
 			$status = 'OUTDATED';
-		return Database::exec("UPDATE configtgz_module SET filepath = :filename, status = :status WHERE moduleid = :id LIMIT 1", array(
+		Database::exec("UPDATE configtgz_module SET filepath = :filename, status = :status WHERE moduleid = :id LIMIT 1", array(
 				'id' => $this->moduleId,
 				'filename' => $this->moduleArchive,
 				'status' => $status
-			)) !== false;
+			));
 	}
 
 	public function dateline_s()
diff --git a/modules-available/sysconfig/inc/configmodule/branding.inc.php b/modules-available/sysconfig/inc/configmodule/branding.inc.php
index 8990dbec..76968ff4 100644
--- a/modules-available/sysconfig/inc/configmodule/branding.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/branding.inc.php
@@ -22,13 +22,12 @@ class ConfigModule_Branding extends ConfigModule
 		if (!$this->validateConfig()) {
 			return $this->archive() !== false && file_exists($this->archive()); // No new temp file given, old archive still exists, pretend it worked...
 		}
-		$task = Taskmanager::submit('MoveFile', array(
+		return Taskmanager::submit('MoveFile', array(
 			'source' => $this->tmpFile,
 			'destination' => $tgz,
 			'parentTask' => $parent,
 			'failOnParentFail' => false
 		));
-		return $task;
 	}
 
 	protected function moduleVersion()
diff --git a/modules-available/sysconfig/inc/configmodule/customodule.inc.php b/modules-available/sysconfig/inc/configmodule/customodule.inc.php
index 8b968336..56aa9277 100644
--- a/modules-available/sysconfig/inc/configmodule/customodule.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/customodule.inc.php
@@ -34,13 +34,12 @@ class ConfigModule_CustomModule extends ConfigModule
 			// Nothing to do
 			return true;
 		}
-		$task = Taskmanager::submit('MoveFile', array(
+		return Taskmanager::submit('MoveFile', array(
 			'source' => $this->tmpFile,
 			'destination' => $tgz,
 			'parentTask' => $parent,
 			'failOnParentFail' => false
 		));
-		return $task;
 	}
 
 	protected function moduleVersion()
diff --git a/modules-available/sysconfig/inc/configmodule/screensaver.inc.php b/modules-available/sysconfig/inc/configmodule/screensaver.inc.php
index ed97941e..08a2c9a1 100644
--- a/modules-available/sysconfig/inc/configmodule/screensaver.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/screensaver.inc.php
@@ -23,13 +23,11 @@ class ConfigModule_Screensaver extends ConfigModule
 		/* Give the Taskmanager the job and create the tgz */
 		$taskId = 'xscreensaver' . mt_rand() . '-' . microtime(true);
 
-		$task = Taskmanager::submit('MakeTarball', array(
+		return Taskmanager::submit('MakeTarball', array(
 			'id' => $taskId,
 			'files' => $this->getFileArray(),
 			'destination' => $tgz,
 		), false);
-
-		return $task;
 	}
 
 	protected function moduleVersion()
diff --git a/modules-available/sysconfig/inc/configtgz.inc.php b/modules-available/sysconfig/inc/configtgz.inc.php
index 67e02503..9ab3ffd3 100644
--- a/modules-available/sysconfig/inc/configtgz.inc.php
+++ b/modules-available/sysconfig/inc/configtgz.inc.php
@@ -10,7 +10,6 @@ class ConfigTgz
 
 	private function __construct()
 	{
-		;
 	}
 
 	public function id()
@@ -134,7 +133,7 @@ class ConfigTgz
 		return $task['id'];
 	}
 
-	public function delete()
+	public function delete(): bool
 	{
 		if ($this->configId === 0)
 			ErrorHandler::traceError('ConfigTgz::delete called with invalid config id!');
@@ -150,14 +149,14 @@ class ConfigTgz
 		return $ret !== false;
 	}
 	
-	public function markOutdated()
+	public function markOutdated(): void
 	{
 		if ($this->configId === 0)
 			ErrorHandler::traceError('ConfigTgz::markOutdated called with invalid config id!');
-		return $this->mark('OUTDATED');
+		$this->mark('OUTDATED');
 	}
 	
-	private function markUpdated($task)
+	private function markUpdated($task): void
 	{
 		if ($this->configId === 0)
 			ErrorHandler::traceError('ConfigTgz::markUpdated called with invalid config id!');
@@ -186,27 +185,28 @@ class ConfigTgz
 				'status' => 'OK',
 				'warnings' => $warnings,
 			]);
-			return 'OK';
+			return;
 		}
-		return $this->mark('OUTDATED');
+		$this->mark('OUTDATED');
 	}
 	
-	private function markFailed()
+	private function markFailed(): void
 	{
 		if ($this->configId === 0)
 			ErrorHandler::traceError('ConfigTgz::markFailed called with invalid config id!');
-		if ($this->file === false || !file_exists($this->file))
-			return $this->mark('MISSING');
-		return $this->mark('OUTDATED');
+		if ($this->file === false || !file_exists($this->file)) {
+			$this->mark('MISSING');
+		} else {
+			$this->mark('OUTDATED');
+		}
 	}
 	
-	private function mark($status)
+	private function mark($status): void
 	{
 		Database::exec("UPDATE configtgz SET status = :status WHERE configid = :configid LIMIT 1", [
 			'configid' => $this->configId,
 			'status' => $status,
 		]);
-		return $status;
 	}
 
 	/*
diff --git a/modules-available/sysconfig/inc/ppd.inc.php b/modules-available/sysconfig/inc/ppd.inc.php
index 8e125ed7..75608c56 100644
--- a/modules-available/sysconfig/inc/ppd.inc.php
+++ b/modules-available/sysconfig/inc/ppd.inc.php
@@ -308,8 +308,8 @@ class Ppd
 		$this->requiredKeywords = array();
 
 		// Parse
-		/* @var $rawOption \PpdOption */
-		/* @var $currentBlock \PpdBlockInternal */
+		/* @var PpdOption $rawOption */
+		/* @var PpdBlockInternal $currentBlock */
 		$currentBlock = false;
 		$inRawBlock = false; // True if in a multi-line InvocationValue or QuotedValue (3.6: Parsing Summary for Values)
 		$wantsEnd = false;
@@ -425,10 +425,7 @@ class Ppd
 						return;
 					}
 					// TODO: Check unique
-					$nb = new PpdBlockInternal($value, $valueTranslation, 'Group', $currentBlock, $lStart);
-					if ($currentBlock !== false) {
-						$currentBlock->childBlocks[] = $nb;
-					}
+					$nb = new PpdBlockInternal($value, $valueTranslation, 'Group', false, $lStart);
 					$currentBlock = $nb;
 					continue;
 				} elseif ($mainKeyword === 'OpenSubGroup') {
@@ -438,9 +435,7 @@ class Ppd
 					}
 					// TODO: Check unique
 					$nb = new PpdBlockInternal($value, $valueTranslation, 'SubGroup', $currentBlock, $lStart);
-					if ($currentBlock !== false) {
-						$currentBlock->childBlocks[] = $nb;
-					}
+					$currentBlock->childBlocks[] = $nb;
 					$currentBlock = $nb;
 					continue;
 				} elseif ($mainKeyword === 'OpenUI' || $mainKeyword === 'JCLOpenUI') {
@@ -542,7 +537,8 @@ class Ppd
 					$option = $this->getOption(substr($mainKeyword, 7), $currentBlock);
 					$option->default = new PpdOption($lStart, $len, $value, $valueTranslation);
 					continue;
-				} elseif (substr($mainKeyword, 0, 17) === 'FoomaticRIPOption') {
+				}
+				if (substr($mainKeyword, 0, 17) === 'FoomaticRIPOption') {
 					if ($optionKeyword === false) {
 						$this->warn($no, "$mainKeyword with no option keyword");
 					} elseif ($currentBlock !== false && isset($this->settings[$optionKeyword])) {
@@ -604,7 +600,7 @@ class Ppd
 			} elseif (strlen(trim($line)) !== 0) {
 				$this->warn($no, 'Invalid format; not empty and not starting with asterisk (*)');
 			}
-		}
+		} // end while loop over ppd contents
 		//
 		if ($currentBlock !== false) {
 			$this->error = 'Block ' . $currentBlock->id . ' (' . $currentBlock->type . ') was never closed.';
@@ -691,10 +687,9 @@ class Ppd
 
 	private function escapeBinaryArray($array)
 	{
-		$chars = array_reduce(array_unique($array), function ($carry, $item) {
+		return array_reduce(array_unique($array), function ($carry, $item) {
 			return $carry . '\x' . dechex(ord($item));
 		}, '');
-		return $chars;
 	}
 
 	private function unhexTranslation($lineNo, $translation)
@@ -957,7 +952,7 @@ class Ppd
 				// $start must lie before range start, otherwise we'd have hit the case above
 				$end = $ranges[$key][1];
 				unset($ranges[$key]);
-				continue;
+				//continue;
 			}
 		}
 		$ranges[] = array($start, $end);
@@ -1107,11 +1102,11 @@ class PpdBlockInternal
 	public $translation;
 	public $type;
 	/**
-	 * @var \PpdBlockInternal[]
+	 * @var PpdBlockInternal[]
 	 */
 	public $childBlocks = array();
 	/**
-	 * @var \PpdBlockInternal
+	 * @var PpdBlockInternal|false
 	 */
 	public $parent;
 
@@ -1148,10 +1143,10 @@ class PpdBlockInternal
 	}
 
 	/**
-	 * @param \PpdBlockInternal $block some other PpdBlock instance
+	 * @param PpdBlockInternal $block some other PpdBlock instance
 	 * @return bool true if this is a child of $block
 	 */
-	public function isChildOf($block)
+	public function isChildOf(PpdBlockInternal $block)
 	{
 		$parent = $this->parent;
 		while ($parent !== false) {