Fix CLI flag parsing (#7430)

* Fix CLI flag parsing
fix https://github.com/FreshRSS/FreshRSS/issues/7428

* Fix other places

* Forgotten debugging

cli/CliOptionsParser.php

@@ -84,7 +84,11 @@ abstract class CliOptionsParser {
 		foreach ($this->inputs as $name => $input) {
 			$values = $input['values'] ?? $input['defaultInput'] ?? null;
 			$types = $this->options[$name]->getTypes();
-			if (!empty($values)) {
+
+			if ($this->options[$name]->getValueTaken() === CliOption::VALUE_NONE) {
+				// @phpstan-ignore property.dynamicName
+				$this->$name = $values !== null;
+			} elseif (!empty($values)) {
 				$validValues = [];
 				$typedValues = [];
 

cli/check.translation.php

@@ -11,9 +11,9 @@ require_once __DIR__ . '/../constants.php';
 $cliOptions = new class extends CliOptionsParser {
 	/** @var array<int,string> $language */
 	public array $language;
-	public string $displayResult;
-	public string $help;
-	public string $displayReport;
+	public bool $displayResult;
+	public bool $help;
+	public bool $displayReport;
 
 	public function __construct() {
 		$this->addOption('language', (new CliOption('language', 'l'))->typeOfArrayOfString());
@@ -27,7 +27,7 @@ $cliOptions = new class extends CliOptionsParser {
 if (!empty($cliOptions->errors)) {
 	fail('FreshRSS error: ' . array_shift($cliOptions->errors) . "\n" . $cliOptions->usage);
 }
-if (isset($cliOptions->help)) {
+if ($cliOptions->help) {
 	checkHelp();
 }
 
@@ -39,8 +39,6 @@ if (isset($cliOptions->language)) {
 } else {
 	$languages = $i18nData->getAvailableLanguages();
 }
-$displayResults = isset($cliOptions->displayResult);
-$displayReport = isset($cliOptions->displayReport);
 
 $isValidated = true;
 $result = [];
@@ -58,7 +56,7 @@ foreach ($languages as $language) {
 	$result[$language] = $i18nValidator->displayResult();
 }
 
-if ($displayResults) {
+if ($cliOptions->displayResult) {
 	foreach ($result as $lang => $value) {
 		echo 'Language: ', $lang, PHP_EOL;
 		print_r($value);
@@ -66,7 +64,7 @@ if ($displayResults) {
 	}
 }
 
-if ($displayReport) {
+if ($cliOptions->displayReport) {
 	foreach ($report as $value) {
 		echo $value;
 	}

cli/create-user.php

@@ -82,7 +82,7 @@ $ok = FreshRSS_user_Controller::createUser(
 	$cliOptions->email ?? null,
 	$cliOptions->password ?? '',
 	$values,
-	!isset($cliOptions->noDefaultFeeds)
+	!$cliOptions->noDefaultFeeds
 );
 
 if (!$ok) {

cli/db-backup.php

@@ -7,7 +7,7 @@ performRequirementCheck(FreshRSS_Context::systemConf()->db['type'] ?? '');
 $ok = true;
 
 $cliOptions = new class extends CliOptionsParser {
-	public string $quiet;
+	public bool $quiet;
 
 	public function __construct() {
 		$this->addOption('quiet', (new CliOption('quiet', 'q'))->withValueNone());
@@ -23,7 +23,7 @@ foreach (listUsers() as $username) {
 	$username = cliInitUser($username);
 	$filename = DATA_PATH . '/users/' . $username . '/backup.sqlite';
 	@unlink($filename);
-	$verbose = !isset($cliOptions->quiet);
+	$verbose = !$cliOptions->quiet;
 
 	if ($verbose) {
 		echo 'FreshRSS backup database to SQLite for user “', $username, "”…\n";

cli/db-restore.php

@@ -6,8 +6,8 @@ require(__DIR__ . '/_cli.php');
 performRequirementCheck(FreshRSS_Context::systemConf()->db['type'] ?? '');
 
 $cliOptions = new class extends CliOptionsParser {
-	public string $deleteBackup;
-	public string $forceOverwrite;
+	public bool $deleteBackup;
+	public bool $forceOverwrite;
 
 	public function __construct() {
 		$this->addOption('deleteBackup', (new CliOption('delete-backup'))->withValueNone());
@@ -49,10 +49,9 @@ foreach (listUsers() as $username) {
 	echo 'FreshRSS restore database from SQLite for user “', $username, "”…\n";
 
 	$databaseDAO = FreshRSS_Factory::createDatabaseDAO($username);
-	$clearFirst = isset($cliOptions->forceOverwrite);
-	$ok &= $databaseDAO->dbCopy($filename, FreshRSS_DatabaseDAO::SQLITE_IMPORT, $clearFirst);
+	$ok &= $databaseDAO->dbCopy($filename, FreshRSS_DatabaseDAO::SQLITE_IMPORT, clearFirst: $cliOptions->forceOverwrite);
 	if ($ok) {
-		if (isset($cliOptions->deleteBackup)) {
+		if ($cliOptions->deleteBackup) {
 			unlink($filename);
 		}
 	} else {

cli/import-sqlite-for-user.php

@@ -8,7 +8,7 @@ performRequirementCheck(FreshRSS_Context::systemConf()->db['type'] ?? '');
 $cliOptions = new class extends CliOptionsParser {
 	public string $user;
 	public string $filename;
-	public string $forceOverwrite;
+	public bool $forceOverwrite;
 
 	public function __construct() {
 		$this->addRequiredOption('user', (new CliOption('user')));
@@ -32,8 +32,7 @@ if (pathinfo($filename, PATHINFO_EXTENSION) !== 'sqlite') {
 echo 'FreshRSS importing database from SQLite for user “', $username, "”…\n";
 
 $databaseDAO = FreshRSS_Factory::createDatabaseDAO($username);
-$clearFirst = isset($cliOptions->forceOverwrite);
-$ok = $databaseDAO->dbCopy($filename, FreshRSS_DatabaseDAO::SQLITE_IMPORT, $clearFirst);
+$ok = $databaseDAO->dbCopy($filename, FreshRSS_DatabaseDAO::SQLITE_IMPORT, clearFirst: $cliOptions->forceOverwrite);
 if (!$ok) {
 	echo 'If you would like to clear the user database first, use the option --force-overwrite', "\n";
 }

cli/manipulate.translation.php

@@ -12,8 +12,8 @@ $cliOptions = new class extends CliOptionsParser {
 	public string $value;
 	public string $language;
 	public string $originLanguage;
-	public string $revert;
-	public string $help;
+	public bool $revert;
+	public bool $help;
 
 	public function __construct() {
 		$this->addRequiredOption('action', (new CliOption('action', 'a')));
@@ -30,7 +30,7 @@ $cliOptions = new class extends CliOptionsParser {
 if (!empty($cliOptions->errors)) {
 	fail('FreshRSS error: ' . array_shift($cliOptions->errors) . "\n" . $cliOptions->usage);
 }
-if (isset($cliOptions->help)) {
+if ($cliOptions->help) {
 	manipulateHelp();
 }
 
@@ -79,7 +79,7 @@ switch ($cliOptions->action) {
 		break;
 	case 'ignore':
 		if (isset($cliOptions->language) && isset($cliOptions->key)) {
-			$i18nData->ignore($cliOptions->key, $cliOptions->language, isset($cliOptions->revert));
+			$i18nData->ignore($cliOptions->key, $cliOptions->language, $cliOptions->revert);
 		} else {
 			error('You need to specify a valid set of options.');
 			exit;
@@ -87,7 +87,7 @@ switch ($cliOptions->action) {
 		break;
 	case 'ignore_unmodified':
 		if (isset($cliOptions->language)) {
-			$i18nData->ignore_unmodified($cliOptions->language, isset($cliOptions->revert));
+			$i18nData->ignore_unmodified($cliOptions->language, $cliOptions->revert);
 		} else {
 			error('You need to specify a valid set of options.');
 			exit;

cli/user-info.php

@@ -8,9 +8,9 @@ const DATA_FORMAT = "%-7s | %-20s | %-5s | %-7s | %-25s | %-15s | %-10s | %-10s
 $cliOptions = new class extends CliOptionsParser {
 	/** @var array<int,string> $user */
 	public array $user;
-	public string $header;
-	public string $json;
-	public string $humanReadable;
+	public bool $header;
+	public bool $json;
+	public bool $humanReadable;
 
 	public function __construct() {
 		$this->addOption('user', (new CliOption('user'))->typeOfArrayOfString());
@@ -29,14 +29,13 @@ $users = $cliOptions->user ?? listUsers();
 
 sort($users);
 
-$formatJson = isset($cliOptions->json);
 $jsonOutput = [];
-if ($formatJson) {
-	unset($cliOptions->header);
-	unset($cliOptions->humanReadable);
+if ($cliOptions->json) {
+	$cliOptions->header = false;
+	$cliOptions->humanReadable = false;
 }
 
-if (isset($cliOptions->header)) {
+if ($cliOptions->header) {
 	printf(
 		DATA_FORMAT,
 		'default',
@@ -85,12 +84,12 @@ foreach ($users as $username) {
 		'lang' => FreshRSS_Context::userConf()->language,
 		'mail_login' => FreshRSS_Context::userConf()->mail_login,
 	];
-	if (isset($cliOptions->humanReadable)) {	//Human format
+	if ($cliOptions->humanReadable) {	//Human format
 		$data['last_user_activity'] = date('c', $data['last_user_activity']);
 		$data['database_size'] = format_bytes($data['database_size']);
 	}
 
-	if ($formatJson) {
+	if ($cliOptions->json) {
 		$data['default'] = !empty($data['default']);
 		$data['admin'] = !empty($data['admin']);
 		$data['enabled'] = !empty($data['enabled']);
@@ -101,7 +100,7 @@ foreach ($users as $username) {
 	}
 }
 
-if ($formatJson) {
+if ($cliOptions->json) {
 	echo json_encode($jsonOutput), "\n";
 }
 

tests/cli/CliOptionsParserTest.php

@@ -15,6 +15,7 @@ final class CliOptionsOptionalTest extends CliOptionsParser {
 	public string $optionalValue = '';
 	public bool $optionalValueWithDefault = false;
 	public string $defaultInputAndOptionalValueWithDefault = '';
+	public bool $flag = false;
 
 	public function __construct() {
 		$this->addOption('string', (new CliOption('string', 's'))->deprecatedAs('deprecated-string'));
@@ -38,7 +39,7 @@ final class CliOptionsOptionalAndRequiredTest extends CliOptionsParser {
 	public string $string = '';
 	public int $int = 0;
 	public bool $bool = false;
-	public string $flag = '';
+	public bool $flag = false;
 
 	public function __construct() {
 		$this->addRequiredOption('required', new CliOption('required'));
@@ -160,6 +161,16 @@ class CliOptionsParserTest extends TestCase {
 		self::assertSame('optional', $result->defaultInputAndOptionalValueWithDefault);
 	}
 
+	public static function testOptionWithFlag(): void {
+		$result = self::runOptionalOptions('--flag');
+		self::assertTrue($result->flag);
+	}
+
+	public static function testOptionWithNoFlag(): void {
+		$result = self::runOptionalOptions('');
+		self::assertFalse($result->flag);
+	}
+
 	public static function testRequiredOptionNotSetReturnsError(): void {
 		$result = self::runOptionalAndRequiredOptions('');
 		self::assertSame(['required' => 'invalid input: required cannot be empty'], $result->errors);