refactor: allow multiple node mapping patterns to be used and their arguments to be validated in variadic functions #350
Conversation
…rguments to be validated in variadic functions
WalkthroughThis set of changes refactors the implementation of custom Doctrine AST function classes to centralize and standardize argument count validation and function metadata. Instead of inline validation and prototype string construction, each function class now overrides methods to specify its name, minimum, and maximum argument counts. The base classes ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DoctrineQuery
participant ASTFunctionClass
participant BaseFunction
participant Exception
User->>DoctrineQuery: Executes DQL with custom function
DoctrineQuery->>ASTFunctionClass: Parses function call
ASTFunctionClass->>BaseFunction: Calls getFunctionName(), getMinArgumentCount(), getMaxArgumentCount()
BaseFunction->>ASTFunctionClass: Validates argument count
alt Invalid argument count
BaseFunction->>Exception: Throws InvalidArgumentForVariadicFunctionException
else Valid arguments
ASTFunctionClass->>DoctrineQuery: Returns parsed function node
end
Possibly related PRs
Suggested labels
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)🧬 Code Graph Analysis (1)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (6)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php (1)
38-47: 💡 Verification agent❓ Verification inconclusive
Test coverage for GREATEST function appears sufficient.
The test covers key use cases for PostgreSQL's GREATEST function including multiple column values, mixed expressions, and error handling for insufficient arguments.
According to PostgreSQL documentation, GREATEST selects the largest value from a list of any comparable expressions, which these tests demonstrate well.
🏁 Script executed:
#!/bin/bash # Check if the tests cover NULL handling for GREATEST as mentioned in PostgreSQL docs rg -A 3 "NULL.*GREATEST|GREATEST.*NULL" --type php tests/Length of output: 59
Add NULL‐handling test for GREATEST
The existing tests cover multiple values, mixed expressions, and insufficient‐argument errors, but they don’t verify how GREATEST behaves when any input is NULL. According to PostgreSQL, GREATEST should return NULL if any argument is NULL.
Please add a test in tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php, for example:
- A new method
public function returns_null_when_any_argument_is_null(): void- Asserting that
SELECT GREATEST(e.integer1, NULL)produces a NULL result or the appropriate SQL.
🧹 Nitpick comments (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php (1)
32-35: Note case difference between documentation and implementationThe documentation comment mentions "DATE_ADD()" (uppercase), but the implementation returns "date_add" (lowercase). This might be intentional if PostgreSQL functions are case-insensitive, but it's worth ensuring consistency in documentation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Arr.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseComparisonFunction.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php(4 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtract.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Greatest.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObject.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObject.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExists.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatch.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQuery.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArray.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirst.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Least.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Row.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsquery.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvector.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Unaccent.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAddTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtractTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExistsTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatchTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php`: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL que...
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL queries for the tested SQL functions and operators.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExistsTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAddTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatchTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtractTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php
🧠 Learnings (3)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
🧬 Code Graph Analysis (8)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.php (1)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TestCase.php (1)
TestCase(15-120)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php (1)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TestCase.php (1)
TestCase(15-120)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Least.php (16)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Arr.php (1)
getFunctionName(22-25)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (1)
getFunctionName(33-36)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtract.php (1)
getFunctionName(32-35)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php (1)
getFunctionName(32-35)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Greatest.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObject.php (1)
getFunctionName(25-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php (1)
getFunctionName(33-36)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObject.php (1)
getFunctionName(25-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
getFunctionName(27-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExists.php (1)
getFunctionName(31-34)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (1)
getFunctionName(37-40)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Row.php (1)
getFunctionName(19-22)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsquery.php (1)
getFunctionName(22-25)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (1)
getFunctionName(32-35)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvector.php (1)
getFunctionName(22-25)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Unaccent.php (1)
getFunctionName(21-24)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (4)
getFunctionName(27-27)getMinArgumentCount(34-34)getMaxArgumentCount(36-36)validateArguments(101-114)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (4)
getFunctionName(32-35)getMinArgumentCount(37-40)getMaxArgumentCount(42-45)validateArguments(47-55)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
validateBoolean(25-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExists.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (4)
getFunctionName(27-27)getMinArgumentCount(34-34)getMaxArgumentCount(36-36)validateArguments(101-114)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatch.php (4)
getFunctionName(33-36)getMinArgumentCount(38-41)getMaxArgumentCount(43-46)validateArguments(48-56)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
validateBoolean(25-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArray.php (7)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (4)
getFunctionName(27-27)getMinArgumentCount(34-34)getMaxArgumentCount(36-36)validateArguments(101-114)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExists.php (4)
getFunctionName(31-34)getMinArgumentCount(36-39)getMaxArgumentCount(41-44)validateArguments(46-54)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatch.php (4)
getFunctionName(33-36)getMinArgumentCount(38-41)getMaxArgumentCount(43-46)validateArguments(48-56)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQuery.php (4)
getFunctionName(32-35)getMinArgumentCount(37-40)getMaxArgumentCount(42-45)validateArguments(47-55)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirst.php (4)
getFunctionName(32-35)getMinArgumentCount(37-40)getMaxArgumentCount(42-45)validateArguments(47-55)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (4)
getFunctionName(37-40)getMinArgumentCount(42-45)getMaxArgumentCount(47-50)validateArguments(52-60)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
validateBoolean(25-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Unaccent.php (12)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (3)
getFunctionName(33-36)getMinArgumentCount(38-41)getMaxArgumentCount(43-46)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtract.php (3)
getFunctionName(32-35)getMinArgumentCount(37-40)getMaxArgumentCount(42-45)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php (3)
getFunctionName(32-35)getMinArgumentCount(37-40)getMaxArgumentCount(42-45)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Greatest.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php (3)
getFunctionName(33-36)getMinArgumentCount(38-41)getMaxArgumentCount(43-46)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (3)
getFunctionName(27-27)getMinArgumentCount(34-34)getMaxArgumentCount(36-36)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Least.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (3)
getFunctionName(37-40)getMinArgumentCount(42-45)getMaxArgumentCount(47-50)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsquery.php (3)
getFunctionName(22-25)getMinArgumentCount(27-30)getMaxArgumentCount(32-35)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (3)
getFunctionName(32-35)getMinArgumentCount(37-40)getMaxArgumentCount(42-45)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvector.php (3)
getFunctionName(22-25)getMinArgumentCount(27-30)getMaxArgumentCount(32-35)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatch.php (2)
getMinArgumentCount(38-41)getMaxArgumentCount(43-46)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (4)
getFunctionName(33-36)getMinArgumentCount(38-41)getMaxArgumentCount(43-46)validateArguments(48-56)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (4)
getFunctionName(27-27)getMinArgumentCount(34-34)getMaxArgumentCount(36-36)validateArguments(101-114)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
validateBoolean(25-37)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: sloth
🔇 Additional comments (92)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
101-113: Minor duplication – maximum‑argument check happens twice
$foundMoreNodesThanMappingExpected(lines 78‑85) already prevents the parser from ever collecting more than the maximum arguments, whilevalidateArguments()(lines 107‑113) performs the same> $maxArgumentCountcheck afterwards.
Although harmless, the second check is redundant and can be removed to streamline the hot‑path.[ suggest_optional_refactor ]
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php (1)
40-41: Test expectation updated correctlyThe message now matches the new lowercase‑function convention introduced in the refactor. ✅
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php (1)
42-43: Test expectation updated correctlyConsistent with the new error‑message format. ✅
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php (3)
44-44: Exception message updated to use lowercase function name.The exception message has been standardized to use the lowercase version of the function name (
jsonb_path_queryinstead ofJSONB_PATH_QUERY), which is consistent with PostgreSQL's own documentation and error messages.
53-53: Exception message updated for argument count validation.The exception message now uses "at least 2 arguments" instead of a fixed range, which better reflects the PostgreSQL function signature. According to PostgreSQL documentation,
jsonb_path_queryaccepts 2-4 arguments:jsonb_path_query(target jsonb, path jsonpath [, vars jsonb [, silent boolean]]).
23-28: Test cases cover all variations of the PostgreSQL function.These test cases provide excellent coverage of all possible argument variations for the
jsonb_path_queryfunction, including the basic form, with vars argument, and with both vars and silent arguments. This aligns well with the PostgreSQL documentation examples.tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatchTest.php (3)
44-44: Exception message updated to use lowercase function name.The exception message has been standardized to use the lowercase version of the function name (
jsonb_path_matchinstead ofJSONB_PATH_MATCH), which is consistent with PostgreSQL's own documentation and error messages.
53-53: Exception message updated for argument count validation.The exception message now uses "at least 2 arguments" instead of a fixed range, which better reflects the PostgreSQL function signature. According to PostgreSQL documentation,
jsonb_path_matchaccepts 2-4 arguments:jsonb_path_match(target jsonb, path jsonpath [, vars jsonb [, silent boolean]]).
24-27: Test cases demonstrate various path matching patterns.These test cases provide excellent coverage of different jsonb path matching patterns, including exists conditions, simple conditions, and various combinations with the optional parameters. This aligns well with real-world PostgreSQL usage examples.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (3)
40-40: Exception message updated to use lowercase function name.The exception message has been standardized to use the lowercase version of the function name (
jsonb_setinstead ofJSONB_SET), which is consistent with PostgreSQL's own documentation and error messages.
49-49: Exception message updated for argument count validation.The exception message now uses "at least 3 arguments" instead of a fixed range, which better reflects the PostgreSQL function signature. According to PostgreSQL documentation,
jsonb_setrequires at least 3 arguments:jsonb_set(target jsonb, path text[], new_value jsonb [, create_missing boolean]).
24-25: Test cases cover both basic and optional parameter scenarios.The test cases demonstrate both the basic usage with 3 arguments and the extended usage with the optional create-if-missing parameter. This covers the full spectrum of functionality for this PostgreSQL function.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (3)
40-40: Exception message updated to use lowercase function name.The exception message has been standardized to use the lowercase version of the function name (
jsonb_insertinstead ofJSONB_INSERT), which is consistent with PostgreSQL's own documentation and error messages.
49-49: Exception message updated for argument count validation.The exception message now uses "at least 3 arguments" instead of a fixed range, which better reflects the PostgreSQL function signature. According to PostgreSQL documentation,
jsonb_insertrequires at least 3 arguments:jsonb_insert(target jsonb, path text[], new_value jsonb [, insert_after boolean]).
24-25: Test cases cover both basic and optional parameter scenarios.The test cases demonstrate both the basic usage with 3 arguments and the extended usage with the optional insert_after parameter. This provides comprehensive coverage for the PostgreSQL function's capabilities.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php (3)
44-44: Test expectation updated to match new error message formatThe exception message now references the lowercase function name (
jsonb_path_query_firstinstead ofJSONB_PATH_QUERY_FIRST), which aligns with the refactoring to standardize error handling and use consistent function naming across the codebase.
53-53: Test expectation updated to match new error message formatThe exception message now states "requires at least 2 arguments" rather than "between 2 and 4 arguments", which aligns with the refactoring to centralize argument validation logic and improve error clarity.
21-39: Confirm test cases against PostgreSQL documentationThe test cases cover the essential use cases for jsonb_path_query_first:
- Basic path extraction with a condition
- Simple path access
- With vars argument
- With both vars and silent parameters
According to the PostgreSQL documentation, this comprehensive coverage addresses the main functionality of the jsonb_path_query_first function, which retrieves the first item from the JSON path query.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php (3)
44-44: Test expectation updated to match new error message formatThe exception message now references the lowercase function name (
jsonb_path_query_arrayinstead ofJSONB_PATH_QUERY_ARRAY), which aligns with the refactoring to standardize error handling and use consistent function naming across the codebase.
53-53: Test expectation updated to match new error message formatThe exception message now states "requires at least 2 arguments" rather than "between 2 and 4 arguments", which aligns with the refactoring to centralize argument validation logic and improve error clarity.
21-39: Confirm test cases against PostgreSQL documentationThe test cases cover the essential use cases for jsonb_path_query_array:
- Array extraction with a condition
- Path-based array extraction
- With vars argument
- With both vars and silent parameters
According to the PostgreSQL documentation, this comprehensive coverage addresses the main functionality of the jsonb_path_query_array function, which retrieves all JSON items from a path query and returns them as a JSON array.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.php (2)
11-11: Class inheritance updated to reflect refactored architectureThe test class now extends
TestCasedirectly instead ofBaseComparisonFunctionTestCase, which aligns with the refactoring to centralize validation logic in base classes. This change reflects the simplified structure where function classes no longer need complex inheritance hierarchies for argument validation.
20-36: Confirm test cases against PostgreSQL documentationThe test cases cover the essential use cases for the PostgreSQL LEAST function:
- Comparing multiple column values
- Comparing a column value with an expression
- Comparing multiple expressions
According to the PostgreSQL documentation, the LEAST function returns the smallest value from a list of any number of expressions, which these test cases appropriately verify.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtractTest.php (3)
42-42: Test expectation updated to match new error message formatThe exception message now references the lowercase function name (
date_subtractinstead ofDATE_SUBTRACT), which aligns with the refactoring to standardize error handling and use consistent function naming across the codebase.
51-51: Test expectation updated to match new error message formatThe exception message now states "requires at least 2 arguments" rather than "between 2 and 3 arguments", which aligns with the refactoring to centralize argument validation logic and improve error clarity.
21-37: Verify test cases cover essential date_subtract functionalityThe test cases cover the main variants of date_subtract usage:
- Subtracting days with a timezone
- Subtracting hours with a timezone
- Subtracting days without a timezone
These cases verify the function's ability to handle different time units and optional timezone handling, providing good coverage of the function's intended functionality.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php (1)
11-11: Base class change is aligned with the centralized validation refactoring.The change from extending a specific base test class to the more general
TestCaseclass is appropriate since the specialized fixture creation method is no longer needed after the refactoring of the underlyingGreatestfunction class.tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExistsTest.php (3)
44-44: Error message updated to use lowercase function name.This change aligns with the standardization of function names in error messages to lowercase format, which is consistent with the PostgreSQL documentation and makes error messages more uniform across the library.
53-53: Error message wording improved for argument count validation.The message now states "requires at least 2 arguments" which is more precise than the previous wording, aligning with the centralized validation approach and making the error more helpful to users.
21-39: Test coverage for jsonb_path_exists looks comprehensive.The tests cover the main use cases described in PostgreSQL documentation for jsonb_path_exists, including path expressions with conditions, nested paths, and the optional vars and silent arguments.
According to PostgreSQL documentation, jsonb_path_exists tests whether a JSON path expression returns any item for the specified JSON value, which these tests verify with various scenarios.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAddTest.php (3)
44-44: Error message updated to use lowercase function name.This change aligns with the standardization of function names in error messages to lowercase format, consistent with PostgreSQL documentation and providing more uniform error messages across the library.
53-53: Error message wording improved for argument count validation.The message now states "requires at least 2 arguments" which is more precise than the previous wording, aligning with the centralized validation approach in the base classes.
21-39: Test coverage for date_add appears comprehensive.The tests cover the main use cases described in PostgreSQL documentation for date_add, including various interval types and the optional timezone parameter. The tests also verify the function's behavior in WHERE clauses.
According to PostgreSQL documentation, date_add adds an interval to a timestamp, which these tests verify with days, hours, and timezone variations.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
47-55: Good addition of factory method for more specific error messaging.The new
unsupportedCombinationfactory method enhances error reporting by allowing more specific explanations of why certain argument combinations are not supported, beyond just count validation.This method follows the consistent pattern of the existing factory methods and provides a clear, formatted error message to help developers understand what went wrong.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Least.php (1)
17-20: Implementation looks goodThe refactored code properly follows the new pattern of implementing
getFunctionName()to return the SQL function name. This approach centralizes function metadata and argument validation in the parent class, which reduces code duplication across function implementations.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Greatest.php (1)
17-20: Implementation looks goodThe refactored code properly follows the new pattern of implementing
getFunctionName()to return the SQL function name. This approach centralizes function metadata and argument validation in the parent class, which reduces code duplication across function implementations.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php (2)
33-46: Implementation follows the new pattern correctlyGood implementation of the new methods
getFunctionName(),getMinArgumentCount(), andgetMaxArgumentCount()to explicitly define function metadata. This declarative approach makes the constraints more visible and maintainable than the previous approach.
48-56: Good refactoring of validation logicThe validation logic has been improved by:
- Delegating basic argument count validation to the parent class
- Adding specific validation for the boolean parameter
- Using the dynamic function name instead of hardcoded strings
This makes the code more maintainable and consistent with other implementations.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObject.php (2)
25-38: Good implementation with helpful commentsThe implementation of
getFunctionName(),getMinArgumentCount(), andgetMaxArgumentCount()is clear and includes helpful comments explaining the constraints (at least one key-value pair, no upper limit but must be even). This makes the code more self-documenting.
40-47: Validation logic looks goodThe validation logic properly delegates to the parent class first, then adds the specific validation that the argument count must be even. Using
$this->getFunctionName()for the exception message ensures consistency if the function name changes in the future.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObject.php (2)
25-38: Well-structured function metadata definitionThe implementation nicely separates function metadata into dedicated methods:
getFunctionName()provides the PostgreSQL function namegetMinArgumentCount()enforces at least one key-value pair (2 arguments)getMaxArgumentCount()allows unlimited arguments but with the comment indicating they must be evenThis approach makes the class more maintainable and follows the single responsibility principle.
40-46: Good validation refactoringExcellent improvements:
- Delegating basic argument count validation to the parent class
- Adding specific validation for even number of arguments
- Using
$this->getFunctionName()in the exception for consistent error messagesThis change reduces code duplication while maintaining the specific validation needs of this function.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (2)
33-46: Clean function metadata implementationThe implementation provides clear function metadata:
- Function name is explicitly defined as 'array_to_json'
- Minimum argument count is set to 1 (required array parameter)
- Maximum argument count is set to 2 (optional pretty_bool parameter)
This approach makes the intent of the function clearer and follows the overall refactoring pattern.
48-55: Improved validation approachGood refactoring of the validation logic:
- Delegating basic argument count validation to the parent class
- Maintaining specific boolean validation for the second parameter
- Using
$this->getFunctionName()for dynamic error messagesThis creates a consistent pattern across the codebase while preserving function-specific validation.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirst.php (2)
32-45: Clear function metadata definitionGood implementation of the metadata methods:
getFunctionName()returns the correct PostgreSQL function namegetMinArgumentCount()properly sets the minimum required arguments to 2getMaxArgumentCount()correctly allows up to 4 arguments as per PostgreSQL documentationThis structure makes it easier to understand the function's requirements at a glance.
47-54: Well-refactored validation logicThe validation logic is properly refactored:
- Delegating basic argument count validation to the parent class
- Focusing only on the specific boolean validation for the fourth parameter
- Using dynamic function name retrieval for error messages
This maintains the special validation requirements while eliminating duplicate code.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseComparisonFunction.php (2)
19-22: Good implementation of customizeFunctionThe implementation dynamically sets the function prototype using the function name from the derived class, which creates a consistent pattern and reduces the need for each subclass to implement this method.
24-32: Appropriate argument constraints for comparison functionsThe class properly defines:
- Minimum argument count of 2, as comparison functions need at least two values to compare
- No maximum limit, allowing for flexible comparison operations
This provides reasonable defaults for all derived comparison functions while still allowing them to override these constraints if needed.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQuery.php (3)
32-35: Consistent implementation of function name method.The implementation correctly returns the lowercase function name, which aligns with PostgreSQL's function naming conventions and ensures consistent error messages.
37-45: Well-defined argument boundaries.The implementation properly defines the minimum and maximum argument counts for jsonb_path_query, correctly documenting that this function requires at least 2 arguments but can accept up to 4 optional parameters.
47-55: Improved argument validation with better delegation to parent class.The refactored validation logic now properly delegates the basic argument count check to the parent class's implementation, while maintaining the specific boolean validation for the fourth parameter when present. Using
getFunctionName()instead of a hardcoded string improves maintainability.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsquery.php (1)
22-35: Clean implementation of function metadata methods.The implementation correctly defines the function name and argument constraints for the PostgreSQL
to_tsqueryfunction. This refactoring simplifies the code by removing redundant validation logic while maintaining proper function definition.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatch.php (2)
33-46: Well-defined function metadata methods.The implementation correctly defines the function name and argument constraints for the PostgreSQL
jsonb_path_matchfunction, matching its documented behavior in PostgreSQL 14.
48-56: Improved argument validation pattern.The validation logic now properly delegates basic argument count validation to the parent class while maintaining specific boolean validation for the fourth parameter. Using
getFunctionName()ensures consistency in error messages and improves maintainability.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (2)
32-45: Consistent implementation of function metadata methods.The implementation correctly defines the function name and argument constraints for the PostgreSQL
row_to_jsonfunction, enforcing that it requires at least 1 argument but can accept up to 2. This matches PostgreSQL's documentation.
47-55: Improved argument validation with better error handling.The refactored validation logic properly delegates the basic argument count checking to the parent class while maintaining the specific boolean validation for the second parameter. Using
getFunctionName()dynamically ensures consistency in error messages and improves maintainability.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (4)
37-40: Well-implemented function name methodThe new
getFunctionName()method provides a clean way to access the function name, which makes the code more maintainable by centralizing this information.
42-45: Correctly defined minimum argument countThe
getMinArgumentCount()method properly defines the minimum number of arguments (3) required for the jsonb_set function, which aligns with PostgreSQL's requirements.
47-50: Correctly defined maximum argument countThe
getMaxArgumentCount()method properly defines the maximum number of arguments (4) allowed for the jsonb_set function, which aligns with PostgreSQL's implementation.
52-60: Good refactoring of argument validationThe validation is now simplified by delegating to the parent class while maintaining the specialized boolean validation for the fourth parameter. Using
$this->getFunctionName()instead of hardcoded strings is a good practice that reduces duplication and potential errors.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtract.php (4)
32-35: Well-implemented function name methodThe
getFunctionName()method properly returns the PostgreSQL function name 'date_subtract', centralizing this information for better maintainability.
37-40: Correctly defined minimum argument countThe
getMinArgumentCount()method properly defines the minimum number of arguments (2) required for the date_subtract function.
42-45: Correctly defined maximum argument countThe
getMaxArgumentCount()method properly defines the maximum number of arguments (3) allowed for the date_subtract function.
47-55: Good refactoring of argument validationThe validation logic has been improved by delegating to the parent class while maintaining the specialized timezone validation for the third parameter. Using
$this->getFunctionName()instead of a hardcoded string enhances maintainability and reduces duplication.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvector.php (3)
22-26: Well-implemented function name methodThe
getFunctionName()method correctly returns the PostgreSQL function name 'to_tsvector', following the same pattern as other AST function classes.
27-30: Correctly defined minimum argument countThe
getMinArgumentCount()method properly defines the minimum number of arguments (1) required for the to_tsvector function.
32-35: Correctly defined maximum argument countThe
getMaxArgumentCount()method properly defines the maximum number of arguments (2) allowed for the to_tsvector function, which aligns with PostgreSQL's implementation.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Unaccent.php (3)
21-24: Well-implemented function name methodThe
getFunctionName()method correctly returns the PostgreSQL function name 'unaccent', following the consistent pattern used across other AST function classes.
26-29: Correctly defined minimum argument countThe
getMinArgumentCount()method properly defines the minimum number of arguments (1) required for the unaccent function.
31-34: Correctly defined maximum argument countThe
getMaxArgumentCount()method properly defines the maximum number of arguments (2) allowed for the unaccent function, which aligns with PostgreSQL's implementation.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExists.php (4)
31-34: Function name definition improves consistencyThe implementation of
getFunctionName()aligns with the PostgreSQL function naming convention and creates consistency across the codebase by moving from hard-coded strings to dedicated methods.
36-39: Minimum argument count properly definedSetting the minimum argument count to 2 is correct according to PostgreSQL documentation - the function requires at minimum a target JSON document and a path expression.
41-44: Maximum argument count correctly constrainedThe maximum of 4 arguments aligns with PostgreSQL's implementation, which can accept optional vars and silent flags in addition to the required parameters.
48-53:❓ Verification inconclusive
Improved validation approach
Delegating to the parent class for basic argument validation while maintaining specific boolean validation for the fourth parameter (silent flag) is a good approach. Using
getFunctionName()instead of a hardcoded string improves maintainability.
🏁 Script executed:
#!/bin/bash # Check PostgreSQL documentation for jsonb_path_exists function ast-grep --pattern '$jsonb_path_exists($_, $_, $_, $_)'Length of output: 57
Improved validation for optional silent flag
Delegating to
parent::validateArgumentsfor basic arity checks while explicitly validating the fourth parameter as a boolean aligns with PostgreSQL’sjsonb_path_existssignature (which accepts an optional silent flag). UsinggetFunctionName()instead of a hardcoded string also enhances maintainability.Please manually verify:
- That
parent::validateArgumentscorrectly permits exactly 3 or 4 arguments for this function.- That PostgreSQL’s
jsonb_path_existsindeed accepts an optional boolean as its fourth parameter.tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php (2)
14-17: Good abstract test case designThe abstract test case provides a solid foundation for testing variadic function implementations. The abstract
createFixture()method ensures that subclasses provide their specific function implementation.
18-41: Comprehensive test for parser behaviorThis test properly verifies that variadic functions throw the appropriate exception when the lexer isn't properly configured. The test thoroughly sets up the mock environment and tests the internal
feedParserWithNodesForNodeMappingPatternmethod.Note that using reflection to access private/protected methods is appropriate in test cases.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArray.php (4)
31-34: Function name properly definedThe implementation of
getFunctionName()correctly returns the PostgreSQL function name, matching the documented function.
36-39: Minimum argument count correctly definedSetting the minimum argument count to 2 matches PostgreSQL's requirements - the function requires at least a target JSON document and a path expression.
41-44: Maximum argument count correctly constrainedThe maximum of 4 arguments aligns with PostgreSQL's implementation which supports 4 parameters: JSON data, path expression, variables, and silent flag.
48-53:✅ Verification successful
Improved validation approach
Delegating to the parent class for basic argument validation while maintaining specific boolean validation for the fourth parameter is consistent with other similar functions. Using
getFunctionName()improves maintainability.
🌐 Web query:
What are the parameters for PostgreSQL's jsonb_path_query_array function?💡 Result:
Parameters for PostgreSQL's
jsonb_path_query_arrayFunctionThe
jsonb_path_query_arrayfunction in PostgreSQL is used to extract all JSON items matching a specified JSONPath expression from a JSONB value, returning the results as a JSON array. Its parameters are as follows:
Parameter Type Required Description target JSONB Yes The input JSONB value to be queried. path JSONPath Yes The JSONPath expression that specifies which items to extract from the JSONB document. vars JSONB No A JSON object providing named variables to be substituted into the JSONPath expression. silent Boolean No If true, suppresses certain errors (such as missing keys or type mismatches) during parsing.Syntax
jsonb_path_query_array( target JSONB, path JSONPATH [, vars JSONB [, silent BOOLEAN]] ) RETURNS JSONBDetails
- target: The JSONB document you want to search in.
- path: The JSONPath string to apply for extraction. It follows SQL/JSON path syntax.
- vars: (Optional) Used to provide variables for the path expression, allowing dynamic queries. Must be a JSON object.
- silent: (Optional) When set to
true, certain errors are suppressed and the function returns an empty array instead of failing[5][4][7].Usage Example
SELECT jsonb_path_query_array( '{"employees":[{"name":"John","age":30},{"name":"Alice","age":35},{"name":"Bob","age":25}]}', '$.employees[*] ? (@.age >= $min && @.age <= $max)', '{"min": 24, "max": 32}' );Result:
[{"age": 30, "name": "John"}, {"age": 25, "name": "Bob"}][4][5][7]
Notes
- If any argument is
NULL, the function returnsNULL.- If no matches are found, the function returns an empty JSON array.
- The function is available in PostgreSQL 12 and later[6].
This function is a powerful tool for querying and extracting multiple objects or values from complex JSON structures directly within PostgreSQL.
Citations:
- 1: https://neon.tech/postgresql/postgresql-json-functions/postgresql-jsonb_path_query_array
- 2: https://www.postgresql.org/docs/current/functions-json.html
- 3: https://neon.tech/postgresql/postgresql-json-functions/postgresql-jsonb_path_query
- 4: https://docs.risingwave.com/sql/functions/json
- 5: https://www.sqliz.com/postgresql-ref/jsonb_path_query_array/
- 6: https://hevodata.com/learn/query-jsonb-array-of-objects/
- 7: https://docs.arenadata.io/en/ADPG/current/how-to/queries/use-complex-types/json-path.html
- 8: https://www.dbvis.com/thetable/postgresql-jsonpath/
- 9: https://postgrespro.com/docs/postgresql/13/functions-json
Approve boolean validation for the fourth parameter
The fourth “silent” argument injsonb_path_query_array(target, path, vars, silent)is indeed optional and Boolean. Delegating core checks toparent::validateArguments()and then specifically validating the fourth argument withvalidateBoolean($arguments[3], $this->getFunctionName())is correct and maintainable.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Row.php (3)
19-22: Function name correctly definedThe implementation of
getFunctionName()appropriately returns 'ROW', which matches PostgreSQL's row constructor syntax.
24-27: Minimum argument count correctly definedSetting the minimum argument count to 1 is correct for row constructors, which need at least one element.
29-32:✅ Verification successful
Maximum argument count properly set to unlimited
Using
PHP_INT_MAXis an appropriate way to indicate no upper limit on arguments, which aligns with PostgreSQL's row constructor that can accept any number of elements.
🌐 Web query:
What is the syntax for PostgreSQL row constructors and how many arguments can they accept?💡 Result:
PostgreSQL Row Constructor Syntax
A PostgreSQL row constructor is an expression that builds a composite value (a row) from a list of field values. The general syntax is:
ROW(value1, value2, ..., valueN)
- The
ROWkeyword is optional if there is more than one field in the expression, so you can also just use parentheses for tuples with two or more values:(value1, value2, ..., valueN)- Each
valueis an expression, and types can differ, as long as their types match the context (table columns, composite types, function parameters, etc.)[1][2][7][10].Examples:
SELECT ROW(1, 'foo', 3.14);SELECT (1, 'foo', 3.14);- For table comparisons or WHERE clauses:
SELECT * FROM table_name WHERE (col1, col2) = (val1, val2); ```[2]Argument Limits for Row Constructors
There is no explicit, documented hard limit to the number of arguments (fields) a row constructor can accept in PostgreSQL. The number of expressions is primarily constrained by:
- The maximum number of columns in a table (1600 columns as of recent PostgreSQL versions, though composite types and anonymous records follow similar limits)[1][2].
- Practical limits such as parser memory and command length for extremely wide rows.
For typical use—defining or manipulating single rows—the number of fields in a row constructor should match the number of columns or attributes in the corresponding table or composite type. There is no arbitrary small cap (like 1000), as might be seen in other systems' bulk insert clauses[1][2].
Advanced Usage Notes
- You can expand existing rows into new constructors with the
.*syntax (e.g.,ROW(t.*, 'A')expands all fields of rowtand adds'A')[3][7].- Row constructors are frequently used for:
- Comparisons (tuple-wise in WHERE and JOIN clauses)
- Batch inserts (as tuples in VALUES)
- Building records for functions returning composite types[2][3].
Summary Table
Feature Syntax Example Notes Basic row constructor ROW(val1, val2, ...)ROWkeyword optional with multiple fieldsTuple for comparison WHERE (col1, col2) = (v1, v2)Field count and types must match Argument limit No explicit limit; practical only Up to table/composite type column limits (~1600 fields) Reference:
- PostgreSQL Documentation, "Value Expressions" and "Composite Types"[1][7][10]
- C# Corner, "Row Constructors in PostgreSQL"[2]
- DatabaseRookies, "Exploring ROW Constructors in PostgreSQL"[3]
Row constructors in PostgreSQL are flexible, powerful, and can accept as many arguments as needed within the practical and type-matching constraints of the context.
Citations:
- 1: https://www.postgresql.org/docs/current/sql-expressions.html
- 2: https://www.c-sharpcorner.com/article/row-constructors-in-postgresql/
- 3: https://databaserookies.wordpress.com/2024/02/27/uncovering-and-exploring-row-constructors-in-postgresql/
- 4: https://www.postgresql.org/docs/current/functions-comparisons.html
- 5: https://postgrespro.com/list/thread-id/1854604
- 6: https://learn.microsoft.com/en-us/sql/t-sql/queries/table-value-constructor-transact-sql?view=sql-server-ver16
- 7: https://postgresql.kr/docs/9.2/sql-expressions.html
- 8: https://www.postgresql.org/docs/current/queries-table-expressions.html
- 9: https://www.postgresql.org/docs/current/arrays.html
- 10: https://www.postgresql.org/docs/current/rowtypes.html
Approved: Unlimited argument count is appropriate
The
getMaxArgumentCount()method returningPHP_INT_MAXcorrectly reflects PostgreSQL’s row constructors, which impose no small arbitrary cap (aside from practical limits like a table’s column count of ≈1600). No changes needed.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Arr.php (4)
22-25: Good implementation of getFunctionName()Excellent approach to centralize the function name definition. This improves maintainability by eliminating hardcoded function names across the codebase.
29-30: Great use of dynamic function nameUsing
getFunctionName()instead of hardcoding 'ARRAY' improves maintainability and consistency. If the function name ever needs to change, it only needs to be updated in one place.
32-35: Good implementation of getMinArgumentCount()Adding this explicit method to define the minimum argument count centralizes validation logic and makes the code more maintainable.
37-40: Good implementation of getMaxArgumentCount()The method clearly defines the maximum argument count allowed. Using PHP_INT_MAX with a comment explaining "No upper limit" makes the intention clear.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php (5)
32-35: Good implementation of getFunctionName()Centralizing the function name definition improves maintainability by eliminating hardcoded strings.
37-40: Good implementation of getMinArgumentCount()Explicitly defining the minimum argument count through this method centralizes validation logic and improves code clarity.
42-45: Good implementation of getMaxArgumentCount()Explicitly defining the maximum argument count through this method centralizes validation logic and improves code clarity.
49-49: Good delegation to parent validationCalling
parent::validateArguments()is a clean approach that eliminates code duplication and centralizes basic argument validation logic in the parent class.
52-54: Good use of dynamic function name in error messagesUsing
getFunctionName()instead of hardcoding the function name for validation messages improves consistency and maintainability.
| protected function customizeFunction(): void | ||
| { | ||
| $this->setFunctionPrototype(\sprintf('%s(%%s)', $this->getFunctionName())); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ 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.
| protected function customizeFunction(): void | |
| { | |
| $this->setFunctionPrototype(\sprintf('%s(%%s)', $this->getFunctionName())); | |
| } | |
| final protected function customizeFunction(): void | |
| { | |
| $this->setFunctionPrototype(\sprintf('%s(%%s)', $this->getFunctionName())); | |
| } |
There was a problem hiding this comment.
This is not true. The Arr class has a different prototype, and it overrides it accordingly for that.
There was a problem hiding this comment.
🧩 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
Arrextends 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
BaseFunctionto support variadic functions with flexible argument counts. - Defines abstract methods:
getFunctionName(): stringgetNodeMappingPattern(): arraygetMinArgumentCount(): intgetMaxArgumentCount(): int
- Implements
customizeFunction()to set the function prototype asfunctionName(%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
DoctrineLexerandDoctrineOrmutilities 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.
feedParserWithNodesparses 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
TokenTypeclass. - Used in
BaseVariadicFunctionandBaseFunctionto 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
Arrclass having a different prototype and overriding accordingly. - The snippet from
Arr.phpshows that it overridescustomizeFunction()to set a prototype with square brackets (ARRAY[%s]), differing from the default parentheses inBaseVariadicFunction. - The
BaseVariadicFunctionclass sets the default prototype with parentheses and handles variadic argument parsing and validation. - The base class
BaseFunctionuses parentheses by default and a fixed node mapping pattern. - Utilities
DoctrineOrmandDoctrineLexerhandle version differences in Doctrine ORM and Lexer. - The exception class is used in
BaseVariadicFunctionto 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.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
87-88:⚠️ Potential issuePotential out-of-bounds array access issue.
If a subclass defines a multi-element node mapping pattern but allows more arguments than the pattern's length, accessing
$nodeMapping[$expectedNodeIndex]could cause an undefined index error.$expectedNodeIndex = $isNodeMappingASimplePattern ? 0 : $nodeIndex; +if (!isset($nodeMapping[$expectedNodeIndex])) { + // Implementation error - max argument count exceeds mapping pattern length + throw InvalidArgumentForVariadicFunctionException::unsupportedCombination( + $this->getFunctionName(), + count($this->nodes) + 1, + 'implementation defines fewer node mapping patterns than maximum allowed arguments' + ); +} $this->nodes[] = $parser->{$nodeMapping[$expectedNodeIndex]}(); // @phpstan-ignore-line
🧹 Nitpick comments (5)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvectorTest.php (1)
44-53: Add test for minimum argument validation.While you've correctly tested for the maximum argument count (max 2), consider adding a test for the minimum argument count validation (min 1) to ensure complete coverage of the validation logic in
BaseVariadicFunction./** * @test */ public function throws_exception_when_too_few_arguments_given(): void { $this->expectException(InvalidArgumentForVariadicFunctionException::class); $dql = 'SELECT TO_TSVECTOR() FROM ' . ContainsTexts::class . ' e'; $this->assertSqlFromDql('', $dql); }tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsqueryTest.php (1)
44-53: Add test for minimum argument validation.While you've correctly tested for the maximum argument count (max 2), consider adding a test for the minimum argument count validation (min 1) to ensure complete coverage of the validation logic in
BaseVariadicFunction./** * @test */ public function throws_exception_when_too_few_arguments_given(): void { $this->expectException(InvalidArgumentForVariadicFunctionException::class); $dql = 'SELECT TO_TSQUERY() FROM ' . ContainsTexts::class . ' e'; $this->assertSqlFromDql('', $dql); }tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObjectTest.php (1)
44-53: Add test for minimum argument validation.While you correctly test for odd numbers of arguments, consider adding a test for when zero arguments are provided, to verify the minimum argument count validation (min 2) in
BaseVariadicFunction./** * @test */ public function throws_exception_when_too_few_arguments_given(): void { $this->expectException(InvalidArgumentForVariadicFunctionException::class); $dql = 'SELECT JSON_BUILD_OBJECT() FROM ' . ContainsJsons::class . ' e'; $this->assertSqlFromDql('', $dql); }tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObjectTest.php (1)
44-53: Add test for minimum argument validation.While you correctly test for odd numbers of arguments, consider adding a test for when zero arguments are provided, to verify the minimum argument count validation (min 2) in
BaseVariadicFunction./** * @test */ public function throws_exception_when_too_few_arguments_given(): void { $this->expectException(InvalidArgumentForVariadicFunctionException::class); $dql = 'SELECT JSONB_BUILD_OBJECT() FROM ' . ContainsJsons::class . ' e'; $this->assertSqlFromDql('', $dql); }src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
22-26: Good centralization of function prototype definition.The implementation provides a standardized way to define function prototypes using the abstract
getFunctionName()method, which reduces code duplication across subclasses.Consider adding a class-level comment documenting that this method can be overridden by subclasses that need specific syntax patterns (like
Arrwhich uses square brackets instead of parentheses).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
fixtures/MartinGeorgiev/Doctrine/Function/TestVariadicFunction.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseComparisonFunction.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAddTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtractTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObjectTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObjectTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExistsTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatchTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php(3 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsqueryTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvectorTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/UnaccentTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- fixtures/MartinGeorgiev/Doctrine/Function/TestVariadicFunction.php
🚧 Files skipped from review as they are similar to previous changes (14)
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryFirstTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GreatestTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LeastTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtractTest.php
- src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseComparisonFunction.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathExistsTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryArrayTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathMatchTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbPathQueryTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAddTest.php
🧰 Additional context used
📓 Path-based instructions (1)
`tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php`: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL que...
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL queries for the tested SQL functions and operators.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObjectTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObjectTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/UnaccentTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvectorTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsqueryTest.php
🧠 Learnings (8)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObjectTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObjectTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/UnaccentTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvectorTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.403Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsqueryTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
🧬 Code Graph Analysis (3)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/UnaccentTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-125)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Unaccent.php (1)
Unaccent(14-35)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php (2)
BaseVariadicFunctionTestCase(14-42)createFixture(16-16)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvectorTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsTexts.php (1)
ORM(10-18)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-125)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvector.php (1)
ToTsvector(15-36)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php (2)
BaseVariadicFunctionTestCase(14-42)createFixture(16-16)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsqueryTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsTexts.php (1)
ORM(10-18)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-125)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsquery.php (1)
ToTsquery(15-36)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php (2)
BaseVariadicFunctionTestCase(14-42)createFixture(16-16)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: sloth
🔇 Additional comments (15)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsvectorTest.php (2)
8-8: LGTM! The refactoring adapts the test to the new variadic function framework.The changes correctly switch to the new
BaseVariadicFunctionTestCaseparent class and provide the requiredcreateFixture()method implementation, aligning with the PR's objective to standardize variadic function argument validation.Also applies to: 12-17
26-33: The test cases provide good coverage of PostgreSQL to_tsvector functionality.The test cases correctly verify:
- Basic conversion of text to tsvector
- Conversion with text transformation (LOWER)
- Conversion with explicit dictionary configuration ('english')
These align well with the PostgreSQL documentation for
to_tsvector().Also applies to: 35-42
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTsqueryTest.php (2)
8-8: LGTM! The refactoring adapts the test to the new variadic function framework.The changes correctly switch to the new
BaseVariadicFunctionTestCaseparent class and provide the requiredcreateFixture()method implementation, aligning with the PR's objective to standardize variadic function argument validation.Also applies to: 12-17
26-33: The test cases provide good coverage of PostgreSQL to_tsquery functionality.The test cases correctly verify:
- Basic conversion to tsquery
- Conversion with text transformation (UPPER)
- Conversion with explicit dictionary configuration ('english')
These align well with the PostgreSQL documentation for
to_tsquery().Also applies to: 35-42
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonBuildObjectTest.php (2)
8-8: LGTM! The refactoring adapts the test to the new variadic function framework.The changes correctly switch to the new
BaseVariadicFunctionTestCaseparent class and provide the requiredcreateFixture()method implementation, aligning with the PR's objective to standardize variadic function argument validation.Also applies to: 12-17
26-33: The test cases provide good coverage of PostgreSQL json_build_object functionality.The test cases correctly verify:
- Building a JSON object with a single key-value pair
- Building a JSON object with multiple key-value pairs using literals and functions
- Building a JSON object with multiple key-value pairs using database columns
These align well with the PostgreSQL documentation for
json_build_object().Also applies to: 35-42
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbBuildObjectTest.php (2)
8-8: LGTM! The refactoring adapts the test to the new variadic function framework.The changes correctly switch to the new
BaseVariadicFunctionTestCaseparent class and provide the requiredcreateFixture()method implementation, aligning with the PR's objective to standardize variadic function argument validation.Also applies to: 12-17
26-33: The test cases provide good coverage of PostgreSQL jsonb_build_object functionality.The test cases correctly verify:
- Building a JSONB object with a single key-value pair
- Building a JSONB object with multiple key-value pairs using literals and functions
- Building a JSONB object with multiple key-value pairs using database columns
These align well with the PostgreSQL documentation for
jsonb_build_object().Also applies to: 35-42
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (3)
27-28: Well-designed abstract methods for function metadata.These new abstract methods create a cleaner contract for subclasses by forcing them to explicitly define their function name and argument count requirements. This approach is more declarative and maintainable than having subclasses implement custom validation logic.
Also applies to: 34-37
78-85: Good early validation of argument count limits.Adding validation before processing each additional argument is a smart approach to fail fast if maximum arguments would be exceeded. This prevents unnecessary parsing and provides clear error messages to users.
98-114: Excellent centralization of validation logic.Moving the argument validation logic to the base class is a significant improvement. It:
- Eliminates code duplication across subclasses
- Ensures consistent validation behavior
- Centralizes exception throwing with standardized messages
- Makes subclasses simpler by removing boilerplate code
This is a good example of the "Don't Repeat Yourself" principle.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowTest.php (1)
11-19: Appropriate test refactoring for new base class design.The test class has been properly updated to:
- Import and use
BaseVariadicFunction- Extend the new
BaseVariadicFunctionTestCase- Implement the required
createFixture()method that returns a newRowinstanceThis aligns with the refactored architecture where variadic functions now declare minimum and maximum argument counts explicitly.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrTest.php (1)
9-16: Proper test adaptation to the new framework.This test refactoring correctly implements the required changes to support the new base variadic function architecture:
- Added the necessary
BaseVariadicFunctionimport- Updated to extend
BaseVariadicFunctionTestCase- Implemented the required
createFixture()methodThis ensures that
ArrTestworks with the updated function architecture.tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/UnaccentTest.php (2)
8-17: Correctly updated test to match the refactored architecture.The
UnaccentTestclass has been properly updated to work with the refactoredBaseVariadicFunctionarchitecture by:
- Adding the import for
BaseVariadicFunction- Extending
BaseVariadicFunctionTestCase- Implementing the required
createFixture()methodThis ensures consistency with the new validation pattern and function metadata approach.
42-51:✅ Verification successful
Test case effectively validates argument limit enforcement.
This test verifies that exceeding the maximum argument count correctly throws an exception. According to the relevant code snippet provided in the context,
Unaccentdefines a maximum of 2 arguments, and this test tries to use 3, triggering the expected validation failure.
🏁 Script executed:
#!/bin/bash # Verify Unaccent's configuration matches the test echo "Checking Unaccent implementation:" grep -A 5 "getMaxArgumentCount" src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Unaccent.phpLength of output: 255
Approve argument limit enforcement test
The test correctly throws
InvalidArgumentForVariadicFunctionExceptionwhen passing three arguments toUNACCENT, which is limited to two (getMaxArgumentCount()returns 2).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (3)
87-88: Consider using array_key_exists for improved semanticsWhen checking for index existence in arrays,
array_key_exists()might be semantically clearer thanisset()as it won't return false for keys with null values.- $argumentCountExceedsMappingPatternExpectation = !isset($nodeMapping[$expectedNodeIndex]); + $argumentCountExceedsMappingPatternExpectation = !\array_key_exists($expectedNodeIndex, $nodeMapping);
66-66: Consider addressing phpstan-ignore annotationsThere are multiple
@phpstan-ignore-lineannotations in the code. While sometimes necessary, they can hide potential type-related issues. Consider:
- Adding appropriate phpdoc type annotations
- Using explicit type assertions if needed
- Refactoring to make the code more type-safe
If these annotations are unavoidable due to Doctrine's API design, consider adding comments explaining why they're needed.
Also applies to: 97-97, 104-104
104-105: Potential redundant validationThis final validation call happens after all arguments are already parsed and previous checks were done at lines 83-85. While redundancy in validation can be beneficial for code safety, consider adding a comment explaining the purpose of this second validation pass to make the intention clear.
- $this->validateArguments(...$this->nodes); // @phpstan-ignore-line + // Final validation ensures all arguments meet requirements, including any special rules in subclass implementations + $this->validateArguments(...$this->nodes); // @phpstan-ignore-line
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.403Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
🔇 Additional comments (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (4)
22-27: Function name management improved via abstractionGood refactoring to centralize function name management through the new abstract
getFunctionName()method. This improves maintainability by requiring subclasses to explicitly define their function name, while ensuring consistent usage in the function prototype.
34-37: Well-structured argument count constraintsThe addition of abstract
getMinArgumentCount()andgetMaxArgumentCount()methods improves the design by:
- Enforcing subclasses to explicitly define their argument constraints
- Providing centralized access to these constraints for validation logic
- Removing the need for validation code duplication across implementations
This change enhances both maintainability and consistency.
78-96: Robust defense against invalid argument counts and potential index errorsExcellent implementation of validation checks that:
- Prevents exceeding maximum allowed arguments early in the parsing process
- Guards against array index out-of-bounds access by checking
isset($nodeMapping[$expectedNodeIndex])- Provides clear, specific error messages when validation fails
This defensive programming approach aligns well with the principle of failing explicitly rather than using fallback behavior that could lead to security issues.
107-123: Well-designed centralized argument validationThe new
validateArguments()method properly centralizes validation logic:
- It provides a consistent place to check argument counts
- Leverages the abstract methods to get constraints
- Uses consistent exception throwing patterns
This approach reduces code duplication and ensures consistent validation across all parsing paths.
…rguments to be validated in variadic functions (martin-georgiev#350)
Summary by CodeRabbit
Refactor
Bug Fixes
Tests