Larastan: proposal: infer models properties

[nunomaduro]

Description

We often have issues with the following code:

$user->age;

And the reason is that Laravel uses magic code to get the property from the model. Would be cool that, somehow, we could infer those properties from the model using migrations, or something else.

[canvural]

I don't think we should support this. Will add to much complexity. And there are tools, like laravel-ide-helper, that can read your models and write doc blocks to your models with all the needed methods and properties.

[nunomaduro]

Depending of the code needed for this we can analyze if adds too much complexity or not.

[CyberiaResurrection]

@nunomaduro , to make sure I grok this, this is only in the absence of existing @property annotations?

[CyberiaResurrection]

On further thought, what happens to properties affected by a mutator? Both fully-synthetic properties (such as the example below) and modifying an existing property.

public function getCollectUnderpantsAttribute()
....

Later...

$result = $foo->collect_underpants;

[mfn]

I'm also on 👎 to add dedicated support for this.

https://github.com/barryvdh/laravel-ide-helper pretty much covers almost everything and should be used by the user of the library for this (or they simple write their own phpdoc of course).

Also: migrations are unreliable, don't have to be there, can be custom SQL, etc.

[canvural]

We can run laravel-ide-helper under the hood and use the generated files for the analysis. This is what the Psalm Laravel plugin does. This can solve quite a lot of issues.

[szepeviktor]

Maybe getting ideas from laravel-ide-helper or copying code is better than using another code's output.

[szepeviktor]

Larastan's code is superior to laravel-ide-helper. Please find another solution.

[canvural]

laravel-ide-helper is a widely used and well-tested library. Why shouldn't we use it? And all the work is already done for us. So why would we invest more time and try to copy the functionality of it?

[szepeviktor]

For example it does not have static analysis in its tests in CI.
That is below my 3-part zero-coverage "test suite" https://twitter.com/szepeviktor/status/1157751604419997696

[mfn]

I'm with @szepeviktor regarding the quality, having worked with this library internals.

OTOH, it's the best thing out there we currently have and it would definitely help to contribute there (wish I had time for _that_ too).

[muglug]

I'm in the process of adding this to Psalm's Laravel plugin, came here to see how Larastan does it (and discovered it doesn't yet).

Psalm's laravel plugin uses barryvdh/laravel-ide-helper under the hood, and it works pretty decently (@szepeviktor I don't consider a lack of static analysis to be a prohibiting factor if the code is heavily used, well-tested, and written by a respected PHP developer).

barryvdh/laravel-ide-helper connects to the database with Doctrine to determine property types. That's not exactly performant to do on every run, though – I wonder if there's another way to figure out column type for a given table? Perhaps it might be possible to aggregate over a bunch of schema migrations to build up a final form?

[canvural]

@muglug Migrations are not too reliable. Developers don't have to use migrations (they can work with SQL backups for example), or migration can use custom SQL which makes it harder to determine column types. But these cases are not very common. So maybe it can be considered.

Yeah, running laravel-ide-helper every time will be costly. We can cache it, but then the question is how often should we update the cache...

We can infer some types from the model by checking the casts and dates properties. But this is not so useful too...

Let me know if you find a solution. Or wanna work together on this :+1:

[muglug]

The migrations will need to be statically analysed due to https://github.com/laravel/framework/issues/31030, so any code I write should be _reasonably_ portable. I'll probably work on this today.

[muglug]

@canvural at least for the project I regularly test on (monicahq/monica) the migrations were sufficient to build up a full picture of all database types.

[muglug]

I've released this, here's the scanning code:

• https://github.com/psalm/laravel-psalm-plugin/blob/0a02a2bcf306f8d7e7352343b610ad372390d7e7/src/AbstractPlugin.php#L148-L150
  
  
  
  • https://github.com/psalm/laravel-psalm-plugin/blob/0a02a2bcf306f8d7e7352343b610ad372390d7e7/src/SchemaAggregator.php.
  
  

The only Psalm-specific dependency there is a reliance on a PhpParser node visitor (a stripped-down version of https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/NodeVisitor/NameResolver.php) that sets a node attribute for all name nodes.

[canvural]

Implemented in #435