[v6] Add additional types & Phpstan (#910)

* init phpstan and larastan * Fix: Unsafe access to private property ... through static:: * fixes and baseline * add phpstan action * init rector * apply code changes by rector * undo Filter contract changes * fixed a bunch of phpstan issues * Fix styling * fix typo --------- Co-authored-by: Nielsvanpach <[email protected]> Co-authored-by: Alex Vanderbist <[email protected]>
spatie · May 10, 2024 · 880e2b1 · 880e2b1
1 parent 47d9561
commit 880e2b1
Show file tree

Hide file tree

Showing 30 changed files with 226 additions and 239 deletions.
diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml
@@ -0,0 +1,26 @@
+name: PHPStan
+
+on:
+    push:
+        paths:
+            - '**.php'
+            - 'phpstan.neon.dist'
+
+jobs:
+    phpstan:
+        name: phpstan
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/checkout@v4
+
+            - name: Setup PHP
+              uses: shivammathur/setup-php@v2
+              with:
+                  php-version: '8.2'
+                  coverage: none
+
+            - name: Install composer dependencies
+              uses: ramsey/composer-install@v2
+
+            - name: Run PHPStan
+              run: ./vendor/bin/phpstan --error-format=github
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
@@ -25,6 +25,7 @@ jobs:
                       testbench: 9.*
                       carbon: ^2.63
                     - laravel: 10.*
+                      php: 8.0
                       testbench: 8.*
 
         name: P${{ matrix.php }} - L${{ matrix.laravel }} - ${{ matrix.stability }} - ${{ matrix.os }}
@@ -52,8 +53,6 @@ jobs:
                     --health-retries 5
 
         steps:
-
-
             - name: Checkout code
               uses: actions/checkout@v4
 

diff --git a/composer.json b/composer.json
@@ -29,7 +29,10 @@
     "require-dev": {
         "ext-json": "*",
         "mockery/mockery": "^1.4",
+        "nunomaduro/larastan": "^2.0",
         "orchestra/testbench": "^7.0|^8.0",
+        "phpunit/phpunit": "^9.6.11",
+        "rector/rector": "^0.18.10",
         "pestphp/pest": "^2.0",
         "spatie/invade": "^2.0",
         "spatie/laravel-ray": "^1.28"
@@ -47,7 +50,9 @@
     },
     "scripts": {
         "test": "vendor/bin/pest",
-        "test-coverage": "phpunit --coverage-html coverage"
+        "test-coverage": "phpunit --coverage-html coverage",
+        "analyse": "vendor/bin/phpstan analyse --ansi --memory-limit=4G",
+        "baseline": "vendor/bin/phpstan analyse --generate-baseline --memory-limit=4G"
     },
     "config": {
         "sort-packages": true,

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -0,0 +1,36 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Parameter \\#1 \\$keys of method Illuminate\\\\Support\\\\Collection\\<int,string\\>\\:\\:except\\(\\) expects array\\<int\\>\\|Illuminate\\\\Support\\\\Enumerable\\<\\(int\\|string\\), int\\>\\|string, int\\<\\-1, max\\> given\\.$#"
+			count: 1
+			path: src/Filters/FiltersExact.php
+
+		-
+			message: "#^Call to an undefined method ReflectionType\\:\\:getName\\(\\)\\.$#"
+			count: 2
+			path: src/Filters/FiltersScope.php
+
+		-
+			message: "#^Call to an undefined method ReflectionType\\:\\:isBuiltin\\(\\)\\.$#"
+			count: 1
+			path: src/Filters/FiltersScope.php
+
+		-
+			message: "#^Call to an undefined method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<TModelClass of Illuminate\\\\Database\\\\Eloquent\\\\Model\\>\\:\\:onlyTrashed\\(\\)\\.$#"
+			count: 1
+			path: src/Filters/FiltersTrashed.php
+
+		-
+			message: "#^Call to an undefined method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<TModelClass of Illuminate\\\\Database\\\\Eloquent\\\\Model\\>\\:\\:withTrashed\\(\\)\\.$#"
+			count: 1
+			path: src/Filters/FiltersTrashed.php
+
+		-
+			message: "#^Call to an undefined method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<TModelClass of Illuminate\\\\Database\\\\Eloquent\\\\Model\\>\\:\\:withoutTrashed\\(\\)\\.$#"
+			count: 1
+			path: src/Filters/FiltersTrashed.php
+
+		-
+			message: "#^PHPDoc tag @return with type Spatie\\\\QueryBuilder\\\\QueryBuilder is not subtype of native type static\\(Spatie\\\\QueryBuilder\\\\QueryBuilder\\)\\.$#"
+			count: 2
+			path: src/QueryBuilder.php
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -0,0 +1,28 @@
+includes:
+    - ./vendor/nunomaduro/larastan/extension.neon
+    - phpstan-baseline.neon
+
+parameters:
+
+    paths:
+        - src/
+        - config/
+        - database/
+
+    # Level 9 is the highest level
+    level: 5
+
+    checkModelProperties: true
+    checkOctaneCompatibility: true
+    checkMissingIterableValueType: false
+    reportUnmatchedIgnoredErrors: false
+    noUnnecessaryCollectionCall: true
+    checkNullables: true
+    checkGenericClassInNonGenericObjectType: false
+    treatPhpDocTypesAsCertain: false
+
+    ignoreErrors:
+        - '#Unsafe usage of new static#'
+        - '#PHPDoc tag @var#'
+
+#    excludePaths:
diff --git a/rector.php b/rector.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector;
+use Rector\Config\RectorConfig;
+use Rector\Php74\Rector\Assign\NullCoalescingOperatorRector;
+use Rector\Php74\Rector\Closure\ClosureToArrowFunctionRector;
+use Rector\Set\ValueObject\LevelSetList;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->paths([
+        __DIR__ . '/config',
+        __DIR__ . '/src',
+        __DIR__ . '/tests',
+    ]);
+
+    // register a single rule
+    //$rectorConfig->rule(InlineConstructorDefaultToPropertyRector::class);
+
+    // define sets of rules
+        $rectorConfig->sets([
+            LevelSetList::UP_TO_PHP_80
+        ]);
+
+        $rectorConfig->skip([
+            ClosureToArrowFunctionRector::class,
+            NullCoalescingOperatorRector::class,
+        ]);
+};
diff --git a/src/AllowedFilter.php b/src/AllowedFilter.php
@@ -14,39 +14,30 @@
 
 class AllowedFilter
 {
-    /** @var Filter */
-    protected $filterClass;
+    protected string $internalName;
 
-    /** @var string */
-    protected $name;
+    protected Filter $filterClass;
 
-    /** @var string */
-    protected $internalName;
-
-    /** @var \Illuminate\Support\Collection */
-    protected $ignored;
+    protected Collection $ignored;
 
     /** @var mixed */
     protected $default;
 
-    /** @var bool */
-    protected $hasDefault = false;
-
-    /** @var bool */
-    protected $nullable = false;
-
-    public function __construct(string $name, Filter $filterClass, ?string $internalName = null)
-    {
-        $this->name = $name;
+    protected bool $hasDefault = false;
 
-        $this->filterClass = $filterClass;
+    protected bool $nullable = false;
 
+    public function __construct(
+        protected string $name,
+        protected Filter $filterClass,
+        ?string $internalName = null
+    ) {
         $this->ignored = Collection::make();
 
         $this->internalName = $internalName ?? $name;
     }
 
-    public function filter(QueryBuilder $query, $value)
+    public function filter(QueryBuilder $query, $value): void
     {
         $valueToFilter = $this->resolveValueForFiltering($value);
 

diff --git a/src/AllowedInclude.php b/src/AllowedInclude.php
@@ -13,19 +13,13 @@
 
 class AllowedInclude
 {
-    /** @var string */
-    protected $name;
+    protected string $internalName;
 
-    /** @var IncludeInterface */
-    protected $includeClass;
-
-    /** @var string|null */
-    protected $internalName;
-
-    public function __construct(string $name, IncludeInterface $includeClass, ?string $internalName = null)
-    {
-        $this->name = $name;
-        $this->includeClass = $includeClass;
+    public function __construct(
+        protected string $name,
+        protected IncludeInterface $includeClass,
+        ?string $internalName = null
+    ) {
         $this->internalName = $internalName ?? $this->name;
     }
 

diff --git a/src/AllowedSort.php b/src/AllowedSort.php
@@ -10,32 +10,22 @@
 
 class AllowedSort
 {
-    /** @var \Spatie\QueryBuilder\Sorts\Sort */
-    protected $sortClass;
+    protected string $defaultDirection;
 
-    /** @var string */
-    protected $name;
+    protected string $internalName;
 
-    /** @var string */
-    protected $defaultDirection;
-
-    /** @var string */
-    protected $internalName;
-
-    public function __construct(string $name, Sort $sortClass, ?string $internalName = null)
+    public function __construct(protected string $name, protected Sort $sortClass, ?string $internalName = null)
     {
         $this->name = ltrim($name, '-');
 
-        $this->sortClass = $sortClass;
-
         $this->defaultDirection = static::parseSortDirection($name);
 
         $this->internalName = $internalName ?? $this->name;
     }
 
     public static function parseSortDirection(string $name): string
     {
-        return strpos($name, '-') === 0 ? SortDirection::DESCENDING : SortDirection::ASCENDING;
+        return str_starts_with($name, '-') ? SortDirection::DESCENDING : SortDirection::ASCENDING;
     }
 
     public function sort(QueryBuilder $query, ?bool $descending = null): void
@@ -75,7 +65,7 @@ public function getInternalName(): string
         return $this->internalName;
     }
 
-    public function defaultDirection(string $defaultDirection)
+    public function defaultDirection(string $defaultDirection): static
     {
         if (! in_array($defaultDirection, [
             SortDirection::ASCENDING,

diff --git a/src/Concerns/AddsFieldsToQuery.php b/src/Concerns/AddsFieldsToQuery.php
@@ -32,7 +32,7 @@ public function allowedFields($fields): static
         return $this;
     }
 
-    protected function addRequestedModelFieldsToQuery()
+    protected function addRequestedModelFieldsToQuery(): void
     {
         $modelTableName = $this->getModel()->getTable();
 
@@ -72,7 +72,7 @@ public function getRequestedFieldsForRelatedTable(string $relation): array
         return $fields;
     }
 
-    protected function ensureAllFieldsExist()
+    protected function ensureAllFieldsExist(): void
     {
         $modelTable = $this->getModel()->getTable();
 

diff --git a/src/Concerns/AddsIncludesToQuery.php b/src/Concerns/AddsIncludesToQuery.php
@@ -52,12 +52,12 @@ public function allowedIncludes($includes): static
         return $this;
     }
 
-    protected function addIncludesToQuery(Collection $includes)
+    protected function addIncludesToQuery(Collection $includes): void
     {
         $includes->each(function ($include) {
             $include = $this->findInclude($include);
 
-            $include->include($this);
+            $include?->include($this);
         });
     }
 
@@ -69,15 +69,15 @@ protected function findInclude(string $include): ?AllowedInclude
             });
     }
 
-    protected function ensureAllIncludesExist()
+    protected function ensureAllIncludesExist(): void
     {
         if (config('query-builder.disable_invalid_includes_query_exception', false)) {
             return;
         }
 
         $includes = $this->request->includes();
 
-        $allowedIncludeNames = $this->allowedIncludes->map(function (AllowedInclude $allowedInclude) {
+        $allowedIncludeNames = $this->allowedIncludes?->map(function (AllowedInclude $allowedInclude) {
             return $allowedInclude->getName();
         });
 

diff --git a/src/Concerns/FiltersQuery.php b/src/Concerns/FiltersQuery.php
@@ -2,13 +2,13 @@
 
 namespace Spatie\QueryBuilder\Concerns;
 
+use Illuminate\Support\Collection;
 use Spatie\QueryBuilder\AllowedFilter;
 use Spatie\QueryBuilder\Exceptions\InvalidFilterQuery;
 
 trait FiltersQuery
 {
-    /** @var \Illuminate\Support\Collection */
-    protected $allowedFilters;
+    protected Collection $allowedFilters;
 
     public function allowedFilters($filters): static
     {
@@ -29,7 +29,7 @@ public function allowedFilters($filters): static
         return $this;
     }
 
-    protected function addFiltersToQuery()
+    protected function addFiltersToQuery(): void
     {
         $this->allowedFilters->each(function (AllowedFilter $filter) {
             if ($this->isFilterRequested($filter)) {
@@ -41,8 +41,6 @@ protected function addFiltersToQuery()
 
             if ($filter->hasDefault()) {
                 $filter->filter($this, $filter->getDefault());
-
-                return;
             }
         });
     }
@@ -60,7 +58,7 @@ protected function isFilterRequested(AllowedFilter $allowedFilter): bool
         return $this->request->filters()->has($allowedFilter->getName());
     }
 
-    protected function ensureAllFiltersExist()
+    protected function ensureAllFiltersExist(): void
     {
         if (config('query-builder.disable_invalid_filter_query_exception', false)) {
             return;