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

fix: Support @property on Model with imported type #170

Merged
merged 2 commits into from
Jul 9, 2021
Merged

fix: Support @property on Model with imported type #170

merged 2 commits into from
Jul 9, 2021

Conversation

caugner
Copy link
Contributor

@caugner caugner commented Jul 6, 2021

Originally reported in vimeo/psalm#6024.

@caugner
Copy link
Contributor Author

caugner commented Jul 6, 2021

@mr-feek The root of the problem is probably located in ModelPropertyAccessorHandler::getPropertyType():

public static function getPropertyType(string $fq_classlike_name, string $property_name, bool $read_mode, ?StatementsSource $source = null, ?Context $context = null): ?Type\Union
{
if (!$source || !$read_mode) {
return null;
}
$codebase = $source->getCodebase();
if (self::accessorExists($codebase, $fq_classlike_name, $property_name)) {
return $codebase->getMethodReturnType($fq_classlike_name . '::get' . str_replace('_', '', $property_name) . 'Attribute', $fq_classlike_name)
?: Type::getMixed();
}
return null;
}

edit: I'm unable to debug the codeception run in PhpStorm, despite having the plugin enabled. Any tips?

@caugner
Copy link
Contributor Author

caugner commented Jul 6, 2021

@mr-feek I have tried to debug with PSALM_ALLOW_XDEBUG=1, as described here, but this didn't seem to work either.

@mr-feek
Copy link
Collaborator

mr-feek commented Jul 6, 2021

@caugner yeah, it's a really frustrating experience. What I have started doing recently is cloning a fresh laravel install and adding the code I want to analyze.

Then I require this plugin via composer, however use a symlink to load it in locally .

Then I'm able to get xdebug working. this is the command I use:
PSALM_ALLOW_XDEBUG=1 php -dxdebug.mode=debug -dxdebug.start_with_request=yes ./vendor/bin/psalm

@caugner
Copy link
Contributor Author

caugner commented Jul 6, 2021

So the reason why the @property CarbonInterface annotation causes trouble on the Model is that it seems to be copied directly to vendor/psalm/plugin-laravel/src/cache/models.stubphp, but without the import.

For example, the cache/models.stubphp contains the following (CarbonInterface without import) when running the test in this branch:

namespace Tests\Psalm\LaravelPlugin\Models{
/**
 * Tests\Psalm\LaravelPlugin\Models\User
 *
 * @property string $id
 * @property CarbonInterface $email_verified_at
 * @property-read \Illuminate\Database\Eloquent\Collection|\Tests\Psalm\LaravelPlugin\Models\Mechanic[] $carsAtMechanic
 * @property-read int|null $cars_at_mechanic_count
 * @property-read \Tests\Psalm\LaravelPlugin\Models\Image|null $image
 * @property-read \Tests\Psalm\LaravelPlugin\Models\Phone|null $phone
 * @property-read \Illuminate\Database\Eloquent\Collection|\Tests\Psalm\LaravelPlugin\Models\Role[] $roles
 * @property-read int|null $roles_count
 * @method static \Illuminate\Database\Eloquent\Builder|User active()
 * @method static \Illuminate\Database\Eloquent\Builder|User newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|User newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|User query()
 */
	final class User extends \Eloquent {}

Now the question is whether this is a bug in laravel-ide-helper or in the Psalm plugin. 😕

@caugner caugner changed the title feature: Model @property support fix: Support @property on Model with imported type Jul 6, 2021
@caugner caugner marked this pull request as ready for review July 6, 2021 19:50
This avoids that existing @Property annotations are copied to
cache/models.stubphp, which caused annotated properties that use an
imported type to break, because the imports are not copied over.
@mr-feek mr-feek added the fix label Jul 9, 2021
@mr-feek mr-feek merged commit 0e2f510 into psalm:master Jul 9, 2021
@caugner caugner deleted the model-@property-support branch July 9, 2021 18:13
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 this pull request may close these issues.

None yet

2 participants