added phpstan

plank · Apr 7, 2024 · b3876f6 · b3876f6
1 parent 2ce3a5e
commit b3876f6
Show file tree

Hide file tree

Showing 18 changed files with 79 additions and 49 deletions.
diff --git a/.github/workflows/automated-test.yml b/.github/workflows/automated-test.yml
@@ -50,6 +50,9 @@ jobs:
           S3_REGION: ${{ secrets.S3_REGION }}
           S3_BUCKET: ${{ secrets.S3_BUCKET }}
 
+      - name: Run PHPStan
+        run: vendor/bin/phpstan -c ./phpstan.neon --memory-limit=1G
+
       - name: Upload coverage results to Coveralls
         env:
           COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,6 +55,7 @@ All SourceAdapter classes have been significantly refactored.
 - Added missing type declarations to most property and method signatures.
 - Removed the `\Plank\Mediable\Stream` class in favor of the `guzzlehttp/psr7` implementation. This removes the direct dependency on the `psr/http-message` library.
 - `\Plank\Mediable\HandlesMediaUploadExceptions::transformMediaUploadException()` parameter and return type changed from `\Exception` to `\Throwable`.
+- Added PHPStan static analysis to the test suite.
 
 ## 5.5.0 - 2022-05-09
 - Filename and pathname sanitization will use the app locale when transliterating UTF-8 characters to ascii.

diff --git a/phpstan.neon b/phpstan.neon
@@ -0,0 +1,5 @@
+parameters:
+	level: 1
+	paths:
+		- src
+		- tests
diff --git a/src/Commands/ImportMediaCommand.php b/src/Commands/ImportMediaCommand.php
@@ -67,7 +67,6 @@ public function handle(): void
 
         $files = $this->listFiles($disk, $directory, $recursive);
         $existing_media = $this->makeModel()
-            ->newQuery()
             ->inDirectory($disk, $directory, $recursive)
             ->get();
 

diff --git a/src/ImageManipulator.php b/src/ImageManipulator.php
@@ -208,7 +208,7 @@ public function createImageVariant(
     /**
      * @param Media $media
      * @param SourceAdapterInterface $source
-     * @param ImageManipulation $variantName
+     * @param ImageManipulation $manipulation
      * @return StreamAdapter
      * @throws ImageManipulationException
      */
@@ -343,7 +343,7 @@ private function checkForDuplicates(
             return;
         }
 
-        if (!$this->filesystem->disk($variant->disk)->has($variant->getDiskPath())) {
+        if (!$this->filesystem->disk($variant->disk)->exists($variant->getDiskPath())) {
             // no conflict, carry on
             return;
         }
@@ -375,7 +375,7 @@ private function generateUniqueFilename(Media $model): string
             }
             $path = "{$model->directory}/{$filename}.{$model->extension}";
             ++$counter;
-        } while ($storage->has($path));
+        } while ($storage->exists($path));
 
         return $filename;
     }

diff --git a/src/Jobs/CreateImageVariants.php b/src/Jobs/CreateImageVariants.php
@@ -32,7 +32,7 @@ class CreateImageVariants implements ShouldQueue
 
     /**
      * CreateImageVariants constructor.
-     * @param Media|Collection|Media[] $model
+     * @param Media|Collection|Media[] $models
      * @param string|string[] $variantNames
      * @throws ImageManipulationException
      */
@@ -77,7 +77,7 @@ public function getModels(): Collection
     }
 
     /**
-     * @param Media $model
+     * @param Collection<Media> $models
      * @param array $variantNames
      * @throws ImageManipulationException
      */
