Improved code quality

.github/workflows/ci.yml

@@ -69,3 +69,12 @@ jobs:
     - name: Upload coverage
       if: ${{ matrix.php-versions == '8.1' }}
       uses: codecov/codecov-action@v3
+    - name: Run rector
+      if: ${{ matrix.php-versions == '8.1' }}
+      run: tools/rector process --dry-run
+    - name: Run psalm
+      if: ${{ matrix.php-versions == '8.1' }}
+      run: tools/psalm
+    - name: Run phpstan
+      if: ${{ matrix.php-versions == '8.1' }}
+      run: tools/phpstan

.gitignore

@@ -30,3 +30,5 @@ reports
 #
 # See https://github.com/littleredbutton/bigbluebutton-api-php/pull/115 for the discussion.
 /composer.lock
+
+/Makefile.iservmake

.php-cs-fixer.php

@@ -5,8 +5,8 @@
 $finder = \PhpCsFixer\Finder::create()
     ->files()
     ->name('*.php')
-    ->in(__DIR__ . '/src')
-    ->in(__DIR__ . '/tests');
+    ->in(__DIR__.'/src')
+    ->in(__DIR__.'/tests');
 
 $config = new PhpCsFixer\Config();
 

composer.json

@@ -91,8 +91,17 @@
   },
   "autoload-dev": {
     "psr-4": {
+      "BigBlueButton\\Tests\\Common\\": [
+        "tests/common"
+      ],
       "BigBlueButton\\Tests\\Functional\\": [
         "tests/functional"
+      ],
+      "BigBlueButton\\Tests\\Integration\\": [
+        "tests/integration"
+      ],
+      "BigBlueButton\\Tests\\Unit\\": [
+        "tests/unit"
       ]
     }
   },
@@ -105,6 +114,9 @@
     "psalm": "tools/psalm --threads=1",
     "psalm-clear": "tools/psalm --clear-cache && tools/psalm --clear-global-cache",
     "psalm-fix": "tools/psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
+    "phpstan": "tools/phpstan analyse",
+    "rector": "tools/rector process --dry-run src/ tests/",
+    "rector-fix": "tools/rector process src/ tests/",
     "post-install-cmd": "tools/composer-git-hooks add --ignore-lock",
     "post-update-cmd": "tools/composer-git-hooks update"
   },
@@ -115,7 +127,9 @@
       ],
       "pre-push": [
         "tools/phpunit --testsuite=\"BigBlueButton unit test suite,BigBlueButton integration test suite\"",
-        "tools/psalm --threads=1"
+        "tools/psalm --threads=1",
+        "tools/phpstan analyse",
+        "tools/rector process --dry-run src/ tests/"
       ],
       "post-merge": "composer install",
       "post-checkout": "composer install"
@@ -128,8 +142,10 @@
         }
       },
       "friendsofphp/php-cs-fixer": "^3.3",
+      "phpstan/phpstan": "^1.10",
       "phpunit/phpunit": "^9",
-      "vimeo/psalm": "^4.22"
+      "rector/rector": "^1.0",
+      "vimeo/psalm": "^5.23"
     }
   }
 }

phpstan.neon

@@ -0,0 +1,17 @@
+parameters:
+    paths:
+        - src
+        - tests
+    parallel:
+        maximumNumberOfProcesses: 4
+    level: 6
+    inferPrivatePropertyTypeFromConstructor: true
+    bootstrapFiles:
+      - tools/bootstrap.php
+    ignoreErrors:
+        -
+            message: '#^Offset ''input'' does not exist on array\{\}\.$#'
+            path: tests/integration/Http/Transport/CurlTransportTest.php
+        -
+            message: '#^Offset ''vars'' does not exist on array\{\}\.$#'
+            path: tests/integration/Http/Transport/CurlTransportTest.php

rector.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector;
+use Rector\DeadCode\Rector\ClassMethod\RemoveUselessParamTagRector;
+use Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnTagRector;
+use Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenRector;
+use Rector\Privatization\Rector\Class_\FinalizeTestCaseClassRector;
+use Rector\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector;
+use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector;
+use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector;
+use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictSetUpRector;
+use Rector\TypeDeclaration\Rector\StmtsAwareInterface\DeclareStrictTypesRector;
+
+return RectorConfig::configure()
+    ->withPaths([
+        __DIR__.'/src',
+        __DIR__.'/tests',
+    ])
+    ->withPhpSets()
+    ->withRules([
+        AddVoidReturnTypeWhereNoReturnRector::class,
+        DeclareStrictTypesRector::class,
+        FinalizeTestCaseClassRector::class,
+        RemoveUnusedVariableAssignRector::class,
+        RemoveUselessParamTagRector::class,
+        RemoveUselessReturnTagRector::class,
+        TypedPropertyFromAssignsRector::class,
+        TypedPropertyFromStrictConstructorRector::class,
+        TypedPropertyFromStrictSetUpRector::class,
+    ])
+    ->withImportNames(importShortClasses: false, removeUnusedImports: true)
+;

