Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions fixtures/MartinGeorgiev/Doctrine/Function/TestVariadicFunction.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

namespace Fixtures\MartinGeorgiev\Doctrine\Function;

use Doctrine\ORM\Query\AST\Literal;
use Doctrine\ORM\Query\AST\Node;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\BaseVariadicFunction;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\BooleanValidationTrait;

/**
* A concrete implementation of BaseVariadicFunction for testing purposes.
*/
class TestVariadicFunction extends BaseVariadicFunction
{
use BooleanValidationTrait;

protected function getNodeMappingPattern(): array
{
return ['StringPrimary'];
}

protected function getFunctionName(): string
{
return 'test_function';
}

protected function getMinArgumentCount(): int
{
return 2;
}

protected function getMaxArgumentCount(): int
{
return 10;
}
}
22 changes: 13 additions & 9 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Arr.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@

namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

use Doctrine\ORM\Query\AST\Node;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException;

/**
* Implementation of PostgreSQL ARRAY[].
*
Expand All @@ -22,16 +19,23 @@ protected function getNodeMappingPattern(): array
return ['StringPrimary'];
}

protected function getFunctionName(): string
{
return 'ARRAY';
}

protected function customizeFunction(): void
{
$this->setFunctionPrototype('ARRAY[%s]');
$this->setFunctionPrototype(\sprintf('%s[%%s]', $this->getFunctionName()));
}

protected function getMinArgumentCount(): int
{
return 1;
}

protected function validateArguments(Node ...$arguments): void
protected function getMaxArgumentCount(): int
{
$argumentCount = \count($arguments);
if ($argumentCount === 0) {
throw InvalidArgumentForVariadicFunctionException::atLeast('ARRAY', 1);
}
return PHP_INT_MAX; // No upper limit
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

use Doctrine\ORM\Query\AST\Node;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\BooleanValidationTrait;

/**
Expand All @@ -31,21 +30,28 @@ protected function getNodeMappingPattern(): array
return ['StringPrimary'];
}

protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('array_to_json(%s)');
return 'array_to_json';
}

protected function getMinArgumentCount(): int
{
return 1;
}

protected function getMaxArgumentCount(): int
{
return 2;
}

protected function validateArguments(Node ...$arguments): void
{
$argumentCount = \count($arguments);
if ($argumentCount < 1 || $argumentCount > 2) {
throw InvalidArgumentForVariadicFunctionException::between('array_to_json', 1, 2);
}
parent::validateArguments(...$arguments);

// Validate that the second parameter is a valid boolean if provided
if ($argumentCount === 2) {
$this->validateBoolean($arguments[1], 'ARRAY_TO_JSON');
if (\count($arguments) === 2) {
$this->validateBoolean($arguments[1], $this->getFunctionName());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,14 @@ protected function getNodeMappingPattern(): array
{
return ['SimpleArithmeticExpression'];
}

protected function getMinArgumentCount(): int
{
return 2;
}

protected function getMaxArgumentCount(): int
{
return PHP_INT_MAX; // No hard limit apart the PHP internals
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,22 @@
*/
abstract class BaseVariadicFunction extends BaseFunction
{
protected function customizeFunction(): void
{
$this->setFunctionPrototype(\sprintf('%s(%%s)', $this->getFunctionName()));
}

Comment on lines +22 to +26
Copy link

@coderabbitai coderabbitai bot Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider declaring customizeFunction() as final

customizeFunction() is now fully implemented in the base class and should never be overridden by descendants – doing so would break the new centralised prototype logic you have just introduced. Marking it final eliminates that risk and makes the intended extension‑points (getFunctionName(), getMinArgumentCount(), getMaxArgumentCount()) explicit.

-    protected function customizeFunction(): void
+    final protected function customizeFunction(): void
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected function customizeFunction(): void
{
$this->setFunctionPrototype(\sprintf('%s(%%s)', $this->getFunctionName()));
}
final protected function customizeFunction(): void
{
$this->setFunctionPrototype(\sprintf('%s(%%s)', $this->getFunctionName()));
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true. The Arr class has a different prototype, and it overrides it accordingly for that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the implementation of the Arr class
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Arr.php

Length of output: 910


@martin-georgiev You're absolutely right. Looking at the Arr class implementation, I can confirm it needs to override customizeFunction() to use a different prototype format:

// In Arr.php
protected function customizeFunction(): void
{
    $this->setFunctionPrototype(\sprintf('%s[%%s]', $this->getFunctionName()));
}

The Arr class uses square brackets (ARRAY[%s]) instead of the default parentheses pattern. Making customizeFunction() final would prevent this type of legitimate customization, which is needed for PostgreSQL array constructor syntax.

Thanks for the correction.


🧬 Code Graph Analysis Results

Relevant snippets for understanding src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php and the user comment:


File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Arr.php

Lines 17-40:

protected function getNodeMappingPattern(): array
{
    return ['StringPrimary'];
}

protected function getFunctionName(): string
{
    return 'ARRAY';
}

protected function customizeFunction(): void
{
    $this->setFunctionPrototype(\sprintf('%s[%%s]', $this->getFunctionName()));
}

protected function getMinArgumentCount(): int
{
    return 1;
}

protected function getMaxArgumentCount(): int
{
    return PHP_INT_MAX; // No upper limit
}
  • This class Arr extends or relates to variadic functions and overrides the function prototype to use square brackets (ARRAY[%s]) instead of parentheses.
  • It defines the function name as 'ARRAY'.
  • It sets minimum argument count to 1 and maximum to unlimited (PHP_INT_MAX).
  • The node mapping pattern is a single parser method 'StringPrimary'.

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php

Lines 1-110 (entire file provided)

  • Abstract class extending BaseFunction to support variadic functions with flexible argument counts.
  • Defines abstract methods:
    • getFunctionName(): string
    • getNodeMappingPattern(): array
    • getMinArgumentCount(): int
    • getMaxArgumentCount(): int
  • Implements customizeFunction() to set the function prototype as functionName(%s) by default.
  • Implements feedParserWithNodes(Parser $parser) which tries multiple node mapping patterns to parse arguments.
  • Implements feedParserWithNodesForNodeMappingPattern(Parser $parser, string $nodeMappingPattern) which parses the arguments according to the pattern and validates argument count.
  • Validates argument count in validateArguments(Node ...$arguments).
  • getSql(SqlWalker $sqlWalker) generates SQL by dispatching nodes and formatting with the function prototype.
  • Uses DoctrineLexer and DoctrineOrm utilities to handle lexer differences between Doctrine versions.

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php

Lines 20-105 (summary):

abstract class BaseFunction extends FunctionNode
{
    protected string $functionPrototype;
    protected array $nodesMapping = [];
    protected array $nodes = [];

    protected function customizeFunction(): void
    {
        // Void by default
    }

    protected function setFunctionPrototype(string $functionPrototype): void
    {
        $this->functionPrototype = $functionPrototype;
    }

    protected function addNodeMapping(string $parserMethod): void
    {
        $this->nodesMapping[] = $parserMethod;
    }

    public function parse(Parser $parser): void
    {
        $shouldUseLexer = DoctrineOrm::isPre219();

        $this->customizeFunction();

        $parser->match($shouldUseLexer ? Lexer::T_IDENTIFIER : TokenType::T_IDENTIFIER);
        $parser->match($shouldUseLexer ? Lexer::T_OPEN_PARENTHESIS : TokenType::T_OPEN_PARENTHESIS);
        $this->feedParserWithNodes($parser);
        $parser->match($shouldUseLexer ? Lexer::T_CLOSE_PARENTHESIS : TokenType::T_CLOSE_PARENTHESIS);
    }

    protected function feedParserWithNodes(Parser $parser): void
    {
        $nodesMappingCount = \count($this->nodesMapping);
        $lastNode = $nodesMappingCount - 1;
        for ($i = 0; $i < $nodesMappingCount; $i++) {
            $parserMethod = $this->nodesMapping[$i];
            $this->nodes[$i] = $parser->{$parserMethod}();
            if ($i < $lastNode) {
                $parser->match(\class_exists(TokenType::class) ? TokenType::T_COMMA : Lexer::T_COMMA);
            }
        }
    }

    public function getSql(SqlWalker $sqlWalker): string
    {
        $dispatched = [];
        foreach ($this->nodes as $node) {
            $dispatched[] = $node instanceof Node ? $node->dispatch($sqlWalker) : 'null';
        }

        return \vsprintf($this->functionPrototype, $dispatched);
    }
}
  • Base class for all functions, providing basic parsing and SQL generation.
  • Uses parentheses by default for function calls.
  • feedParserWithNodes parses nodes according to a fixed mapping pattern.
  • customizeFunction() is empty by default but overridden in subclasses.

File: src/MartinGeorgiev/Utils/DoctrineOrm.php

Lines 12-18:

final class DoctrineOrm
{
    public static function isPre219(): bool
    {
        return !\class_exists(TokenType::class);
    }
}
  • Utility to detect Doctrine ORM version before 2.1.9 by checking existence of TokenType class.
  • Used in BaseVariadicFunction and BaseFunction to handle lexer differences.

File: src/MartinGeorgiev/Utils/DoctrineLexer.php

Lines 12-58 (summary):

final class DoctrineLexer
{
    public static function isPre200(Lexer $lexer): bool
    {
        return \is_array($lexer->lookahead);
    }

    public static function getLookaheadType(Lexer $lexer)
    {
        if (self::isPre200($lexer)) {
            return $lexer->lookahead['type'];
        }
        return $lexer->lookahead?->type;
    }

    public static function getTokenValue(Lexer $lexer)
    {
        if (self::isPre200($lexer)) {
            if ($lexer->token === null) {
                return null;
            }
            return $lexer->token['value'];
        }
        return $lexer->token?->value;
    }
}
  • Utility class to abstract differences in Doctrine Lexer versions before 2.0.0.
  • Provides methods to get lookahead token type and token value safely.

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php

Lines 7-56 (summary):

class InvalidArgumentForVariadicFunctionException extends \InvalidArgumentException
{
    public static function exactCount(string $functionName, int $expected): self { ... }
    public static function atLeast(string $functionName, int $min): self { ... }
    public static function between(string $functionName, int $min, int $max): self { ... }
    public static function evenNumber(string $functionName): self { ... }
    public static function unsupportedCombination(string $functionName, int $count, string $note): self { ... }
}
  • Exception class for invalid argument counts or combinations in variadic functions.
  • Provides static factory methods for different error messages related to argument count validation.

Summary of relevance:

  • The user comment refers to the Arr class having a different prototype and overriding accordingly.
  • The snippet from Arr.php shows that it overrides customizeFunction() to set a prototype with square brackets (ARRAY[%s]), differing from the default parentheses in BaseVariadicFunction.
  • The BaseVariadicFunction class sets the default prototype with parentheses and handles variadic argument parsing and validation.
  • The base class BaseFunction uses parentheses by default and a fixed node mapping pattern.
  • Utilities DoctrineOrm and DoctrineLexer handle version differences in Doctrine ORM and Lexer.
  • The exception class is used in BaseVariadicFunction to throw errors on invalid argument counts.

These snippets provide the necessary context to understand the difference in function prototype handling between BaseVariadicFunction and the Arr class, which overrides the prototype accordingly.


✏️ Learnings added
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:22-26
Timestamp: 2025-04-18T10:20:43.983Z
Learning: The `customizeFunction()` method in `BaseVariadicFunction` should not be marked as final because some child classes like `Arr` need to override it to use different SQL syntax patterns (e.g., square brackets instead of parentheses).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

abstract protected function getFunctionName(): string;

/**
* @return array<int|string, string>
* @return array<string>
*/
abstract protected function getNodeMappingPattern(): array;

/**
* @throws ParserException
*/
abstract protected function getMinArgumentCount(): int;

abstract protected function getMaxArgumentCount(): int;

protected function feedParserWithNodes(Parser $parser): void
{
foreach ($this->getNodeMappingPattern() as $nodeMappingPattern) {
Expand All @@ -38,22 +46,24 @@ protected function feedParserWithNodes(Parser $parser): void
// swallow and continue with next pattern
}
}

$this->validateArguments(...$this->nodes); // @phpstan-ignore-line
}

/**
* @throws InvalidArgumentForVariadicFunctionException
* @throws ParserException
*/
private function feedParserWithNodesForNodeMappingPattern(Parser $parser, string $nodeMappingPattern): void
{
$nodeMapping = \explode(',', $nodeMappingPattern);
$lexer = $parser->getLexer();

try {
// @phpstan-ignore-next-line
$this->nodes[] = $parser->{$nodeMapping[0]}();
$lookaheadType = DoctrineLexer::getLookaheadType($lexer);
if ($lookaheadType === null) {
throw ParserException::missingLookaheadType();
throw InvalidArgumentForVariadicFunctionException::atLeast($this->getFunctionName(), $this->getMinArgumentCount());
}

$this->nodes[] = $parser->{$nodeMapping[0]}(); // @phpstan-ignore-line
} catch (\Throwable $throwable) {
throw ParserException::withThrowable($throwable);
}
Expand All @@ -64,13 +74,53 @@ private function feedParserWithNodesForNodeMappingPattern(Parser $parser, string
while (($shouldUseLexer ? Lexer::T_CLOSE_PARENTHESIS : TokenType::T_CLOSE_PARENTHESIS) !== $lookaheadType) {
if (($shouldUseLexer ? Lexer::T_COMMA : TokenType::T_COMMA) === $lookaheadType) {
$parser->match($shouldUseLexer ? Lexer::T_COMMA : TokenType::T_COMMA);
// @phpstan-ignore-next-line
$this->nodes[] = $parser->{$nodeMapping[$isNodeMappingASimplePattern ? 0 : $nodeIndex]}();

// Check if we're about to exceed the maximum number of arguments
// nodeIndex starts at 1 and counts up for each argument after the first
// So when nodeIndex=1, we're about to add the 2nd argument (total: 2)
// When nodeIndex=2, we're about to add the 3rd argument (total: 3)
$foundMoreNodesThanMappingExpected = ($nodeIndex + 1) > $this->getMaxArgumentCount();
if ($foundMoreNodesThanMappingExpected) {
throw InvalidArgumentForVariadicFunctionException::between($this->getFunctionName(), $this->getMinArgumentCount(), $this->getMaxArgumentCount());
}

$expectedNodeIndex = $isNodeMappingASimplePattern ? 0 : $nodeIndex;
$argumentCountExceedsMappingPatternExpectation = !\array_key_exists($expectedNodeIndex, $nodeMapping);
if ($argumentCountExceedsMappingPatternExpectation) {
throw InvalidArgumentForVariadicFunctionException::unsupportedCombination(
$this->getFunctionName(),
\count($this->nodes) + 1,
'implementation defines fewer node mappings than the actually provided argument count'
);
}

$this->nodes[] = $parser->{$nodeMapping[$expectedNodeIndex]}(); // @phpstan-ignore-line
$nodeIndex++;
}

$lookaheadType = DoctrineLexer::getLookaheadType($lexer);
}

// Final validation ensures all arguments meet requirements, including any special rules in subclass implementations
$this->validateArguments(...$this->nodes); // @phpstan-ignore-line
}

/**
* @throws InvalidArgumentForVariadicFunctionException
*/
protected function validateArguments(Node ...$arguments): void
{
$minArgumentCount = $this->getMinArgumentCount();
$maxArgumentCount = $this->getMaxArgumentCount();
$argumentCount = \count($arguments);

if ($argumentCount < $minArgumentCount) {
throw InvalidArgumentForVariadicFunctionException::atLeast($this->getFunctionName(), $this->getMinArgumentCount());
}

if ($argumentCount > $maxArgumentCount) {
throw InvalidArgumentForVariadicFunctionException::between($this->getFunctionName(), $this->getMinArgumentCount(), $this->getMaxArgumentCount());
}
}

public function getSql(SqlWalker $sqlWalker): string
Expand All @@ -82,11 +132,4 @@ public function getSql(SqlWalker $sqlWalker): string

return \sprintf($this->functionPrototype, \implode(', ', $dispatched));
}

/**
* Validates the arguments passed to the function.
*
* @throws InvalidArgumentForVariadicFunctionException
*/
abstract protected function validateArguments(Node ...$arguments): void;
}
24 changes: 15 additions & 9 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

