Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.2 - Ide helper files seem not recognized #1402

Closed
karinarastsinskagia opened this issue Jan 10, 2023 · 4 comments · Fixed by #1428
Closed

PHP 8.2 - Ide helper files seem not recognized #1402

karinarastsinskagia opened this issue Jan 10, 2023 · 4 comments · Fixed by #1428
Labels

Comments

@karinarastsinskagia
Copy link

Versions:

  • ide-helper Version: 2.12.13
  • Laravel Version: 9.44
  • PHP Version: 8.2

Description:

After the update to PHP 8.2 all @Property annotations which include in _ide_helper_models.php are not recognized and an Access to an undefined property is thrown. More specifically, I have the following model and the respective _ide_helper_models.php

  • Article Model
<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

/**
 * @mixin IdeHelperArticle
 */
class Article extends Model
{

    protected $fillable = ['title'];

    public function comments(): HasMany
    {
        return $this->hasMany(Comment::class);
    }
}
  • Generated ide_helper_models.php
<?php

// @formatter:off
/**
 * A helper file for your Eloquent Models
 * Copy the phpDocs from this file to the correct Model,
 * And remove them from this file, to prevent double declarations.
 *
 * @author Barry vd. Heuvel <[email protected]>
 */


namespace App\Models{
/**
 * App\Models\Article
 *
 * @property int $id
 * @property string $title
 * @property string $created_at
 * @property-read \Illuminate\Database\Eloquent\Collection|\App\Models\Comment[] $comments
 * @property-read int|null $comments_count
 * @method static \Database\Factories\ArticleFactory factory(...$parameters)
 * @method static \Illuminate\Database\Eloquent\Builder|Article newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Article newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Article query()
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereCreatedAt($value)
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereId($value)
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereTitle($value)
 * @mixin \Eloquent
 * @noinspection PhpFullyQualifiedNameUsageInspection
 * @noinspection PhpUnnecessaryFullyQualifiedNameInspection
 */
	class IdeHelperArticle {}
}
  • phpstan.neon
includes:
    - ./vendor/nunomaduro/larastan/extension.neon

parameters:
    paths:
        - src

    level: 6

    tmpDir: build/phpstan
    reportStaticMethodSignatures: true
    checkModelProperties: true
    checkMissingIterableValueType: false
    checkGenericClassInNonGenericObjectType: false
    checkOctaneCompatibility: true

    scanFiles:
    	- _ide_helper.php
    	- _ide_helper_models.php
  • Code Example
 
$article = new Article();
$article->title = 'Article 1';
echo $article->title;

All works fine with PHP Version 8.1. Although, using PHP 8.2 I get the mentioned error message (Access to an undefined property App\Models\Article::$title).

About the issue, I have tried 3 different approaches. Please find them below

  • Approach : Trying to insert all @Property inside Model class
    Result : Works
    Comment : I prefer to keep my first structure using the @mixin IdeHelperArticle.
<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

/**
 * * App\Models\Article
 *
 * @property int $id
 * @property string $title
 * @property string $created_at
 * @property-read \Illuminate\Database\Eloquent\Collection|\App\Models\Comment[] $comments
 * @property-read int|null $comments_count
 * @method static \Database\Factories\ArticleFactory factory(...$parameters)
 * @method static \Illuminate\Database\Eloquent\Builder|Article newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Article newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Article query()
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereCreatedAt($value)
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereId($value)
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereTitle($value)
 * @mixin \Eloquent
 * @noinspection PhpFullyQualifiedNameUsageInspection
 * @noinspection PhpUnnecessaryFullyQualifiedNameInspection
 */
class Article extends Model
{

    protected $fillable = ['title'];

    public function comments(): HasMany
    {
        return $this->hasMany(Comment::class);
    }

}
  • Approach : Add #[\AllowDynamicProperties] before model class
    Result : Not work
    Comment : I found out that PHP 8.2 needs to have #[\AllowDynamicProperties] above the class to work however this approach does not solve the issue
<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

/**
 * @mixin IdeHelperArticle
 */
 
#[\AllowDynamicProperties] 
class Article extends Model
{

    protected $fillable = ['title'];

    public function comments(): HasMany
    {
        return $this->hasMany(Comment::class);
    }

}
  • Approach : Use #[\AllowDynamicProperties] inside ide_helper_models.php
    Result : works
    Comment : I don't know why and how to enforce this feature at generation time of ide_helper_models.php file
<?php

// @formatter:off
/**
 * A helper file for your Eloquent Models
 * Copy the phpDocs from this file to the correct Model,
 * And remove them from this file, to prevent double declarations.
 *
 * @author Barry vd. Heuvel <[email protected]>
 */


namespace App\Models{
/**
 * App\Models\Article
 *
 * @property int $id
 * @property string $title
 * @property string $created_at
 * @property-read \Illuminate\Database\Eloquent\Collection|\App\Models\Comment[] $comments
 * @property-read int|null $comments_count
 * @method static \Database\Factories\ArticleFactory factory(...$parameters)
 * @method static \Illuminate\Database\Eloquent\Builder|Article newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Article newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Article query()
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereCreatedAt($value)
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereId($value)
 * @method static \Illuminate\Database\Eloquent\Builder|Article whereTitle($value)
 * @mixin \Eloquent
 * @noinspection PhpFullyQualifiedNameUsageInspection
 * @noinspection PhpUnnecessaryFullyQualifiedNameInspection
 */
 #[\AllowDynamicProperties] class IdeHelperArticle {}
}

Is there any setting that I have to enable in order to make ide_helper_models.php be compatible with PHP 8.2 or is there any other solution in order to keep the functionality as it is in PHP 8.1?
Does anyone know why my Model class doesn't work in the same way after the update to PHP 8.2?

@Jefemy
Copy link
Contributor

Jefemy commented Feb 22, 2023

This seems to just be an issue in general with using mixins in this way now. I initially thought this was a bug too and created an issue in phpstan/phpstan#8939 but after some discussion it makes sense. PHPStan is assuming that because the IdeHelper class has no __get/__set the properties attached to it are dynamic and since it has no #[\AllowDynamicProperties] by default it is ignoring those entirely on the main class.

ide-helper could theoretically get around this by adding a fake get/set or AllowDynamicProperties to its generated class but I feel like both of those are just workarounds for the fact that I don't think mixins were made to do something like this.

@mfn
Copy link
Collaborator

mfn commented Sep 18, 2023

Wouldn't it be technically more correct if the classes generated in the helper file would actually extend \Illuminate\Database\Eloquent\Model?

There the __get is implemented.

@karinarastsinskagia
Copy link
Author

Wouldn't it be technically more correct if the classes generated in the helper file would actually extend \Illuminate\Database\Eloquent\Model?

There the __get is implemented.

Well, it works, however I don't if, technically it's more correct

@karinarastsinskagia
Copy link
Author

@barryvdh

Do you have any news on this?

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants