Ci/phpstan

.github/workflows/phpstan.yml

@@ -0,0 +1,46 @@
+# This workflow is provided via the organization template repository
+#
+# https://github.com/nextcloud/.github
+# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization
+#
+# SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+# SPDX-License-Identifier: MIT
+
+name: PHPStan
+
+on: pull_request
+
+concurrency:
+  group: phpstan-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  phpstan-static-analysis:
+    runs-on: ubuntu-latest
+
+    name: static-phpstan-analysis
+    steps:
+      - name: Checkout
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          persist-credentials: false
+
+      - name: Get php version
+        id: versions
+        uses: icewind1991/nextcloud-version-matrix@58becf3b4bb6dc6cef677b15e2fd8e7d48c0908f # v1.3.1
+
+      - name: Set up php${{ steps.versions.outputs.php-available }}
+        uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1
+        with:
+          php-version: ${{ steps.versions.outputs.php-available }}
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
+          coverage: none
+          ini-file: development
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Install dependencies
+        run: composer i
+
+      - name: Run static analysis
+        run: composer run phpstan

composer.json

@@ -23,6 +23,7 @@
 		"cs:check": "php-cs-fixer fix --dry-run --diff",
 		"cs:fix": "php-cs-fixer fix",
 		"openapi": "generate-spec",
+		"phpstan": "vendor/bin/phpstan analyse -c phpstan.neon",
 		"psalm": "psalm --no-cache --threads=$(nproc)",
 		"psalm:dev": "@psalm",
 		"psalm:update-baseline": "psalm --threads=1 --update-baseline",

lib/BackgroundJob/RetentionJob.php

@@ -103,7 +103,7 @@ public function run($argument): void {
 
 		$offset = '';
 		$limit = 1000;
-		while ($offset !== null) {
+		while (true) {
 			$fileIds = $this->tagMapper->getObjectIdsForTags((string)$tag, 'files', $limit, $offset);
 			$this->logger->debug('Checking retention for ' . count($fileIds) . ' files in this chunk');
 

lib/Controller/APIController.php

@@ -86,10 +86,10 @@ public function getRetentions(): DataResponse {
 	 * Create a retention rule
 	 *
 	 * @param int $tagid Tag the retention is based on
-	 * @param 0|1|2|3 $timeunit Time unit of the retention (days, weeks, months, years)
+	 * @param int<0, 3> $timeunit Time unit of the retention (days, weeks, months, years)
 	 * @psalm-param Constants::UNIT_* $timeunit
-	 * @param positive-int $timeamount Amount of time units that have to be passed
-	 * @param 0|1 $timeafter Whether retention time is based creation time (0) or modification time (1)
+	 * @param int<1, 32000> $timeamount Amount of time units that have to be passed
+	 * @param int<0, 1> $timeafter Whether retention time is based creation time (0) or modification time (1)
 	 * @psalm-param Constants::MODE_* $timeafter
 	 * @return DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'tagid'|'timeunit'|'timeamount'|'timeafter'}, array{}>|DataResponse<Http::STATUS_CREATED, Files_RetentionRule, array{}>
 	 *
@@ -103,16 +103,6 @@ public function addRetention(int $tagid, int $timeunit, int $timeamount, int $ti
 			return new DataResponse(['error' => 'tagid'], Http::STATUS_BAD_REQUEST);
 		}
 
-		if ($timeunit < 0 || $timeunit > 3) {
-			return new DataResponse(['error' => 'timeunit'], Http::STATUS_BAD_REQUEST);
-		}
-		if ($timeamount < 1 || $timeamount > 32_000) {
-			return new DataResponse(['error' => 'timeamount'], Http::STATUS_BAD_REQUEST);
-		}
-		if ($timeafter < 0 || $timeafter > 1) {
-			return new DataResponse(['error' => 'timeafter'], Http::STATUS_BAD_REQUEST);
-		}
-
 		// Fetch tag_id
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('tag_id')
@@ -154,7 +144,7 @@ public function addRetention(int $tagid, int $timeunit, int $timeamount, int $ti
 	 * Delete a retention rule
 	 *
 	 * @param int $id Retention rule to delete
-	 * @return DataResponse<Http::STATUS_NO_CONTENT|Http::STATUS_NOT_FOUND, list<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_NO_CONTENT|Http::STATUS_NOT_FOUND, array{}, array{}>
 	 *
 	 * 204: Retention rule deleted
 	 * 404: Retention rule not found

openapi.json

@@ -253,28 +253,23 @@
                                     "timeunit": {
                                         "type": "integer",
                                         "format": "int64",
-                                        "enum": [
-                                            0,
-                                            1,
-                                            2,
-                                            3
-                                        ],
-                                        "description": "Time unit of the retention (days, weeks, months, years)"
+                                        "description": "Time unit of the retention (days, weeks, months, years)",
+                                        "minimum": 0,
+                                        "maximum": 3
                                     },
                                     "timeamount": {
                                         "type": "integer",
                                         "format": "int64",
                                         "description": "Amount of time units that have to be passed",
-                                        "minimum": 1
+                                        "minimum": 1,
+                                        "maximum": 32000
                                     },
                                     "timeafter": {
                                         "type": "integer",
                                         "format": "int64",
-                                        "enum": [
-                                            0,
-                                            1
-                                        ],
-                                        "description": "Whether retention time is based creation time (0) or modification time (1)"
+                                        "description": "Whether retention time is based creation time (0) or modification time (1)",
+                                        "minimum": 0,
+                                        "maximum": 1
                                     }
                                 }
                             }
@@ -489,7 +484,9 @@
                                                 "meta": {
                                                     "$ref": "#/components/schemas/OCSMeta"
                                                 },
-                                                "data": {}
+                                                "data": {
+                                                    "type": "object"
+                                                }
                                             }
                                         }
                                     }

phpstan.neon

@@ -0,0 +1,12 @@
+# SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+# SPDX-License-Identifier: AGPL-3.0-or-later
+parameters:
+	level: 5
+	paths:
+		- lib
+	scanFiles:
+		- tests/stub.phpstub
+	scanDirectories:
+		- vendor/nextcloud/ocp
+	ignoreErrors:
+		- '#on an unknown class Doctrine\\DBAL\\Schema\\Table#'

tests/lib/Contoller/APIControllerTest.php

@@ -67,6 +67,7 @@ public function testAddRetentionInvalidTag(): void {
 	}
 
 	public function testAddRetentionInvalidTimeUnit(): void {
+		$this->markTestSkipped('Test is no longer applicable as the Dispatcher is limiting values');
 		$response = $this->api->addRetention(42, -1, 1);
 		$this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus());
 
@@ -75,6 +76,7 @@ public function testAddRetentionInvalidTimeUnit(): void {
 	}
 
 	public function testAddRetentionInvalidTimeAmount(): void {
+		$this->markTestSkipped('Test is no longer applicable as the Dispatcher is limiting values');
 		$response = $this->api->addRetention(42, 3, 128_000);
 		$this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus());
 	}

vendor-bin/phpstan/composer.json

@@ -0,0 +1,10 @@
+{
+	"config": {
+		"platform": {
+			"php": "8.1"
+		}
+	},
+	"require-dev": {
+		"phpstan/phpstan": "^2.1.0"
+	}
+}