feat: property check on dynamic wheres (larastan#686)

bepsvpt-fork · Oct 16, 2020 · aa4ceda · aa4ceda
1 parent 585f53d
commit aa4ceda
Show file tree

Hide file tree

Showing 17 changed files with 225 additions and 148 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+
+- Support fot checking model properties on dynamic wheres ([#686](https://github.com/nunomaduro/larastan/pull/686))
+
 ## [0.6.5] - 2020-10-15
 
 This release introduces a new rule that can check the arguments of methods that expects a model property name, and can warn you if the passed argument is not actually a property of the model. You can read the details about the rule [here](https://github.com/nunomaduro/larastan/blob/master/docs/rules.md#modelpropertyrule).

diff --git a/extension.neon b/extension.neon
@@ -332,3 +332,7 @@ services:
     -
         class: NunoMaduro\Larastan\Rules\ModelProperties\ModelPropertiesRuleHelper
 
+    -
+        class: NunoMaduro\Larastan\Methods\BuilderHelper
+        arguments:
+            checkProperties: %checkModelProperties%
diff --git a/src/Methods/BuilderHelper.php b/src/Methods/BuilderHelper.php
@@ -9,14 +9,15 @@
 use Illuminate\Database\Query\Builder as QueryBuilder;
 use Illuminate\Support\Str;
 use NunoMaduro\Larastan\Reflection\EloquentBuilderMethodReflection;
-use PHPStan\Broker\Broker;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\FunctionVariantWithPhpDocs;
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\Reflection\MissingMethodFromReflectionException;
 use PHPStan\Reflection\ParametersAcceptorSelector;
 use PHPStan\Reflection\Php\DummyParameter;
+use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\ShouldNotHappenException;
+use PHPStan\TrinaryLogic;
 use PHPStan\Type\Generic\GenericObjectType;
 use PHPStan\Type\Generic\TemplateTypeHelper;
 use PHPStan\Type\Generic\TemplateTypeMap;
@@ -34,14 +35,16 @@ class BuilderHelper
     /** @var string[] */
     public const MODEL_CREATION_METHODS = ['make', 'create', 'forceCreate', 'findOrNew', 'firstOrNew', 'updateOrCreate', 'firstOrCreate'];
 
-    /**
-     * @var Broker
-     */
-    private $broker;
+    /** @var ReflectionProvider */
+    private $reflectionProvider;
 
-    public function __construct(Broker $broker)
+    /** @var bool */
+    private $checkProperties;
+
+    public function __construct(ReflectionProvider $reflectionProvider, bool $checkProperties)
     {
-        $this->broker = $broker;
+        $this->reflectionProvider = $reflectionProvider;
+        $this->checkProperties = $checkProperties;
     }
 
     public function dynamicWhere(
@@ -52,14 +55,47 @@ public function dynamicWhere(
             return null;
         }
 
-        $classReflection = $this->broker->getClass(QueryBuilder::class);
+        if ($returnObject instanceof GenericObjectType && $this->checkProperties) {
+            $returnClassReflection = $returnObject->getClassReflection();
+
+            if ($returnClassReflection !== null) {
+                $modelType = $returnClassReflection->getActiveTemplateTypeMap()->getType('TModelClass');
+
+                if ($modelType === null) {
+                    $modelType = $returnClassReflection->getActiveTemplateTypeMap()->getType('TRelatedModel');
+                }
+
+                if ($modelType !== null) {
+                    $finder = substr($methodName, 5);
+
+                    $segments = preg_split(
+                        '/(And|Or)(?=[A-Z])/', $finder, -1, PREG_SPLIT_DELIM_CAPTURE
+                    );
+
+                    if ($segments !== false) {
+                        $trinaryLogic = TrinaryLogic::createYes();
+
+                        foreach ($segments as $segment) {
+                            if ($segment !== 'And' && $segment !== 'Or') {
+                                $trinaryLogic = $trinaryLogic->and($modelType->hasProperty(Str::snake($segment)));
+                            }
+                        }
+
+                        if (! $trinaryLogic->yes()) {
+                            return null;
+                        }
+                    }
+                }
+            }
+        }
+
+        $classReflection = $this->reflectionProvider->getClass(QueryBuilder::class);
 
         $methodReflection = $classReflection->getNativeMethod('dynamicWhere');
 
         /** @var FunctionVariantWithPhpDocs $originalDynamicWhereVariant */
         $originalDynamicWhereVariant = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants());
 
-        /** @var \PHPStan\Reflection\ParameterReflectionWithPhpDocs $originalParameter */
         $originalParameter = $originalDynamicWhereVariant->getParameters()[1];
 
         $actualParameter = new DummyParameter($originalParameter->getName(), new MixedType(), $originalParameter->isOptional(), $originalParameter->passedByReference(), $originalParameter->isVariadic(), $originalParameter->getDefaultValue());
@@ -76,7 +112,7 @@ public function dynamicWhere(
 
     public function searchOnEloquentBuilder(ClassReflection $eloquentBuilder, string $methodName, string $modelClassName): ?MethodReflection
     {
-        $model = $this->broker->getClass($modelClassName);
+        $model = $this->reflectionProvider->getClass($modelClassName);
 
         if ($model->hasNativeMethod('scope'.ucfirst($methodName))) {
             $methodReflection = $model->getNativeMethod('scope'.ucfirst($methodName));
@@ -121,7 +157,7 @@ public function searchOnEloquentBuilder(ClassReflection $eloquentBuilder, string
 
     public function searchOnQueryBuilder(string $methodName, string $modelClassName): ?MethodReflection
     {
-        $queryBuilder = $this->broker->getClass(QueryBuilder::class);
+        $queryBuilder = $this->reflectionProvider->getClass(QueryBuilder::class);
 
         if ($queryBuilder->hasNativeMethod($methodName)) {
             return $queryBuilder->getNativeMethod($methodName);
@@ -139,7 +175,7 @@ public function searchOnQueryBuilder(string $methodName, string $modelClassName)
      */
     public function determineBuilderType(string $modelClassName): string
     {
-        $method = $this->broker->getClass($modelClassName)->getNativeMethod('newEloquentBuilder');
+        $method = $this->reflectionProvider->getClass($modelClassName)->getNativeMethod('newEloquentBuilder');
 
         $returnType = ParametersAcceptorSelector::selectSingle($method->getVariants())->getReturnType();
 
@@ -161,7 +197,7 @@ public function getMethodReflectionFromBuilder(
         Type $customReturnType
     ): ?EloquentBuilderMethodReflection {
         $methodReflection = null;
-        $model = $this->broker->getClass($modelName);
+        $model = $this->reflectionProvider->getClass($modelName);
 
         // This can be a custom EloquentBuilder or the normal one
         $builderName = $this->determineBuilderType($modelName);
@@ -222,7 +258,7 @@ public function getMethodReflectionFromBuilder(
 
     public function determineCollectionClassName(string $modelClassName): string
     {
-        $newCollectionMethod = $this->broker->getClass($modelClassName)->getNativeMethod('newCollection');
+        $newCollectionMethod = $this->reflectionProvider->getClass($modelClassName)->getNativeMethod('newCollection');
 
         $returnType = ParametersAcceptorSelector::selectSingle($newCollectionMethod->getVariants())->getReturnType();
 

diff --git a/src/Methods/EloquentBuilderForwardsCallsExtension.php b/src/Methods/EloquentBuilderForwardsCallsExtension.php
@@ -36,6 +36,14 @@ final class EloquentBuilderForwardsCallsExtension implements MethodsClassReflect
         'exists', 'doesntExist', 'count', 'min', 'max', 'avg', 'average', 'sum', 'getConnection',
     ];
 
+    /** @var BuilderHelper */
+    private $builderHelper;
+
+    public function __construct(BuilderHelper $builderHelper)
+    {
+        $this->builderHelper = $builderHelper;
+    }
+
     private function getBuilderReflection(): ClassReflection
     {
         return $this->broker->getClass(QueryBuilder::class);
@@ -117,15 +125,13 @@ public function getMethod(ClassReflection $classReflection, string $methodName):
         }
 
         if ($modelType instanceof ObjectType) {
-            $builderHelper = new BuilderHelper($this->getBroker());
-
             if ($classReflection->isSubclassOf(EloquentBuilder::class)) {
                 $eloquentBuilderClass = $classReflection->getName();
             } else {
-                $eloquentBuilderClass = $builderHelper->determineBuilderType($modelType->getClassName());
+                $eloquentBuilderClass = $this->builderHelper->determineBuilderType($modelType->getClassName());
             }
 
-            $returnMethodReflection = $builderHelper->getMethodReflectionFromBuilder(
+            $returnMethodReflection = $this->builderHelper->getMethodReflectionFromBuilder(
                 $classReflection,
                 $methodName,
                 $modelType->getClassName(),

diff --git a/src/Methods/ModelForwardsCallsExtension.php b/src/Methods/ModelForwardsCallsExtension.php
@@ -4,28 +4,23 @@
 
 namespace NunoMaduro\Larastan\Methods;
 
-use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
 use Illuminate\Database\Eloquent\Model;
-use Illuminate\Database\Query\Builder as QueryBuilder;
-use Illuminate\Support\Str;
-use NunoMaduro\Larastan\Concerns;
-use PHPStan\Reflection\BrokerAwareExtension;
 use PHPStan\Reflection\ClassReflection;
-use PHPStan\Reflection\Dummy\DummyMethodReflection;
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\Reflection\MethodsClassReflectionExtension;
 use PHPStan\Reflection\MissingMethodFromReflectionException;
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\Generic\GenericObjectType;
 use PHPStan\Type\ObjectType;
 
-final class ModelForwardsCallsExtension implements MethodsClassReflectionExtension, BrokerAwareExtension
+final class ModelForwardsCallsExtension implements MethodsClassReflectionExtension
 {
-    use Concerns\HasBroker;
+    /** @var BuilderHelper */
+    private $builderHelper;
 
-    private function getBuilderReflection(): ClassReflection
+    public function __construct(BuilderHelper $builderHelper)
     {
-        return $this->broker->getClass(EloquentBuilder::class);
+        $this->builderHelper = $builderHelper;
     }
 
     public function hasMethod(ClassReflection $classReflection, string $methodName): bool
@@ -34,26 +29,16 @@ public function hasMethod(ClassReflection $classReflection, string $methodName):
             return false;
         }
 
-        if (in_array($methodName, ['increment', 'decrement', 'paginate', 'simplePaginate'], true)) {
-            return true;
-        }
-
-        // Model scopes
-        if ($classReflection->hasNativeMethod('scope'.ucfirst($methodName))) {
-            return true;
-        }
+        $customBuilderName = $this->builderHelper->determineBuilderType($classReflection->getName());
 
-        // Dynamic wheres
-        if (Str::startsWith($methodName, 'where')) {
-            return true;
-        }
-
-        $builderHelper = new BuilderHelper($this->getBroker());
-        $customBuilder = $builderHelper->determineBuilderType($classReflection->getName());
+        $returnMethodReflection = $this->builderHelper->getMethodReflectionFromBuilder(
+            $classReflection,
+            $methodName,
+            $classReflection->getName(),
+            new GenericObjectType($customBuilderName, [new ObjectType($classReflection->getName())])
+        );
 
-        return $this->getBuilderReflection()->hasNativeMethod($methodName)
-            || $this->broker->getClass(QueryBuilder::class)->hasNativeMethod($methodName)
-            || ($customBuilder && $this->broker->getClass($customBuilder)->hasNativeMethod($methodName));
+        return $returnMethodReflection !== null;
     }
 
     /**
@@ -66,20 +51,19 @@ public function hasMethod(ClassReflection $classReflection, string $methodName):
      */
     public function getMethod(ClassReflection $originalModelReflection, string $methodName): MethodReflection
     {
-        $builderHelper = new BuilderHelper($this->getBroker());
-        $customBuilderName = $builderHelper->determineBuilderType($originalModelReflection->getName());
+        $customBuilderName = $this->builderHelper->determineBuilderType($originalModelReflection->getName());
 
-        $returnMethodReflection = $builderHelper->getMethodReflectionFromBuilder(
+        $returnMethodReflection = $this->builderHelper->getMethodReflectionFromBuilder(
             $originalModelReflection,
             $methodName,
             $originalModelReflection->getName(),
             new GenericObjectType($customBuilderName, [new ObjectType($originalModelReflection->getName())])
         );
 
-        if ($returnMethodReflection !== null) {
-            return $returnMethodReflection;
+        if ($returnMethodReflection === null) {
+            throw new ShouldNotHappenException();
         }
 
-        return new DummyMethodReflection($methodName);
+        return $returnMethodReflection;
     }
 }
diff --git a/src/Methods/Pipes/Mixins.php b/src/Methods/Pipes/Mixins.php
@@ -8,8 +8,9 @@
 use NunoMaduro\Larastan\Concerns;
 use NunoMaduro\Larastan\Contracts\Methods\PassableContract;
 use NunoMaduro\Larastan\Contracts\Methods\Pipes\PipeContract;
-use PHPStan\Broker\Broker;
+use PHPStan\Broker\ClassNotFoundException;
 use PHPStan\Reflection\ClassReflection;
+use PHPStan\Reflection\ReflectionProvider;
 
 /**
  * @internal
@@ -46,13 +47,13 @@ public function handle(PassableContract $passable, Closure $next): void
     }
 
     /**
-     * @param \PHPStan\Broker\Broker              $broker
-     * @param \PHPStan\Reflection\ClassReflection $classReflection
+     * @param ReflectionProvider $reflectionProvider
+     * @param ClassReflection    $classReflection
      *
      * @return string[]
-     * @throws \PHPStan\Broker\ClassNotFoundException
+     * @throws ClassNotFoundException
      */
-    public function getMixinsFromClass(Broker $broker, ClassReflection $classReflection): array
+    public function getMixinsFromClass(ReflectionProvider $reflectionProvider, ClassReflection $classReflection): array
     {
         $phpdocs = (string) $classReflection->getNativeReflection()->getDocComment();
 
@@ -65,7 +66,7 @@ public function getMixinsFromClass(Broker $broker, ClassReflection $classReflect
                 $this->resolve('config')->get('larastan.mixins')[$classReflection->getName()] ?? []
         );
 
-        $mixins = array_filter($mixins, function ($mixin) use ($classReflection) {
+        $mixins = array_filter($mixins, static function ($mixin) use ($classReflection) {
             try {
                 return (new \ReflectionClass($mixin))->getName() !== $classReflection->getName();
             } catch (\ReflectionException $e) {
@@ -81,7 +82,7 @@ public function getMixinsFromClass(Broker $broker, ClassReflection $classReflect
                      */
                     self::$resolved[$mixin] = [];
 
-                    self::$resolved[$mixin] = $this->getMixinsFromClass($broker, $broker->getClass($mixin));
+                    self::$resolved[$mixin] = $this->getMixinsFromClass($reflectionProvider, $reflectionProvider->getClass($mixin));
                 }
                 $mixins = array_merge($mixins, self::$resolved[$mixin]);
             }
@@ -100,7 +101,7 @@ private function getMixinsFromPhpDocs(string $phpdocs, string $pattern): array
     {
         preg_match_all($pattern, $phpdocs, $mixins);
 
-        return array_map(function ($mixin) {
+        return array_map(static function ($mixin) {
             return preg_replace('#^\\\\#', '', $mixin);
         }, $mixins[1]);
     }