phpstan level 4

phpstan.neon

@@ -1,11 +1,37 @@
 parameters:
-  level: 3
+  level: 4
   paths:
     - resources
     - test
   ignoreErrors:
+    # $this, $data comes from UnityMailer
     - messages:
         - '#Variable \$this might not be defined.#'
         - '#Variable \$data might not be defined.#'
       paths:
         - resources/mail/*
+    # these functions are called with call_user_func
+    - messages:
+        - '#Method SSHKeyAddTest::addSshKeysPaste\(\) is unused.#'
+        - '#Method SSHKeyAddTest::addSshKeysImport\(\) is unused.#'
+        - '#Method SSHKeyAddTest::addSshKeysGenerate\(\) is unused.#'
+        - '#Method SSHKeyAddTest::addSshKeysGithub\(\) is unused.#'
+      paths:
+        - test/functional/SSHKeyAddTest.php
+    # assertTrue(true) makes phpunit shut up about tests doing no assertions
+    - messages:
+        - '#Call to method PHPUnit\\Framework\\Assert::assertTrue\(\) with true will always evaluate to true.#'
+      paths:
+        - test/functional/InvalidEPPNTest.php
+        - test/functional/PageLoadTest.php
+    # I cannot seem to make this error go away no matter how many functions I add @phpstan-impure to
+    - messages:
+        - '#Negated boolean expression is always false\.#'
+      paths:
+        - test/functional/PiRemoveUserTest.php
+        - test/phpunit-bootstrap.php
+    # $Subject is written by mail templates
+    - messages:
+        - '#Property UnityWebPortal\\lib\\UnityWebhook::\$Subject is never written, only read\.#'
+      paths:
+        - resources/lib/UnityWebhook.php

resources/lib/PosixGroup.php

@@ -25,13 +25,6 @@ public function getDN(): string
 
     public function equals(PosixGroup $other_group): bool
     {
-        if (!is_a($other_group, self::class)) {
-            throw new Exception(
-                "Unable to check equality because the parameter is not a " .
-                    self::class .
-                    " object",
-            );
-        }
         return $this->getDN() == $other_group->getDN();
     }
 

resources/lib/UnityGroup.php

@@ -250,7 +250,6 @@ public function newUserRequest(UnityUser $new_user, bool $send_mail = true): voi
         }
         if ($this->SQL->accDeletionRequestExists($new_user->uid)) {
             throw new Exception("user '$new_user' requested account deletion");
-            return;
         }
         $this->addRequest($new_user->uid);
         if ($send_mail) {

resources/lib/UnityHTTPD.php

@@ -245,9 +245,6 @@ public static function getPostData(string $key): mixed
     /* returns null if not found and not $throw_if_not_found */
     public static function getQueryParameter(string $key, bool $throw_if_not_found = true): mixed
     {
-        if (!is_array($_GET)) {
-            throw new RuntimeException('$_GET is not an array!');
-        }
         if (!array_key_exists($key, $_GET)) {
             if ($throw_if_not_found) {
                 self::badRequest("\$_GET has no array key '$key'");
@@ -263,13 +260,16 @@ public static function getUploadedFileContents(
         bool $do_delete_tmpfile_after_read = true,
         string $encoding = "UTF-8",
     ): string {
-        try {
-            $tmpfile_path = $_FILES[$filename]["tmp_name"];
-        } catch (ArrayKeyException $e) {
-            self::badRequest("no such uploaded file", $e, [
-                '$_FILES' => $_FILES,
-            ]);
+        if (!array_key_exists($filename, $_FILES)) {
+            self::badRequest("\$_FILES has no array key '$filename'", data: ['$_FILES' => $_FILES]);
+        }
+        if (!array_key_exists("tmp_name", $_FILES[$filename])) {
+            self::badRequest(
+                "\$_FILES[$filename] has no array key 'tmp_name'",
+                data: ['$_FILES' => $_FILES],
+            );
         }
+        $tmpfile_path = $_FILES[$filename]["tmp_name"];
         $contents = file_get_contents($tmpfile_path);
         if ($contents === false) {
             throw new \Exception("Failed to read file: " . $tmpfile_path);

resources/lib/UnityMailer.php

@@ -76,7 +76,7 @@ public function __construct()
         }
     }
 
-    public function sendMail(string $recipients, ?string $template = null, mixed $data = null)
+    public function sendMail(string|array $recipients, ?string $template = null, mixed $data = null)
     {
         if (isset($template)) {
             $this->setFrom($this->MSG_SENDER_EMAIL, $this->MSG_SENDER_NAME);

resources/lib/UnityUser.php

@@ -35,14 +35,6 @@ public function __construct(
 
     public function equals(UnityUser $other_user): bool
     {
-        if (!is_a($other_user, self::class)) {
-            throw new Exception(
-                "Unable to check equality because the parameter is not a " .
-                    self::class .
-                    " object",
-            );
-        }
-
         return $this->uid == $other_user->uid;
     }
 
@@ -411,18 +403,6 @@ public function hasRequestedAccountDeletion(): bool
      */
     public function isInGroup(string $uid, UnityGroup $group): bool
     {
-        if (gettype($group) == "string") {
-            $group_checked = new UnityGroup(
-                $group,
-                $this->LDAP,
-                $this->SQL,
-                $this->MAILER,
-                $this->WEBHOOK,
-            );
-        } else {
-            $group_checked = $group;
-        }
-
-        return in_array($uid, $group_checked->getMemberUIDs());
+        return in_array($uid, $group->getMemberUIDs());
     }
 }

resources/lib/UnityWebhook.php

@@ -49,7 +49,11 @@ public function sendWebhook(?string $template = null, mixed $data = null): bool
         curl_setopt($ch, CURLOPT_POST, 1);
         curl_setopt($ch, CURLOPT_HTTPHEADER, ["Content-Type: application/json"]);
         curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
-        curl_setopt($ch, CURLOPT_POSTFIELDS, \jsonEncode(["text" => $message]));
+        curl_setopt(
+            $ch,
+            CURLOPT_POSTFIELDS,
+            \jsonEncode(["subject" => $this->Subject, "text" => $message]),
+        );
         $result = curl_exec($ch);
         curl_close($ch);
         return $result;

test/functional/PIBecomeApproveTest.php

@@ -36,7 +36,7 @@ public function testApprovePI()
         switchuser(...$user_to_qualify_args);
         $pi_group = $USER->getPIGroup();
         $this->assertTrue($USER->exists());
-        $this->assertTrue(!$pi_group->exists());
+        $this->assertFalse($pi_group->exists());
         try {
             $this->requestGroupCreation();
             $this->assertRequestedPIGroup(true);

test/functional/PiMemberApproveTest.php

@@ -76,7 +76,7 @@ public function testApproveMemberByPI()
         $this->assertTrue($USER->exists());
         $this->assertTrue($pi_group->exists());
         $this->assertGroupMembers($pi_group, [$pi_uid]);
-        $this->assertTrue(!$pi_group->memberUIDExists($USER->uid));
+        $this->assertFalse($pi_group->memberUIDExists($USER->uid));
         $this->assertRequestedMembership(false, $gid);
         try {
             $this->requestGroupMembership($pi_group->gid);
@@ -103,7 +103,7 @@ public function testApproveMemberByPI()
             $this->approveUserByPI($approve_uid);
             switchUser(...$user_to_approve_args);
 
-            $this->assertTrue(!$pi_group->requestExists($USER));
+            $this->assertFalse($pi_group->requestExists($USER));
             $this->assertRequestedMembership(false, $gid);
             $this->assertTrue($pi_group->memberUIDExists($USER->uid));
             $this->assertTrue($USER->getFlag(UserFlag::QUALIFIED));
@@ -116,7 +116,7 @@ public function testApproveMemberByPI()
             // }
             // $this->assertTrue($third_request_failed);
             $this->assertRequestedMembership(false, $gid);
-            $this->assertTrue(!$pi_group->requestExists($USER));
+            $this->assertFalse($pi_group->requestExists($USER));
         } finally {
             switchUser(...$user_to_approve_args);
             ensureUserNotInPIGroup($pi_group);
@@ -136,7 +136,7 @@ public function testApproveMemberByAdmin()
         $this->assertTrue($USER->exists());
         $this->assertTrue($pi_group->exists());
         $this->assertGroupMembers($pi_group, [$pi_uid]);
-        $this->assertTrue(!$pi_group->memberUIDExists($USER->uid));
+        $this->assertFalse($pi_group->memberUIDExists($USER->uid));
         $this->assertRequestedMembership(false, $gid);
         try {
             $this->requestGroupMembership($pi_group->gid);
@@ -163,7 +163,7 @@ public function testApproveMemberByAdmin()
             $this->approveUserByAdmin($gid, $approve_uid);
             switchUser(...$user_to_approve_args);
 
-            $this->assertTrue(!$pi_group->requestExists($USER));
+            $this->assertFalse($pi_group->requestExists($USER));
             $this->assertRequestedMembership(false, $gid);
             $this->assertTrue($pi_group->memberUIDExists($USER->uid));
             $this->assertTrue($USER->getFlag(UserFlag::QUALIFIED));
@@ -176,7 +176,7 @@ public function testApproveMemberByAdmin()
             // }
             // $this->assertTrue($third_request_failed);
             $this->assertRequestedMembership(false, $gid);
-            $this->assertTrue(!$pi_group->requestExists($USER));
+            $this->assertFalse($pi_group->requestExists($USER));
         } finally {
             switchUser(...$user_to_approve_args);
             ensureUserNotInPIGroup($pi_group);

test/functional/RegisterUserTest.php

@@ -26,9 +26,9 @@ public function testRegisterUserAndCreateOrg($user_to_register_args, $expected_u
         $user_entry = $LDAP->getUserEntry($USER->uid);
         $user_group_entry = $LDAP->getGroupEntry($USER->uid);
         $org_entry = $LDAP->getOrgGroupEntry($SSO["org"]);
-        $this->assertTrue(!$user_entry->exists());
-        $this->assertTrue(!$user_group_entry->exists());
-        $this->assertTrue(!$org_entry->exists());
+        $this->assertFalse($user_entry->exists());
+        $this->assertFalse($user_group_entry->exists());
+        $this->assertFalse($org_entry->exists());
         try {
             $this->register();
             $this->assertTrue($user_entry->exists());

test/unit/CSRFTokenTest.php

@@ -21,7 +21,6 @@ protected function tearDown(): void
     public function testGenerateCreatesToken(): void
     {
         $token = CSRFToken::generate();
-        $this->assertIsString($token);
         $this->assertEquals(64, strlen($token));
         $this->assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $token);
     }