mikebronner / laravel-model-caching

Eloquent model-caching made easy.
MIT License
2.24k stars 213 forks source link

PivotEventTrait method not applied #361

Closed Tjab closed 4 years ago

Tjab commented 4 years ago

Describe the bug My PHPStorm shows an error:

image

Environment

Additional context Shouldn't be my problem to fix right? As soon as I place (in my class that uses the Cachable trait):

protected function newBelongsToMany(Builder $query, Model $parent, $table, $foreignPivotKey, $relatedPivotKey, $parentKey, $relatedKey, $relationName = null)
{
}

It's fixed, but I believe it should be fixed in the trait itself?

mikebronner commented 4 years ago

@Tjab could you provide your model code for review? Thanks!

Tjab commented 4 years ago

@mikebronner sure, but now you make me doubt as to if it's something that I'm doing wrong! Haha :)

As soon as I create a class and use the trait, the error shows.

<?php

namespace app\Models;

use GeneaLabs\LaravelModelCaching\Traits\Cachable;
use Illuminate\Database\Eloquent\Model;

class ExampleClass extends Model
{
    use Cachable;
}

image

mikebronner commented 4 years ago

@Tjab Thanks for the update! :) I will take a look, but am wondering if it isn't a bug in PHPStorm, as I haven't had this error show up when running code.

mikebronner commented 4 years ago

Revisiting the code, I think its a PHPStorm bug. There are only two places this function exists in regard:

However, in Cachable we are overriding the method from laravel-pivot-events to use the one from ModelCaching. I believe it is this that PHPStorm has issue with interpreting correctly:

    use PivotEventTrait {
        ModelCaching::newBelongsToMany insteadof PivotEventTrait;
    }

All tests pass without any errors when running the code, so it's likely something other than the code in this case. I'll close for now. Thanks for bringing this up, I hadn't seen this before, as I don't use PHPStorm anymore.

Tjab commented 4 years ago

Thanks for the work/help! I've reported it at PHPStorm :) Hopefully they can fix it easily, it's very annoying! Thanks again!

Edit: It was already reported (I made a double).

https://youtrack.jetbrains.com/issue/WI-54284

Thanks again for your reply! Lets hope they update this fast ;)

mikebronner commented 4 years ago

@Tjab Good luck! Let me know what they say. :) Thanks :)