Merge pull request #326 from Art4/add-phpstan

Add phpstan
kbsali · Sep 22, 2023 · 8665629 · 8665629
2 parents 3e2dcb2 + 0c91f76
commit 8665629
Show file tree

Hide file tree

Showing 18 changed files with 229 additions and 149 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -36,6 +36,10 @@ jobs:
       - name: "Install Composer dependencies"
         uses: "ramsey/composer-install@v2"
 
+      - name: Run static code analysis
+        if: ${{ matrix.php-versions == '8.2' }}
+        run: composer run phpstan -- --error-format=github
+
       - name: Run tests
         run: vendor/bin/phpunit --coverage-clover .phpunit.cache/clover.xml
 

diff --git a/.phpstan.neon b/.phpstan.neon
@@ -0,0 +1,9 @@
+parameters:
+    level: 4
+
+    paths:
+        - src/
+        - tests/
+
+    scanDirectories:
+        - vendor
diff --git a/composer.json b/composer.json
@@ -32,7 +32,8 @@
         "friendsofphp/php-cs-fixer": "^3",
         "phpunit/phpunit": "^9 || 10.2.*",
         "guzzlehttp/psr7": "^2",
-        "php-mock/php-mock-phpunit": "^2.6"
+        "php-mock/php-mock-phpunit": "^2.6",
+        "phpstan/phpstan": "^1.10"
     },
     "autoload": {
         "psr-4": {
@@ -46,6 +47,7 @@
     },
     "scripts": {
         "coverage": "phpunit --coverage-html=\".phpunit.cache/code-coverage\"",
+        "phpstan": "phpstan analyze --memory-limit 512M --configuration .phpstan.neon",
         "test": "phpunit"
     }
 }
diff --git a/src/Redmine/Api/Group.php b/src/Redmine/Api/Group.php
@@ -63,7 +63,7 @@ public function listing($forceUpdate = false)
      *
      * @throws MissingParameterException Missing mandatory parameters
      *
