Merge pull request #726 from nextcloud/backport/psalm-level-4

Backport: add config for the psalm to check unused variables and parameter (#655)

lib/Controller/ConfigController.php

@@ -155,7 +155,7 @@ public function setConfig(array $values): DataResponse {
 		$result = [];
 
 		if (isset($values['token'])) {
-			if ($values['token'] && $values['token'] !== '') {
+			if ($values['token']) {
 				$result = $this->storeUserInfo();
 			} else {
 				$this->clearUserInfo();
@@ -192,7 +192,7 @@ private function setIntegrationConfig(array $values): array {
 		];
 		// if values contains a key that is not in the allowedKeys array,
 		// return a response with status code 400 and an error message
-		foreach ($values as $key => $value) {
+		foreach (array_keys($values) as $key) {
 			if (!in_array($key, $allowedKeys)) {
 				throw new InvalidArgumentException('Invalid key');
 			}
@@ -247,7 +247,7 @@ private function setIntegrationConfig(array $values): array {
 			if ($key === 'setup_project_folder' || $key === 'setup_app_password') {
 				continue;
 			}
-			$this->config->setAppValue(Application::APP_ID, $key, trim($value));
+			$this->config->setAppValue(Application::APP_ID, $key, trim((string)$value));
 		}
 
 		// if the OpenProject OAuth URL has changed
@@ -266,7 +266,7 @@ private function setIntegrationConfig(array $values): array {
 					Application::APP_ID, 'nc_oauth_client_id', ''
 				);
 				$this->oauthService->setClientRedirectUri(
-					(int)$oauthClientInternalId, $values['openproject_instance_url']
+					(int)$oauthClientInternalId, (string)$values['openproject_instance_url']
 				);
 			}
 		}

lib/Controller/DirectUploadController.php

@@ -181,7 +181,6 @@ public function directUpload(string $token):DataResponse {
 				throw new NotFoundException('invalid token');
 			}
 			$tokenInfo = $this->directUploadService->getTokenInfo($token);
-			$fileId = null;
 			$directUploadFile = $this->request->getUploadedFile('file');
 			if (empty($directUploadFile)) {
 				throw new OpenprojectFileNotUploadedException(

lib/Migration/Version2001Date20221213083550.php

@@ -50,7 +50,7 @@ public function __construct(IConfig $config) {
 	/**
 	 * @param IOutput $output
 	 * @param Closure(): ISchemaWrapper $schemaClosure
-	 * @param array<string, string|null> $options
+	 * @param array $options
 	 * @return null|ISchemaWrapper
 	 */
 	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {

lib/Migration/Version2310Date20230116153411.php

@@ -37,7 +37,7 @@ class Version2310Date20230116153411 extends SimpleMigrationStep {
 	/**
 	 * @param IOutput $output
 	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
-	 * @param array<string, string|null> $options
+	 * @param array $options
 	 * @return null|ISchemaWrapper
 	 * @throws SchemaException
 	 */

lib/Migration/Version2400Date20230504144300.php

@@ -50,7 +50,7 @@ public function __construct(IConfig $config) {
 	/**
 	 * @param IOutput $output
 	 * @param Closure(): ISchemaWrapper $schemaClosure
-	 * @param array<string, string|null> $options
+	 * @param array $options
 	 * @return null|ISchemaWrapper
 	 */
 	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {

lib/Migration/Version2640Date20240628114301.php

@@ -55,7 +55,7 @@ public function __construct(
 	/**
 	 * @param IOutput $output
 	 * @param Closure(): ISchemaWrapper $schemaClosure
-	 * @param array<string, string|null> $options
+	 * @param array $options
 	 * @return null|ISchemaWrapper
 	 * @throws DoesNotExistException
 	 * @throws Exception

lib/Service/OpenProjectAPIService.php

@@ -242,7 +242,6 @@ public function searchWorkPackage(
 		bool $onlyLinkableWorkPackages = true,
 		int $workPackageId = null
 	): array {
-		$resultsById = [];
 		$filters = [];
 
 		// search by description
@@ -608,7 +607,7 @@ public static function validateIntegrationSetupInformation(?array $values, bool
 				throw new InvalidArgumentException('invalid data');
 			}
 		} else {
-			foreach ($values as $key => $value) {
+			foreach (array_keys($values) as $key) {
 				if (!in_array($key, $opKeys)) {
 					throw new InvalidArgumentException('invalid key');
 				}
@@ -765,7 +764,7 @@ public function linkWorkPackageToFile(
 			'workpackageId',
 			'fileinfo'
 		];
-		foreach ($values as $key => $value) {
+		foreach (array_keys($values) as $key) {
 			if (!in_array($key, $allowedKeys)) {
 				throw new InvalidArgumentException('invalid key');
 			}

lib/Settings/AdminSection.php

@@ -49,9 +49,9 @@ public function getPriority(): int {
 	}
 
 	/**
-	 * @return ?string The relative path to a an icon describing the section
+	 * @return string The relative path to a an icon describing the section
 	 */
-	public function getIcon(): ?string {
+	public function getIcon(): string {
 		return $this->urlGenerator->imagePath('integration_openproject', 'app-dark.svg');
 	}
 }

lib/Settings/PersonalSection.php

@@ -49,9 +49,9 @@ public function getPriority(): int {
 	}
 
 	/**
-	 * @return ?string The relative path to a an icon describing the section
+	 * @return string The relative path to a an icon describing the section
 	 */
-	public function getIcon(): ?string {
+	public function getIcon(): string {
 		return $this->urlGenerator->imagePath('integration_openproject', 'app-dark.svg');
 	}
 }

psalm.xml

@@ -1,12 +1,13 @@
 <?xml version="1.0"?>
 <psalm
-	errorLevel="6"
+	errorLevel="4"
 	resolveFromConfigFile="true"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xmlns="https://getpsalm.org/schema/config"
 	xsi:schemaLocation="https://getpsalm.org/schema/config"
 	findUnusedBaselineEntry="false"
 	findUnusedCode="false"
+	findUnusedVariablesAndParams="true"
 	autoloader="bootstrap.php"
 >
 	<projectFiles>
@@ -17,7 +18,8 @@
 		</ignoreFiles>
 	</projectFiles>
 	<stubs>
-		<file name="tests/stub.phpstub" preloadClasses="true"/>
+		<file name="tests/stub/doctrine_cacheItem.phpstub" preloadClasses="true"/>
+		<file name="tests/stub/timejob_joblist.phpstub" preloadClasses="true"/>
 	</stubs>
 	<issueHandlers>
 		<UndefinedClass>

tests/acceptance/features/bootstrap/DirectUploadContext.php

@@ -1,6 +1,7 @@
 <?php
 
 use Behat\Behat\Context\Context;
+use Behat\Behat\Context\Environment\InitializedContextEnvironment;
 use Behat\Behat\Hook\Scope\BeforeScenarioScope;
 use Behat\Gherkin\Node\TableNode;
 use PHPUnit\Framework\Assert;
@@ -215,6 +216,8 @@ public function before(BeforeScenarioScope $scope):void {
 		$environment = $scope->getEnvironment();
 
 		// Get all the contexts you need in this context
-		$this->featureContext = $environment->getContext('FeatureContext');
+		if ($environment instanceof InitializedContextEnvironment) {
+			$this->featureContext = $environment->getContext('FeatureContext');
+		}
 	}
 }

tests/acceptance/features/bootstrap/FeatureContext.php

@@ -1,6 +1,7 @@
 <?php
 
 use Behat\Behat\Context\Context;
+use Behat\Behat\Context\Environment\InitializedContextEnvironment;
 use Behat\Behat\Hook\Scope\BeforeScenarioScope;
 use Behat\Gherkin\Node\PyStringNode;
 use Behat\Gherkin\Node\TableNode;
@@ -32,7 +33,7 @@ class FeatureContext implements Context {
 	private SharingContext $sharingContext;
 	private DirectUploadContext $directUploadContext;
 	/**
-	 * @var array<int>
+	 * @var array<string|null>
 	 */
 	private array $createdFiles = [];
 
@@ -127,21 +128,21 @@ private function createUserWithRetry(string $user, array $userAttributes): void
 				$isUserCreated = true;
 				break;
 			} elseif ($response->getStatusCode() === 400 && getenv('CI')) {
-				var_dump("Error: " . $response->getBody()->getContents());
-				var_dump('Creating user ' . $user . ' failed!');
-				var_dump('Deleting the file system of ' . $user . ' and retrying the user creation again...');
+				echo("Error: " . $response->getBody()->getContents());
+				echo('Creating user ' . $user . ' failed!');
+				echo('Deleting the file system of ' . $user . ' and retrying the user creation again...');
 				exec(
 					"docker exec nextcloud  /bin/bash -c 'rm -rf data/$user'",
 					$output,
 					$command_result_code
 				);
 				if ($command_result_code === 0) {
-					var_dump('File system for user ' . $user . ' has been deleted successfully!');
+					echo('File system for user ' . $user . ' has been deleted successfully!');
 				}
 			} else {
 				// in case of any other error we just log the response
-				var_dump("Status Code: " . $response->getStatusCode());
-				var_dump("Error: " . $response->getBody()->getContents());
+				echo("Status Code: " . $response->getStatusCode());
+				echo("Error: " . $response->getBody()->getContents());
 			}
 			sleep(2);
 			$retryCreate++;
@@ -161,7 +162,7 @@ public function userHasBeenCreated(string $user, string $displayName = null):voi
 			$userAttributes['displayName'] = $displayName;
 		}
 		$this->createUserWithRetry($user, $userAttributes);
-		$userid = \strtolower((string)$user);
+		$userid = \strtolower($user);
 		$this->createdUsers[$userid] = $userAttributes;
 		$this->response = $this->makeDavRequest(
 			$user,
@@ -552,9 +553,9 @@ public function theDataOfTheOCSResponseShouldMatch(
 		PyStringNode $schemaString
 	): void {
 		$responseAsJson = json_decode($this->response->getBody()->getContents());
-		$responseAsJson = $responseAsJson->ocs->data;
+		$_responseAsJson = $responseAsJson->ocs->data;
 		JsonAssertions::assertJsonDocumentMatchesSchema(
-			$responseAsJson,
+			$_responseAsJson,
 			$this->getJSONSchema($schemaString)
 		);
 	}
@@ -568,9 +569,9 @@ public function theDataOfTheOCSResponseShouldMatch(
 	public function theDataOfTheResponseShouldMatch(
 		PyStringNode $schemaString
 	): void {
-		$responseAsJson = json_decode($this->response->getBody()->getContents());
+		$_responseAsJson = json_decode($this->response->getBody()->getContents());
 		JsonAssertions::assertJsonDocumentMatchesSchema(
-			$responseAsJson,
+			$_responseAsJson,
 			$this->getJSONSchema($schemaString)
 		);
 	}
@@ -579,7 +580,7 @@ public function uploadFileWithContent(
 		string $user,
 		?string $content,
 		string $destination
-	): int {
+	): ?string {
 		$this->response = $this->makeDavRequest(
 			$user,
 			$this->regularUserPassword,
@@ -613,18 +614,15 @@ public function theFollowingHeadersShouldBeSet(TableNode $table):void {
 			$expectedHeaderValue = $header['value'];
 			$returnedHeader = $this->response->getHeader($headerName);
 
-			$headerValue = $returnedHeader;
-			if (\is_array($returnedHeader)) {
-				if (empty($returnedHeader)) {
-					throw new Exception(
-						\sprintf(
-							"Missing expected header '%s'",
-							$headerName
-						)
-					);
-				}
-				$headerValue = $returnedHeader[0];
+			if (empty($returnedHeader)) {
+				throw new Exception(
+					\sprintf(
+						"Missing expected header '%s'",
+						$headerName
+					)
+				);
 			}
+			$headerValue = $returnedHeader[0];
 
 			Assert::assertEquals(
 				$expectedHeaderValue,
@@ -712,7 +710,7 @@ public function propfindFileOrFolder(string $user, string $path): ResponseInterf
 		);
 	}
 
-	public function getIdOfFileOrFolder(string $user, string $path): int {
+	public function getIdOfFileOrFolder(string $user, string $path): string {
 		$propfindResponse = $this->propfindFileOrFolder($user, $path);
 		// Ensure PROPFIND returned status 207
 		$this->theHTTPStatusCodeShouldBe(207, "", $propfindResponse);
@@ -721,7 +719,10 @@ public function getIdOfFileOrFolder(string $user, string $path): int {
 			'oc',
 			'http://owncloud.org/ns'
 		);
-		return (int)(string)$responseXmlObject->xpath('//oc:fileid')[0];
+		$fileId = $responseXmlObject->xpath('//oc:fileid')[0];
+		Assert::assertNotNull($fileId, __METHOD__ . " file $path user $user not found (the file may not exist)");
+
+		return (string) $fileId;
 	}
 
 	public function fileOrFolderExists(string $user, string $path): bool {
@@ -1091,6 +1092,11 @@ public function sendRequestsToAppEndpoint(
 		$fullUrl = $this->getBaseUrl();
 		$fullUrl .= "index.php/apps/integration_openproject/" . $endpoint;
 
+		// Handle PyStringNode
+		if ($data instanceof PyStringNode) {
+			$data = (string)$data;
+		}
+
 		// don't set content-type for multipart requests
 		if (is_array($data) && $headers === null) {
 			$options['multipart'] = $data;
@@ -1158,8 +1164,10 @@ public function before(BeforeScenarioScope $scope):void {
 		$environment = $scope->getEnvironment();
 
 		// Get all the contexts you need in this context
-		$this->sharingContext = $environment->getContext('SharingContext');
-		$this->directUploadContext = $environment->getContext('DirectUploadContext');
+		if ($environment instanceof InitializedContextEnvironment) {
+			$this->sharingContext = $environment->getContext('SharingContext');
+			$this->directUploadContext = $environment->getContext('DirectUploadContext');
+		}
 	}
 
 	/**

tests/acceptance/features/bootstrap/FilesVersionsContext.php

@@ -1,6 +1,7 @@
 <?php
 
 use Behat\Behat\Context\Context;
+use Behat\Behat\Context\Environment\InitializedContextEnvironment;
 use Behat\Behat\Hook\Scope\BeforeScenarioScope;
 use GuzzleHttp\Exception\GuzzleException;
 use PHPUnit\Framework\Assert;
@@ -28,23 +29,22 @@ public function theVersionFolderOfFileShouldContainElements(
 		int $count
 	):void {
 		$fileId = $this->featureContext->getIdOfFileOrFolder($user, $path);
-		Assert::assertNotNull($fileId, __METHOD__ . " file $path user $user not found (the file may not exist)");
 		$this->theVersionFolderOfFileIdShouldContainElements($user, $fileId, $count);
 	}
 
 	/**
 	 * assert file versions count
 	 *
 	 * @param string $user
-	 * @param int $fileId
+	 * @param string $fileId
 	 * @param int $count
 	 *
 	 * @return void
 	 * @throws GuzzleException
 	 */
 	public function theVersionFolderOfFileIdShouldContainElements(
 		string $user,
-		int $fileId,
+		string $fileId,
 		int $count
 	):void {
 		$responseXml = $this->listVersionFolder($user, $fileId);
@@ -60,15 +60,15 @@ public function theVersionFolderOfFileIdShouldContainElements(
 	 * returns the result parsed into an SimpleXMLElement
 	 *
 	 * @param string $user
-	 * @param int $fileId
+	 * @param string $fileId
 	 *
 	 * @return SimpleXMLElement
 	 * @throws GuzzleException
 	 * @throws Exception
 	 */
 	public function listVersionFolder(
 		string $user,
-		int $fileId
+		string $fileId
 	):SimpleXMLElement {
 		$password = $this->featureContext->getRegularUserPassword();
 		$fullUrl = $this->featureContext->sanitizeUrl(
@@ -110,6 +110,8 @@ public function before(BeforeScenarioScope $scope):void {
 		$environment = $scope->getEnvironment();
 
 		// Get all the contexts you need in this context
-		$this->featureContext = $environment->getContext('FeatureContext');
+		if ($environment instanceof InitializedContextEnvironment) {
+			$this->featureContext = $environment->getContext('FeatureContext');
+		}
 	}
 }