From 463bb64b996535d6c06bebc3c7280f78cdeb8ef0 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 09:56:45 +1100 Subject: [PATCH 1/5] Restore case-insensitive magic properties on Predicate The class documents AND, OR, NEST and UNNEST alongside their lower-case forms, and the docs promise the names are case-insensitive, but __get() lost its strtolower() in an earlier refactor. Upper-case names fell through to the default and returned the predicate unchanged, so $where->OR->equalTo(...) was silently combined with AND. The private $unnest property is renamed $parent. It shared its name with the @property Predicate $unnest tag, so the analyzer read every access as the magic property and reported the null guard as impossible. The in(), notIn() and expression() docblocks now carry the value types their predicates accept. --- src/Sql/Predicate/Predicate.php | 49 +++++++++-------- test/unit/Sql/Predicate/PredicateTest.php | 64 +++++++++++++++++++++++ 2 files changed, 91 insertions(+), 22 deletions(-) diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php index f2317ba1..94715247 100644 --- a/src/Sql/Predicate/Predicate.php +++ b/src/Sql/Predicate/Predicate.php @@ -8,7 +8,11 @@ use PhpDb\Sql\ExpressionInterface; use PhpDb\Sql\Predicate\Exception\RuntimeException; +use function strtolower; + /** + * @psalm-import-type ExpressionParameter from \PhpDb\Sql\Expression + * * @property Predicate $and * @property Predicate $or * @property Predicate $AND @@ -20,7 +24,7 @@ */ class Predicate extends PredicateSet { - private ?Predicate $unnest = null; + private ?Predicate $parent = null; protected ?string $nextPredicateCombineOperator = null; @@ -62,6 +66,7 @@ public function equalTo( /** * Create an expression, with parameter placeholders * + * @param string|float|int|ArgumentInterface|ExpressionInterface|array|null $parameters * @return $this Provides a fluent interface */ public function expression( @@ -116,6 +121,7 @@ public function greaterThanOrEqualTo( * Create "IN" predicate * Utilizes In predicate * + * @param list|ArgumentInterface $valueSet * @return $this Provides a fluent interface */ public function in(string|ArgumentInterface $identifier, array|ArgumentInterface $valueSet): static @@ -280,6 +286,7 @@ public function notEqualTo( * Create "NOT IN" predicate * Utilizes NotIn predicate * + * @param list|ArgumentInterface $valueSet * @return $this Provides a fluent interface */ public function notIn(string|ArgumentInterface $identifier, array|ArgumentInterface $valueSet): static @@ -334,8 +341,7 @@ public function predicate(PredicateInterface $predicate): static */ public function setUnnest(?Predicate $predicate = null): void { - /** @psalm-suppress PossiblyNullPropertyAssignmentValue */ - $this->unnest = $predicate; + $this->parent = $predicate; } /** @@ -345,15 +351,14 @@ public function setUnnest(?Predicate $predicate = null): void */ public function unnest(): self { - if (! $this->unnest instanceof Predicate) { + if (! $this->parent instanceof Predicate) { throw RuntimeException::forNotNested(); } - $unnest = $this->unnest; - /** @psalm-suppress PossiblyNullPropertyAssignmentValue */ - $this->unnest = null; + $parent = $this->parent; + $this->parent = null; - return $unnest; + return $parent; } protected function getNextPredicateCombineOperator(): string @@ -364,6 +369,13 @@ protected function getNextPredicateCombineOperator(): string return $operator; } + private function combineNextWith(string $operator): self + { + $this->nextPredicateCombineOperator = $operator; + + return $this; + } + /** * Overloading * Overloads "or", "and", "nest", and "unnest" @@ -372,19 +384,12 @@ protected function getNextPredicateCombineOperator(): string */ public function __get(string $name): self { - switch ($name) { - case 'or': - $this->nextPredicateCombineOperator = self::OP_OR; - break; - case 'and': - $this->nextPredicateCombineOperator = self::OP_AND; - break; - case 'nest': - return $this->nest(); - case 'unnest': - return $this->unnest(); - } - - return $this; + return match (strtolower($name)) { + 'or' => $this->combineNextWith(self::OP_OR), + 'and' => $this->combineNextWith(self::OP_AND), + 'nest' => $this->nest(), + 'unnest' => $this->unnest(), + default => $this, + }; } } diff --git a/test/unit/Sql/Predicate/PredicateTest.php b/test/unit/Sql/Predicate/PredicateTest.php index 2d668c55..6e5b9e3c 100644 --- a/test/unit/Sql/Predicate/PredicateTest.php +++ b/test/unit/Sql/Predicate/PredicateTest.php @@ -60,6 +60,20 @@ public static function fluentParameterProvider(): array ]; } + /** + * @return array + */ + public static function magicCombinationProvider(): array + { + return [ + 'lower-case or' => ['or', Predicate::OP_AND, '%s IS NULL OR %s IS NULL'], + 'upper-case OR' => ['OR', Predicate::OP_AND, '%s IS NULL OR %s IS NULL'], + 'mixed-case Or' => ['Or', Predicate::OP_AND, '%s IS NULL OR %s IS NULL'], + 'lower-case and' => ['and', Predicate::OP_OR, '%s IS NULL AND %s IS NULL'], + 'upper-case AND' => ['AND', Predicate::OP_OR, '%s IS NULL AND %s IS NULL'], + ]; + } + /** * @throws ErrorException */ @@ -415,6 +429,28 @@ public function literalCreatesLiteralPredicate(): void static::assertSame('foo.bar = ?', $expressionData['spec']); } + #[Test] + public function magicCombinationAppliesOnlyToTheNextPredicate(): void + { + $predicate = new Predicate(); + $predicate->isNull('a')->OR->isNull('b')->isNull('c'); + + static::assertSame('%s IS NULL OR %s IS NULL AND %s IS NULL', $predicate->getExpressionData()['spec']); + } + + #[Test] + #[DataProvider('magicCombinationProvider')] + public function magicCombinationPropertyIgnoresCase( + string $property, + string $defaultCombination, + string $expectedSpec, + ): void { + $predicate = new Predicate(null, $defaultCombination); + $predicate->isNull('a')->{$property}->isNull('b'); + + static::assertSame($expectedSpec, $predicate->getExpressionData()['spec']); + } + #[Test] public function magicGetNestReturnsNestedPredicate(): void { @@ -426,6 +462,14 @@ public function magicGetNestReturnsNestedPredicate(): void static::assertNotSame($predicate, $nested); } + #[Test] + public function magicGetUnknownPropertyReturnsSamePredicate(): void + { + $predicate = new Predicate(); + + static::assertSame($predicate, $predicate->__get('unknown')); + } + #[Test] public function magicGetUnnestReturnsParentPredicate(): void { @@ -437,6 +481,16 @@ public function magicGetUnnestReturnsParentPredicate(): void static::assertSame($predicate, $parent); } + #[Test] + public function magicGetUpperCaseNestAndUnnestReturnToParent(): void + { + $predicate = new Predicate(); + $nested = $predicate->NEST; + + static::assertNotSame($predicate, $nested); + static::assertSame($predicate, $nested->UNNEST); + } + #[Test] public function notEqualToCreatesOperatorPredicate(): void { @@ -500,6 +554,16 @@ public function predicateMethodAddsCustomPredicateInterface(): void static::assertCount(1, $predicate); } + #[Test] + public function unnestReturnsThePredicateGivenToSetUnnest(): void + { + $parent = new Predicate(); + $predicate = new Predicate(); + $predicate->setUnnest($parent); + + static::assertSame($parent, $predicate->unnest()); + } + #[Test] public function unnestThrowsWhenNotNested(): void { From cd4e15a40545d5f04b715addc05d3d9fdf40d5e0 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 09:56:45 +1100 Subject: [PATCH 2/5] Build a predicate for every list entry addPredicates() accepts A string with a placeholder and no key became a Sql\Expression, which is not a PredicateInterface, so addPredicates(['a = ?']) threw a TypeError. It now becomes a Predicate\Expression, as it did before the refactor. A Select bound to a placeholder key is wrapped in an Argument\Select so it reaches the expression as a parameter. A list entry that is not a predicate, an expression or a string now throws InvalidArgumentException::forInvalidListPredicate() rather than a TypeError from str_contains(). Entries are built by two private helpers, one per key type, typed against a PredicateValue alias that the where() and having() docblocks on Select, Update and Delete now share. getExpressionData() drops its special cases for zero and one predicate; the loop produces the same result for both. --- src/Sql/Delete.php | 3 +- .../Exception/InvalidArgumentException.php | 10 ++ src/Sql/Predicate/PredicateSet.php | 126 ++++++++---------- src/Sql/Select.php | 5 +- src/Sql/Update.php | 3 +- .../InvalidArgumentExceptionTest.php | 6 + test/unit/Sql/Predicate/PredicateSetTest.php | 58 ++++++++ 7 files changed, 136 insertions(+), 75 deletions(-) diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index 139e509e..21def71f 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -17,6 +17,7 @@ /** * @property Where $where * @psalm-import-type Specification from AbstractSql + * @psalm-import-type PredicateValue from Predicate\PredicateSet */ class Delete extends AbstractPreparableSql { @@ -79,7 +80,7 @@ public function getRawState(?string $key = null): mixed /** * Create where clause * - * @param PredicateInterface|array|Closure|string|Where $predicate + * @param PredicateInterface|array|Closure|string|Where $predicate * @param string $combination One of the OP_* constants from Predicate\PredicateSet */ public function where( diff --git a/src/Sql/Predicate/Exception/InvalidArgumentException.php b/src/Sql/Predicate/Exception/InvalidArgumentException.php index e94e87db..df5081b3 100644 --- a/src/Sql/Predicate/Exception/InvalidArgumentException.php +++ b/src/Sql/Predicate/Exception/InvalidArgumentException.php @@ -6,10 +6,15 @@ use PhpDb\Sql\Exception; +/** + * @api + */ class InvalidArgumentException extends Exception\InvalidArgumentException { final public const string INVALID_COMBINATION = "Invalid combination: expected 'AND' or 'OR'"; + final public const string INVALID_LIST_PREDICATE = 'A predicate without a string key must be a predicate, an expression or a string'; + final public const string MISSING_IDENTIFIER = 'Identifier must be specified'; final public const string MISSING_LEFT_EXPRESSION = 'Left expression must be specified'; @@ -31,6 +36,11 @@ public static function forInvalidCombination(): self return new self(self::INVALID_COMBINATION); } + public static function forInvalidListPredicate(): self + { + return new self(self::INVALID_LIST_PREDICATE); + } + public static function forMissingIdentifier(): self { return new self(self::MISSING_IDENTIFIER); diff --git a/src/Sql/Predicate/PredicateSet.php b/src/Sql/Predicate/PredicateSet.php index 96f6bc06..58457b76 100644 --- a/src/Sql/Predicate/PredicateSet.php +++ b/src/Sql/Predicate/PredicateSet.php @@ -7,18 +7,23 @@ use Closure; use Countable; use Override; +use PhpDb\Sql\Argument\Select; use PhpDb\Sql\Expression; use PhpDb\Sql\Predicate\Expression as PredicateExpression; +use PhpDb\Sql\SqlInterface; use ReturnTypeWillChange; +use function array_push; use function count; use function implode; use function is_array; use function is_string; use function str_contains; -// phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse - +/** + * @psalm-import-type ExpressionParameter from Expression + * @psalm-type PredicateValue = ExpressionParameter|PredicateInterface|SqlInterface + */ class PredicateSet implements PredicateInterface, Countable { final public const string OP_AND = 'AND'; @@ -45,10 +50,8 @@ public function __construct(?array $predicates = null, string $defaultCombinatio { $this->defaultCombination = $defaultCombination; - if (null !== $predicates) { - foreach ($predicates as $predicate) { - $this->addPredicate($predicate); - } + foreach ($predicates ?? [] as $predicate) { + $this->addPredicate($predicate); } } @@ -71,54 +74,24 @@ public function addPredicate(PredicateInterface $predicate, ?string $combination /** * Add predicates to set * - * @param PredicateInterface|Closure|string|array $predicates + * @param PredicateInterface|Closure|string|array $predicates * @throws Exception\InvalidArgumentException */ public function addPredicates( PredicateInterface|Closure|string|array $predicates, string $combination = self::OP_AND, ): static { - if ($predicates instanceof PredicateInterface) { - $this->addPredicate($predicates, $combination); - - return $this; - } - if ($predicates instanceof Closure) { $predicates($this); return $this; } - if (is_string($predicates)) { - $predicate = str_contains($predicates, Expression::PLACEHOLDER) - ? new PredicateExpression($predicates) - : new Literal($predicates); - $this->addPredicate($predicate, $combination); - - return $this; - } - - foreach ($predicates as $pkey => $pvalue) { - $predicate = match (true) { - is_string($pkey) => match (true) { - str_contains($pkey, '?') => new PredicateExpression($pkey, $pvalue), - null === $pvalue => new IsNull($pkey), - is_array($pvalue) => new In($pkey, $pvalue), - $pvalue instanceof PredicateInterface - => throw Exception\InvalidArgumentException::forPredicateWithStringKey(), - default => new Operator($pkey, Operator::OP_EQ, $pvalue), - }, - $pvalue instanceof PredicateInterface => $pvalue, - $pvalue instanceof Expression => new PredicateExpression( - $pvalue->getExpression(), - $pvalue->getParameters(), - ), - str_contains($pvalue, Expression::PLACEHOLDER) => new Expression($pvalue), - default => new Literal($pvalue), - }; - - $this->addPredicate($predicate, $combination); + foreach (is_array($predicates) ? $predicates : [$predicates] as $key => $value) { + $this->addPredicate( + is_string($key) ? $this->createColumnPredicate($key, $value) : $this->createListPredicate($value), + $combination, + ); } return $this; @@ -148,29 +121,8 @@ public function count(): int #[Override] public function getExpressionData(): array { - $predicateCount = count($this->predicates); - - if (0 === $predicateCount) { - return ['spec' => '', 'values' => []]; - } - - if (1 === $predicateCount) { - [$operator, $predicate] = $this->predicates[0]; - $expressionData = $predicate->getExpressionData(); - - if ($predicate instanceof self) { - return [ - 'spec' => "({$expressionData['spec']})", - 'values' => $expressionData['values'], - ]; - } - - return $expressionData; - } - $specParts = []; $allValues = []; - $first = true; foreach ($this->predicates as [$operator, $predicate]) { $expressionData = $predicate->getExpressionData(); @@ -179,15 +131,9 @@ public function getExpressionData(): array ? "({$expressionData['spec']})" : $expressionData['spec']; - $specParts[] = $first ? $spec : "{$operator} {$spec}"; - $first = false; + $specParts[] = [] === $specParts ? $spec : "{$operator} {$spec}"; - $values = $expressionData['values']; - if ([] !== $values) { - foreach ($values as $value) { - $allValues[] = $value; - } - } + array_push($allValues, ...$expressionData['values']); } return [ @@ -215,4 +161,42 @@ public function orPredicate(PredicateInterface $predicate): static return $this; } + + /** + * Build the predicate for an entry keyed by a column name or an expression with placeholders + * + * @param PredicateValue $value + * @throws Exception\InvalidArgumentException + */ + private function createColumnPredicate(string $key, mixed $value): PredicateInterface + { + return match (true) { + str_contains($key, Expression::PLACEHOLDER) => new PredicateExpression( + $key, + $value instanceof SqlInterface ? new Select($value) : $value, + ), + null === $value => new IsNull($key), + is_array($value) => new In($key, $value), + $value instanceof PredicateInterface + => throw Exception\InvalidArgumentException::forPredicateWithStringKey(), + default => new Operator($key, Operator::OP_EQ, $value), + }; + } + + /** + * Build the predicate for an entry without a string key + * + * @param PredicateValue $value + * @throws Exception\InvalidArgumentException + */ + private function createListPredicate(mixed $value): PredicateInterface + { + return match (true) { + $value instanceof PredicateInterface => $value, + $value instanceof Expression => new PredicateExpression($value->getExpression(), $value->getParameters()), + ! is_string($value) => throw Exception\InvalidArgumentException::forInvalidListPredicate(), + str_contains($value, Expression::PLACEHOLDER) => new PredicateExpression($value), + default => new Literal($value), + }; + } } diff --git a/src/Sql/Select.php b/src/Sql/Select.php index 7f551cf1..cbc72b69 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -36,6 +36,7 @@ * @property Having $having * @property Join $joins * @psalm-import-type Specification from AbstractSql + * @psalm-import-type PredicateValue from Predicate\PredicateSet * @psalm-import-type JoinName from Join * @psalm-type TableReference = string|TableIdentifier|array * @psalm-type ColumnList = array @@ -281,7 +282,7 @@ public function group(mixed $group): static /** * Create having clause * - * @param Having|PredicateInterface|array|Closure|string $predicate + * @param Having|PredicateInterface|array|Closure|string $predicate * @param string $combination One of the OP_* constants from Predicate\PredicateSet */ public function having( @@ -449,7 +450,7 @@ public function setSpecification(string $index, array|string $specification): st /** * Create where clause * - * @param PredicateInterface|array|string|Closure $predicate + * @param PredicateInterface|array|string|Closure $predicate * @param string $combination One of the OP_* constants from Predicate\PredicateSet * @throws Exception\InvalidArgumentException */ diff --git a/src/Sql/Update.php b/src/Sql/Update.php index b99c130f..d2c692cb 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -23,6 +23,7 @@ /** * @property Where $where * @psalm-import-type Specification from AbstractSql + * @psalm-import-type PredicateValue from Predicate\PredicateSet * @psalm-import-type JoinName from Join */ class Update extends AbstractPreparableSql @@ -145,7 +146,7 @@ public function table(TableIdentifier|string|array $table): static /** * Create where clause * - * @param PredicateInterface|array|Closure|string|Where $predicate + * @param PredicateInterface|array|Closure|string|Where $predicate * @param string $combination One of the OP_* constants from Predicate\PredicateSet * @throws Exception\InvalidArgumentException */ diff --git a/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php b/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php index 8160e359..7bdf4ee8 100644 --- a/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php +++ b/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php @@ -16,6 +16,7 @@ #[Group('unit')] #[CoversMethod(InvalidArgumentException::class, 'forInvalidCombination')] +#[CoversMethod(InvalidArgumentException::class, 'forInvalidListPredicate')] #[CoversMethod(InvalidArgumentException::class, 'forMissingIdentifier')] #[CoversMethod(InvalidArgumentException::class, 'forMissingLeftExpression')] #[CoversMethod(InvalidArgumentException::class, 'forMissingLikeExpression')] @@ -35,6 +36,11 @@ public static function namedConstructorProvider(): array [], InvalidArgumentException::INVALID_COMBINATION, ], + 'invalid list predicate' => [ + 'forInvalidListPredicate', + [], + InvalidArgumentException::INVALID_LIST_PREDICATE, + ], 'missing identifier' => [ 'forMissingIdentifier', [], diff --git a/test/unit/Sql/Predicate/PredicateSetTest.php b/test/unit/Sql/Predicate/PredicateSetTest.php index 27a4eeb6..e74052b2 100644 --- a/test/unit/Sql/Predicate/PredicateSetTest.php +++ b/test/unit/Sql/Predicate/PredicateSetTest.php @@ -15,8 +15,10 @@ use PhpDb\Sql\Predicate\Operator; use PhpDb\Sql\Predicate\PredicateInterface; use PhpDb\Sql\Predicate\PredicateSet; +use PhpDb\Sql\Select; use PhpDbTest\DeprecatedAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\Attributes\Test; @@ -34,10 +36,25 @@ #[CoversMethod(PredicateSet::class, 'andPredicate')] #[CoversMethod(PredicateSet::class, 'getExpressionData')] #[CoversMethod(PredicateSet::class, 'count')] +#[CoversMethod(PredicateSet::class, 'createColumnPredicate')] +#[CoversMethod(PredicateSet::class, 'createListPredicate')] final class PredicateSetTest extends TestCase { use DeprecatedAssertionsTrait; + /** + * @return array + */ + public static function invalidListPredicateProvider(): array + { + return [ + 'integer' => [1], + 'null' => [null], + 'boolean' => [true], + 'list' => [[1, 2]], + ]; + } + /** * @throws ReflectionException */ @@ -94,6 +111,38 @@ public function addPredicates(): void $predicateSet->addPredicates(null); } + #[Test] + public function addPredicatesBuildsPredicateExpressionForPlaceholderStringWithoutKey(): void + { + $predicateSet = new PredicateSet(); + $predicateSet->addPredicates(['a = ?']); + + static::assertInstanceOf(Expression::class, $predicateSet->getPredicates()[0][1]); + } + + #[Test] + public function addPredicatesPassesTheSetToAClosure(): void + { + $predicateSet = new PredicateSet(); + $predicateSet->addPredicates(static function (PredicateSet $set): void { + $set->addPredicate(new IsNull('a')); + }); + + static::assertCount(1, $predicateSet); + } + + #[Test] + #[DataProvider('invalidListPredicateProvider')] + public function addPredicatesThrowsForListEntryThatCannotBecomeAPredicate(mixed $value): void + { + $predicateSet = new PredicateSet(); + + self::expectException(InvalidArgumentException::class); + self::expectExceptionMessage(InvalidArgumentException::INVALID_LIST_PREDICATE); + /** @psalm-suppress MixedArgumentTypeCoercion */ + $predicateSet->addPredicates([$value]); + } + #[Test] public function addPredicatesThrowsWhenStringKeyUsedWithPredicateInterface(): void { @@ -155,6 +204,15 @@ public function addPredicatesWithMultipleExpressions(): void static::assertInstanceOf(Expression::class, $predicates[1][1]); } + #[Test] + public function addPredicatesWrapsSelectBoundToPlaceholder(): void + { + $predicateSet = new PredicateSet(); + $predicateSet->addPredicates(['id IN (?)' => new Select('t')]); + + static::assertInstanceOf(Argument\Select::class, $predicateSet->getExpressionData()['values'][0]); + } + #[Test] public function addPredicateThrowsOnInvalidCombination(): void { From a8cdc50c7213a46d2533d5e3a8f72a01c0afa69e Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 09:56:53 +1100 Subject: [PATCH 3/5] Tag the Predicate component @api and clear it from the baselines Every class and interface under Sql\Predicate is public API that callers build and extend, so each is marked @api rather than final. Mago's class finality check accepts @api, which settles both the class-must-be-final and missing-api-or-internal findings without closing the classes to the platform packages. Predicate joins the too-many-methods exclusions. It is the fluent WHERE and HAVING builder, one method per predicate type, so its method count is the size of its public API. Sql\Predicate now leaves both baselines entirely. Typing the predicate arrays moves six findings onto the TableGateway and RowGateway callers of where(), which are left for the gateway pass. --- analyzer-baseline.toml | 204 ++++-------------- lint-baseline.toml | 18 -- mago.toml | 12 +- src/Sql/Predicate/Between.php | 3 + .../Predicate/Exception/RuntimeException.php | 3 + src/Sql/Predicate/Expression.php | 3 + src/Sql/Predicate/In.php | 3 + src/Sql/Predicate/IsNotNull.php | 3 + src/Sql/Predicate/IsNull.php | 3 + src/Sql/Predicate/Like.php | 3 + src/Sql/Predicate/Literal.php | 3 + src/Sql/Predicate/NotBetween.php | 3 + src/Sql/Predicate/NotIn.php | 3 + src/Sql/Predicate/NotLike.php | 3 + src/Sql/Predicate/Operator.php | 3 + src/Sql/Predicate/Predicate.php | 2 + src/Sql/Predicate/PredicateInterface.php | 3 + src/Sql/Predicate/PredicateSet.php | 2 + 18 files changed, 89 insertions(+), 188 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 8b23a15d..cca21de4 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1506,6 +1506,24 @@ code = "invalid-array-element-key" message = "Invalid array key type." count = 1 +[[issues]] +file = "src/RowGateway/AbstractRowGateway.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Delete::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `array` is less specific.' +count = 1 + +[[issues]] +file = "src/RowGateway/AbstractRowGateway.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|array|null|scalar>|string`, but provided type `array` is less specific.' +count = 1 + +[[issues]] +file = "src/RowGateway/AbstractRowGateway.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `array` is less specific.' +count = 1 + [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "less-specific-return-statement" @@ -2598,174 +2616,6 @@ code = "missing-api-or-internal" message = 'Interface `PhpDb\Sql\Platform\PlatformDecoratorInterface` is missing an `@api` or `@internal` annotation.' count = 1 -[[issues]] -file = "src/Sql/Predicate/Exception/InvalidArgumentException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\Exception\InvalidArgumentException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Exception/RuntimeException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\Exception\RuntimeException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Expression.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\Expression` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/IsNotNull.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\IsNotNull` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Literal.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\Literal` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/NotBetween.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\NotBetween` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/NotIn.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\NotIn` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/NotLike.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\NotLike` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Operator.php" -code = "class-must-be-final" -message = 'Class `PhpDb\Sql\Predicate\Operator` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "impossible-condition" -message = "This condition (type `false`) will always evaluate to false." -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "imprecise-type" -message = "Type `array` in parameter `$parameters` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "imprecise-type" -message = "Type `array` in parameter `$valueSet` is imprecise, equivalent to `array`." -count = 2 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "invalid-property-assignment-value" -message = 'Invalid type for property `$unnest`: expected `PhpDb\Sql\Predicate\Predicate`, but got `PhpDb\Sql\Predicate\Predicate|null`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "invalid-property-assignment-value" -message = 'Invalid type for property `$unnest`: expected `PhpDb\Sql\Predicate\Predicate`, but got `null`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #2 of `PhpDb\Sql\Expression::__construct`: expected `PhpDb\Sql\ArgumentInterface|PhpDb\Sql\ExpressionInterface|array|null|scalar>|list|null|scalar`, but provided type `PhpDb\Sql\ArgumentInterface|PhpDb\Sql\ExpressionInterface|float|int|non-empty-array|null|string` is less specific.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #2 of `PhpDb\Sql\Predicate\In::__construct`: expected `PhpDb\Sql\ArgumentInterface|PhpDb\Sql\Select|list|null`, but possibly received `PhpDb\Sql\ArgumentInterface|array`.' -count = 2 - -[[issues]] -file = "src/Sql/Predicate/PredicateInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\Sql\Predicate\PredicateInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\Expression::__construct`: expected `string`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\Literal::__construct`: expected `string`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `str_contains`: expected `string`, but found `mixed`." -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #2 of `PhpDb\Sql\Expression::__construct`: expected `PhpDb\Sql\ArgumentInterface|PhpDb\Sql\ExpressionInterface|array|null|scalar>|list|null|scalar`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #3 of `PhpDb\Sql\Predicate\Operator::__construct`: expected `PhpDb\Sql\ArgumentInterface|PhpDb\Sql\ExpressionInterface|PhpDb\Sql\SqlInterface|null|scalar`, but found `nonnull`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\Predicate\PredicateSet::addPredicate`: expected `PhpDb\Sql\Predicate\PredicateInterface`, but possibly received `PhpDb\Sql\Expression|PhpDb\Sql\Predicate\Expression|PhpDb\Sql\Predicate\In|PhpDb\Sql\Predicate\IsNull|PhpDb\Sql\Predicate\Literal|PhpDb\Sql\Predicate\Operator|PhpDb\Sql\Predicate\PredicateInterface`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #2 of `PhpDb\Sql\Predicate\In::__construct`: expected `PhpDb\Sql\ArgumentInterface|PhpDb\Sql\Select|list|null`, but possibly received `array`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "possibly-null-destructuring-source" -message = 'Destructuring a possibly null value of type `array{0: string, 1: PhpDb\Sql\Predicate\PredicateInterface}|null`.' -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "possibly-undefined-int-array-index" -message = 'Possibly undefined array index accessed on `list`.' -count = 1 - [[issues]] file = "src/Sql/PreparableSqlInterface.php" code = "missing-api-or-internal" @@ -3234,12 +3084,30 @@ code = "less-specific-nested-argument-type" message = 'Argument type mismatch for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but provided type `PhpDb\Adapter\Driver\ResultInterface|null` is less specific.' count = 1 +[[issues]] +file = "src/TableGateway/AbstractTableGateway.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Delete::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `PhpDb\Sql\Where|array|string` is less specific.' +count = 1 + [[issues]] file = "src/TableGateway/AbstractTableGateway.php" code = "less-specific-nested-argument-type" message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::columns`: expected `array`, but provided type `non-empty-array` is less specific.' count = 1 +[[issues]] +file = "src/TableGateway/AbstractTableGateway.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|array|null|scalar>|string`, but provided type `PhpDb\Sql\Where|array|string` is less specific.' +count = 1 + +[[issues]] +file = "src/TableGateway/AbstractTableGateway.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `(closure(...mixed=): mixed)|PhpDb\Sql\Where|array|string` is less specific.' +count = 1 + [[issues]] file = "src/TableGateway/AbstractTableGateway.php" code = "missing-api-or-internal" diff --git a/lint-baseline.toml b/lint-baseline.toml index 98558a29..c92c2258 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -426,24 +426,6 @@ code = "too-many-methods" message = "Class has too many methods." count = 1 -[[issues]] -file = "src/Sql/Predicate/Predicate.php" -code = "too-many-methods" -message = "Class has too many methods." -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "halstead" -message = "Method has a high halstead difficulty" -count = 1 - -[[issues]] -file = "src/Sql/Predicate/PredicateSet.php" -code = "kan-defect" -message = "Class has a high kan defect score." -count = 1 - [[issues]] file = "src/Sql/Select.php" code = "cyclomatic-complexity" diff --git a/mago.toml b/mago.toml index d91e430c..d5f6b80a 100644 --- a/mago.toml +++ b/mago.toml @@ -15,8 +15,16 @@ baseline = "lint-baseline.toml" # Exception classes are message catalogues: one template and one named constructor per # condition, per the convention in docs/book/exceptions.md. The method count tracks the # number of distinct failures a component can report, so the maintainability signal does -# not apply. "test/" repeats the phpdb-qa-tools default, which this key replaces. -too-many-methods = { exclude = ["test/", "src/Sql/Exception/", "src/TableGateway/Exception/", "src/Adapter/Exception/"] } +# not apply. Predicate is the fluent WHERE/HAVING builder: one public method per predicate +# type, so its method count is the size of its public API. "test/" repeats the +# phpdb-qa-tools default, which this key replaces. +too-many-methods = { exclude = [ + "test/", + "src/Sql/Exception/", + "src/TableGateway/Exception/", + "src/Adapter/Exception/", + "src/Sql/Predicate/Predicate.php", +] } # The capability interfaces return `ResultSetInterface&`, and PHP rejects # `self` as part of an intersection type, so this rule's advice cannot be taken there. diff --git a/src/Sql/Predicate/Between.php b/src/Sql/Predicate/Between.php index 138e9154..5f9be4a6 100644 --- a/src/Sql/Predicate/Between.php +++ b/src/Sql/Predicate/Between.php @@ -11,6 +11,9 @@ use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; +/** + * @api + */ class Between extends AbstractExpression implements PredicateInterface { protected string $operator = 'BETWEEN'; diff --git a/src/Sql/Predicate/Exception/RuntimeException.php b/src/Sql/Predicate/Exception/RuntimeException.php index 0ade086d..3793da0c 100644 --- a/src/Sql/Predicate/Exception/RuntimeException.php +++ b/src/Sql/Predicate/Exception/RuntimeException.php @@ -6,6 +6,9 @@ use PhpDb\Sql\Exception; +/** + * @api + */ class RuntimeException extends Exception\RuntimeException { final public const string NOT_NESTED = 'Not nested'; diff --git a/src/Sql/Predicate/Expression.php b/src/Sql/Predicate/Expression.php index 8752b263..add5448a 100644 --- a/src/Sql/Predicate/Expression.php +++ b/src/Sql/Predicate/Expression.php @@ -6,4 +6,7 @@ use PhpDb\Sql\Expression as BaseExpression; +/** + * @api + */ class Expression extends BaseExpression implements PredicateInterface {} diff --git a/src/Sql/Predicate/In.php b/src/Sql/Predicate/In.php index bc744c70..8e4c898b 100644 --- a/src/Sql/Predicate/In.php +++ b/src/Sql/Predicate/In.php @@ -13,6 +13,9 @@ use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; use PhpDb\Sql\Select; +/** + * @api + */ class In extends AbstractExpression implements PredicateInterface { protected ?ArgumentInterface $identifier = null; diff --git a/src/Sql/Predicate/IsNotNull.php b/src/Sql/Predicate/IsNotNull.php index 4da9f2ca..9023c584 100644 --- a/src/Sql/Predicate/IsNotNull.php +++ b/src/Sql/Predicate/IsNotNull.php @@ -4,6 +4,9 @@ namespace PhpDb\Sql\Predicate; +/** + * @api + */ class IsNotNull extends IsNull { protected string $operator = 'IS NOT NULL'; diff --git a/src/Sql/Predicate/IsNull.php b/src/Sql/Predicate/IsNull.php index 2b561d01..c949a453 100644 --- a/src/Sql/Predicate/IsNull.php +++ b/src/Sql/Predicate/IsNull.php @@ -10,6 +10,9 @@ use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; +/** + * @api + */ class IsNull extends AbstractExpression implements PredicateInterface { protected string $operator = 'IS NULL'; diff --git a/src/Sql/Predicate/Like.php b/src/Sql/Predicate/Like.php index f0e87d83..2d5591ed 100644 --- a/src/Sql/Predicate/Like.php +++ b/src/Sql/Predicate/Like.php @@ -11,6 +11,9 @@ use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; +/** + * @api + */ class Like extends AbstractExpression implements PredicateInterface { protected string $operator = 'LIKE'; diff --git a/src/Sql/Predicate/Literal.php b/src/Sql/Predicate/Literal.php index 475e2759..ef4c6138 100644 --- a/src/Sql/Predicate/Literal.php +++ b/src/Sql/Predicate/Literal.php @@ -6,4 +6,7 @@ use PhpDb\Sql\Literal as BaseLiteral; +/** + * @api + */ class Literal extends BaseLiteral implements PredicateInterface {} diff --git a/src/Sql/Predicate/NotBetween.php b/src/Sql/Predicate/NotBetween.php index e42ce86e..dd4baec6 100644 --- a/src/Sql/Predicate/NotBetween.php +++ b/src/Sql/Predicate/NotBetween.php @@ -4,6 +4,9 @@ namespace PhpDb\Sql\Predicate; +/** + * @api + */ class NotBetween extends Between { protected string $operator = 'NOT BETWEEN'; diff --git a/src/Sql/Predicate/NotIn.php b/src/Sql/Predicate/NotIn.php index 45fc2573..c68ba2cc 100644 --- a/src/Sql/Predicate/NotIn.php +++ b/src/Sql/Predicate/NotIn.php @@ -4,6 +4,9 @@ namespace PhpDb\Sql\Predicate; +/** + * @api + */ class NotIn extends In { protected string $operator = 'NOT IN'; diff --git a/src/Sql/Predicate/NotLike.php b/src/Sql/Predicate/NotLike.php index 01ef6cea..4194d50f 100644 --- a/src/Sql/Predicate/NotLike.php +++ b/src/Sql/Predicate/NotLike.php @@ -4,6 +4,9 @@ namespace PhpDb\Sql\Predicate; +/** + * @api + */ class NotLike extends Like { protected string $operator = 'NOT LIKE'; diff --git a/src/Sql/Predicate/Operator.php b/src/Sql/Predicate/Operator.php index 4d98e855..1344e062 100644 --- a/src/Sql/Predicate/Operator.php +++ b/src/Sql/Predicate/Operator.php @@ -14,6 +14,9 @@ use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; use PhpDb\Sql\SqlInterface; +/** + * @api + */ class Operator extends AbstractExpression implements PredicateInterface { final public const string OPERATOR_EQUAL_TO = '='; diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php index 94715247..f3e3a1b1 100644 --- a/src/Sql/Predicate/Predicate.php +++ b/src/Sql/Predicate/Predicate.php @@ -11,6 +11,8 @@ use function strtolower; /** + * @api + * * @psalm-import-type ExpressionParameter from \PhpDb\Sql\Expression * * @property Predicate $and diff --git a/src/Sql/Predicate/PredicateInterface.php b/src/Sql/Predicate/PredicateInterface.php index d1935e85..dd753885 100644 --- a/src/Sql/Predicate/PredicateInterface.php +++ b/src/Sql/Predicate/PredicateInterface.php @@ -6,4 +6,7 @@ use PhpDb\Sql\ExpressionInterface; +/** + * @api + */ interface PredicateInterface extends ExpressionInterface {} diff --git a/src/Sql/Predicate/PredicateSet.php b/src/Sql/Predicate/PredicateSet.php index 58457b76..a3f791dd 100644 --- a/src/Sql/Predicate/PredicateSet.php +++ b/src/Sql/Predicate/PredicateSet.php @@ -21,6 +21,8 @@ use function str_contains; /** + * @api + * * @psalm-import-type ExpressionParameter from Expression * @psalm-type PredicateValue = ExpressionParameter|PredicateInterface|SqlInterface */ From 23a81caa9cf89e011eb7a1ca3c6d62cb64c383a0 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 10:42:23 +1100 Subject: [PATCH 4/5] Match Predicate's magic properties exactly and refuse mixed case __get() matches the lower- and upper-case names its @property tags declare instead of lower-casing every name it is given. A mixed-case form of a known name, such as Or, now throws InvalidArgumentException::forMixedCaseMagicProperty() rather than falling through and silently leaving the next predicate's combination unchanged. Other unknown names still return the predicate. The where-having guide described the names as case-insensitive. It now documents lower and upper case and the exception, and its example loses a stray quote. The Predicate exception catalogue joins the too-many-methods exclusions alongside src/Sql/Exception/. --- docs/book/sql/where-having.md | 7 +++-- mago.toml | 1 + .../Exception/InvalidArgumentException.php | 9 ++++++ src/Sql/Predicate/Predicate.php | 19 +++++++----- .../InvalidArgumentExceptionTest.php | 6 ++++ test/unit/Sql/Predicate/PredicateTest.php | 29 +++++++++++++++++-- 6 files changed, 59 insertions(+), 12 deletions(-) diff --git a/docs/book/sql/where-having.md b/docs/book/sql/where-having.md index 02ab5303..31573e4f 100644 --- a/docs/book/sql/where-having.md +++ b/docs/book/sql/where-having.md @@ -614,11 +614,12 @@ Produces: WHERE status = 'active' AND age > 18 OR role = 'admin' ``` -The properties are case-insensitive for convenience: +The properties can be written in lower or upper case. Mixed case, such as `Or`, throws +`PhpDb\Sql\Predicate\Exception\InvalidArgumentException`: -```php title="Case-insensitive magic property usage" +```php title="Lower- and upper-case magic property usage" $where->and->equalTo('a', 1); -$where->AND->equalTo('b', 2'); +$where->AND->equalTo('b', 2); ``` ### Deep nesting of predicates diff --git a/mago.toml b/mago.toml index d5f6b80a..4780bd50 100644 --- a/mago.toml +++ b/mago.toml @@ -21,6 +21,7 @@ baseline = "lint-baseline.toml" too-many-methods = { exclude = [ "test/", "src/Sql/Exception/", + "src/Sql/Predicate/Exception/", "src/TableGateway/Exception/", "src/Adapter/Exception/", "src/Sql/Predicate/Predicate.php", diff --git a/src/Sql/Predicate/Exception/InvalidArgumentException.php b/src/Sql/Predicate/Exception/InvalidArgumentException.php index df5081b3..4209eb36 100644 --- a/src/Sql/Predicate/Exception/InvalidArgumentException.php +++ b/src/Sql/Predicate/Exception/InvalidArgumentException.php @@ -6,6 +6,8 @@ use PhpDb\Sql\Exception; +use function sprintf; + /** * @api */ @@ -15,6 +17,8 @@ class InvalidArgumentException extends Exception\InvalidArgumentException final public const string INVALID_LIST_PREDICATE = 'A predicate without a string key must be a predicate, an expression or a string'; + final public const string MIXED_CASE_MAGIC_PROPERTY = 'Magic property "%s" must be given in lower or upper case'; + final public const string MISSING_IDENTIFIER = 'Identifier must be specified'; final public const string MISSING_LEFT_EXPRESSION = 'Left expression must be specified'; @@ -76,6 +80,11 @@ public static function forMissingValueSet(): self return new self(self::MISSING_VALUE_SET); } + public static function forMixedCaseMagicProperty(string $name): self + { + return new self(sprintf(self::MIXED_CASE_MAGIC_PROPERTY, $name)); + } + public static function forPredicateWithStringKey(): self { return new self(self::PREDICATE_WITH_STRING_KEY); diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php index f3e3a1b1..c9d2afb7 100644 --- a/src/Sql/Predicate/Predicate.php +++ b/src/Sql/Predicate/Predicate.php @@ -6,8 +6,10 @@ use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\ExpressionInterface; +use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; use PhpDb\Sql\Predicate\Exception\RuntimeException; +use function in_array; use function strtolower; /** @@ -380,18 +382,21 @@ private function combineNextWith(string $operator): self /** * Overloading - * Overloads "or", "and", "nest", and "unnest" + * Overloads "or", "and", "nest" and "unnest", in lower or upper case * * @throws RuntimeException If "unnest" is accessed on a predicate that is not nested. + * @throws InvalidArgumentException If one of those names is given in mixed case. */ public function __get(string $name): self { - return match (strtolower($name)) { - 'or' => $this->combineNextWith(self::OP_OR), - 'and' => $this->combineNextWith(self::OP_AND), - 'nest' => $this->nest(), - 'unnest' => $this->unnest(), - default => $this, + return match ($name) { + 'or', 'OR' => $this->combineNextWith(self::OP_OR), + 'and', 'AND' => $this->combineNextWith(self::OP_AND), + 'nest', 'NEST' => $this->nest(), + 'unnest', 'UNNEST' => $this->unnest(), + default => in_array(strtolower($name), ['or', 'and', 'nest', 'unnest'], strict: true) + ? throw InvalidArgumentException::forMixedCaseMagicProperty($name) + : $this, }; } } diff --git a/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php b/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php index 7bdf4ee8..78a97583 100644 --- a/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php +++ b/test/unit/Sql/Predicate/Exception/InvalidArgumentExceptionTest.php @@ -24,6 +24,7 @@ #[CoversMethod(InvalidArgumentException::class, 'forMissingMinValue')] #[CoversMethod(InvalidArgumentException::class, 'forMissingRightExpression')] #[CoversMethod(InvalidArgumentException::class, 'forMissingValueSet')] +#[CoversMethod(InvalidArgumentException::class, 'forMixedCaseMagicProperty')] #[CoversMethod(InvalidArgumentException::class, 'forPredicateWithStringKey')] final class InvalidArgumentExceptionTest extends TestCase { @@ -76,6 +77,11 @@ public static function namedConstructorProvider(): array [], InvalidArgumentException::MISSING_VALUE_SET, ], + 'mixed case magic property' => [ + 'forMixedCaseMagicProperty', + ['Or'], + InvalidArgumentException::MIXED_CASE_MAGIC_PROPERTY, + ], 'predicate with string key' => [ 'forPredicateWithStringKey', [], diff --git a/test/unit/Sql/Predicate/PredicateTest.php b/test/unit/Sql/Predicate/PredicateTest.php index 6e5b9e3c..f392440d 100644 --- a/test/unit/Sql/Predicate/PredicateTest.php +++ b/test/unit/Sql/Predicate/PredicateTest.php @@ -10,6 +10,7 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Expression; use PhpDb\Sql\Predicate\Between; +use PhpDb\Sql\Predicate\Exception\InvalidArgumentException; use PhpDb\Sql\Predicate\Exception\RuntimeException; use PhpDb\Sql\Predicate\In; use PhpDb\Sql\Predicate\IsNull; @@ -30,6 +31,7 @@ use function array_diff; use function array_map; +use function sprintf; #[Group('unit')] final class PredicateTest extends TestCase @@ -68,12 +70,24 @@ public static function magicCombinationProvider(): array return [ 'lower-case or' => ['or', Predicate::OP_AND, '%s IS NULL OR %s IS NULL'], 'upper-case OR' => ['OR', Predicate::OP_AND, '%s IS NULL OR %s IS NULL'], - 'mixed-case Or' => ['Or', Predicate::OP_AND, '%s IS NULL OR %s IS NULL'], 'lower-case and' => ['and', Predicate::OP_OR, '%s IS NULL AND %s IS NULL'], 'upper-case AND' => ['AND', Predicate::OP_OR, '%s IS NULL AND %s IS NULL'], ]; } + /** + * @return array + */ + public static function mixedCaseMagicPropertyProvider(): array + { + return [ + 'Or' => ['Or'], + 'aND' => ['aND'], + 'Nest' => ['Nest'], + 'unNest' => ['unNest'], + ]; + } + /** * @throws ErrorException */ @@ -440,7 +454,7 @@ public function magicCombinationAppliesOnlyToTheNextPredicate(): void #[Test] #[DataProvider('magicCombinationProvider')] - public function magicCombinationPropertyIgnoresCase( + public function magicCombinationPropertyAcceptsLowerOrUpperCase( string $property, string $defaultCombination, string $expectedSpec, @@ -462,6 +476,17 @@ public function magicGetNestReturnsNestedPredicate(): void static::assertNotSame($predicate, $nested); } + #[Test] + #[DataProvider('mixedCaseMagicPropertyProvider')] + public function magicGetThrowsForMixedCaseName(string $name): void + { + $predicate = new Predicate(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf(InvalidArgumentException::MIXED_CASE_MAGIC_PROPERTY, $name)); + $predicate->__get($name); + } + #[Test] public function magicGetUnknownPropertyReturnsSamePredicate(): void { From ebb162c1698f9d5f870dbd48c0ceaf060e0890a9 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 13:16:14 +1100 Subject: [PATCH 5/5] Accept the interface's resource in AbstractPdo::createStatement() Mago 1.56.2 reports the PDO driver's docblock as narrower than DriverInterface::createStatement(), which documents resource|string. The PDO driver now documents resource as well. It already treats any argument that is not a PDOStatement or a string as no SQL and prepares a fresh statement, so behaviour is unchanged. --- src/Adapter/Driver/Pdo/AbstractPdo.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Driver/Pdo/AbstractPdo.php b/src/Adapter/Driver/Pdo/AbstractPdo.php index a73ade04..de5d80c9 100644 --- a/src/Adapter/Driver/Pdo/AbstractPdo.php +++ b/src/Adapter/Driver/Pdo/AbstractPdo.php @@ -53,7 +53,7 @@ public function checkEnvironment(): bool /** * todo: this needs improved * - * @param PDOStatement|string $sqlOrResource + * @param PDOStatement|resource|string|null $sqlOrResource A non-PDO resource is ignored, as with null */ #[Override] public function createStatement($sqlOrResource = null): StatementInterface