src/BigBlueButton.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * BigBlueButton open source conferencing system - https://www.bigbluebutton.org/.
  *
@@ -72,40 +75,17 @@ class BigBlueButton
     public const CONNECTION_ERROR_BASEURL = 1;
     public const CONNECTION_ERROR_SECRET = 2;
 
-    /**
-     * @var string
-     */
-    protected $securitySecret;
-
-    /**
-     * @var string
-     */
-    protected $bbbServerBaseUrl;
+    protected string $securitySecret;
 
-    /**
-     * @var string
-     */
-    protected $hashingAlgorithm;
+    protected string $bbbServerBaseUrl;
 
-    /**
-     * @var UrlBuilder
-     */
-    protected $urlBuilder;
+    protected UrlBuilder $urlBuilder;
 
-    /**
-     * @var string|null
-     */
-    protected $jSessionId;
+    protected ?string $jSessionId = null;
 
-    /**
-     * @var int|null
-     */
-    protected $connectionError;
+    protected ?int $connectionError = null;
 
-    /**
-     * @var TransportInterface
-     */
-    protected $transport;
+    protected TransportInterface $transport;
 
     /**
      * @param string|null             $baseUrl   (optional) If not given, it will be retrieved from the environment
@@ -114,13 +94,30 @@ class BigBlueButton
      *
      * @throws ConfigException
      */
-    public function __construct(?string $baseUrl = null, ?string $secret = null, ?TransportInterface $transport = null, HashingAlgorithm $hashingAlgorithm = HashingAlgorithm::SHA_1)
+    public function __construct(?string $baseUrl = null, ?string $secret = null, ?TransportInterface $transport = null, protected HashingAlgorithm $hashingAlgorithm = HashingAlgorithm::SHA_1)
     {
         // Keeping backward compatibility with older deployed versions
-        $this->securitySecret = $secret ?: getenv('BBB_SECURITY_SALT') ?: getenv('BBB_SECRET');
-        $this->bbbServerBaseUrl = $baseUrl ?: getenv('BBB_SERVER_BASE_URL');
+        $securitySecret = $secret ?: getenv('BBB_SECURITY_SALT') ?: getenv('BBB_SECRET');
+
+        if (false === $securitySecret) {
+            // @codeCoverageIgnoreStart
+            @trigger_error(sprintf('Constructing "%s" without passing a secret is deprecated since 6.0 and will throw an exception in 7.0.', self::class), \E_USER_DEPRECATED);
+            $this->securitySecret = ''; // previous behaviour
+        // @codeCoverageIgnoreEnd
+        } else {
+            $this->securitySecret = $securitySecret;
+        }
 
-        $this->hashingAlgorithm = $hashingAlgorithm;
+        $bbbServerBaseUrl = $baseUrl ?: getenv('BBB_SERVER_BASE_URL');
+
+        if (false === $bbbServerBaseUrl) {
+            // @codeCoverageIgnoreStart
+            @trigger_error(sprintf('Constructing "%s" without passing a server base URL is deprecated since 6.0 and will throw an exception 7.0.', self::class), \E_USER_DEPRECATED);
+            $this->bbbServerBaseUrl = ''; // previous behaviour
+        // @codeCoverageIgnoreEnd
+        } else {
+            $this->bbbServerBaseUrl = $bbbServerBaseUrl;
+        }
 
         if (empty($this->bbbServerBaseUrl)) {
             throw new ConfigException('Base url required');
@@ -170,7 +167,7 @@ public function isConnectionWorking(): bool
             }
 
             // HTTP exception or XML parse
-        } catch (\Exception $e) {
+        } catch (\Exception) {
         }
 
         $this->connectionError = self::CONNECTION_ERROR_BASEURL;

src/Core/ApiMethod.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * BigBlueButton open source conferencing system - https://www.bigbluebutton.org/.
  *
@@ -19,24 +22,24 @@
 
 namespace BigBlueButton\Core;
 