use Doctrine\ORM\Query\AST\Node;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\TimezoneValidationTrait;

/**
Expand All @@ -30,21 +29,28 @@ protected function getNodeMappingPattern(): array
return ['StringPrimary'];
}

protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('date_add(%s)');
return 'date_add';
}

protected function getMinArgumentCount(): int
{
return 2;
}

protected function getMaxArgumentCount(): int
{
return 3;
}

protected function validateArguments(Node ...$arguments): void
{
$argumentCount = \count($arguments);
if ($argumentCount < 2 || $argumentCount > 3) {
throw InvalidArgumentForVariadicFunctionException::between('date_add', 2, 3);
}
parent::validateArguments(...$arguments);

// Validate that the third parameter is a valid timezone if provided
if ($argumentCount === 3) {
$this->validateTimezone($arguments[2], 'DATE_ADD');
if (\count($arguments) === 3) {
$this->validateTimezone($arguments[2], $this->getFunctionName());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

use Doctrine\ORM\Query\AST\Node;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\TimezoneValidationTrait;

/**
Expand All @@ -30,21 +29,28 @@ protected function getNodeMappingPattern(): array
return ['StringPrimary'];
}

protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('date_subtract(%s)');
return 'date_subtract';
}

protected function getMinArgumentCount(): int
{
return 2;
}

protected function getMaxArgumentCount(): int
{
return 3;
}

protected function validateArguments(Node ...$arguments): void
{
$argumentCount = \count($arguments);
if ($argumentCount < 2 || $argumentCount > 3) {
throw InvalidArgumentForVariadicFunctionException::between('date_subtract', 2, 3);
}
parent::validateArguments(...$arguments);

// Validate that the third parameter is a valid timezone if provided
if ($argumentCount === 3) {
$this->validateTimezone($arguments[2], 'DATE_SUBTRACT');
if (\count($arguments) === 3) {
$this->validateTimezone($arguments[2], $this->getFunctionName());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,14 @@ public static function evenNumber(string $functionName): self
$functionName
));
}

public static function unsupportedCombination(string $functionName, int $count, string $note): self
{
return new self(\sprintf(
'%s() cannot be called with %d arguments, because %s',
$functionName,
$count,
$note,
));
}
}
15 changes: 2 additions & 13 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Greatest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@

namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

use Doctrine\ORM\Query\AST\Node;
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException;

/**
* Implementation of PostgreSQL GREATEST().
*
Expand All @@ -17,16 +14,8 @@
*/
class Greatest extends BaseComparisonFunction
{
protected function customizeFunction(): void
{
$this->setFunctionPrototype('greatest(%s)');
}

protected function validateArguments(Node ...$arguments): void
protected function getFunctionName(): string
{
$argumentCount = \count($arguments);
if ($argumentCount < 2) {
throw InvalidArgumentForVariadicFunctionException::atLeast('greatest', 2);
}
return 'greatest';
}
}
Loading
Loading