-     * @return \SimpleXMLElement
+     * @return string|false
      */
     public function create(array $params = [])
     {

diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php
@@ -76,7 +76,7 @@ public function show($id, array $params = [])
      *
      * @param array $params the new issue data
      *
-     * @return \SimpleXMLElement
+     * @return string|false
      */
     public function create(array $params = [])
     {
@@ -174,6 +174,7 @@ public function removeWatcher($id, $watcherUserId)
      */
     public function setIssueStatus($id, $status)
     {
+        /** @var IssueStatus */
         $api = $this->client->getApi('issue_status');
 
         return $this->update($id, [
@@ -204,31 +205,37 @@ public function addNoteToIssue($id, $note, $privateNote = false)
     private function cleanParams(array $params = [])
     {
         if (isset($params['project'])) {
+            /** @var Project */
             $apiProject = $this->client->getApi('project');
             $params['project_id'] = $apiProject->getIdByName($params['project']);
             unset($params['project']);
             if (isset($params['category'])) {
+                /** @var IssueCategory */
                 $apiIssueCategory = $this->client->getApi('issue_category');
                 $params['category_id'] = $apiIssueCategory->getIdByName($params['project_id'], $params['category']);
                 unset($params['category']);
             }
         }
         if (isset($params['status'])) {
+            /** @var IssueStatus */
             $apiIssueStatus = $this->client->getApi('issue_status');
             $params['status_id'] = $apiIssueStatus->getIdByName($params['status']);
             unset($params['status']);
         }
         if (isset($params['tracker'])) {
+            /** @var Tracker */
             $apiTracker = $this->client->getApi('tracker');
             $params['tracker_id'] = $apiTracker->getIdByName($params['tracker']);
             unset($params['tracker']);
         }
         if (isset($params['assigned_to'])) {
+            /** @var User */
             $apiUser = $this->client->getApi('user');
             $params['assigned_to_id'] = $apiUser->getIdByUsername($params['assigned_to']);
             unset($params['assigned_to']);
         }
         if (isset($params['author'])) {
+            /** @var User */
             $apiUser = $this->client->getApi('user');
             $params['author_id'] = $apiUser->getIdByUsername($params['author']);
             unset($params['author']);

diff --git a/src/Redmine/Api/Membership.php b/src/Redmine/Api/Membership.php
@@ -121,7 +121,7 @@ public function removeMember($projectId, $userId, array $params = [])
     {
         $memberships = $this->all($projectId, $params);
         if (!isset($memberships['memberships']) || !is_array($memberships['memberships'])) {
-            return;
+            return false;
         }
         $removed = false;
         foreach ($memberships['memberships'] as $membership) {

diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php
@@ -106,7 +106,7 @@ public function show($id, array $params = [])
      *
      * @throws MissingParameterException
      *
-     * @return \SimpleXMLElement
+     * @return string|false
      */
     public function create(array $params = [])
     {

diff --git a/src/Redmine/Client/NativeCurlClient.php b/src/Redmine/Client/NativeCurlClient.php
@@ -4,7 +4,6 @@
 
 namespace Redmine\Client;
 
-use Redmine\Api;
 use Redmine\Exception\ClientException;
 
 /**
@@ -246,7 +245,9 @@ private function request(string $method, string $path, string $body = ''): bool
     /**
      * Prepare the request by setting the cURL options.
      *
-     * @return resource a cURL handle on success, <b>FALSE</b> on errors
+     * BC for PHP 7.4: Do not add the return type because CurlHandle was introduced in PHP 8.0
+     *
+     * @return \CurlHandle a cURL handle on success, <b>FALSE</b> on errors
      */
     private function createCurl(string $method, string $path, string $body = '')
     {
@@ -281,13 +282,13 @@ private function createCurl(string $method, string $path, string $body = '')
                     $curlOptions[CURLOPT_POSTFIELDS] = $filedata;
                     $curlOptions[CURLOPT_INFILE] = $file;
                     $curlOptions[CURLOPT_INFILESIZE] = $size;
-                } elseif (isset($body)) {
+                } elseif ($body !== '') {
                     $curlOptions[CURLOPT_POSTFIELDS] = $body;
                 }
                 break;
             case 'put':
                 $curlOptions[CURLOPT_CUSTOMREQUEST] = 'PUT';
-                if (isset($body)) {
+                if ($body !== '') {
                     $curlOptions[CURLOPT_POSTFIELDS] = $body;
                 }
                 break;

diff --git a/tests/Fixtures/MockClient.php b/tests/Fixtures/MockClient.php
@@ -12,10 +12,15 @@
  * The runRequest method of this client class just returns the value of
  * the path, method and data or the $runRequestReturnValue value if set.
  */
-class MockClient implements Client
+final class MockClient implements Client
 {
     use ClientApiTrait;
 
+    public static function create()
+    {
+        return new self();
+    }
+
     /**
      * Return value the mocked runRequest method should return.
      *
@@ -34,22 +39,7 @@ class MockClient implements Client
     public $responseCodeMock;
     public $responseContentTypeMock;
 
-    private string $url;
-    private string $apikeyOrUsername;
-    private ?string $password;
-
-    /**
-     * $apikeyOrUsername should be your ApiKey, but it could also be your username.
-     * $password needs to be set if a username is given (not recommended).
-     */
-    public function __construct(
-        string $url,
-        string $apikeyOrUsername,
-        string $password = null
-    ) {
-        $this->url = $url;
-        $this->apikeyOrUsername = $apikeyOrUsername;
-        $this->password = $password;
+    private function __construct() {
     }
 
     /**

diff --git a/tests/Integration/GroupXmlTest.php b/tests/Integration/GroupXmlTest.php
@@ -9,19 +9,10 @@
 
 class GroupXmlTest extends TestCase
 {
-    /**
-     * @var MockClient
-     */
-    private $client;
-
-    public function setup(): void
-    {
-        $this->client = new MockClient('http://test.local', 'asdf');
-    }
-
     public function testCreateBlank()
     {
-        $api = $this->client->getApi('group');
+        /** @var \Redmine\Api\Group */
+        $api = MockClient::create()->getApi('group');
         $this->assertInstanceOf('Redmine\Api\Group', $api);
 
         $this->expectException(MissingParameterException::class);
@@ -32,7 +23,9 @@ public function testCreateBlank()
 
     public function testCreateComplex()
     {
-        $res = $this->client->getApi('group')->create([
+        /** @var \Redmine\Api\Group */
+        $api = MockClient::create()->getApi('group');
+        $res = $api->create([
             'name' => 'Developers',
             'user_ids' => [3, 5],
         ]);
@@ -57,7 +50,8 @@ public function testCreateComplex()
 
     public function testUpdateNotImplemented()
     {
-        $api = $this->client->getApi('group');
+        /** @var \Redmine\Api\Group */
+        $api = MockClient::create()->getApi('group');
         $this->assertInstanceOf('Redmine\Api\Group', $api);
 
         $this->expectException(Exception::class);

diff --git a/tests/Integration/IssueCategoryXmlTest.php b/tests/Integration/IssueCategoryXmlTest.php
@@ -8,19 +8,10 @@
 
 class IssueCategoryXmlTest extends TestCase
 {
-    /**
-     * @var MockClient
-     */
-    private $client;
-
-    public function setup(): void
-    {
-        $this->client = new MockClient('http://test.local', 'asdf');
-    }
-
     public function testCreateBlank()
     {
-        $api = $this->client->getApi('issue_category');
+        /** @var \Redmine\Api\IssueCategory */
+        $api = MockClient::create()->getApi('issue_category');
         $this->assertInstanceOf('Redmine\Api\IssueCategory', $api);
 
         $this->expectException(MissingParameterException::class);
@@ -31,7 +22,8 @@ public function testCreateBlank()
 
     public function testCreateComplex()
     {
-        $api = $this->client->getApi('issue_category');
+        /** @var \Redmine\Api\IssueCategory */
+        $api = MockClient::create()->getApi('issue_category');
         $res = $api->create('otherProject', [
             'name' => 'test category',
         ]);
@@ -52,7 +44,8 @@ public function testCreateComplex()
 
     public function testUpdate()
     {
-        $api = $this->client->getApi('issue_category');
+        /** @var \Redmine\Api\IssueCategory */
+        $api = MockClient::create()->getApi('issue_category');
         $res = $api->update(1, [
             'name' => 'new category name',
         ]);

diff --git a/tests/Integration/IssueXmlTest.php b/tests/Integration/IssueXmlTest.php
@@ -7,19 +7,10 @@
 
 class IssueXmlTest extends TestCase
 {
-    /**
-     * @var MockClient
-     */
-    private $client;
-
-    public function setup(): void
-    {
-        $this->client = new MockClient('http://test.local', 'asdf');
-    }
-
     public function testCreateBlank()
     {
-        $api = $this->client->getApi('issue');
+        /** @var \Redmine\Api\Issue */
+        $api = MockClient::create()->getApi('issue');
         $this->assertInstanceOf('Redmine\Api\Issue', $api);
 
         $res = $api->create();
@@ -38,7 +29,8 @@ public function testCreateBlank()
 
     public function testCreateComplexWithUpload()
     {
-        $api = $this->client->getApi('issue');
+        /** @var \Redmine\Api\Issue */
+        $api = MockClient::create()->getApi('issue');
         $res = $api->create([
             'project_id' => 'myproject',
             'subject' => 'A test issue',
@@ -79,7 +71,8 @@ public function testCreateComplexWithUpload()
 
     public function testCreateComplex()
     {
-        $api = $this->client->getApi('issue');
+        /** @var \Redmine\Api\Issue */
+        $api = MockClient::create()->getApi('issue');
         $res = $api->create([
             'project_id' => 'test',
             'subject' => 'test api (xml) 3',
@@ -129,7 +122,8 @@ public function testCreateComplex()
 
     public function testCreateComplexWithLineBreakInDescription()
     {
-        $api = $this->client->getApi('issue');
+        /** @var \Redmine\Api\Issue */
+        $api = MockClient::create()->getApi('issue');
         $res = $api->create([
             'project_id' => 'test',
             'subject' => 'test api (xml) 3',
@@ -180,7 +174,8 @@ public function testCreateComplexWithLineBreakInDescription()
 
     public function testUpdateIssue()
     {
-        $api = $this->client->getApi('issue');
+        /** @var \Redmine\Api\Issue */
+        $api = MockClient::create()->getApi('issue');
         $res = $api->update(1, [
             'subject' => 'test note (xml) 1',
             'notes' => 'test note api',
@@ -215,7 +210,8 @@ public function testUpdateIssue()
 
     public function testAddNoteToIssue()
     {
-        $api = $this->client->getApi('issue');
+        /** @var \Redmine\Api\Issue */
+        $api = MockClient::create()->getApi('issue');
         $res = $api->addNoteToIssue(1, 'some comment');
         $response = json_decode($res, true);
 

diff --git a/tests/Integration/MembershipXmlTest.php b/tests/Integration/MembershipXmlTest.php
@@ -8,19 +8,10 @@
 
 class MembershipXmlTest extends TestCase
 {
-    /**
-     * @var MockClient
-     */
-    private $client;
-
-    public function setup(): void
-    {
-        $this->client = new MockClient('http://test.local', 'asdf');
-    }
-
     public function testCreateBlank()
     {
-        $api = $this->client->getApi('membership');
+        /** @var \Redmine\Api\Membership */
+        $api = MockClient::create()->getApi('membership');
         $this->assertInstanceOf('Redmine\Api\Membership', $api);
 
         $this->expectException(MissingParameterException::class);
@@ -31,7 +22,8 @@ public function testCreateBlank()
 
     public function testCreateComplex()
     {
-        $api = $this->client->getApi('membership');
+        /** @var \Redmine\Api\Membership */
+        $api = MockClient::create()->getApi('membership');
         $res = $api->create('otherProject', [
             'user_id' => 1,
             'role_ids' => [1, 2],
@@ -57,7 +49,8 @@ public function testCreateComplex()
 
     public function testUpdate()
     {
-        $api = $this->client->getApi('membership');
+        /** @var \Redmine\Api\Membership */
+        $api = MockClient::create()->getApi('membership');
         $res = $api->update(1, [
             'role_ids' => [1, 2],
         ]);