-final class ApiMethod
+enum ApiMethod: string
 {
-    public const CREATE = 'create';
-    public const JOIN = 'join';
-    public const ENTER = 'enter';
-    public const END = 'end';
-    public const IS_MEETING_RUNNING = 'isMeetingRunning';
-    public const GET_MEETING_INFO = 'getMeetingInfo';
-    public const GET_MEETINGS = 'getMeetings';
-    public const SIGN_OUT = 'signOut';
-    public const GET_RECORDINGS = 'getRecordings';
-    public const PUBLISH_RECORDINGS = 'publishRecordings';
-    public const DELETE_RECORDINGS = 'deleteRecordings';
-    public const UPDATE_RECORDINGS = 'updateRecordings';
-    public const GET_RECORDING_TEXT_TRACKS = 'getRecordingTextTracks';
-    public const PUT_RECORDING_TEXT_TRACK = 'putRecordingTextTrack';
-    public const HOOKS_CREATE = 'hooks/create';
-    public const HOOKS_LIST = 'hooks/list';
-    public const HOOKS_DESTROY = 'hooks/destroy';
-    public const INSERT_DOCUMENT = 'insertDocument';
+    case CREATE = 'create';
+    case JOIN = 'join';
+    case ENTER = 'enter';
+    case END = 'end';
+    case IS_MEETING_RUNNING = 'isMeetingRunning';
+    case GET_MEETING_INFO = 'getMeetingInfo';
+    case GET_MEETINGS = 'getMeetings';
+    case SIGN_OUT = 'signOut';
+    case GET_RECORDINGS = 'getRecordings';
+    case PUBLISH_RECORDINGS = 'publishRecordings';
+    case DELETE_RECORDINGS = 'deleteRecordings';
+    case UPDATE_RECORDINGS = 'updateRecordings';
+    case GET_RECORDING_TEXT_TRACKS = 'getRecordingTextTracks';
+    case PUT_RECORDING_TEXT_TRACK = 'putRecordingTextTrack';
+    case HOOKS_CREATE = 'hooks/create';
+    case HOOKS_LIST = 'hooks/list';
+    case HOOKS_DESTROY = 'hooks/destroy';
+    case INSERT_DOCUMENT = 'insertDocument';
 }

src/Core/Attendee.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * BigBlueButton open source conferencing system - https://www.bigbluebutton.org/.
  *
@@ -21,50 +24,24 @@
 
 class Attendee
 {
-    /**
-     * @var string
-     */
-    private $userId;
-
-    /**
-     * @var string
-     */
-    private $fullName;
-
-    /**
-     * @var string
-     */
-    private $role;
-
-    /**
-     * @var bool
-     */
-    private $isPresenter;
-
-    /**
-     * @var bool
-     */
-    private $isListeningOnly;
-
-    /**
-     * @var bool
-     */
-    private $hasJoinedVoice;
-
-    /**
-     * @var bool
-     */
-    private $hasVideo;
-
-    /**
-     * @var array
-     */
-    private $customData = [];
-
-    /**
-     * @var string
-     */
-    private $clientType;
+    private readonly string $userId;
+
+    private readonly string $fullName;
+
+    private readonly string $role;
+
+    private readonly bool $isPresenter;
+
+    private readonly bool $isListeningOnly;
+
+    private readonly bool $hasJoinedVoice;
+
+    private readonly bool $hasVideo;
+
+    /** @var array<string,string> */
+    private array $customData = [];
+
+    private readonly string $clientType;
 
     public function __construct(\SimpleXMLElement $xml)
     {
@@ -124,6 +101,7 @@ public function getClientType(): string
         return $this->clientType;
     }
 
+    /** @return array<string,string> */
     public function getCustomData(): array
     {
         return $this->customData;

src/Core/GuestPolicy.php

@@ -22,10 +22,10 @@
 
 namespace BigBlueButton\Core;
 
-final class GuestPolicy
+enum GuestPolicy: string
 {
-    public const ALWAYS_ACCEPT = 'ALWAYS_ACCEPT';
-    public const ALWAYS_DENY = 'ALWAYS_DENY';
-    public const ASK_MODERATOR = 'ASK_MODERATOR';
-    public const ALWAYS_ACCEPT_AUTH = 'ALWAYS_ACCEPT_AUTH';
+    case ALWAYS_ACCEPT = 'ALWAYS_ACCEPT';
+    case ALWAYS_DENY = 'ALWAYS_DENY';
+    case ASK_MODERATOR = 'ASK_MODERATOR';
+    case ALWAYS_ACCEPT_AUTH = 'ALWAYS_ACCEPT_AUTH';
 }