@@ -107,7 +107,7 @@ public function getForceRecreate(): bool
 
     /**
      * @param Media|Collection|Media[] $models
-     * @return bool
+     * @return Collection
      */
     private function collect($models): Collection
     {

diff --git a/src/Media.php b/src/Media.php
@@ -246,7 +246,7 @@ public function scopeInDirectory(Builder $q, string $disk, string $directory, bo
 
     /**
      * Query scope for finding media in a particular directory or one of its subdirectories.
-     * @param  Builder|Media $q
+     * @param  Builder $q
      * @param  string $disk Filesystem disk to search in
      * @param  string $directory Path relative to disk
      * @return void
@@ -372,7 +372,7 @@ public function getTemporaryUrl(\DateTimeInterface $expiry): string
      */
     public function fileExists(): bool
     {
-        return $this->storage()->has($this->getDiskPath());
+        return $this->storage()->exists($this->getDiskPath());
     }
 
     /**
@@ -450,12 +450,17 @@ public function makeOriginal(): self
         return $this;
     }
 
+    /**
+     * @param Media|string|int $media
+     * @param string $variantName
+     * @return $this
+     */
     public function makeVariantOf($media, string $variantName): self
     {
-        if (!$media instanceof static) {
+        if (!$media instanceof self) {
             $media = $this->newQuery()->findOrFail($media);
         }
-
+        /** @var Media $media */
         $this->variant_name = $variantName;
         $this->original_media_id = $media->isOriginal()
             ? $media->getKey()
@@ -508,7 +513,7 @@ public function copyTo(string $destination, string $filename = null): self
      *
      * Will invoke the `save()` method on the model after the associated file has been moved to prevent synchronization errors
      * @param  string $disk the disk to move the file to
-     * @param  string $directory directory relative to disk root
+     * @param  string $destination directory relative to disk root
      * @param  string $filename filename. Do not include extension
      * @return void
      * @throws MediaMoveException If attempting to change the file extension or a file with the same name already exists at the destination
@@ -528,9 +533,8 @@ public function moveToDisk(
      *
      * This method creates a new Media object as well as duplicates the associated file on the disk.
      *
-     * @param  Media $media The media to copy from
      * @param  string $disk the disk to copy the file to
-     * @param  string $directory directory relative to disk root
+     * @param  string $destination directory relative to disk root
      * @param  string $filename optional filename. Do not include extension
      *
      * @return Media
@@ -554,14 +558,16 @@ protected function getMediaMover(): MediaMover
     protected function handleMediaDeletion(): void
     {
         // optionally detach mediable relationships on soft delete
-        if (static::hasGlobalScope(SoftDeletingScope::class) && !$this->forceDeleting) {
+        if (static::hasGlobalScope(SoftDeletingScope::class)
+            && (!property_exists($this, 'forceDeleting') || !$this->forceDeleting)
+        ) {
             if (config('mediable.detach_on_soft_delete')) {
                 $this->newBaseQueryBuilder()
                     ->from(config('mediable.mediables_table', 'mediables'))
                     ->where('media_id', $this->getKey())
                     ->delete();
             }
-        } elseif ($this->storage()->has($this->getDiskPath())) {
+        } elseif ($this->storage()->exists($this->getDiskPath())) {
             // unlink associated file on delete
             $this->storage()->delete($this->getDiskPath());
         }

diff --git a/src/MediaMover.php b/src/MediaMover.php
@@ -38,7 +38,7 @@ public function move(Media $media, string $directory, string $filename = null):
         $directory = File::sanitizePath($directory);
         $targetPath = $directory . '/' . $filename . '.' . $media->extension;
 
-        if ($storage->has($targetPath)) {
+        if ($storage->exists($targetPath)) {
             throw MediaMoveException::destinationExists($targetPath);
         }
 
@@ -80,7 +80,7 @@ public function moveToDisk(
         $directory = File::sanitizePath($directory);
         $targetPath = $directory . '/' . $filename . '.' . $media->extension;
 
-        if ($targetStorage->has($targetPath)) {
+        if ($targetStorage->exists($targetPath)) {
             throw MediaMoveException::destinationExistsOnDisk($disk, $targetPath);
         }
 
@@ -122,7 +122,7 @@ public function copyTo(Media $media, string $directory, string $filename = null)
 
         $targetPath = $directory . '/' . $filename . '.' . $media->extension;
 
-        if ($storage->has($targetPath)) {
+        if ($storage->exists($targetPath)) {
             throw MediaMoveException::destinationExists($targetPath);
         }
 
@@ -174,7 +174,7 @@ public function copyToDisk(
         $directory = File::sanitizePath($directory);
         $targetPath = $directory . '/' . $filename . '.' . $media->extension;
 
-        if ($targetStorage->has($targetPath)) {
+        if ($targetStorage->exists($targetPath)) {
             throw MediaMoveException::destinationExistsOnDisk($disk, $targetPath);
         }
 

diff --git a/src/MediaUploader.php b/src/MediaUploader.php
@@ -472,7 +472,7 @@ public function getVisibility(): string
      * The original image will not be preserved.
      *
      * Note this will manipulate the image as part of the upload process, which may be slow.
-     * @param string|ImageManipulation $variant Either a defined ImageManipulation variant name
+     * @param string|ImageManipulation $imageManipulation Either a defined ImageManipulation variant name
      *   or an ImageManipulation instance
      * @return $this
      */
@@ -873,14 +873,17 @@ private function verifySource(): void
 
     private function inferMimeType(Filesystem $filesystem, string $path): string
     {
+        $mimeType = null;
         try {
-            $mime = $filesystem->mimeType($path);
+            if (method_exists($filesystem, 'mimeType')) {
+                $mimeType = $filesystem->mimeType($path);
+            }
         } catch (UnableToRetrieveMetadata $e) {
             // previous versions of flysystem would default to octet-stream when
             // the file was unrecognized. Maintain the behaviour for now
             return 'application/octet-stream';
         }
-        return $mime ?: 'application/octet-stream';
+        return $mimeType ?: 'application/octet-stream';
     }
 
     private function selectMimeType(): string

diff --git a/src/Mediable.php b/src/Mediable.php
@@ -100,15 +100,15 @@ public function scopeWhereHasMedia(Builder $q, $tags = [], bool $matchAll = fals
      * @param  string|string[] $tags
      * @return void
      */
-    public function scopeWhereHasMediaMatchAll(Builder $q, array $tags): void
+    public function scopeWhereHasMediaMatchAll(Builder $q, $tags): void
     {
         $this->scopeWhereHasMedia($q, $tags, true);
     }
 
     /**
      * Query scope to eager load attached media.
      *
-     * @param Builder|Mediable $q
+     * @param Builder $q
      * @param string|string[] $tags If one or more tags are specified, only media attached to those tags will be loaded.
      * @param bool $matchAll Only load media matching all provided tags
      * @param bool $withVariants If true, also load the variants and/or originalMedia relation of each Media
@@ -434,7 +434,7 @@ public function firstMedia($tags, bool $matchAll = false): ?Media
      * @see \Plank\Mediable\Mediable::getMedia()
      * @return Media|null
      */
-    public function lastMedia($tags, $matchAll = false): ?Media
+    public function lastMedia($tags, bool $matchAll = false): ?Media
     {
         return $this->getMedia($tags, $matchAll)->last();
     }
@@ -565,7 +565,9 @@ protected function addMatchAllToEagerLoadQuery(MorphToMany $q, $tags = []): void
     protected function handleMediableDeletion(): void
     {
         // only cascade soft deletes when configured
-        if (static::hasGlobalScope(SoftDeletingScope::class) && !$this->forceDeleting) {
+        if (static::hasGlobalScope(SoftDeletingScope::class)
+            && (!property_exists($this, 'forceDeleting') || !$this->forceDeleting)
+        ) {
             if (config('mediable.detach_on_soft_delete')) {
                 $this->media()->detach();
             }

diff --git a/src/MediableCollection.php b/src/MediableCollection.php
@@ -13,8 +13,8 @@
  * Collection of Mediable Models.
  *
  * @template TKey of array-key
- * @template TModel of Model
- * @extends Collection<TKey, TModel>
+ * @template TMedia of Model&MediableInterface
+ * @extends Collection<TKey, TMedia>
  */
 class MediableCollection extends Collection
 {
@@ -47,7 +47,9 @@ public function loadMedia(
 
         if ($matchAll) {
             $closure = function (MorphToMany $q) use ($tags, $withVariants) {
-                $this->addMatchAllToEagerLoadQuery($q, $tags);
+                if (method_exists($this, 'addMatchAllToEagerLoadQuery')) {
+                    $this->addMatchAllToEagerLoadQuery($q, $tags);
+                }
 
                 if ($withVariants) {
                     $q->with(['originalMedia.variants', 'variants']);
@@ -121,7 +123,7 @@ public function delete(): void
         $classes = [];
 
         $this->each(
-            function (Model $item) use ($query, $relation, &$classes) {
+            function (Model $item) use (&$classes) {
                 // collect list of ids of each class in case not all
                 // items belong to the same class
                 $classes[get_class($item)][] = $item->getKey();
@@ -130,6 +132,10 @@ function (Model $item) use ($query, $relation, &$classes) {
 
         // delete each item by class
         collect($classes)->each(
+            /**
+             * @param array<int> $ids
+             * @param class-string<Model> $class
+             */
             function (array $ids, string $class) use ($query, $relation) {
                 // select pivots matching each item for deletion
                 $query->orWhere(
@@ -142,7 +148,7 @@ function (Builder $q) use ($class, $ids, $relation) {
                     }
                 );
 
-                $class::whereIn((new $class)->getKeyName(), $ids)->delete();
+                $class::query()->whereIn((new $class)->getKeyName(), $ids)->delete();
             }
         );
 

diff --git a/src/MediableInterface.php b/src/MediableInterface.php
@@ -7,6 +7,7 @@
 use Illuminate\Database\Eloquent\Relations\MorphToMany;
 
 /**
+ * @property Collection<Media> $media
  * @method static Builder withMedia($tags = [], bool $matchAll = false, bool $withVariants = false)
  * @method static Builder withMediaAndVariants($tags = [], bool $matchAll = false)
  * @method static Builder withMediaMatchAll($tags = [], bool $withVariants = false)

diff --git a/src/SourceAdapters/StreamAdapter.php b/src/SourceAdapters/StreamAdapter.php
@@ -232,7 +232,7 @@ private function scanFile(string $hashAlgorithm = 'md5'): void
             $this->hash[$hashAlgorithm] = hash_final($hash);
             $this->source->rewind();
         } finally {
-            if ($finfo) {
+            if (!empty($finfo)) {
                 finfo_close($finfo);
             }
         }

diff --git a/tests/Factories/ModelFactory.php b/tests/Factories/ModelFactory.php
@@ -11,7 +11,7 @@
 
     return [
         'disk' => 'tmp',
-        'directory' => implode('/', $faker->words($faker->randomDigit)),
+        'directory' => implode('/', $faker->words($faker->randomDigit())),
         'filename' => $faker->word,
         'extension' => $faker->randomElement($types[$type]['extensions']),
         'mime_type' => $faker->randomElement($types[$type]['mime_types']),
@@ -27,7 +27,7 @@
 
     return [
         'disk' => 'tmp',
-        'directory' => implode('/', $faker->words($faker->randomDigit)),
+        'directory' => implode('/', $faker->words($faker->randomDigit())),
         'filename' => $faker->word,
         'extension' => $faker->randomElement($types[$type]['extensions']),
         'mime_type' => $faker->randomElement($types[$type]['mime_types']),

diff --git a/tests/Integration/Commands/ImportMediaCommandTest.php b/tests/Integration/Commands/ImportMediaCommandTest.php
@@ -38,7 +38,7 @@ public function test_it_creates_media_for_unmatched_files(): void
         $this->assertEquals("Imported 1 file(s).\n", $artisan->output());
         $this->assertEquals(
             ['bar', 'foo'],
-            Media::orderBy('filename')->pluck('filename')->toArray()
+            Media::query()->orderBy('filename')->pluck('filename')->toArray()
         );
     }
 
@@ -57,7 +57,7 @@ public function test_it_creates_media_for_unmatched_files_in_directory(): void
         $artisan->call('media:import', ['disk' => 'tmp', '--directory' => 'a/b']);
 
         $this->assertEquals("Imported 1 file(s).\n", $artisan->output());
-        $this->assertEquals(['bar'], Media::pluck('filename')->toArray());
+        $this->assertEquals(['bar'], Media::query()->pluck('filename')->toArray());
     }
 
     public function test_it_creates_media_for_unmatched_files_non_recursively(): void
@@ -78,7 +78,7 @@ public function test_it_creates_media_for_unmatched_files_non_recursively(): voi
         );
 
         $this->assertEquals("Imported 1 file(s).\n", $artisan->output());
-        $this->assertEquals(['foo'], Media::pluck('filename')->toArray());
+        $this->assertEquals(['foo'], Media::query()->pluck('filename')->toArray());
     }
 
     public function test_it_skips_files_of_unmatched_aggregate_type(): void
@@ -133,7 +133,7 @@ public function test_it_updates_existing_media(): void
         $artisan->call('media:import', ['disk' => 'tmp', '--force' => true]);
         $this->assertEquals(
             ['image', 'image'],
-            Media::pluck('aggregate_type')->toArray()
+            Media::query()->pluck('aggregate_type')->toArray()
         );
         $this->assertEquals(
             "Imported 0 file(s).\nUpdated 1 record(s).\nSkipped 1 file(s).\n",