Property check (larastan#673)

* feat: new model property types and rules
bepsvpt-fork · Oct 6, 2020 · ec8d1b1 · ec8d1b1
1 parent 8d25ba7
commit ec8d1b1
Show file tree

Hide file tree

Showing 40 changed files with 1,227 additions and 83 deletions.
diff --git a/docs/custom-types.md b/docs/custom-types.md
@@ -30,3 +30,11 @@ If the string is not an existing blade view, the following error will be display
 ```
 Parameter #1 $view of method TestClass::renderView() expects view-string, string given.  
 ```
+
+## model-property
+`model-property` extends the built-in `string` type and acts like a string in the type level. But during the analysis if Larastan finds that an argument of the method or a function has a `model-property<ModelName>`, it'll try to check that the given argument value is actually a property of the model.
+
+All of the Laravel core methods have this type thanks to the stubs. So whenever you use a Eloquent builder, relation or a model method that expects a column, it'll be checked by Larastan if the column actually exists. But you can also typehint any argument with `model-property` in your code.
+
+The actual check is done by the `ModelPropertyRule`. You can read the details [here](rules.md#ModelPropertyRule).
+
diff --git a/docs/rules.md b/docs/rules.md
@@ -54,3 +54,49 @@ parameters:
     noUnnecessaryCollectionCallExcept: ['contains']
 ```
 
+## ModelPropertyRule
+
+This rule checks every argument of a method or a function, and if the argument has the type `model-property`, it will try to check the given value against the model properties. And if the model does not have the given property, it'll produce an error.
+
+### Basic example
+
+```php
+User::create([
+    'name' => 'John Doe',
+    'emaiil' => '[email protected]'
+]);
+```
+
+Here we have a typo in `email` column. So if we run analysis on this file Larastan will generate the following error:
+
+```
+Property 'emaiil' does not exist in App\User model.
+```
+
+This check will be done automatically on Laravel's core methods where a property is expected. But you can also typehint the `model-property` in your own code to take advantage of this analysis.
+
+You can define a function like this:
+```php
+/**
+ * @phpstan-param model-property<\App\User> $property
+ */
+function takesOnlyUserModelProperties(string $property)
+{
+    // ...
+}
+```
+
+And if you call the function above with a property that does not exist in User model, Larastan will warn you about it.
+
+```php
+// Property 'emaiil' does not exist in App\User model.
+takesOnlyUserModelProperties('emaiil');
+```
+
+### Configuration
+This rule is disabled by default. You can enable it by putting
+```
+parameters:
+    checkModelProperties: true
+```
+to your `phpstan.neon` file.
diff --git a/extension.neon b/extension.neon
@@ -42,16 +42,22 @@ parameters:
     noUnnecessaryCollectionCallOnly: []
     noUnnecessaryCollectionCallExcept: []
     databaseMigrationsPath: null
+    checkModelProperties: false
 
 parametersSchema:
     noUnnecessaryCollectionCall: bool()
     noUnnecessaryCollectionCallOnly: listOf(string())
     noUnnecessaryCollectionCallExcept: listOf(string())
     databaseMigrationsPath: schema(anyOf(string()), nullable())
+    checkModelProperties: bool()
 
 conditionalTags:
     NunoMaduro\Larastan\Rules\NoUnnecessaryCollectionCallRule:
         phpstan.rules.rule: %noUnnecessaryCollectionCall%
+    NunoMaduro\Larastan\Rules\ModelProperties\ModelPropertyRule:
+        phpstan.rules.rule: %checkModelProperties%
+    NunoMaduro\Larastan\Rules\ModelProperties\ModelPropertyStaticCallRule:
+        phpstan.rules.rule: %checkModelProperties%
 
 services:
     -
@@ -292,11 +298,28 @@ services:
             onlyMethods: %noUnnecessaryCollectionCallOnly%
             excludeMethods: %noUnnecessaryCollectionCallExcept%
 
+    -
+        class: NunoMaduro\Larastan\Rules\ModelProperties\ModelPropertyRule
+        tags:
+            - phpstan.rules.rule
+
+    -
+        class: NunoMaduro\Larastan\Rules\ModelProperties\ModelPropertyStaticCallRule
+        tags:
+            - phpstan.rules.rule
+
     -
         class: NunoMaduro\Larastan\Types\GenericEloquentBuilderTypeNodeResolverExtension
         tags:
             - phpstan.phpDoc.typeNodeResolverExtension
 
+    -
+        class: NunoMaduro\Larastan\Types\ModelProperty\ModelPropertyTypeNodeResolverExtension
+        tags:
+            - phpstan.phpDoc.typeNodeResolverExtension
+        arguments:
+            active: %checkModelProperties%
+
     -
         class: NunoMaduro\Larastan\Types\RelationParserHelper
 
@@ -305,3 +328,7 @@ services:
         arguments:
             databaseMigrationPath: %databaseMigrationsPath%
             currentWorkingDirectory: %currentWorkingDirectory%
+
+    -
+        class: NunoMaduro\Larastan\Rules\ModelProperties\ModelPropertiesRuleHelper
+
diff --git a/src/Methods/BuilderHelper.php b/src/Methods/BuilderHelper.php
@@ -67,6 +67,7 @@ public function dynamicWhere(
         return new EloquentBuilderMethodReflection(
             $methodName,
             $classReflection,
+            $methodReflection,
             [$actualParameter],
             $returnObject,
             true
@@ -89,7 +90,9 @@ public function searchOnEloquentBuilder(ClassReflection $eloquentBuilder, string
             $returnType = $parametersAcceptor->getReturnType();
 
             return new EloquentBuilderMethodReflection(
-                'scope'.ucfirst($methodName), $methodReflection->getDeclaringClass(),
+                'scope'.ucfirst($methodName),
+                $methodReflection->getDeclaringClass(),
+                $methodReflection,
                 $parameters,
                 $returnType,
                 $parametersAcceptor->isVariadic()
@@ -106,7 +109,7 @@ public function searchOnEloquentBuilder(ClassReflection $eloquentBuilder, string
             $returnType = ModelTypeHelper::replaceStaticTypeWithModel($parametersAcceptor->getReturnType(), $modelClassName);
 
             return new EloquentBuilderMethodReflection(
-                $methodName, $eloquentBuilder,
+                $methodName, $eloquentBuilder, $methodReflection,
                 $parametersAcceptor->getParameters(),
                 $returnType,
                 $parametersAcceptor->isVariadic()
@@ -163,7 +166,8 @@ public function getMethodReflectionFromBuilder(
         // This can be a custom EloquentBuilder or the normal one
         $builderName = $this->determineBuilderType($modelName);
 
-        $builderReflection = $this->broker->getClass($builderName);
+        /** @var ClassReflection $builderReflection */
+        $builderReflection = (new GenericObjectType($builderName, [new ObjectType($modelName)]))->getClassReflection();
 
         $methodReflection = $this->searchOnEloquentBuilder($builderReflection, $methodName, $modelName);
 
@@ -199,8 +203,14 @@ public function getMethodReflectionFromBuilder(
                 $returnType = new GenericObjectType(Collection::class, [new ObjectType($modelName)]);
             }
 
+            $originalMethodReflection = $methodReflection;
+
+            if ($originalMethodReflection instanceof EloquentBuilderMethodReflection) {
+                $originalMethodReflection = $originalMethodReflection->getOriginalMethodReflection();
+            }
+
             return new EloquentBuilderMethodReflection(
-                $methodName, $classReflection,
+                $methodName, $classReflection, $originalMethodReflection,
                 $parametersAcceptor->getParameters(),
                 $returnType,
                 $parametersAcceptor->isVariadic()

diff --git a/src/Methods/EloquentBuilderForwardsCallsExtension.php b/src/Methods/EloquentBuilderForwardsCallsExtension.php
@@ -81,7 +81,7 @@ public function getMethod(ClassReflection $classReflection, string $methodName):
             }
 
             return new EloquentBuilderMethodReflection(
-                $methodName, $classReflection,
+                $methodName, $classReflection, $methodReflection,
                 $parametersAcceptor->getParameters(),
                 $returnType,
                 $parametersAcceptor->isVariadic()
@@ -109,7 +109,7 @@ public function getMethod(ClassReflection $classReflection, string $methodName):
             $parametersAcceptor = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants());
 
             return new EloquentBuilderMethodReflection(
-                $methodName, $classReflection,
+                $methodName, $classReflection, $methodReflection,
                 $parametersAcceptor->getParameters(),
                 new GenericObjectType($builderClass, [$modelType]),
                 $parametersAcceptor->isVariadic()

diff --git a/src/Properties/SchemaAggregator.php b/src/Properties/SchemaAggregator.php
@@ -58,7 +58,7 @@ private function addUpMethodStatements(array $stmts): void
                 && $stmt->expr instanceof PhpParser\Node\Expr\StaticCall
                 && ($stmt->expr->class instanceof PhpParser\Node\Name)
                 && $stmt->expr->name instanceof PhpParser\Node\Identifier
-                && ($stmt->expr->class->toCodeString() === '\\Illuminate\Support\Facades\Schema' || $stmt->expr->class->toCodeString() === '\Schema')
+                && ($stmt->expr->class->toCodeString() === '\Illuminate\Support\Facades\Schema' || $stmt->expr->class->toCodeString() === '\Schema')
             ) {
                 switch ($stmt->expr->name->name) {
                     case 'create':

diff --git a/src/Reflection/EloquentBuilderMethodReflection.php b/src/Reflection/EloquentBuilderMethodReflection.php
@@ -27,6 +27,9 @@ final class EloquentBuilderMethodReflection implements MethodReflection
      */
     private $classReflection;
 
+    /** @var MethodReflection */
+    private $originalMethodReflection;
+
     /**
      * @var ParameterReflection[]
      */
@@ -45,14 +48,16 @@ final class EloquentBuilderMethodReflection implements MethodReflection
     /**
      * @param string                $methodName
      * @param ClassReflection       $classReflection
+     * @param MethodReflection      $originalMethodReflection
      * @param ParameterReflection[] $parameters
      * @param Type|null             $returnType
      * @param bool                  $isVariadic
      */
-    public function __construct(string $methodName, ClassReflection $classReflection, array $parameters, ?Type $returnType = null, bool $isVariadic = false)
+    public function __construct(string $methodName, ClassReflection $classReflection, MethodReflection $originalMethodReflection, array $parameters, ?Type $returnType = null, bool $isVariadic = false)
     {
         $this->methodName = $methodName;
         $this->classReflection = $classReflection;
+        $this->originalMethodReflection = $originalMethodReflection;
         $this->parameters = $parameters;
         $this->returnType = $returnType ?? new ObjectType(Builder::class);
         $this->isVariadic = $isVariadic;
@@ -138,4 +143,12 @@ public function hasSideEffects(): TrinaryLogic
     {
         return TrinaryLogic::createMaybe();
     }
+
+    /**
+     * @return MethodReflection
+     */
+    public function getOriginalMethodReflection(): MethodReflection
+    {
+        return $this->originalMethodReflection;
+    }
 }