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

Eloquent Collection groupBy has incorrect return type when grouping by multiple fields #1813

Open
simon-tma opened this issue Jan 10, 2024 · 0 comments · May be fixed by #1860
Open

Eloquent Collection groupBy has incorrect return type when grouping by multiple fields #1813

simon-tma opened this issue Jan 10, 2024 · 0 comments · May be fixed by #1860

Comments

@simon-tma
Copy link
Contributor

simon-tma commented Jan 10, 2024

  • Larastan Version: 2.8.1
  • --level used: 3

Description

Collection->groupBy accepts a list of fields to group by. If more than one field is passed, it uses the higher-order ->map->groupBy to recurse. This causes the return to change from being static to always being Illuminate\Support\Collection.

Also, there are extra levels of nesting for each additional field that is grouped on.

Laravel code where the issue was found

use App\Models\User;
use Illuminate\Support\Collection;

/** @return Collection<int|string, Collection<int|string, Collection<int, User>>> */
function example(): Collection {
  return User::all()->groupBy(['id', 'name']);
}

Potential Fix

I've created this extension for my project, which appears to generate the correct return type for groupBy with multiple arguments.

use Illuminate\Support\Collection;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\IntegerRangeType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\UnionType;

class CollectionGroupByReturnExtension implements DynamicMethodReturnTypeExtension {
    public function getClass(): string {
        return Collection::class;
    }

    public function isMethodSupported(MethodReflection $methodReflection): bool {
        return $methodReflection->getName() === 'groupBy';
    }

    public function getTypeFromMethodCall(
        MethodReflection $methodReflection,
        MethodCall $methodCall,
        Scope $scope
    ): ?Type {
        $fields = $methodCall->getArgs();
        if (\count($fields) < 1) {
            return null;
        }

        $groupBy = $fields[0]->value;
        $groupByType = $scope->getType($groupBy);

        $collectionType = $scope->getType($methodCall->var);

        if (
            // @phpstan-ignore-next-line
            ! $collectionType instanceof GenericObjectType ||
            ! $groupByType->isIterable()->yes()
        ) {
            return null;
        }

        $limit = $groupByType->getArraySize();
        if ($limit instanceof ConstantIntegerType) {
            $limit = $limit->getValue();
        } elseif ($limit instanceof IntegerRangeType) {
            $limit = $limit->getMin();
        }

        if (! \is_int($limit) || $limit < 2) {
            return null;
        }

        $inner = new GenericObjectType(
            Collection::class,
            $collectionType->getTypes(),
        );

        while ($limit > 0) {
            $inner = new GenericObjectType(
                Collection::class,
                [
                    new UnionType([new IntegerType(), new StringType()]),
                    $inner,
                ],
            );

            $limit -= 1;
        }

        return $inner;
    }
}
@mad-briller mad-briller linked a pull request Feb 19, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant