From 717a9ac1694da684603f72c1eea80d236b491f28 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 16:52:42 +1000 Subject: [PATCH 01/22] Add typed extractors for array assertions in Sql tests - Add ExpressionDataAssertionsTrait with argumentAt()/predicateEntryAt() - Replace unchecked fixed-index access in Predicate tests and SelectTest - Document Expression::getParameters() return type --- src/Sql/Expression.php | 1 + .../Sql/ExpressionDataAssertionsTrait.php | 47 ++++++++++++ test/unit/Sql/Predicate/BetweenTest.php | 39 +++++----- test/unit/Sql/Predicate/ExpressionTest.php | 42 ++++++----- test/unit/Sql/Predicate/InTest.php | 63 ++++++++-------- test/unit/Sql/Predicate/IsNullTest.php | 9 ++- test/unit/Sql/Predicate/LikeTest.php | 27 ++++--- test/unit/Sql/Predicate/NotBetweenTest.php | 40 +++++----- test/unit/Sql/Predicate/NotInTest.php | 52 ++++++------- test/unit/Sql/Predicate/NotLikeTest.php | 15 ++-- test/unit/Sql/Predicate/OperatorTest.php | 15 ++-- test/unit/Sql/Predicate/PredicateSetTest.php | 54 ++++++------- test/unit/Sql/Predicate/PredicateTest.php | 75 ++++++++++--------- test/unit/Sql/SelectTest.php | 70 ++++++++--------- 14 files changed, 315 insertions(+), 234 deletions(-) create mode 100644 test/unit/Sql/ExpressionDataAssertionsTrait.php diff --git a/src/Sql/Expression.php b/src/Sql/Expression.php index 9903e21d..f3f9fd81 100644 --- a/src/Sql/Expression.php +++ b/src/Sql/Expression.php @@ -96,6 +96,7 @@ public function setParameters( return $this; } + /** @return ArgumentInterface[] */ public function getParameters(): array { return $this->parameters; diff --git a/test/unit/Sql/ExpressionDataAssertionsTrait.php b/test/unit/Sql/ExpressionDataAssertionsTrait.php new file mode 100644 index 00000000..02928be7 --- /dev/null +++ b/test/unit/Sql/ExpressionDataAssertionsTrait.php @@ -0,0 +1,47 @@ + $values + */ + private static function argumentAt(array $values, int $index): ArgumentInterface + { + Assert::assertArrayHasKey($index, $values); + Assert::assertInstanceOf(ArgumentInterface::class, $values[$index] ?? null); + + return $values[$index]; + } + + /** + * @param array $predicates + * @return array{string, PredicateInterface} + */ + private static function predicateEntryAt(array $predicates, int $index): array + { + Assert::assertArrayHasKey($index, $predicates); + Assert::assertIsArray($predicates[$index] ?? null); + + $entry = $predicates[$index]; + Assert::assertArrayHasKey(0, $entry); + Assert::assertArrayHasKey(1, $entry); + Assert::assertIsString($entry[0] ?? null); + Assert::assertInstanceOf(PredicateInterface::class, $entry[1] ?? null); + + return [$entry[0], $entry[1]]; + } +} \ No newline at end of file diff --git a/test/unit/Sql/Predicate/BetweenTest.php b/test/unit/Sql/Predicate/BetweenTest.php index c00ece8c..153cb577 100644 --- a/test/unit/Sql/Predicate/BetweenTest.php +++ b/test/unit/Sql/Predicate/BetweenTest.php @@ -10,6 +10,7 @@ use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Predicate\Between; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; @@ -25,6 +26,8 @@ #[CoversMethod(Between::class, 'getExpressionData')] final class BetweenTest extends TestCase { + use ExpressionDataAssertionsTrait; + protected Between $between; #[Override] @@ -193,19 +196,19 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(3, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo.bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo.bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify min value argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals(10, $values[1]->getValue()); - self::assertEquals(ArgumentType::Value, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals(10, $value1->getValue()); + self::assertEquals(ArgumentType::Value, $value1->getType()); // Verify max value argument - self::assertInstanceOf(ArgumentInterface::class, $values[2]); - self::assertEquals(19, $values[2]->getValue()); - self::assertEquals(ArgumentType::Value, $values[2]->getType()); + $value2 = self::argumentAt($values, 2); + self::assertEquals(19, $value2->getValue()); + self::assertEquals(ArgumentType::Value, $value2->getType()); $this->between->setIdentifier(Argument::value(10)) ->setMinValue(Argument::identifier('foo.bar')) @@ -221,19 +224,19 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(3, $values); // Verify identifier argument (passed as Value type) - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals(10, $values[0]->getValue()); - self::assertEquals(ArgumentType::Value, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals(10, $value0->getValue()); + self::assertEquals(ArgumentType::Value, $value0->getType()); // Verify min value argument (passed as Identifier type) - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('foo.bar', $values[1]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('foo.bar', $value1->getValue()); + self::assertEquals(ArgumentType::Identifier, $value1->getType()); // Verify max value argument (passed as Identifier type) - self::assertInstanceOf(ArgumentInterface::class, $values[2]); - self::assertEquals('foo.baz', $values[2]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[2]->getType()); + $value2 = self::argumentAt($values, 2); + self::assertEquals('foo.baz', $value2->getValue()); + self::assertEquals(ArgumentType::Identifier, $value2->getType()); } public function testGetExpressionDataThrowsExceptionWhenIdentifierNotSet(): void diff --git a/test/unit/Sql/Predicate/ExpressionTest.php b/test/unit/Sql/Predicate/ExpressionTest.php index f82cbd27..2c02f471 100644 --- a/test/unit/Sql/Predicate/ExpressionTest.php +++ b/test/unit/Sql/Predicate/ExpressionTest.php @@ -7,15 +7,17 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Argument\Select; use PhpDb\Sql\Argument\Value; -use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Predicate\Expression; use PhpDb\Sql\Predicate\IsNull; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; final class ExpressionTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testEmptyConstructorYieldsEmptyLiteralAndParameter(): void { $expression = new Expression(); @@ -156,12 +158,14 @@ public function testParameterIsMutable(): void // Verify the first mutation occurred - getParameters returns an array $parameters1 = $expression->getParameters(); self::assertCount(2, $parameters1); - self::assertInstanceOf(ArgumentInterface::class, $parameters1[0]); - self::assertEquals('foo', $parameters1[0]->getValue()); - self::assertEquals(ArgumentType::Value, $parameters1[0]->getType()); - self::assertInstanceOf(ArgumentInterface::class, $parameters1[1]); - self::assertEquals('bar', $parameters1[1]->getValue()); - self::assertEquals(ArgumentType::Value, $parameters1[1]->getType()); + + $value0 = self::argumentAt($parameters1, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Value, $value0->getType()); + + $value1 = self::argumentAt($parameters1, 1); + self::assertEquals('bar', $value1->getValue()); + self::assertEquals(ArgumentType::Value, $value1->getType()); // Second mutation with different data to verify mutability $expression->setParameters(['baz', 'qux', 'quux']); @@ -169,19 +173,21 @@ public function testParameterIsMutable(): void // Verify the instance was actually mutated - parameters are accumulated $parameters2 = $expression->getParameters(); self::assertCount(5, $parameters2); // 2 original + 3 new = 5 total + // First two are still there - self::assertEquals('foo', $parameters2[0]->getValue()); - self::assertEquals('bar', $parameters2[1]->getValue()); + self::assertEquals('foo', self::argumentAt($parameters2, 0)->getValue()); + self::assertEquals('bar', self::argumentAt($parameters2, 1)->getValue()); + // New ones were appended - self::assertInstanceOf(ArgumentInterface::class, $parameters2[2]); - self::assertEquals('baz', $parameters2[2]->getValue()); - self::assertEquals(ArgumentType::Value, $parameters2[2]->getType()); - self::assertInstanceOf(ArgumentInterface::class, $parameters2[3]); - self::assertEquals('qux', $parameters2[3]->getValue()); - self::assertEquals(ArgumentType::Value, $parameters2[3]->getType()); - self::assertInstanceOf(ArgumentInterface::class, $parameters2[4]); - self::assertEquals('quux', $parameters2[4]->getValue()); - self::assertEquals(ArgumentType::Value, $parameters2[4]->getType()); + $value2 = self::argumentAt($parameters2, 2); + self::assertEquals('baz', $value2->getValue()); + self::assertEquals(ArgumentType::Value, $value2->getType()); + $value3 = self::argumentAt($parameters2, 3); + self::assertEquals('qux', $value3->getValue()); + self::assertEquals(ArgumentType::Value, $value3->getType()); + $value4 = self::argumentAt($parameters2, 4); + self::assertEquals('quux', $value4->getValue()); + self::assertEquals(ArgumentType::Value, $value4->getType()); } public function testRetrievingWherePartsReturnsSpecificationArrayOfLiteralAndParametersAndArrayOfTypes(): void diff --git a/test/unit/Sql/Predicate/InTest.php b/test/unit/Sql/Predicate/InTest.php index 691eacdc..a7e98298 100644 --- a/test/unit/Sql/Predicate/InTest.php +++ b/test/unit/Sql/Predicate/InTest.php @@ -12,6 +12,7 @@ use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\Predicate\In; use PhpDb\Sql\Select; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; @@ -25,6 +26,8 @@ #[Group('unit')] final class InTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testEmptyConstructorYieldsNullIdentifierAndValueSet(): void { $in = new In(); @@ -134,14 +137,14 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo.bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo.bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify value set argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals([1, 2, 3], $values[1]->getValue()); - self::assertEquals(ArgumentType::Values, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals([1, 2, 3], $value1->getValue()); + self::assertEquals(ArgumentType::Values, $value1->getType()); // Test with typed value sets $in->setIdentifier('foo.bar') @@ -161,18 +164,18 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo.bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo.bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify value set argument with types - self::assertInstanceOf(ArgumentInterface::class, $values[1]); + $value1 = self::argumentAt($values, 1); self::assertEquals([ [1 => ArgumentType::Literal], [2 => ArgumentType::Value], [3 => ArgumentType::Literal], - ], $values[1]->getValue()); - self::assertEquals(ArgumentType::Values, $values[1]->getType()); + ], $value1->getValue()); + self::assertEquals(ArgumentType::Values, $value1->getType()); } public function testGetExpressionDataWithSubselect(): void @@ -190,14 +193,14 @@ public function testGetExpressionDataWithSubselect(): void self::assertCount(2, $values); // Verify value argument (passed as value type) - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo', $values[0]->getValue()); - self::assertEquals(ArgumentType::Value, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Value, $value0->getType()); // Verify subselect argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertSame($select, $values[1]->getValue()); - self::assertEquals(ArgumentType::Select, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertSame($select, $value1->getValue()); + self::assertEquals(ArgumentType::Select, $value1->getType()); } public function testGetExpressionDataWithEmptyValues(): void @@ -225,14 +228,14 @@ public function testGetExpressionDataWithSubselectAndIdentifier(): void self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify subselect argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertSame($select, $values[1]->getValue()); - self::assertEquals(ArgumentType::Select, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertSame($select, $value1->getValue()); + self::assertEquals(ArgumentType::Select, $value1->getType()); } public function testGetExpressionDataWithSubselectAndArrayIdentifier(): void @@ -250,14 +253,14 @@ public function testGetExpressionDataWithSubselectAndArrayIdentifier(): void self::assertCount(2, $values); // Verify array identifiers argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals(['foo', 'bar'], $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifiers, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals(['foo', 'bar'], $value0->getValue()); + self::assertEquals(ArgumentType::Identifiers, $value0->getType()); // Verify subselect argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertSame($select, $values[1]->getValue()); - self::assertEquals(ArgumentType::Select, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertSame($select, $value1->getValue()); + self::assertEquals(ArgumentType::Select, $value1->getType()); } public function testGetExpressionDataThrowsExceptionWhenIdentifierNotSet(): void diff --git a/test/unit/Sql/Predicate/IsNullTest.php b/test/unit/Sql/Predicate/IsNullTest.php index 4659bf48..ff1e47c0 100644 --- a/test/unit/Sql/Predicate/IsNullTest.php +++ b/test/unit/Sql/Predicate/IsNullTest.php @@ -10,6 +10,7 @@ use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\Predicate\IsNotNull; use PhpDb\Sql\Predicate\IsNull; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; @@ -23,6 +24,8 @@ #[Group('unit')] final class IsNullTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testEmptyConstructorYieldsNullIdentifier(): void { $isNotNull = new IsNotNull(); @@ -94,9 +97,9 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(1, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo.bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo.bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); } public function testGetExpressionDataThrowsExceptionWhenIdentifierNotSet(): void diff --git a/test/unit/Sql/Predicate/LikeTest.php b/test/unit/Sql/Predicate/LikeTest.php index 70534e90..c0d71109 100644 --- a/test/unit/Sql/Predicate/LikeTest.php +++ b/test/unit/Sql/Predicate/LikeTest.php @@ -9,6 +9,7 @@ use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\Predicate\Like; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; @@ -22,6 +23,8 @@ #[CoversMethod(Like::class, 'getExpressionData')] final class LikeTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testConstructEmptyArgs(): void { $like = new Like(); @@ -110,14 +113,14 @@ public function testGetExpressionData(): void self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify like expression argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('Foo%', $values[1]->getValue()); - self::assertEquals(ArgumentType::Value, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('Foo%', $value1->getValue()); + self::assertEquals(ArgumentType::Value, $value1->getType()); $like = new Like(Argument::value('Foo%'), Argument::identifier('bar')); @@ -131,14 +134,14 @@ public function testGetExpressionData(): void self::assertCount(2, $values); // Verify identifier argument (now with Value type) - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('Foo%', $values[0]->getValue()); - self::assertEquals(ArgumentType::Value, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('Foo%', $value0->getValue()); + self::assertEquals(ArgumentType::Value, $value0->getType()); // Verify like expression argument (now with Identifier type) - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('bar', $values[1]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('bar', $value1->getValue()); + self::assertEquals(ArgumentType::Identifier, $value1->getType()); } public function testInstanceOfPerSetters(): void diff --git a/test/unit/Sql/Predicate/NotBetweenTest.php b/test/unit/Sql/Predicate/NotBetweenTest.php index ffbb0601..53df3591 100644 --- a/test/unit/Sql/Predicate/NotBetweenTest.php +++ b/test/unit/Sql/Predicate/NotBetweenTest.php @@ -6,9 +6,9 @@ use Override; use PhpDb\Sql\Argument; -use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Predicate\NotBetween; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; @@ -16,6 +16,8 @@ #[CoversMethod(NotBetween::class, 'getExpressionData')] final class NotBetweenTest extends TestCase { + use ExpressionDataAssertionsTrait; + protected NotBetween $notBetween; #[Override] @@ -46,19 +48,19 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(3, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo.bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo.bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify min value argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals(10, $values[1]->getValue()); - self::assertEquals(ArgumentType::Value, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals(10, $value1->getValue()); + self::assertEquals(ArgumentType::Value, $value1->getType()); // Verify max value argument - self::assertInstanceOf(ArgumentInterface::class, $values[2]); - self::assertEquals(19, $values[2]->getValue()); - self::assertEquals(ArgumentType::Value, $values[2]->getType()); + $value2 = self::argumentAt($values, 2); + self::assertEquals(19, $value2->getValue()); + self::assertEquals(ArgumentType::Value, $value2->getType()); $this->notBetween ->setIdentifier(Argument::value(10)) @@ -75,18 +77,18 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(3, $values); // Verify identifier argument (passed as Value type) - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals(10, $values[0]->getValue()); - self::assertEquals(ArgumentType::Value, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals(10, $value0->getValue()); + self::assertEquals(ArgumentType::Value, $value0->getType()); // Verify min value argument (passed as Identifier type) - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('foo.bar', $values[1]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('foo.bar', $value1->getValue()); + self::assertEquals(ArgumentType::Identifier, $value1->getType()); // Verify max value argument (passed as Identifier type) - self::assertInstanceOf(ArgumentInterface::class, $values[2]); - self::assertEquals('foo.baz', $values[2]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[2]->getType()); + $value2 = self::argumentAt($values, 2); + self::assertEquals('foo.baz', $value2->getValue()); + self::assertEquals(ArgumentType::Identifier, $value2->getType()); } } diff --git a/test/unit/Sql/Predicate/NotInTest.php b/test/unit/Sql/Predicate/NotInTest.php index c75f1287..f4460d85 100644 --- a/test/unit/Sql/Predicate/NotInTest.php +++ b/test/unit/Sql/Predicate/NotInTest.php @@ -5,14 +5,16 @@ namespace PhpDbTest\Sql\Predicate; use PhpDb\Sql\Argument; -use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Predicate\NotIn; use PhpDb\Sql\Select; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\TestCase; final class NotInTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAndValuesAndArrayOfTypes(): void { $in = new NotIn(); @@ -29,14 +31,14 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo.bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo.bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify value set argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals([1, 2, 3], $values[1]->getValue()); - self::assertEquals(ArgumentType::Values, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals([1, 2, 3], $value1->getValue()); + self::assertEquals(ArgumentType::Values, $value1->getType()); } public function testGetExpressionDataWithSubselect(): void @@ -54,14 +56,14 @@ public function testGetExpressionDataWithSubselect(): void self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify subselect argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertSame($select, $values[1]->getValue()); - self::assertEquals(ArgumentType::Select, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertSame($select, $value1->getValue()); + self::assertEquals(ArgumentType::Select, $value1->getType()); } public function testGetExpressionDataWithSubselectAndIdentifier(): void @@ -79,14 +81,14 @@ public function testGetExpressionDataWithSubselectAndIdentifier(): void self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify subselect argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertSame($select, $values[1]->getValue()); - self::assertEquals(ArgumentType::Select, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertSame($select, $value1->getValue()); + self::assertEquals(ArgumentType::Select, $value1->getType()); } public function testGetExpressionDataWithSubselectAndArrayIdentifier(): void @@ -104,13 +106,13 @@ public function testGetExpressionDataWithSubselectAndArrayIdentifier(): void self::assertCount(2, $values); // Verify array identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals(['foo', 'bar'], $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifiers, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals(['foo', 'bar'], $value0->getValue()); + self::assertEquals(ArgumentType::Identifiers, $value0->getType()); // Verify subselect argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertSame($select, $values[1]->getValue()); - self::assertEquals(ArgumentType::Select, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertSame($select, $value1->getValue()); + self::assertEquals(ArgumentType::Select, $value1->getType()); } } diff --git a/test/unit/Sql/Predicate/NotLikeTest.php b/test/unit/Sql/Predicate/NotLikeTest.php index 4f7228f7..aae87b96 100644 --- a/test/unit/Sql/Predicate/NotLikeTest.php +++ b/test/unit/Sql/Predicate/NotLikeTest.php @@ -8,10 +8,13 @@ use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Predicate\Like; use PhpDb\Sql\Predicate\NotLike; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\TestCase; final class NotLikeTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testConstructEmptyArgs(): void { $notLike = new NotLike(); @@ -100,14 +103,14 @@ public function testGetExpressionData(): void self::assertCount(2, $values); // Verify identifier argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('bar', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('bar', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify like expression argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('Foo%', $values[1]->getValue()); - self::assertEquals(ArgumentType::Value, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('Foo%', $value1->getValue()); + self::assertEquals(ArgumentType::Value, $value1->getType()); } public function testInstanceOfPerSetters(): void diff --git a/test/unit/Sql/Predicate/OperatorTest.php b/test/unit/Sql/Predicate/OperatorTest.php index 6afeaf4d..5f5805cc 100644 --- a/test/unit/Sql/Predicate/OperatorTest.php +++ b/test/unit/Sql/Predicate/OperatorTest.php @@ -12,6 +12,7 @@ use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\Expression; use PhpDb\Sql\Predicate\Operator; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; @@ -27,6 +28,8 @@ #[Group('unit')] final class OperatorTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testEmptyConstructorYieldsNullLeftAndRightValues(): void { $operator = new Operator(); @@ -162,14 +165,14 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfLeftAndRightA self::assertCount(2, $values); // Verify left argument - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo', $values[0]->getValue()); - self::assertEquals(ArgumentType::Value, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Value, $value0->getType()); // Verify right argument - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('foo.bar', $values[1]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('foo.bar', $value1->getValue()); + self::assertEquals(ArgumentType::Identifier, $value1->getType()); } public function testGetExpressionDataThrowsExceptionWhenLeftNotSet(): void diff --git a/test/unit/Sql/Predicate/PredicateSetTest.php b/test/unit/Sql/Predicate/PredicateSetTest.php index d5e36985..4bd53385 100644 --- a/test/unit/Sql/Predicate/PredicateSetTest.php +++ b/test/unit/Sql/Predicate/PredicateSetTest.php @@ -16,6 +16,7 @@ use PhpDb\Sql\Predicate\PredicateInterface; use PhpDb\Sql\Predicate\PredicateSet; use PhpDbTest\DeprecatedAssertionsTrait; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -36,6 +37,7 @@ final class PredicateSetTest extends TestCase { use DeprecatedAssertionsTrait; + use ExpressionDataAssertionsTrait; public function testEmptyConstructorYieldsCountOfZero(): void { @@ -129,33 +131,33 @@ public function testAddPredicates(): void $predicates = (array) $this->readAttribute($predicateSet, 'predicates'); self::assertCount(7, $predicates); - self::assertIsArray($predicates[0]); - self::assertEquals('AND', $predicates[0][0]); - self::assertInstanceOf(Literal::class, $predicates[0][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Literal::class, $predicate); - self::assertIsArray($predicates[1]); - self::assertEquals('AND', $predicates[1][0]); - self::assertInstanceOf(Expression::class, $predicates[1][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 1); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Expression::class, $predicate); - self::assertIsArray($predicates[2]); - self::assertEquals('AND', $predicates[2][0]); - self::assertInstanceOf(Operator::class, $predicates[2][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 2); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Operator::class, $predicate); - self::assertIsArray($predicates[3]); - self::assertEquals('OR', $predicates[3][0]); - self::assertInstanceOf(Literal::class, $predicates[3][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 3); + self::assertEquals('OR', $operator); + self::assertInstanceOf(Literal::class, $predicate); - self::assertIsArray($predicates[4]); - self::assertEquals('AND', $predicates[4][0]); - self::assertInstanceOf(IsNull::class, $predicates[4][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 4); + self::assertEquals('AND', $operator); + self::assertInstanceOf(IsNull::class, $predicate); - self::assertIsArray($predicates[5]); - self::assertEquals('AND', $predicates[5][0]); - self::assertInstanceOf(In::class, $predicates[5][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 5); + self::assertEquals('AND', $operator); + self::assertInstanceOf(In::class, $predicate); - self::assertIsArray($predicates[6]); - self::assertEquals('AND', $predicates[6][0]); - self::assertInstanceOf(IsNotNull::class, $predicates[6][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 6); + self::assertEquals('AND', $operator); + self::assertInstanceOf(IsNotNull::class, $predicate); $predicateSet->addPredicates(function (PredicateSet $what) use ($predicateSet): void { self::assertSame($predicateSet, $what); @@ -183,10 +185,10 @@ public function testAddPredicatesWithExpression(): void $predicates = (array) $this->readAttribute($predicateSet, 'predicates'); self::assertCount(1, $predicates); - self::assertIsArray($predicates[0]); - self::assertEquals('AND', $predicates[0][0]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertEquals('AND', $operator); // Should be wrapped in a Predicate\Expression - self::assertInstanceOf(Expression::class, $predicates[0][1]); + self::assertInstanceOf(Expression::class, $predicate); // Verify the expression data is preserved $expressionData = $predicateSet->getExpressionData(); @@ -210,8 +212,8 @@ public function testAddPredicatesWithMultipleExpressions(): void $predicates = (array) $this->readAttribute($predicateSet, 'predicates'); self::assertCount(2, $predicates); - self::assertInstanceOf(Expression::class, $predicates[0][1]); - self::assertInstanceOf(Expression::class, $predicates[1][1]); + self::assertInstanceOf(Expression::class, self::predicateEntryAt($predicates, 0)[1]); + self::assertInstanceOf(Expression::class, self::predicateEntryAt($predicates, 1)[1]); } public function testAddPredicateThrowsOnInvalidCombination(): void diff --git a/test/unit/Sql/Predicate/PredicateTest.php b/test/unit/Sql/Predicate/PredicateTest.php index 91eabbc9..74a56e49 100644 --- a/test/unit/Sql/Predicate/PredicateTest.php +++ b/test/unit/Sql/Predicate/PredicateTest.php @@ -13,6 +13,7 @@ use PhpDb\Sql\Predicate\Predicate; use PhpDb\Sql\Predicate\PredicateInterface; use PhpDb\Sql\Select; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; @@ -20,6 +21,8 @@ #[Group('unit')] final class PredicateTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testEqualToCreatesOperatorPredicate(): void { $predicate = new Predicate(); @@ -32,8 +35,8 @@ public function testEqualToCreatesOperatorPredicate(): void self::assertEquals('%s = %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testNotEqualToCreatesOperatorPredicate(): void @@ -48,8 +51,8 @@ public function testNotEqualToCreatesOperatorPredicate(): void self::assertEquals('%s != %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testLessThanCreatesOperatorPredicate(): void @@ -64,8 +67,8 @@ public function testLessThanCreatesOperatorPredicate(): void self::assertEquals('%s < %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testGreaterThanCreatesOperatorPredicate(): void @@ -80,8 +83,8 @@ public function testGreaterThanCreatesOperatorPredicate(): void self::assertEquals('%s > %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testLessThanOrEqualToCreatesOperatorPredicate(): void @@ -96,8 +99,8 @@ public function testLessThanOrEqualToCreatesOperatorPredicate(): void self::assertEquals('%s <= %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testGreaterThanOrEqualToCreatesOperatorPredicate(): void @@ -112,8 +115,8 @@ public function testGreaterThanOrEqualToCreatesOperatorPredicate(): void self::assertEquals('%s >= %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testLikeCreatesLikePredicate(): void @@ -128,8 +131,8 @@ public function testLikeCreatesLikePredicate(): void self::assertEquals('%s LIKE %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testNotLikeCreatesLikePredicate(): void @@ -144,8 +147,8 @@ public function testNotLikeCreatesLikePredicate(): void self::assertEquals('%s NOT LIKE %s', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testLiteralCreatesLiteralPredicate(): void @@ -170,7 +173,7 @@ public function testIsNullCreatesIsNullPredicate(): void self::assertEquals('%s IS NULL', $expressionData['spec']); self::assertCount(1, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); } public function testIsNotNullCreatesIsNotNullPredicate(): void @@ -184,7 +187,7 @@ public function testIsNotNullCreatesIsNotNullPredicate(): void self::assertEquals('%s IS NOT NULL', $expressionData['spec']); self::assertCount(1, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); } public function testInCreatesInPredicate(): void @@ -199,8 +202,8 @@ public function testInCreatesInPredicate(): void self::assertEquals('%s IN (%s, %s)', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testNotInCreatesNotInPredicate(): void @@ -215,8 +218,8 @@ public function testNotInCreatesNotInPredicate(): void self::assertEquals('%s NOT IN (%s, %s)', $expressionData['spec']); self::assertCount(2, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($expression, $expressionData['values'][1]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($expression, self::argumentAt($expressionData['values'], 1)); } public function testBetweenCreatesBetweenPredicate(): void @@ -232,9 +235,9 @@ public function testBetweenCreatesBetweenPredicate(): void self::assertEquals('%s BETWEEN %s AND %s', $expressionData['spec']); self::assertCount(3, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($minValue, $expressionData['values'][1]); - self::assertEquals($maxValue, $expressionData['values'][2]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($minValue, self::argumentAt($expressionData['values'], 1)); + self::assertEquals($maxValue, self::argumentAt($expressionData['values'], 2)); } public function testBetweenCreatesNotBetweenPredicate(): void @@ -250,9 +253,9 @@ public function testBetweenCreatesNotBetweenPredicate(): void self::assertEquals('%s NOT BETWEEN %s AND %s', $expressionData['spec']); self::assertCount(3, $expressionData['values']); - self::assertEquals($identifier, $expressionData['values'][0]); - self::assertEquals($minValue, $expressionData['values'][1]); - self::assertEquals($maxValue, $expressionData['values'][2]); + self::assertEquals($identifier, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($minValue, self::argumentAt($expressionData['values'], 1)); + self::assertEquals($maxValue, self::argumentAt($expressionData['values'], 2)); } public function testCanChainPredicateFactoriesBetweenOperators(): void @@ -275,10 +278,10 @@ public function testCanChainPredicateFactoriesBetweenOperators(): void self::assertCount(4, $expressionData['values']); // Verify combined spec self::assertEquals('%s IS NULL OR %s IS NOT NULL AND %s = %s', $expressionData['spec']); - self::assertEquals($identifier1, $expressionData['values'][0]); - self::assertEquals($identifier2, $expressionData['values'][1]); - self::assertEquals($identifier3, $expressionData['values'][2]); - self::assertEquals($expression3, $expressionData['values'][3]); + self::assertEquals($identifier1, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($identifier2, self::argumentAt($expressionData['values'], 1)); + self::assertEquals($identifier3, self::argumentAt($expressionData['values'], 2)); + self::assertEquals($expression3, self::argumentAt($expressionData['values'], 3)); } public function testCanNestPredicates(): void @@ -302,10 +305,10 @@ public function testCanNestPredicates(): void self::assertCount(4, $expressionData['values']); // Verify combined spec with nested brackets self::assertEquals('%s IS NULL AND (%s IS NOT NULL AND %s = %s)', $expressionData['spec']); - self::assertEquals($identifier1, $expressionData['values'][0]); - self::assertEquals($identifier2, $expressionData['values'][1]); - self::assertEquals($identifier3, $expressionData['values'][2]); - self::assertEquals($expression3, $expressionData['values'][3]); + self::assertEquals($identifier1, self::argumentAt($expressionData['values'], 0)); + self::assertEquals($identifier2, self::argumentAt($expressionData['values'], 1)); + self::assertEquals($identifier3, self::argumentAt($expressionData['values'], 2)); + self::assertEquals($expression3, self::argumentAt($expressionData['values'], 3)); } #[TestDox('Unit test: Test expression() is chainable and returns proper values')] diff --git a/test/unit/Sql/SelectTest.php b/test/unit/Sql/SelectTest.php index 1dcd508b..07cd4b94 100644 --- a/test/unit/Sql/SelectTest.php +++ b/test/unit/Sql/SelectTest.php @@ -74,6 +74,7 @@ final class SelectTest extends TestCase { use AdapterTestTrait; + use ExpressionDataAssertionsTrait; public function testConstruct(): void { @@ -228,10 +229,10 @@ public function testWhereArgument1IsString(): void $where = $select->getRawState('where'); $predicates = $where->getPredicates(); self::assertCount(1, $predicates); - self::assertIsArray($predicates[0]); - self::assertInstanceOf(Predicate\Expression::class, $predicates[0][1]); - self::assertEquals(Predicate\PredicateSet::OP_AND, $predicates[0][0]); - self::assertEquals('x = ?', $predicates[0][1]->getExpression()); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Predicate\Expression::class, $predicate); + self::assertEquals(Predicate\PredicateSet::OP_AND, $operator); + self::assertEquals('x = ?', $predicate->getExpression()); $select = new Select(); $select->where('x = y'); @@ -239,8 +240,8 @@ public function testWhereArgument1IsString(): void /** @var Where $where */ $where = $select->getRawState('where'); $predicates = $where->getPredicates(); - self::assertIsArray($predicates[0]); - self::assertInstanceOf(Literal::class, $predicates[0][1]); + [, $literal] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Literal::class, $literal); } #[TestDox('unit test: Test where() will accept an array with a string key (containing ?) used as an @@ -256,11 +257,11 @@ public function testWhereArgument1IsAssociativeArrayContainingReplacementCharact $expression = new Value(5); self::assertCount(1, $predicates); - self::assertIsArray($predicates[0]); - self::assertInstanceOf(Predicate\Expression::class, $predicates[0][1]); - self::assertEquals(Predicate\PredicateSet::OP_AND, $predicates[0][0]); - self::assertEquals('foo > ?', $predicates[0][1]->getExpression()); - self::assertEquals([$expression], $predicates[0][1]->getParameters()); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Predicate\Expression::class, $predicate); + self::assertEquals(Predicate\PredicateSet::OP_AND, $operator); + self::assertEquals('foo > ?', $predicate->getExpression()); + self::assertEquals([$expression], $predicate->getParameters()); } #[TestDox('unit test: Test where() will accept any array with string key (without ?) to be used @@ -279,18 +280,17 @@ public function testWhereArgument1IsAssociativeArrayNotContainingReplacementChar $where = $select->getRawState('where'); $predicates = $where->getPredicates(); self::assertCount(2, $predicates); - self::assertIsArray($predicates[0]); - self::assertIsArray($predicates[1]); - - self::assertInstanceOf(Operator::class, $predicates[0][1]); - self::assertEquals(Predicate\PredicateSet::OP_AND, $predicates[0][0]); - self::assertEquals($identifier1, $predicates[0][1]->getLeft()); - self::assertEquals($expression1, $predicates[0][1]->getRight()); - - self::assertInstanceOf(Operator::class, $predicates[1][1]); - self::assertEquals(Predicate\PredicateSet::OP_AND, $predicates[1][0]); - self::assertEquals($identifier2, $predicates[1][1]->getLeft()); - self::assertEquals($expression2, $predicates[1][1]->getRight()); + [$operator1, $predicate1] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Operator::class, $predicate1); + self::assertEquals(Predicate\PredicateSet::OP_AND, $operator1); + self::assertEquals($identifier1, $predicate1->getLeft()); + self::assertEquals($expression1, $predicate1->getRight()); + + [$operator2, $predicate2] = self::predicateEntryAt($predicates, 1); + self::assertInstanceOf(Operator::class, $predicate2); + self::assertEquals(Predicate\PredicateSet::OP_AND, $operator2); + self::assertEquals($identifier2, $predicate2->getLeft()); + self::assertEquals($expression2, $predicate2->getRight()); $select = new Select(); $select->where(['x = y']); @@ -298,8 +298,8 @@ public function testWhereArgument1IsAssociativeArrayNotContainingReplacementChar /** @var Where $where */ $where = $select->getRawState('where'); $predicates = $where->getPredicates(); - self::assertIsArray($predicates[0]); - self::assertInstanceOf(Literal::class, $predicates[0][1]); + [, $literal] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Literal::class, $literal); } #[TestDox(' @@ -327,10 +327,10 @@ public function testWhereArgument1IsIndexedArray(): void $where = $select->getRawState('where'); $predicates = $where->getPredicates(); self::assertCount(1, $predicates); - self::assertIsArray($predicates[0]); - self::assertInstanceOf(Literal::class, $predicates[0][1]); - self::assertEquals(Predicate\PredicateSet::OP_AND, $predicates[0][0]); - self::assertEquals('name = "Ralph"', $predicates[0][1]->getLiteral()); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Literal::class, $predicate); + self::assertEquals(Predicate\PredicateSet::OP_AND, $operator); + self::assertEquals('name = "Ralph"', $predicate->getLiteral()); } #[TestDox('unit test: Test where() will accept an indexed array to be used by joining string expressions, @@ -344,10 +344,10 @@ public function testWhereArgument1IsIndexedArrayArgument2IsOr(): void $where = $select->getRawState('where'); $predicates = $where->getPredicates(); self::assertCount(1, $predicates); - self::assertIsArray($predicates[0]); - self::assertInstanceOf(Literal::class, $predicates[0][1]); - self::assertEquals(Predicate\PredicateSet::OP_OR, $predicates[0][0]); - self::assertEquals('name = "Ralph"', $predicates[0][1]->getLiteral()); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertInstanceOf(Literal::class, $predicate); + self::assertEquals(Predicate\PredicateSet::OP_OR, $operator); + self::assertEquals('name = "Ralph"', $predicate->getLiteral()); } #[TestDox('unit test: Test where() will accept a closure to be executed with Where object as argument')] @@ -375,8 +375,8 @@ public function testWhereArgument1IsPredicate(): void /** @var Where $where */ $where = $select->getRawState('where'); $predicates = $where->getPredicates(); - self::assertIsArray($predicates[0]); - self::assertSame($predicate, $predicates[0][1]); + [, $actualPredicate] = self::predicateEntryAt($predicates, 0); + self::assertSame($predicate, $actualPredicate); } #[TestDox('unit test: Test where() will accept a Where object')] From 5079ce31d8e344cf418f3562b00be14c38056633 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 17:08:10 +1000 Subject: [PATCH 02/22] Replace reflection with ConcreteSql test asset in AbstractSqlTest - Add ConcreteSql exposing AbstractSql protected methods with typed wrappers - Replace ReflectionMethod invocations with direct calls in AbstractSqlTest - Convert Ddl constraint/column tests to argumentAt() - Guard preg_match captures and cast key() results --- test/unit/Sql/AbstractSqlTest.php | 252 ++++-------------- test/unit/Sql/Ddl/Column/DecimalTest.php | 9 +- test/unit/Sql/Ddl/Column/TimestampTest.php | 15 +- .../Sql/Ddl/Constraint/ForeignKeyTest.php | 40 +-- test/unit/TestAsset/ConcreteSql.php | 96 +++++++ 5 files changed, 182 insertions(+), 230 deletions(-) create mode 100644 test/unit/TestAsset/ConcreteSql.php diff --git a/test/unit/Sql/AbstractSqlTest.php b/test/unit/Sql/AbstractSqlTest.php index ba288fe2..a925b210 100644 --- a/test/unit/Sql/AbstractSqlTest.php +++ b/test/unit/Sql/AbstractSqlTest.php @@ -9,7 +9,6 @@ use PhpDb\Adapter\Driver\DriverInterface; use PhpDb\Adapter\Driver\StatementInterface; use PhpDb\Adapter\ParameterContainer; -use PhpDb\Adapter\StatementContainer; use PhpDb\Sql\AbstractSql; use PhpDb\Sql\Argument; use PhpDb\Sql\Argument\Identifier; @@ -23,6 +22,7 @@ use PhpDb\Sql\Predicate; use PhpDb\Sql\Select; use PhpDb\Sql\TableIdentifier; +use PhpDbTest\TestAsset\ConcreteSql; use PhpDbTest\TestAsset\SelectDecorator; use PhpDbTest\TestAsset\TrustingSql92Platform; use PHPUnit\Framework\Attributes\CoversMethod; @@ -32,8 +32,6 @@ use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use ReflectionException; -use ReflectionMethod; use function count; use function current; @@ -60,7 +58,7 @@ #[CoversMethod(AbstractSql::class, 'localizeVariables')] final class AbstractSqlTest extends TestCase { - protected AbstractSql&MockObject $abstractSql; + private ConcreteSql $abstractSql; protected DriverInterface&MockObject $mockDriver; @@ -70,7 +68,7 @@ final class AbstractSqlTest extends TestCase #[Override] protected function setUp(): void { - $this->abstractSql = $this->getMockBuilder(AbstractSql::class)->onlyMethods([])->getMock(); + $this->abstractSql = new ConcreteSql(); $this->mockDriver = $this->getMockBuilder(DriverInterface::class)->getMock(); $this->mockDriver @@ -80,12 +78,9 @@ protected function setUp(): void $this->mockDriver ->expects($this->any()) ->method('formatParameterName') - ->willReturnCallback(fn($x): string => ':' . $x); + ->willReturnCallback(fn(string $x): string => ':' . $x); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWithoutParameterContainer(): void { $expression = new Expression('? > ? AND y < ?', [new Identifier('x'), 5, 10]); @@ -94,9 +89,6 @@ public function testProcessExpressionWithoutParameterContainer(): void self::assertEquals("\"x\" > '5' AND y < '10'", $sqlAndParams); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWithParameterContainerAndParameterizationTypeNamed(): void { $parameterContainer = new ParameterContainer(); @@ -109,13 +101,14 @@ public function testProcessExpressionWithParameterContainerAndParameterizationTy self::assertMatchesRegularExpression('#"x" > :expr\d+Param1 AND y < :expr\d+Param2#', $sqlAndParams); // Verify parameter names and values - preg_match('#expr(\d+)Param1#', key($parameters), $matches); + preg_match('#expr(\d+)Param1#', (string) key($parameters), $matches); + self::assertTrue(isset($matches[1])); $expressionNumber = $matches[1]; - self::assertMatchesRegularExpression('#expr\d+Param1#', key($parameters)); + self::assertMatchesRegularExpression('#expr\d+Param1#', (string) key($parameters)); self::assertEquals(5, current($parameters)); next($parameters); - self::assertMatchesRegularExpression('#expr\d+Param2#', key($parameters)); + self::assertMatchesRegularExpression('#expr\d+Param2#', (string) key($parameters)); self::assertEquals(10, current($parameters)); // Verify next invocation increments expression number @@ -124,15 +117,13 @@ public function testProcessExpressionWithParameterContainerAndParameterizationTy $parameters = $parameterContainer->getNamedArray(); - preg_match('#expr(\d+)Param1#', key($parameters), $matches); + preg_match('#expr(\d+)Param1#', (string) key($parameters), $matches); + self::assertTrue(isset($matches[1])); $expressionNumberNext = $matches[1]; self::assertEquals(1, (int) $expressionNumberNext - (int) $expressionNumber); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWorksWithExpressionContainingStringParts(): void { $expression = new Predicate\Expression('x = ?', 5); @@ -143,13 +134,10 @@ public function testProcessExpressionWorksWithExpressionContainingStringParts(): self::assertEquals("(x = '5')", $sqlAndParams); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWorksWithExpressionContainingSelectObject(): void { $select = new Select(); - $select->from('x')->where->like('bar', 'Foo%'); + $select->from('x')->where(new Predicate\Like('bar', 'Foo%')); $expression = new Predicate\In('x', $select); $predicateSet = new Predicate\PredicateSet([new Predicate\PredicateSet([$expression])]); @@ -158,9 +146,6 @@ public function testProcessExpressionWorksWithExpressionContainingSelectObject() self::assertEquals('("x" IN (SELECT "x".* FROM "x" WHERE "bar" LIKE \'Foo%\'))', $sqlAndParams); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWorksWithExpressionContainingExpressionObject(): void { $expression = new Predicate\Operator( @@ -173,9 +158,6 @@ public function testProcessExpressionWorksWithExpressionContainingExpressionObje self::assertEquals('"release_date" = FROM_UNIXTIME(\'100000000\')', $sqlAndParams); } - /** - * @throws ReflectionException - */ #[Group('7407')] public function testProcessExpressionWorksWithExpressionObjectWithPercentageSigns(): void { @@ -186,9 +168,6 @@ public function testProcessExpressionWorksWithExpressionObjectWithPercentageSign self::assertSame($expressionString, $sqlString); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWorksWithNamedParameterPrefix(): void { $parameterContainer = new ParameterContainer(); @@ -199,9 +178,6 @@ public function testProcessExpressionWorksWithNamedParameterPrefix(): void self::assertSame($namedParameterPrefix . '1', (string) key($parameterContainer->getNamedArray())); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWorksWithNamedParameterPrefixContainingWhitespace(): void { $parameterContainer = new ParameterContainer(); @@ -212,40 +188,25 @@ public function testProcessExpressionWorksWithNamedParameterPrefixContainingWhit self::assertSame('string__containing__white__space1', key($parameterContainer->getNamedArray())); } - /** - * @throws ReflectionException - */ public function testResolveColumnValueWithNull(): void { - $method = new ReflectionMethod($this->abstractSql, 'resolveColumnValue'); - - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callResolveColumnValue( null, new TrustingSql92Platform(), - $this->mockDriver, - null, - null + $this->mockDriver ); self::assertEquals('NULL', $result); } - /** - * @throws ReflectionException - */ public function testResolveColumnValueWithSelect(): void { $select = new Select('foo'); - $method = new ReflectionMethod($this->abstractSql, 'resolveColumnValue'); - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callResolveColumnValue( $select, new TrustingSql92Platform(), - $this->mockDriver, - null, - null + $this->mockDriver ); self::assertStringContainsString('SELECT', $result); @@ -253,84 +214,60 @@ public function testResolveColumnValueWithSelect(): void self::assertStringEndsWith(')', $result); } - /** - * @throws ReflectionException - */ public function testResolveColumnValueWithArrayAndFromTable(): void { - $method = new ReflectionMethod($this->abstractSql, 'resolveColumnValue'); - - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callResolveColumnValue( [ 'column' => 'id', 'isIdentifier' => true, 'fromTable' => 'table.', ], new TrustingSql92Platform(), - $this->mockDriver, - null, - null + $this->mockDriver ); self::assertStringContainsString('table.', $result); self::assertStringContainsString('id', $result); } - /** - * @throws ReflectionException - */ public function testResolveTableWithTableIdentifierAndSchema(): void { - $table = new TableIdentifier('users', 'public'); - $method = new ReflectionMethod($this->abstractSql, 'resolveTable'); + $table = new TableIdentifier('users', 'public'); - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callResolveTable( $table, new TrustingSql92Platform(), - $this->mockDriver, - null + $this->mockDriver ); + self::assertIsString($result); self::assertStringContainsString('public', $result); self::assertStringContainsString('users', $result); } - /** - * @throws ReflectionException - */ public function testResolveTableWithSelect(): void { $select = new Select('foo'); - $method = new ReflectionMethod($this->abstractSql, 'resolveTable'); - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callResolveTable( $select, new TrustingSql92Platform(), - $this->mockDriver, - null + $this->mockDriver ); + self::assertIsString($result); self::assertStringStartsWith('(', $result); self::assertStringEndsWith(')', $result); self::assertStringContainsString('SELECT', $result); } - /** - * @throws ReflectionException - */ public function testProcessSubSelectWithParameterContainer(): void { $select = new Select('foo'); $select->where(['id' => 5]); - $method = new ReflectionMethod($this->abstractSql, 'processSubSelect'); - $parameterContainer = new ParameterContainer(); - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callProcessSubSelect( $select, new TrustingSql92Platform(), $this->mockDriver, @@ -341,29 +278,19 @@ public function testProcessSubSelectWithParameterContainer(): void self::assertGreaterThan(0, count($parameterContainer->getNamedArray())); } - /** - * @throws ReflectionException - */ public function testProcessSubSelectWithoutParameterContainer(): void { $select = new Select('foo'); - $method = new ReflectionMethod($this->abstractSql, 'processSubSelect'); - - $result = $method->invoke( - $this->abstractSql, + $result = $this->abstractSql->callProcessSubSelect( $select, new TrustingSql92Platform(), - $this->mockDriver, - null + $this->mockDriver ); self::assertStringContainsString('SELECT', $result); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWithValuesArgument(): void { $expression = new Expression( @@ -384,9 +311,6 @@ public function testProcessExpressionWithValuesArgument(): void self::assertStringContainsString('"id"', $sqlAndParams); } - /** - * @throws ReflectionException - */ public function testProcessExpressionWithIdentifiersArgument(): void { $expression = new Expression('? IN (SELECT col1, col2 FROM bar)', [ @@ -400,12 +324,10 @@ public function testProcessExpressionWithIdentifiersArgument(): void } /** - * @throws ReflectionException + * @throws RuntimeException */ public function testCreateSqlFromSpecificationThrowsOnParameterCountMismatch(): void { - $method = new ReflectionMethod($this->abstractSql, 'createSqlFromSpecificationAndParameters'); - $specifications = [ 'SELECT %1$s FROM %2$s' => [ [1 => '%1$s', 'combinedby' => ', '], @@ -415,20 +337,15 @@ public function testCreateSqlFromSpecificationThrowsOnParameterCountMismatch(): $this->expectException(RuntimeException::class); $this->expectExceptionMessage('A number of parameters was found that is not supported by this specification'); - $method->invoke($this->abstractSql, $specifications, ['col1', 'table', 'extra']); + $this->abstractSql->callCreateSqlFromSpecificationAndParameters($specifications, ['col1', 'table', 'extra']); } - /** - * @throws ReflectionException - */ protected function invokeProcessExpressionMethod( ExpressionInterface $expression, ParameterContainer|null $parameterContainer = null, string|null $namedParameterPrefix = null - ): string|StatementContainer { - $method = new ReflectionMethod($this->abstractSql, 'processExpression'); - return $method->invoke( - $this->abstractSql, + ): string { + return $this->abstractSql->callProcessExpression( $expression, new TrustingSql92Platform(), $this->mockDriver, @@ -437,147 +354,86 @@ protected function invokeProcessExpressionMethod( ); } - /** - * @throws ReflectionException - */ public function testProcessJoinWithArrayAlias(): void { $join = new Join(); $join->join(['b' => 'bar'], 'foo.id = b.foo_id'); - $method = new ReflectionMethod($this->abstractSql, 'processJoin'); - $result = $method->invoke( - $this->abstractSql, - $join, - new TrustingSql92Platform(), - null, - null - ); + $result = $this->abstractSql->callProcessJoin($join, new TrustingSql92Platform()); self::assertNotNull($result); + self::assertIsString($result[0][0][1] ?? null); self::assertStringContainsString('AS', $result[0][0][1]); } - /** - * @throws ReflectionException - */ public function testProcessJoinWithTableIdentifier(): void { $join = new Join(); $join->join(new TableIdentifier('bar', 'myschema'), 'foo.id = bar.foo_id'); - $method = new ReflectionMethod($this->abstractSql, 'processJoin'); - $result = $method->invoke( - $this->abstractSql, - $join, - new TrustingSql92Platform(), - null, - null - ); + $result = $this->abstractSql->callProcessJoin($join, new TrustingSql92Platform()); self::assertNotNull($result); + self::assertIsString($result[0][0][1] ?? null); self::assertStringContainsString('"myschema"', $result[0][0][1]); self::assertStringContainsString('"bar"', $result[0][0][1]); } - /** - * @throws ReflectionException - */ public function testProcessJoinWithPredicateExpressionOnClause(): void { $join = new Join(); $join->join('bar', new Predicate\Expression('foo.id = bar.foo_id AND bar.active = 1')); - $method = new ReflectionMethod($this->abstractSql, 'processJoin'); - $result = $method->invoke( - $this->abstractSql, - $join, - new TrustingSql92Platform(), - null, - null - ); + $result = $this->abstractSql->callProcessJoin($join, new TrustingSql92Platform()); self::assertNotNull($result); + self::assertIsString($result[0][0][2] ?? null); self::assertStringContainsString('foo.id = bar.foo_id', $result[0][0][2]); } - /** - * @throws ReflectionException - */ public function testProcessJoinReturnsNullWhenEmpty(): void { - $method = new ReflectionMethod($this->abstractSql, 'processJoin'); - $result = $method->invoke( - $this->abstractSql, - null, - new TrustingSql92Platform(), - null, - null - ); + $result = $this->abstractSql->callProcessJoin(null, new TrustingSql92Platform()); self::assertNull($result); } - /** - * @throws ReflectionException - */ public function testRenderTableWithAlias(): void { - $method = new ReflectionMethod($this->abstractSql, 'renderTable'); - $result = $method->invoke($this->abstractSql, '"foo"', '"f"'); + $result = $this->abstractSql->callRenderTable('"foo"', '"f"'); self::assertSame('"foo" AS "f"', $result); } - /** - * @throws ReflectionException - */ public function testProcessJoinWithExpressionNameViaArray(): void { $join = new Join(); $join->join(['x' => new Expression('LATERAL(SELECT 1)')], 'true'); - $method = new ReflectionMethod($this->abstractSql, 'processJoin'); - $result = $method->invoke( - $this->abstractSql, - $join, - new TrustingSql92Platform(), - null, - null - ); + $result = $this->abstractSql->callProcessJoin($join, new TrustingSql92Platform()); + self::assertIsString($result[0][0][1] ?? null); self::assertStringContainsString('LATERAL(SELECT 1)', $result[0][0][1]); } - /** - * @throws ReflectionException - */ public function testProcessJoinWithSelectSubqueryViaArray(): void { $subselect = new Select('bar'); $join = new Join(); $join->join(['b' => $subselect], 'foo.id = b.foo_id'); - $method = new ReflectionMethod($this->abstractSql, 'processJoin'); - $result = $method->invoke( - $this->abstractSql, - $join, - new TrustingSql92Platform(), - null, - null - ); + $result = $this->abstractSql->callProcessJoin($join, new TrustingSql92Platform()); + self::assertIsString($result[0][0][1] ?? null); self::assertStringContainsString('SELECT', $result[0][0][1]); self::assertStringContainsString('AS', $result[0][0][1]); } /** - * @throws ReflectionException + * @throws RuntimeException */ public function testCreateSqlFromSpecWithCombinedByScalarParam(): void { - $method = new ReflectionMethod($this->abstractSql, 'createSqlFromSpecificationAndParameters'); - $spec = [ 'SELECT %1$s FROM %2$s' => [ [1 => '%1$s', 'combinedby' => ', '], @@ -586,18 +442,16 @@ public function testCreateSqlFromSpecWithCombinedByScalarParam(): void ]; $params = [['col1'], 'table1']; - $result = $method->invoke($this->abstractSql, $spec, $params); + $result = $this->abstractSql->callCreateSqlFromSpecificationAndParameters($spec, $params); self::assertSame('SELECT col1 FROM table1', $result); } /** - * @throws ReflectionException + * @throws RuntimeException */ public function testCreateSqlFromSpecWithCombinedByThrowsOnUnsupportedCount(): void { - $method = new ReflectionMethod($this->abstractSql, 'createSqlFromSpecificationAndParameters'); - $spec = [ 'SELECT %1$s FROM %2$s' => [ [1 => '%1$s', 'combinedby' => ', '], @@ -608,16 +462,14 @@ public function testCreateSqlFromSpecWithCombinedByThrowsOnUnsupportedCount(): v $this->expectException(RuntimeException::class); $this->expectExceptionMessage('A number of parameters (2)'); - $method->invoke($this->abstractSql, $spec, $params); + $this->abstractSql->callCreateSqlFromSpecificationAndParameters($spec, $params); } /** - * @throws ReflectionException + * @throws RuntimeException */ public function testCreateSqlFromSpecWithNonCombinedByParam(): void { - $method = new ReflectionMethod($this->abstractSql, 'createSqlFromSpecificationAndParameters'); - $spec = [ 'FROM %1$s' => [ [1 => '%1$s'], @@ -625,18 +477,16 @@ public function testCreateSqlFromSpecWithNonCombinedByParam(): void ]; $params = [['my_table']]; - $result = $method->invoke($this->abstractSql, $spec, $params); + $result = $this->abstractSql->callCreateSqlFromSpecificationAndParameters($spec, $params); self::assertSame('FROM my_table', $result); } /** - * @throws ReflectionException + * @throws RuntimeException */ public function testCreateSqlFromSpecNonCombinedByThrowsOnUnsupportedCount(): void { - $method = new ReflectionMethod($this->abstractSql, 'createSqlFromSpecificationAndParameters'); - $spec = [ 'FROM %1$s' => [ [1 => '%1$s'], @@ -646,7 +496,7 @@ public function testCreateSqlFromSpecNonCombinedByThrowsOnUnsupportedCount(): vo $this->expectException(RuntimeException::class); $this->expectExceptionMessage('A number of parameters (2)'); - $method->invoke($this->abstractSql, $spec, $params); + $this->abstractSql->callCreateSqlFromSpecificationAndParameters($spec, $params); } public function testProcessExpressionThrowsOnUnknownArgumentType(): void diff --git a/test/unit/Sql/Ddl/Column/DecimalTest.php b/test/unit/Sql/Ddl/Column/DecimalTest.php index 27ee7845..7446176a 100644 --- a/test/unit/Sql/Ddl/Column/DecimalTest.php +++ b/test/unit/Sql/Ddl/Column/DecimalTest.php @@ -7,6 +7,7 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Ddl\Column\AbstractPrecisionColumn; use PhpDb\Sql\Ddl\Column\Decimal; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; @@ -19,6 +20,8 @@ #[CoversMethod(AbstractPrecisionColumn::class, 'getLengthExpression')] final class DecimalTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testGetExpressionData(): void { $column = new Decimal('foo', 10, 5); @@ -69,10 +72,10 @@ public function testGetExpressionDataWithNullDecimal(): void // Without decimal, length expression should be just the digits (as string) $values = $expressionData['values']; self::assertCount(3, $values); - self::assertEquals(Argument::identifier('amount'), $values[0]); - self::assertEquals(Argument::literal('DECIMAL'), $values[1]); + self::assertEquals(Argument::identifier('amount'), self::argumentAt($values, 0)); + self::assertEquals(Argument::literal('DECIMAL'), self::argumentAt($values, 1)); // The third value should be "10" (string representation) - self::assertEquals(Argument::literal((string) 10), $values[2]); + self::assertEquals(Argument::literal((string) 10), self::argumentAt($values, 2)); } public function testInheritanceFromAbstractPrecisionColumn(): void diff --git a/test/unit/Sql/Ddl/Column/TimestampTest.php b/test/unit/Sql/Ddl/Column/TimestampTest.php index 7712588f..a1dc0f9a 100644 --- a/test/unit/Sql/Ddl/Column/TimestampTest.php +++ b/test/unit/Sql/Ddl/Column/TimestampTest.php @@ -7,9 +7,9 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Argument\Identifier; use PhpDb\Sql\Argument\Literal; -use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Ddl\Column\AbstractTimestampColumn; use PhpDb\Sql\Ddl\Column\Timestamp; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; @@ -17,6 +17,8 @@ #[CoversMethod(AbstractTimestampColumn::class, 'getExpressionData')] final class TimestampTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testGetExpressionData(): void { $column = new Timestamp('foo'); @@ -45,13 +47,12 @@ public function testGetExpressionDataWithOnUpdateOption(): void // Should have 3 values: identifier, type, and ON UPDATE argument self::assertCount(3, $values); - self::assertEquals(new Identifier('created_at'), $values[0]); - self::assertEquals(new Literal('TIMESTAMP'), $values[1]); + self::assertEquals(new Identifier('created_at'), self::argumentAt($values, 0)); + self::assertEquals(new Literal('TIMESTAMP'), self::argumentAt($values, 1)); // Third value should be the ON UPDATE argument - self::assertInstanceOf(ArgumentInterface::class, $values[2]); // Verify it equals the expected Argument using factory method for consistency - self::assertEquals(new Literal('ON UPDATE CURRENT_TIMESTAMP'), $values[2]); + self::assertEquals(new Literal('ON UPDATE CURRENT_TIMESTAMP'), self::argumentAt($values, 2)); } public function testGetExpressionDataWithoutOnUpdateOption(): void @@ -63,8 +64,8 @@ public function testGetExpressionDataWithoutOnUpdateOption(): void // Should have 2 values: identifier and type (no ON UPDATE) $values = $expressionData['values']; self::assertCount(2, $values); - self::assertEquals(new Identifier('updated_at'), $values[0]); - self::assertEquals(Argument::literal('TIMESTAMP'), $values[1]); + self::assertEquals(new Identifier('updated_at'), self::argumentAt($values, 0)); + self::assertEquals(Argument::literal('TIMESTAMP'), self::argumentAt($values, 1)); } public function testGetExpressionDataWithCurrentTimestampDefault(): void diff --git a/test/unit/Sql/Ddl/Constraint/ForeignKeyTest.php b/test/unit/Sql/Ddl/Constraint/ForeignKeyTest.php index 60b52066..0e5e5e6d 100644 --- a/test/unit/Sql/Ddl/Constraint/ForeignKeyTest.php +++ b/test/unit/Sql/Ddl/Constraint/ForeignKeyTest.php @@ -4,10 +4,10 @@ namespace PhpDbTest\Sql\Ddl\Constraint; -use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\ArgumentType; use PhpDb\Sql\Ddl\Constraint\AbstractConstraint; use PhpDb\Sql\Ddl\Constraint\ForeignKey; +use PhpDbTest\Sql\ExpressionDataAssertionsTrait; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; @@ -32,6 +32,8 @@ #[CoversMethod(ForeignKey::class, 'getExpressionData')] final class ForeignKeyTest extends TestCase { + use ExpressionDataAssertionsTrait; + public function testSetName(): void { $fk = new ForeignKey('foo', 'bar', 'baz', 'bam'); @@ -149,33 +151,33 @@ public function testGetExpressionData(): void self::assertCount(6, $values); // Verify constraint name - self::assertInstanceOf(ArgumentInterface::class, $values[0]); - self::assertEquals('foo', $values[0]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[0]->getType()); + $value0 = self::argumentAt($values, 0); + self::assertEquals('foo', $value0->getValue()); + self::assertEquals(ArgumentType::Identifier, $value0->getType()); // Verify column name - self::assertInstanceOf(ArgumentInterface::class, $values[1]); - self::assertEquals('bar', $values[1]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[1]->getType()); + $value1 = self::argumentAt($values, 1); + self::assertEquals('bar', $value1->getValue()); + self::assertEquals(ArgumentType::Identifier, $value1->getType()); // Verify reference table - self::assertInstanceOf(ArgumentInterface::class, $values[2]); - self::assertEquals('baz', $values[2]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[2]->getType()); + $value2 = self::argumentAt($values, 2); + self::assertEquals('baz', $value2->getValue()); + self::assertEquals(ArgumentType::Identifier, $value2->getType()); // Verify reference column - self::assertInstanceOf(ArgumentInterface::class, $values[3]); - self::assertEquals('bam', $values[3]->getValue()); - self::assertEquals(ArgumentType::Identifier, $values[3]->getType()); + $value3 = self::argumentAt($values, 3); + self::assertEquals('bam', $value3->getValue()); + self::assertEquals(ArgumentType::Identifier, $value3->getType()); // Verify on delete rule - self::assertInstanceOf(ArgumentInterface::class, $values[4]); - self::assertEquals('CASCADE', $values[4]->getValue()); - self::assertEquals(ArgumentType::Literal, $values[4]->getType()); + $value4 = self::argumentAt($values, 4); + self::assertEquals('CASCADE', $value4->getValue()); + self::assertEquals(ArgumentType::Literal, $value4->getType()); // Verify on update rule - self::assertInstanceOf(ArgumentInterface::class, $values[5]); - self::assertEquals('SET NULL', $values[5]->getValue()); - self::assertEquals(ArgumentType::Literal, $values[5]->getType()); + $value5 = self::argumentAt($values, 5); + self::assertEquals('SET NULL', $value5->getValue()); + self::assertEquals(ArgumentType::Literal, $value5->getType()); } } diff --git a/test/unit/TestAsset/ConcreteSql.php b/test/unit/TestAsset/ConcreteSql.php new file mode 100644 index 00000000..21a0291a --- /dev/null +++ b/test/unit/TestAsset/ConcreteSql.php @@ -0,0 +1,96 @@ +processExpression($expression, $platform, $driver, $parameterContainer, $namedParameterPrefix); + } + + /** + * @param Select|array|string|int|bool|ExpressionInterface|null $column + */ + public function callResolveColumnValue( + Select|array|string|int|bool|ExpressionInterface|null $column, + PlatformInterface $platform, + ?DriverInterface $driver = null, + ?ParameterContainer $parameterContainer = null, + ?string $namedParameterPrefix = null + ): string { + return $this->resolveColumnValue($column, $platform, $driver, $parameterContainer, $namedParameterPrefix); + } + + /** + * @return string|array|null + */ + public function callResolveTable( + Select|string|TableIdentifier|null $table, + PlatformInterface $platform, + ?DriverInterface $driver = null, + ?ParameterContainer $parameterContainer = null + ): string|array|null { + return $this->resolveTable($table, $platform, $driver, $parameterContainer); + } + + public function callProcessSubSelect( + Select $subselect, + PlatformInterface $platform, + ?DriverInterface $driver = null, + ?ParameterContainer $parameterContainer = null + ): string { + return $this->processSubSelect($subselect, $platform, $driver, $parameterContainer); + } + + /** + * @param array|string $specifications + * @param array $parameters + * @throws RuntimeException + */ + public function callCreateSqlFromSpecificationAndParameters( + array|string $specifications, + array $parameters + ): string { + return $this->createSqlFromSpecificationAndParameters($specifications, $parameters); + } + + /** + * @return null|string[][][] + */ + public function callProcessJoin( + ?Join $joins, + PlatformInterface $platform, + ?DriverInterface $driver = null, + ?ParameterContainer $parameterContainer = null + ): array|null { + return $this->processJoin($joins, $platform, $driver, $parameterContainer); + } + + public function callRenderTable(string $table, ?string $alias = null): string + { + return $this->renderTable($table, $alias); + } +} \ No newline at end of file From 410f89678fe0bdab5c6837aef7f06fd0f8ba526b Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 17:10:22 +1000 Subject: [PATCH 03/22] Guard remaining fixed-index array access in Sql tests - Coalesce keyed state and decorator lookups inside assertions - Assert decorator tuple keys in AbstractSqlFunctionalTestCase --- test/unit/Sql/AbstractSqlFunctionalTestCase.php | 6 ++++-- test/unit/Sql/DeleteTest.php | 6 +++--- test/unit/Sql/ExpressionTest.php | 2 +- test/unit/Sql/Platform/AbstractPlatformTest.php | 2 +- test/unit/Sql/Platform/PlatformTest.php | 2 +- test/unit/Sql/UpdateTest.php | 10 +++++----- 6 files changed, 15 insertions(+), 13 deletions(-) diff --git a/test/unit/Sql/AbstractSqlFunctionalTestCase.php b/test/unit/Sql/AbstractSqlFunctionalTestCase.php index c93ddaae..5a416439 100644 --- a/test/unit/Sql/AbstractSqlFunctionalTestCase.php +++ b/test/unit/Sql/AbstractSqlFunctionalTestCase.php @@ -241,7 +241,7 @@ public static function dataProvider(): array /** @psalm-suppress MixedAssignment */ foreach ($testExpected as $platform => $expected) { $res[$index . '->' . $platform] = [ - 'sqlObject' => $test['sqlObject'], + 'sqlObject' => $test['sqlObject'] ?? null, 'platform' => $platform, 'expected' => $expected, ]; @@ -270,7 +270,7 @@ public function test(PreparableSqlInterface|SqlInterface $sqlObject, string $pla } } - $expectedString = is_string($expected) ? $expected : (string) $expected['string']; + $expectedString = is_string($expected) ? $expected : (string) ($expected['string'] ?? ''); if ($expectedString !== '') { self::assertInstanceOf(SqlInterface::class, $sqlObject); $actual = $sql->buildSqlString($sqlObject); @@ -294,6 +294,8 @@ protected function resolveDecorator( PlatformDecoratorInterface|array $decorator ): PlatformDecoratorInterface|MockObject|null { if (is_array($decorator)) { + self::assertTrue(isset($decorator[0], $decorator[1])); + /** @var class-string $classString */ $classString = $decorator[0]; $decoratorMock = $this->getMockBuilder($classString) diff --git a/test/unit/Sql/DeleteTest.php b/test/unit/Sql/DeleteTest.php index b11e428d..2d1ce488 100644 --- a/test/unit/Sql/DeleteTest.php +++ b/test/unit/Sql/DeleteTest.php @@ -242,9 +242,9 @@ public function testGetRawState(): void self::assertArrayHasKey('where', $rawState); self::assertArrayHasKey('emptyWhereProtection', $rawState); - self::assertEquals('foo', $rawState['table']); - self::assertInstanceOf(Where::class, $rawState['where']); - self::assertTrue($rawState['emptyWhereProtection']); + self::assertEquals('foo', $rawState['table'] ?? null); + self::assertInstanceOf(Where::class, $rawState['where'] ?? null); + self::assertTrue($rawState['emptyWhereProtection'] ?? null); } public function testGetRawStateWithKey(): void diff --git a/test/unit/Sql/ExpressionTest.php b/test/unit/Sql/ExpressionTest.php index 2ee05788..99ed8cbd 100644 --- a/test/unit/Sql/ExpressionTest.php +++ b/test/unit/Sql/ExpressionTest.php @@ -224,7 +224,7 @@ public function testSetParametersWrapsArrayInValuesArgument(): void $data = $expression->getExpressionData(); self::assertCount(2, $data['values']); - self::assertInstanceOf(Argument\Values::class, $data['values'][1]); + self::assertInstanceOf(Argument\Values::class, $data['values'][1] ?? null); } public function testGetExpressionDataUsesRegexWhenPlaceholderCountMismatches(): void diff --git a/test/unit/Sql/Platform/AbstractPlatformTest.php b/test/unit/Sql/Platform/AbstractPlatformTest.php index fe5429e8..de30d259 100644 --- a/test/unit/Sql/Platform/AbstractPlatformTest.php +++ b/test/unit/Sql/Platform/AbstractPlatformTest.php @@ -50,7 +50,7 @@ public function testSetAndGetTypeDecorator(): void $decorators = $this->platform->getDecorators(); self::assertArrayHasKey(Select::class, $decorators); - self::assertSame($decorator, $decorators[Select::class]); + self::assertSame($decorator, $decorators[Select::class] ?? null); } public function testGetTypeDecoratorReturnsSubjectWhenNoMatch(): void diff --git a/test/unit/Sql/Platform/PlatformTest.php b/test/unit/Sql/Platform/PlatformTest.php index ae6066dc..6bde4a47 100644 --- a/test/unit/Sql/Platform/PlatformTest.php +++ b/test/unit/Sql/Platform/PlatformTest.php @@ -113,7 +113,7 @@ public function testSetTypeDecoratorRegistersDecorator(): void $decorators = $platform->getDecorators(); self::assertArrayHasKey(Select::class, $decorators); - self::assertSame($decorator, $decorators[Select::class]); + self::assertSame($decorator, $decorators[Select::class] ?? null); } public function testGetTypeDecoratorReturnsSubjectWhenNoDecoratorRegistered(): void diff --git a/test/unit/Sql/UpdateTest.php b/test/unit/Sql/UpdateTest.php index d653fb69..3009e315 100644 --- a/test/unit/Sql/UpdateTest.php +++ b/test/unit/Sql/UpdateTest.php @@ -478,11 +478,11 @@ public function testGetRawStateReturnsAllState(): void self::assertArrayHasKey('emptyWhereProtection', $rawState); self::assertArrayHasKey('joins', $rawState); - self::assertEquals('foo', $rawState['table']); - self::assertEquals(['bar' => 'baz'], $rawState['set']); - self::assertInstanceOf(Where::class, $rawState['where']); - self::assertInstanceOf(Join::class, $rawState['joins']); - self::assertTrue($rawState['emptyWhereProtection']); + self::assertEquals('foo', $rawState['table'] ?? null); + self::assertEquals(['bar' => 'baz'], $rawState['set'] ?? null); + self::assertInstanceOf(Where::class, $rawState['where'] ?? null); + self::assertInstanceOf(Join::class, $rawState['joins'] ?? null); + self::assertTrue($rawState['emptyWhereProtection'] ?? null); } public function testJoinWithTableIdentifier(): void From a5f2533800ff697440aa6159b13d8b36d49316d1 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 17:23:03 +1000 Subject: [PATCH 04/22] Add project mago.toml - Port analyzer, linter, and formatter config from global settings - Pin php-version 8.2 - Register PHPUnit setUp/setUpBeforeClass as class initializers - Scope invalid-enum-case-value ignore to src/Sql/ --- mago.toml | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 mago.toml diff --git a/mago.toml b/mago.toml new file mode 100644 index 00000000..b9fe4f5f --- /dev/null +++ b/mago.toml @@ -0,0 +1,126 @@ +php-version = "8.2" + +[source] +paths = ["src", "test"] +includes = ["vendor"] + +[source.glob] +literal-separator = false + +[formatter] +preset = "default" +preserve-breaking-member-access-chain = true +preserve-breaking-member-access-chain-first-method-on-same-line = true +preserve-breaking-argument-list = true +preserve-breaking-array-like = true +preserve-breaking-parameter-list = true +preserve-breaking-attribute-list = true +preserve-breaking-conditional-expression = true +preserve-breaking-condition-expression = true +preserve-redundant-logical-binary-expression-parentheses = true +preserve-breaking-binary-expression = true +space-after-logical-not-unary-prefix-operator = true +always-break-named-arguments-list = true +align-assignment-like = true +sort-class-methods = true + +[linter] +minimum-fail-level = "Help" +integrations = ["laminas", "phpunit"] + +[linter.rules] +literal-named-argument = { enabled = true } +halstead = { effort-threshold = 7000 } +valid-docblock = { enabled = true } +no-negated-ternary = { enabled = true } +no-short-bool-cast = { enabled = true } +no-inline = { enabled = true } +no-array-accumulation-in-loop = { enabled = true } +no-literal-namespace-string = { enabled = true } +no-parameter-shadowing = { enabled = true } +no-side-effects-with-declarations = { enabled = true } +prefer-array-spread = { enabled = true } +prefer-explode-over-preg-split = { enabled = true } +prefer-first-class-callable = { check-functions = true } +prefer-test-attribute = { enabled = true } +require-namespace = { enabled = true } +prefer-pre-increment = { enabled = true } +prefer-self-return-type = { enabled = true } +sorted-integer-keys = { enabled = true } +yoda-conditions = { enabled = true } +file-name = { enabled = true } +array-style = { level = "Help" } +block-statement = { level = "Help" } +braced-string-interpolation = { enabled = true, level = "Help" } +no-alias-function = { level = "Help" } +no-php-tag-terminator = { level = "Help" } +no-trailing-space = { level = "Help" } +string-style = { enabled = true, level = "Help" } +ambiguous-constant-access = { enabled = true } +ambiguous-function-call = { enabled = true } +class-name = { psr = true } +function-name = { camel = true, exclude = ["src/functions/*"] } +interface-name = { psr = true } +method-name = { enabled = true, camel = true } +no-fully-qualified-global-class-like = { enabled = true } +no-fully-qualified-global-constant = { enabled = true } +no-fully-qualified-global-function = { enabled = true } +property-name = { enabled = true } +trait-name = { psr = true } +variable-name = { enabled = true, camel = true, either = false } +too-many-methods = { exclude = ["test/"] } +no-iterator-to-array-in-foreach = { enabled = true } +no-duplicate-match-arm = { level = "Help" } +no-empty-comment = { level = "Help" } +no-empty-loop = { level = "Help" } +no-is-null = { enabled = true, level = "Help" } +no-null-property-init = { enabled = true } +no-redundant-else = { enabled = true } +disallowed-type-instantiation = { enabled = true } +no-debug-symbols = { level = "Help" } +no-assign-in-argument = { enabled = true } +no-dead-store = { enabled = true } +no-redundant-variable = { enabled = true } +no-unused-closure-capture = { enabled = true } +no-unused-global = { enabled = true } +no-unused-static = { enabled = true } +switch-continue-to-break = { enabled = true } +invalid-open-tag = { level = "Help" } + +[analyzer] +plugins = ["psl", "psr-container"] +find-unused-definitions = true +find-unused-expressions = true +find-overly-wide-return-types = true +analyze-dead-code = true +memoize-properties = true +check-throws = true +unchecked-exceptions = [ + 'Error', + 'LogicException', + 'PHPUnit\Framework\Exception', + 'PHPUnit\Framework\MockObject\Exception', + 'PHPUnit\Event\NoPreviousThrowableException', +] +unchecked-exception-classes = [] +check-missing-override = true +find-unused-parameters = true +strict-list-index-checks = true +strict-array-index-existence = true +allow-array-truthy-operand = false +no-boolean-literal-comparison = true +check-missing-type-hints = true +check-arrow-function-missing-type-hints = true +register-super-globals = false +check-property-initialization = true +class-initializers = [ + 'PHPUnit\Framework\TestCase::setUp', + 'PHPUnit\Framework\TestCase::setUpBeforeClass', +] +check-use-statements = true +check-name-casing = true +enforce-class-finality = true +require-api-or-internal = true +ignore = [ + { code = "invalid-enum-case-value", in = "src/Sql/" }, +] \ No newline at end of file From 8c2607a018cc83706ee5e863e38f132ad0c2334b Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 22:02:16 +1000 Subject: [PATCH 05/22] Type Sql builder shapes and clear AbstractSql/Join/Select analysis - Define the specification/parameter/column array shapes as @phpstan-type on AbstractSql and import them where consumed - Tighten ExpressionInterface::getExpressionData() values, Expression and the Argument flatten helper to list - Widen processExpressionParameterName() to accept null bound values and return a non-null name - Rework createSqlFromSpecificationAndParameters() to interpret the spec DSL through typed locals instead of unchecked array access - Type the Join specification structure (JoinName/JoinSpecification) and guard the iterator and constructor edges - Type Select properties, process* returns and setter params; guard the mixed access in the process bodies and fix the resolveTable tuple type Signed-off-by: Simon Mundy --- src/Sql/AbstractSql.php | 122 ++++++++++++++++++++++---------- src/Sql/Expression.php | 4 +- src/Sql/ExpressionInterface.php | 2 +- src/Sql/Join.php | 42 +++++++++-- src/Sql/Select.php | 108 ++++++++++++++++++++-------- 5 files changed, 203 insertions(+), 75 deletions(-) diff --git a/src/Sql/AbstractSql.php b/src/Sql/AbstractSql.php index eb5350a6..250e246a 100644 --- a/src/Sql/AbstractSql.php +++ b/src/Sql/AbstractSql.php @@ -31,6 +31,16 @@ use function strtoupper; use function vsprintf; +/** + * @phpstan-type ParameterSpecification array + * @phpstan-type SpecificationArray array> + * @phpstan-type SpecResult array> + * @phpstan-type ColumnDescriptor array{ + * column: Select|string|int|bool|ExpressionInterface|null, + * fromTable?: string, + * isIdentifier?: bool + * } + */ abstract class AbstractSql implements SqlInterface { protected SqlInterface|PreparableSqlInterface|null $subject = null; @@ -38,7 +48,7 @@ abstract class AbstractSql implements SqlInterface /** * Specifications for Sql String generation * - * @var string[]|array[] + * @var array */ protected array $specifications = []; @@ -49,6 +59,7 @@ abstract class AbstractSql implements SqlInterface */ protected array $processInfo = ['paramPrefix' => '', 'subselectCount' => 0]; + /** @var array */ protected array $instanceParameterIndex = []; /** @@ -72,6 +83,10 @@ protected function buildSqlString( $sqls = []; foreach ($this->specifications as $name => $specification) { + /** + * @var SpecResult|string|null $result + * @mago-expect analysis:string-member-selector + */ $result = $this->{'process' . $name}( $platform, $driver, @@ -127,9 +142,7 @@ protected function processExpression( . str_replace([' ', "\t", "\n", "\r"], '__', $namedParameterPrefix); } - if (! isset($this->instanceParameterIndex[$namedParameterPrefix])) { - $this->instanceParameterIndex[$namedParameterPrefix] = 1; - } + $this->instanceParameterIndex[$namedParameterPrefix] ??= 1; $expressionParamIndex = &$this->instanceParameterIndex[$namedParameterPrefix]; $expressionValues = $this->flattenExpressionValues($expressionValues); @@ -137,7 +150,8 @@ protected function processExpression( foreach ($expressionValues as $vIndex => $argument) { $values[] = match (true) { - $argument instanceof Value => $parameterContainer instanceof ParameterContainer + $argument instanceof Value => + $parameterContainer instanceof ParameterContainer && $driver instanceof DriverInterface ? $this->processExpressionParameterName( $argument->getValue(), $namedParameterPrefix, @@ -167,8 +181,8 @@ protected function processExpression( /** * Flattens expression values, expanding Values arguments * - * @param ArgumentInterface[] $arguments - * @return ArgumentInterface[] + * @param list $arguments + * @return list */ protected function flattenExpressionValues(array $arguments): array { @@ -227,6 +241,7 @@ protected function processIdentifiersArgument( ArgumentInterface $argument, PlatformInterface $platform ): string { + /** @var list $identifiers */ $identifiers = $argument->getValue(); $processedIdentifiers = []; @@ -238,12 +253,12 @@ protected function processIdentifiersArgument( } protected function processExpressionParameterName( - int|float|string|bool $value, + int|float|string|bool|null $value, string $namedParameterPrefix, int &$expressionParamIndex, DriverInterface $driver, ParameterContainer $parameterContainer - ): ?string { + ): string { $name = $namedParameterPrefix . $expressionParamIndex++; $parameterContainer->offsetSet($name, $value); @@ -251,25 +266,31 @@ protected function processExpressionParameterName( } /** + * @param SpecificationArray|string $specifications + * @param SpecResult $parameters * @throws Exception\RuntimeException */ protected function createSqlFromSpecificationAndParameters(array|string $specifications, array $parameters): string { if (is_string($specifications)) { - return vsprintf($specifications, $parameters); + /** @var array $flatParameters */ + $flatParameters = $parameters; + return vsprintf($specifications, $flatParameters); } $parametersCount = count($parameters); - foreach ($specifications as $specificationString => $paramSpecs) { - if ($parametersCount === count($paramSpecs)) { + $specificationString = null; + $paramSpecs = []; + foreach ($specifications as $candidateSpec => $candidateParamSpecs) { + if ($parametersCount === count($candidateParamSpecs)) { + $specificationString = $candidateSpec; + $paramSpecs = $candidateParamSpecs; break; } - - unset($specificationString, $paramSpecs); } - if (! isset($specificationString)) { + if ($specificationString === null) { throw new Exception\RuntimeException( 'A number of parameters was found that is not supported by this specification' ); @@ -277,9 +298,13 @@ protected function createSqlFromSpecificationAndParameters(array|string $specifi $topParameters = []; foreach ($parameters as $position => $paramsForPosition) { - if (isset($paramSpecs[$position]['combinedby'])) { + $paramSpec = $paramSpecs[$position] ?? null; + + if ($paramSpec !== null && isset($paramSpec['combinedby'])) { $multiParamValues = []; - foreach ($paramsForPosition as $multiParamsForPosition) { + /** @var array> $rows */ + $rows = $paramsForPosition; + foreach ($rows as $multiParamsForPosition) { if (is_array($multiParamsForPosition)) { $ppCount = count($multiParamsForPosition); } else { @@ -287,33 +312,40 @@ protected function createSqlFromSpecificationAndParameters(array|string $specifi $multiParamsForPosition = [$multiParamsForPosition]; } - if (! isset($paramSpecs[$position][$ppCount])) { + $format = $paramSpec[$ppCount] ?? null; + if ($format === null) { throw new Exception\RuntimeException(sprintf( 'A number of parameters (%d) was found that is not supported by this specification', $ppCount )); } - $multiParamValues[] = vsprintf($paramSpecs[$position][$ppCount], $multiParamsForPosition); + $multiParamValues[] = vsprintf($format, $multiParamsForPosition); } - $topParameters[] = implode($paramSpecs[$position]['combinedby'], $multiParamValues); - } elseif ($paramSpecs[$position] !== null) { - $ppCount = count($paramsForPosition); - if (! isset($paramSpecs[$position][$ppCount])) { + $topParameters[] = implode($paramSpec['combinedby'], $multiParamValues); + } elseif ($paramSpec !== null) { + /** @var array $paramsForRow */ + $paramsForRow = $paramsForPosition; + $ppCount = count($paramsForRow); + + $format = $paramSpec[$ppCount] ?? null; + if ($format === null) { throw new Exception\RuntimeException(sprintf( 'A number of parameters (%d) was found that is not supported by this specification', $ppCount )); } - $topParameters[] = vsprintf($paramSpecs[$position][$ppCount], $paramsForPosition); + $topParameters[] = vsprintf($format, $paramsForRow); } else { $topParameters[] = $paramsForPosition; } } - return vsprintf($specificationString, $topParameters); + /** @var array $flatTopParameters */ + $flatTopParameters = $topParameters; + return vsprintf($specificationString, $flatTopParameters); } protected function processSubSelect( @@ -364,7 +396,7 @@ protected function processJoin( $joinNameValue = $join['name']; if (is_array($joinNameValue)) { $joinName = current($joinNameValue); - $joinAs = $platform->quoteIdentifier(key($joinNameValue)); + $joinAs = $platform->quoteIdentifier((string) key($joinNameValue)); } else { $joinName = $joinNameValue; } @@ -379,7 +411,7 @@ protected function processJoin( } elseif ($joinName instanceof Select) { $joinName = '(' . $this->processSubSelect($joinName, $platform, $driver, $parameterContainer) . ')'; } else { - $joinName = $platform->quoteIdentifier($joinName); + $joinName = $platform->quoteIdentifier((string) $joinName); } $joinSpecArgArray[$j] = [ @@ -406,6 +438,9 @@ protected function processJoin( return [$joinSpecArgArray]; } + /** + * @param Select|string|int|bool|ExpressionInterface|null|ColumnDescriptor $column + */ protected function resolveColumnValue( Select|array|string|int|bool|ExpressionInterface|null $column, PlatformInterface $platform, @@ -419,7 +454,7 @@ protected function resolveColumnValue( $isIdentifier = false; $fromTable = ''; if (is_array($column)) { - $isIdentifier = (bool) ($column['isIdentifier'] ?? false); + $isIdentifier = $column['isIdentifier'] ?? false; $fromTable = $column['fromTable'] ?? ''; $column = $column['column']; } @@ -437,10 +472,13 @@ protected function resolveColumnValue( } return $isIdentifier - ? $fromTable . $platform->quoteIdentifierInFragment($column) - : $platform->quoteValue($column); + ? $fromTable . $platform->quoteIdentifierInFragment((string) $column) + : $platform->quoteValue((string) $column); } + /** + * @return string|null + */ protected function resolveTable( Select|string|TableIdentifier|null $table, PlatformInterface $platform, @@ -449,29 +487,37 @@ protected function resolveTable( ): string|array|null { $schema = null; if ($table instanceof TableIdentifier) { - [$table, $schema] = $table->getTableAndSchema(); + $schema = $table->getSchema(); + $table = $table->getTable(); } if ($table instanceof Select) { - $table = '(' . $this->processSubSelect($table, $platform, $driver, $parameterContainer) . ')'; + $rendered = '(' . $this->processSubSelect($table, $platform, $driver, $parameterContainer) . ')'; } elseif ($table) { - $table = $platform->quoteIdentifier($table); + $rendered = $platform->quoteIdentifier($table); + } else { + $rendered = $table; } - if ($schema && $table) { - $table = $platform->quoteIdentifier($schema) . $platform->getIdentifierSeparator() . $table; + if ($schema && $rendered) { + $rendered = $platform->quoteIdentifier($schema) . $platform->getIdentifierSeparator() . $rendered; } - return $table; + return $rendered; } protected function localizeVariables(): void { - if (! $this instanceof PlatformDecoratorInterface) { + if (! $this instanceof PlatformDecoratorInterface || $this->subject === null) { return; } - foreach (get_object_vars($this->subject) as $name => $value) { + /** @var array $subjectVars */ + $subjectVars = get_object_vars($this->subject); + + /** @mago-expect analysis:mixed-assignment */ + foreach ($subjectVars as $name => $value) { + /** @mago-expect analysis:string-member-selector */ $this->{$name} = $value; } } diff --git a/src/Sql/Expression.php b/src/Sql/Expression.php index f3f9fd81..2778e0e7 100644 --- a/src/Sql/Expression.php +++ b/src/Sql/Expression.php @@ -27,7 +27,7 @@ class Expression extends AbstractExpression protected string $expression = ''; - /** @var ArgumentInterface[] */ + /** @var list */ protected array $parameters = []; /** @@ -96,7 +96,7 @@ public function setParameters( return $this; } - /** @return ArgumentInterface[] */ + /** @return list */ public function getParameters(): array { return $this->parameters; diff --git a/src/Sql/ExpressionInterface.php b/src/Sql/ExpressionInterface.php index 8121ddc3..eff6202a 100644 --- a/src/Sql/ExpressionInterface.php +++ b/src/Sql/ExpressionInterface.php @@ -9,7 +9,7 @@ interface ExpressionInterface /** * Returns raw expression data as array for optimised processing * - * @return array{spec: string, values: ArgumentInterface[]} + * @return array{spec: string, values: list} */ public function getExpressionData(): array; } diff --git a/src/Sql/Join.php b/src/Sql/Join.php index 2af0df5e..d5616d9d 100644 --- a/src/Sql/Join.php +++ b/src/Sql/Join.php @@ -6,12 +6,15 @@ use Countable; use Iterator; +use LogicException; use Override; use ReturnTypeWillChange; use function array_shift; use function count; +use function get_debug_type; use function is_array; +use function is_scalar; use function is_string; use function key; use function sprintf; @@ -25,6 +28,15 @@ * `Select::SQL_STAR`. * - type: the type of JOIN being performed; see the `JOIN_*` constants; * defaults to `JOIN_INNER` + * + * @phpstan-type JoinName string|TableIdentifier|non-empty-array + * @phpstan-type JoinSpecification array{ + * name: JoinName, + * on: string|Predicate\PredicateInterface, + * columns: array, + * type: string, + * } + * @implements Iterator */ class Join implements Iterator, Countable { @@ -49,6 +61,8 @@ class Join implements Iterator, Countable /** * JOIN specifications + * + * @var list */ protected array $joins = []; @@ -64,12 +78,20 @@ public function rewind(): void /** * Return current join specification. + * + * @return JoinSpecification */ #[Override] #[ReturnTypeWillChange] public function current(): array { - return $this->joins[$this->position]; + $current = $this->joins[$this->position] ?? null; + + if ($current === null) { + throw new LogicException('Join iterator is not at a valid position.'); + } + + return $current; } /** @@ -102,19 +124,22 @@ public function valid(): bool return isset($this->joins[$this->position]); } + /** + * @return list + */ public function getJoins(): array { return $this->joins; } /** - * @param array|string|TableIdentifier $name A table name on which to join, or a single + * @param JoinName $name A table name on which to join, or a single * element associative array, of the form alias => table, or TableIdentifier instance - * @param string|Predicate\Expression $on A specification describing the fields to join on. - * @param string|string[] $columns A single column name, an array + * @param string|Predicate\PredicateInterface $on A specification describing the fields to join on. + * @param array|string $columns A single column name, an array * of column names, or (a) specification(s) such as SQL_STAR representing * the columns to join. - * @param string $type The JOIN type to use; see the JOIN_* constants. + * @param string $type The JOIN type to use; see the JOIN_* constants. * @throws Exception\InvalidArgumentException For invalid $name values. */ // phpcs:ignore Generic.NamingConventions.ConstructorName.OldStyle @@ -125,8 +150,13 @@ public function join( string $type = self::JOIN_INNER ): static { if (is_array($name) && (! is_string(key($name)) || count($name) !== 1)) { + $offending = array_shift($name); + throw new Exception\InvalidArgumentException( - sprintf("join() expects '%s' as a single element associative array", array_shift($name)) + sprintf( + "join() expects '%s' as a single element associative array", + is_scalar($offending) ? $offending : get_debug_type($offending) + ) ); } diff --git a/src/Sql/Select.php b/src/Sql/Select.php index de499976..c74923c6 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -11,6 +11,7 @@ use PhpDb\Sql\Predicate\PredicateInterface; use function array_key_exists; +use function array_values; use function count; use function current; use function explode; @@ -35,6 +36,10 @@ * @property Where $where * @property Having $having * @property Join $joins + * @phpstan-import-type SpecificationArray from AbstractSql + * @phpstan-import-type ColumnDescriptor from AbstractSql + * @phpstan-import-type SpecResult from AbstractSql + * @phpstan-import-type JoinName from Join */ class Select extends AbstractPreparableSql { @@ -97,7 +102,7 @@ class Select extends AbstractPreparableSql final public const COMBINE_INTERSECT = 'intersect'; - /** @var string[]|array[] $specifications */ + /** @var array */ protected array $specifications = [ 'statementStart' => '%1$s', self::SELECT => [ @@ -141,18 +146,22 @@ class Select extends AbstractPreparableSql protected bool $prefixColumnsWithTable = true; + /** @var string|array|TableIdentifier|null */ protected string|array|TableIdentifier|null $table = null; protected string|ExpressionInterface|null $quantifier = null; + /** @var array */ protected array $columns = [self::SQL_STAR]; protected ?Join $joins = null; protected ?Where $where = null; + /** @var array */ protected array $order = []; + /** @var list|null */ protected array|null $group = null; protected ?Having $having = null; @@ -161,10 +170,13 @@ class Select extends AbstractPreparableSql protected string|int|null $offset = null; + /** @var array{select: Select, type: string, modifier: string}|array{} */ protected array $combine = []; /** * Constructor + * + * @param string|array|TableIdentifier|null $table */ public function __construct(array|string|TableIdentifier|null $table = null) { @@ -192,6 +204,7 @@ private function getHaving(): Having /** * Create from clause * + * @param string|array|TableIdentifier $table * @throws Exception\InvalidArgumentException */ public function from(array|string|TableIdentifier $table): static @@ -231,6 +244,8 @@ public function quantifier(ExpressionInterface|string $quantifier): static * array(string => value, ...) * key string will be use as alias, * value can be string or Expression objects + * + * @param array $columns */ public function columns(array $columns, bool $prefixColumnsWithTable = true): static { @@ -242,7 +257,9 @@ public function columns(array $columns, bool $prefixColumnsWithTable = true): st /** * Create join clause * - * @param string $type one of the JOIN_* constants + * @param JoinName $name + * @param array|string $columns + * @param string $type one of the JOIN_* constants * @throws Exception\InvalidArgumentException */ public function join( @@ -259,7 +276,7 @@ public function join( /** * Create where clause * - * @param string $combination One of the OP_* constants from Predicate\PredicateSet + * @param PredicateInterface|array|string|Closure $predicate * @throws Exception\InvalidArgumentException */ public function where( @@ -277,12 +294,11 @@ public function where( public function group(mixed $group): static { - if (is_array($group)) { - foreach ($group as $o) { - $this->group[] = $o; - } - } else { - $this->group[] = $group; + /** @var list $values */ + $values = is_array($group) ? array_values($group) : [$group]; + + foreach ($values as $value) { + $this->group[] = $value; } return $this; @@ -291,6 +307,7 @@ public function group(mixed $group): static /** * Create having clause * + * @param Having|PredicateInterface|array|Closure|string $predicate * @param string $combination One of the OP_* constants from Predicate\PredicateSet */ public function having( @@ -306,10 +323,13 @@ public function having( return $this; } + /** + * @param ExpressionInterface|array|string $order + */ public function order(ExpressionInterface|array|string $order): static { if (is_string($order)) { - $order = str_contains($order, ',') ? preg_split('#,\s+#', $order) : (array) $order; + $order = str_contains($order, ',') ? (preg_split('#,\s+#', $order) ?: []) : (array) $order; } elseif (! is_array($order)) { $order = [$order]; } @@ -429,7 +449,7 @@ public function reset(string $part): static } /** - * @param string|array $specification + * @param string|SpecificationArray $specification */ public function setSpecification(string $index, array|string $specification): static { @@ -489,6 +509,8 @@ protected function processStatementEnd(): ?array /** * Process the select part + * + * @return SpecResult */ protected function processSelect( PlatformInterface $platform, @@ -528,7 +550,7 @@ protected function processSelect( foreach ($this->getJoins()->getJoins() as $join) { $joinName = is_array($join['name']) ? key($join['name']) : $join['name']; - $joinName = parent::resolveTable($joinName, $platform, $driver, $parameterContainer); + $joinName = parent::resolveTable($joinName, $platform, $driver, $parameterContainer) ?? ''; foreach ($join['columns'] as $jKey => $jColumn) { $jColumns = []; @@ -548,7 +570,7 @@ protected function processSelect( ); if (is_string($jKey)) { $jColumns[] = $platform->quoteIdentifier($jKey); - } elseif ($jColumn !== self::SQL_STAR) { + } elseif (is_string($jColumn) && $jColumn !== self::SQL_STAR) { $jColumns[] = $platform->quoteIdentifier($jColumn); } @@ -556,6 +578,7 @@ protected function processSelect( } } + $quantifier = null; if ($this->quantifier) { $quantifier = $this->quantifier instanceof ExpressionInterface ? $this->processExpression($this->quantifier, $platform, $driver, $parameterContainer, 'quantifier') @@ -580,6 +603,9 @@ protected function processJoins( return $this->processJoin($this->joins, $platform, $driver, $parameterContainer); } + /** + * @return SpecResult|null + */ protected function processWhere( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -594,6 +620,9 @@ protected function processWhere( ]; } + /** + * @return SpecResult|null + */ protected function processGroup( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -620,6 +649,9 @@ protected function processGroup( return [$groups]; } + /** + * @return SpecResult|null + */ protected function processHaving( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -634,6 +666,9 @@ protected function processHaving( ]; } + /** + * @return SpecResult|null + */ protected function processOrder( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -654,7 +689,9 @@ protected function processOrder( if (is_int($k)) { if (str_contains($v, ' ')) { - [$k, $v] = explode(' ', $v, 2); + $parts = explode(' ', $v, 2); + $k = $parts[0]; + $v = $parts[1] ?? self::ORDER_ASCENDING; } else { $k = $v; $v = self::ORDER_ASCENDING; @@ -671,6 +708,9 @@ protected function processOrder( return [$orders]; } + /** + * @return SpecResult|null + */ protected function processLimit( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -680,15 +720,18 @@ protected function processLimit( return null; } - if ($parameterContainer instanceof ParameterContainer) { + if ($parameterContainer instanceof ParameterContainer && $driver instanceof DriverInterface) { $paramPrefix = $this->processInfo['paramPrefix']; $parameterContainer->offsetSet($paramPrefix . 'limit', $this->limit, ParameterContainer::TYPE_INTEGER); return [$driver->formatParameterName($paramPrefix . 'limit')]; } - return [$platform->quoteValue($this->limit)]; + return [$platform->quoteValue((string) $this->limit)]; } + /** + * @return SpecResult|null + */ protected function processOffset( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -698,15 +741,18 @@ protected function processOffset( return null; } - if ($parameterContainer instanceof ParameterContainer) { + if ($parameterContainer instanceof ParameterContainer && $driver instanceof DriverInterface) { $paramPrefix = $this->processInfo['paramPrefix']; $parameterContainer->offsetSet($paramPrefix . 'offset', $this->offset, ParameterContainer::TYPE_INTEGER); return [$driver->formatParameterName($paramPrefix . 'offset')]; } - return [$platform->quoteValue($this->offset)]; + return [$platform->quoteValue((string) $this->offset)]; } + /** + * @return SpecResult|null + */ protected function processCombine( PlatformInterface $platform, ?DriverInterface $driver = null, @@ -716,13 +762,15 @@ protected function processCombine( return null; } - $type = $this->combine['modifier'] - ? "{$this->combine['type']} {$this->combine['modifier']}" - : $this->combine['type']; + /** @var array{select: Select, type: string, modifier: string} $combine */ + $combine = $this->combine; + $type = $combine['modifier'] + ? "{$combine['type']} {$combine['modifier']}" + : $combine['type']; return [ strtoupper($type), - $this->processSubSelect($this->combine['select'], $platform, $driver, $parameterContainer), + $this->processSubSelect($combine['select'], $platform, $driver, $parameterContainer), ]; } @@ -745,10 +793,8 @@ public function __get(string $name): Where|Join|Having * __clone * * Resets the where object each time the Select is cloned. - * - * @return void */ - public function __clone() + public function __clone(): void { if ($this->where !== null) { $this->where = clone $this->where; @@ -762,8 +808,13 @@ public function __clone() } /** - * @return array{0: string, 1: string} - * @phpstan-return array{0: string, 1: string} + * Overrides the base method to return a `[table, fromTable]` tuple rather + * than a plain string; the differing return type is intentional. + * + * @param Select|string|array|TableIdentifier|null $table + * @return array{0: string|null, 1: string} + * @phpstan-return array{0: string|null, 1: string} + * @mago-expect analysis:incompatible-return-type */ protected function resolveTable( Select|string|array|TableIdentifier|null $table, @@ -775,6 +826,7 @@ protected function resolveTable( if (is_array($table)) { $alias = key($table); + /** @var string|Select|TableIdentifier $table */ $table = current($table); } @@ -782,7 +834,7 @@ protected function resolveTable( if ($alias) { $fromTable = $platform->quoteIdentifier($alias); - $table = $this->renderTable($table, $fromTable); + $table = $this->renderTable((string) $table, $fromTable); } else { $fromTable = $table; } From c2f9e13af8f4b99dbc5253cc4de2d0398d6ab369 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 23:09:03 +1000 Subject: [PATCH 06/22] Type Insert/Update/Delete and centralise table alias handling - Move aliased-array table stripping into AbstractSql::resolveTable so Insert/Update/Delete no longer reimplement it; Select stays the only override, as it alone preserves the alias as a [table, fromTable] tuple - Widen resolveColumnValue() to accept float bound values - Type the Insert/Update/Delete specifications, table, column and predicate members and narrow the spec format to string at each str_replace call - Guard driver-dependent parameter binding, cast column keys used as identifiers, and document the genuinely-mixed value maps Signed-off-by: Simon Mundy --- src/Sql/AbstractSql.php | 18 +++++++++--- src/Sql/Delete.php | 24 ++++++++++------ src/Sql/Insert.php | 61 +++++++++++++++++++++++++++++------------ src/Sql/Update.php | 50 +++++++++++++++++++++------------ 4 files changed, 105 insertions(+), 48 deletions(-) diff --git a/src/Sql/AbstractSql.php b/src/Sql/AbstractSql.php index 250e246a..1886dec0 100644 --- a/src/Sql/AbstractSql.php +++ b/src/Sql/AbstractSql.php @@ -36,7 +36,7 @@ * @phpstan-type SpecificationArray array> * @phpstan-type SpecResult array> * @phpstan-type ColumnDescriptor array{ - * column: Select|string|int|bool|ExpressionInterface|null, + * column: Select|string|int|float|bool|ExpressionInterface|null, * fromTable?: string, * isIdentifier?: bool * } @@ -439,10 +439,10 @@ protected function processJoin( } /** - * @param Select|string|int|bool|ExpressionInterface|null|ColumnDescriptor $column + * @param Select|string|int|float|bool|ExpressionInterface|null|ColumnDescriptor $column */ protected function resolveColumnValue( - Select|array|string|int|bool|ExpressionInterface|null $column, + Select|array|string|int|float|bool|ExpressionInterface|null $column, PlatformInterface $platform, ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null, @@ -477,14 +477,24 @@ protected function resolveColumnValue( } /** + * Resolves a table specification to its rendered string form. + * + * An aliased `[alias => table]` array is reduced to the underlying table; + * only {@see Select} overrides this to preserve the alias. + * + * @param Select|string|array|TableIdentifier|null $table * @return string|null */ protected function resolveTable( - Select|string|TableIdentifier|null $table, + Select|string|array|TableIdentifier|null $table, PlatformInterface $platform, ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null ): string|array|null { + if (is_array($table)) { + $table = current($table) ?: ''; + } + $schema = null; if ($table instanceof TableIdentifier) { $schema = $table->getSchema(); diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index 28dd1000..34b24543 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -18,6 +18,8 @@ /** * @property Where $where + * + * @phpstan-import-type SpecificationArray from AbstractSql */ class Delete extends AbstractPreparableSql { @@ -28,16 +30,13 @@ class Delete extends AbstractPreparableSql final public const SPECIFICATION_WHERE = 'where'; - /**@#-*/ - - /** - * {@inheritDoc} - */ + /** @var array */ protected array $specifications = [ self::SPECIFICATION_DELETE => 'DELETE FROM %1$s', self::SPECIFICATION_WHERE => 'WHERE %1$s', ]; + /** @var TableIdentifier|string|array */ protected TableIdentifier|string|array $table = ''; protected bool $emptyWhereProtection = true; @@ -61,6 +60,8 @@ private function getWhere(): Where /** * Create from statement + * + * @param TableIdentifier|string|array $table */ public function from(TableIdentifier|string|array $table): static { @@ -81,6 +82,7 @@ public function getRawState(?string $key = null): mixed /** * Create where clause * + * @param PredicateInterface|array|Closure|string|Where $predicate * @param string $combination One of the OP_* constants from Predicate\PredicateSet */ public function where( @@ -101,10 +103,13 @@ protected function processDelete( ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null ): string { + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_DELETE] ?? ''; + return str_replace( '%1$s', - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), - $this->specifications[static::SPECIFICATION_DELETE] + $this->resolveTable($this->table, $platform, $driver, $parameterContainer) ?? '', + $specification ); } @@ -117,10 +122,13 @@ protected function processWhere( return null; } + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_WHERE] ?? ''; + return str_replace( '%1$s', $this->processExpression($this->where, $platform, $driver, $parameterContainer, 'where'), - $this->specifications[static::SPECIFICATION_WHERE] + $specification ); } diff --git a/src/Sql/Insert.php b/src/Sql/Insert.php index 6c0e3a2d..40177fd2 100644 --- a/src/Sql/Insert.php +++ b/src/Sql/Insert.php @@ -21,6 +21,10 @@ use function range; use function str_replace; +/** + * @phpstan-import-type ColumnDescriptor from AbstractSql + * @phpstan-import-type SpecificationArray from AbstractSql + */ class Insert extends AbstractPreparableSql { /** @@ -36,16 +40,19 @@ class Insert extends AbstractPreparableSql final public const VALUES_SET = 'set'; - /** @var string[]|array[] $specifications */ + /** @var array */ protected array $specifications = [ self::SPECIFICATION_INSERT => 'INSERT INTO %1$s (%2$s) VALUES (%3$s)', self::SPECIFICATION_SELECT => 'INSERT INTO %1$s %2$s %3$s', ]; + /** @var TableIdentifier|string|array */ protected TableIdentifier|string|array $table = ''; + /** @var array */ protected array $columns = []; + /** @var Select|null */ protected null|array|Select $select = null; /** @@ -60,6 +67,8 @@ public function __construct(string|TableIdentifier|null $table = null) /** * Create INTO clause + * + * @param TableIdentifier|string|array $table */ public function into(TableIdentifier|string|array $table): static { @@ -69,6 +78,8 @@ public function into(TableIdentifier|string|array $table): static /** * Specify columns + * + * @param list $columns */ public function columns(array $columns): static { @@ -79,7 +90,8 @@ public function columns(array $columns): static /** * Specify values to insert * - * @param string $flag one of VALUES_MERGE or VALUES_SET; defaults to VALUES_SET + * @param array|Select $values + * @param string $flag one of VALUES_MERGE or VALUES_SET; defaults to VALUES_SET * @throws Exception\InvalidArgumentException */ public function values(array|Select $values, string $flag = self::VALUES_SET): static @@ -107,6 +119,7 @@ public function values(array|Select $values, string $flag = self::VALUES_SET): s ? $values : array_combine(array_keys($this->columns), array_values($values)); } else { + /** @mago-expect analysis:mixed-assignment */ foreach ($values as $column => $value) { $this->columns[$column] = $value; } @@ -119,6 +132,8 @@ public function values(array|Select $values, string $flag = self::VALUES_SET): s * Simple test for an associative array * * @link http://stackoverflow.com/questions/173400/how-to-check-if-php-array-is-associative-or-sequential + * + * @param array $array */ private function isAssocativeArray(array $array): bool { @@ -135,6 +150,8 @@ public function select(Select $select): static /** * Get raw state + * + * @return TableIdentifier|string|array */ public function getRawState(?string $key = null): TableIdentifier|string|array { @@ -164,20 +181,23 @@ protected function processInsert( $i = 0; $isPdoDriver = $driver instanceof PdoDriverInterface; + /** @mago-expect analysis:mixed-assignment */ foreach ($this->columns as $column => $value) { - $columns[] = $platform->quoteIdentifier($column); - if (is_scalar($value) && $parameterContainer) { + $columns[] = $platform->quoteIdentifier((string) $column); + if (is_scalar($value) && $parameterContainer && $driver instanceof DriverInterface) { // use incremental value instead of column name for PDO // @see https://github.com/zendframework/zend-db/issues/35 if ($isPdoDriver) { $column = 'c_' . $i++; } - $values[] = $driver->formatParameterName($column); + $values[] = $driver->formatParameterName((string) $column); $parameterContainer->offsetSet($column, $value); } else { - $values[] = $this->resolveColumnValue( - $value, + /** @var Select|string|int|float|bool|ExpressionInterface|null $columnValue */ + $columnValue = $value; + $values[] = $this->resolveColumnValue( + $columnValue, $platform, $driver, $parameterContainer @@ -185,14 +205,17 @@ protected function processInsert( } } + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_INSERT] ?? ''; + return str_replace( ['%1$s', '%2$s', '%3$s'], [ - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), + $this->resolveTable($this->table, $platform, $driver, $parameterContainer) ?? '', implode(', ', $columns), implode(', ', $values), ], - $this->specifications[static::SPECIFICATION_INSERT] + $specification ); } @@ -207,17 +230,23 @@ protected function processSelect( $selectSql = $this->processSubSelect($this->select, $platform, $driver, $parameterContainer); - $columns = array_map([$platform, 'quoteIdentifier'], array_keys($this->columns)); + $columns = array_map( + static fn (int|string $column): string => $platform->quoteIdentifier((string) $column), + array_keys($this->columns) + ); $columns = implode(', ', $columns); + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_SELECT] ?? ''; + return str_replace( ['%1$s', '%2$s', '%3$s'], [ - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), + $this->resolveTable($this->table, $platform, $driver, $parameterContainer) ?? '', $columns ? "({$columns})" : '', $selectSql, ], - $this->specifications[static::SPECIFICATION_SELECT] + $specification ); } @@ -237,9 +266,8 @@ public function __set(string $name, mixed $value): void * Proxies to values and columns * * @throws Exception\InvalidArgumentException - * @return void */ - public function __unset(string $name) + public function __unset(string $name): void { if (! array_key_exists($name, $this->columns)) { throw new Exception\InvalidArgumentException( @@ -254,10 +282,8 @@ public function __unset(string $name) * Overloading: variable isset * * Proxies to columns; does a column of that name exist? - * - * @return bool */ - public function __isset(string $name) + public function __isset(string $name): bool { return array_key_exists($name, $this->columns); } @@ -267,7 +293,6 @@ public function __isset(string $name) * Retrieves value by column name * * @throws Exception\InvalidArgumentException - * @return string */ public function __get(string $name): mixed { diff --git a/src/Sql/Update.php b/src/Sql/Update.php index 8361a701..9b825c48 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -25,6 +25,9 @@ /** * @property Where $where + * @phpstan-import-type ColumnDescriptor from AbstractSql + * @phpstan-import-type SpecificationArray from AbstractSql + * @phpstan-import-type JoinName from Join */ class Update extends AbstractPreparableSql { @@ -45,7 +48,7 @@ class Update extends AbstractPreparableSql /**@#-**/ - /** @var array|array */ + /** @var array */ protected array $specifications = [ self::SPECIFICATION_UPDATE => 'UPDATE %1$s', self::SPECIFICATION_JOIN => [ @@ -57,6 +60,7 @@ class Update extends AbstractPreparableSql self::SPECIFICATION_WHERE => 'WHERE %1$s', ]; + /** @var TableIdentifier|string|array */ protected TableIdentifier|string|array $table = ''; protected bool $emptyWhereProtection = true; @@ -98,6 +102,8 @@ private function getJoins(): Join /** * Specify table for statement + * + * @param TableIdentifier|string|array $table */ public function table(TableIdentifier|string|array $table): static { @@ -108,8 +114,7 @@ public function table(TableIdentifier|string|array $table): static /** * Set key/value pairs to update * - * @param array $values Associative array of key values - * @param string $flag One of the VALUES_* constants + * @param array $values Associative array of key values * @throws Exception\InvalidArgumentException */ public function set(array $values, string|int $flag = self::VALUES_SET): static @@ -119,7 +124,8 @@ public function set(array $values, string|int $flag = self::VALUES_SET): static $set->clear(); } - $priority = is_numeric($flag) ? $flag : 0; + $priority = is_numeric($flag) ? (int) $flag : 0; + /** @mago-expect analysis:mixed-assignment */ foreach ($values as $k => $v) { if (! is_string($k)) { throw new Exception\InvalidArgumentException('set() expects a string for the value key'); @@ -134,6 +140,7 @@ public function set(array $values, string|int $flag = self::VALUES_SET): static /** * Create where clause * + * @param PredicateInterface|array|Closure|string|Where $predicate * @throws Exception\InvalidArgumentException */ public function where( @@ -152,6 +159,7 @@ public function where( /** * Create join clause * + * @param JoinName $name * @throws Exception\InvalidArgumentException */ public function join(array|string|TableIdentifier $name, string $on, string $type = Join::JOIN_INNER): static @@ -178,10 +186,13 @@ protected function processUpdate( ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null ): string { + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_UPDATE] ?? ''; + return str_replace( '%1$s', - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), - $this->specifications[static::SPECIFICATION_UPDATE] + $this->resolveTable($this->table, $platform, $driver, $parameterContainer) ?? '', + $specification ); } @@ -194,6 +205,7 @@ protected function processSet( $i = 0; $isPdoDriver = $driver instanceof PdoDriverInterface; + /** @mago-expect analysis:mixed-assignment */ foreach ($this->getSet() as $column => $value) { $prefix = $this->resolveColumnValue( [ @@ -207,7 +219,7 @@ protected function processSet( 'column' ); $prefix .= ' = '; - if (is_scalar($value) && $parameterContainer) { + if (is_scalar($value) && $parameterContainer && $driver instanceof DriverInterface) { // use incremental value instead of column name for PDO // @see https://github.com/zendframework/zend-db/issues/35 if ($isPdoDriver) { @@ -217,8 +229,10 @@ protected function processSet( $setSql[] = $prefix . $driver->formatParameterName($column); $parameterContainer->offsetSet($column, $value); } else { - $setSql[] = $prefix . $this->resolveColumnValue( - $value, + /** @var Select|string|int|float|bool|ExpressionInterface|null $columnValue */ + $columnValue = $value; + $setSql[] = $prefix . $this->resolveColumnValue( + $columnValue, $platform, $driver, $parameterContainer @@ -226,11 +240,10 @@ protected function processSet( } } - return str_replace( - '%1$s', - implode(', ', $setSql), - $this->specifications[static::SPECIFICATION_SET] - ); + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_SET] ?? ''; + + return str_replace('%1$s', implode(', ', $setSql), $specification); } protected function processWhere( @@ -242,10 +255,13 @@ protected function processWhere( return null; } + /** @var string $specification */ + $specification = $this->specifications[static::SPECIFICATION_WHERE] ?? ''; + return str_replace( '%1$s', $this->processExpression($this->where, $platform, $driver, $parameterContainer, 'where'), - $this->specifications[static::SPECIFICATION_WHERE] + $specification ); } @@ -275,10 +291,8 @@ public function __get(string $name): ?Where * __clone * * Resets the where object each time the Update is cloned. - * - * @return void */ - public function __clone() + public function __clone(): void { if ($this->where !== null) { $this->where = clone $this->where; From a6ab5ccb6edc9be0ba26cec6c821ee24a6688c02 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 8 Jul 2026 23:22:44 +1000 Subject: [PATCH 07/22] Add specificationString() accessor for plain-string specifications Replaces the repeated `@var string` narrowing of the specifications map in Insert/Update/Delete with a single typed accessor, so the format string is narrowed once at a named boundary rather than at each call. Signed-off-by: Simon Mundy --- src/Sql/AbstractSql.php | 15 +++++++++++++++ src/Sql/Delete.php | 10 ++-------- src/Sql/Insert.php | 10 ++-------- src/Sql/Update.php | 15 +++------------ 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/Sql/AbstractSql.php b/src/Sql/AbstractSql.php index 1886dec0..be315536 100644 --- a/src/Sql/AbstractSql.php +++ b/src/Sql/AbstractSql.php @@ -103,6 +103,21 @@ protected function buildSqlString( return rtrim(implode(' ', $sqls), "\n ,"); } + /** + * Returns the plain format string for a specification key. + * + * The `$specifications` map also allows the array form consumed by + * {@see createSqlFromSpecificationAndParameters()}; statements whose + * specifications are plain `sprintf`/`str_replace` templates use this to + * read them as strings. + */ + protected function specificationString(string $name): string + { + $specification = $this->specifications[$name] ?? ''; + + return is_string($specification) ? $specification : ''; + } + /** * Render table with alias in from/join parts * diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index 34b24543..f022ee25 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -103,13 +103,10 @@ protected function processDelete( ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null ): string { - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_DELETE] ?? ''; - return str_replace( '%1$s', $this->resolveTable($this->table, $platform, $driver, $parameterContainer) ?? '', - $specification + $this->specificationString(static::SPECIFICATION_DELETE) ); } @@ -122,13 +119,10 @@ protected function processWhere( return null; } - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_WHERE] ?? ''; - return str_replace( '%1$s', $this->processExpression($this->where, $platform, $driver, $parameterContainer, 'where'), - $specification + $this->specificationString(static::SPECIFICATION_WHERE) ); } diff --git a/src/Sql/Insert.php b/src/Sql/Insert.php index 40177fd2..d484e300 100644 --- a/src/Sql/Insert.php +++ b/src/Sql/Insert.php @@ -205,9 +205,6 @@ protected function processInsert( } } - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_INSERT] ?? ''; - return str_replace( ['%1$s', '%2$s', '%3$s'], [ @@ -215,7 +212,7 @@ protected function processInsert( implode(', ', $columns), implode(', ', $values), ], - $specification + $this->specificationString(static::SPECIFICATION_INSERT) ); } @@ -236,9 +233,6 @@ protected function processSelect( ); $columns = implode(', ', $columns); - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_SELECT] ?? ''; - return str_replace( ['%1$s', '%2$s', '%3$s'], [ @@ -246,7 +240,7 @@ protected function processSelect( $columns ? "({$columns})" : '', $selectSql, ], - $specification + $this->specificationString(static::SPECIFICATION_SELECT) ); } diff --git a/src/Sql/Update.php b/src/Sql/Update.php index 9b825c48..a81cf110 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -186,13 +186,10 @@ protected function processUpdate( ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null ): string { - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_UPDATE] ?? ''; - return str_replace( '%1$s', $this->resolveTable($this->table, $platform, $driver, $parameterContainer) ?? '', - $specification + $this->specificationString(static::SPECIFICATION_UPDATE) ); } @@ -240,10 +237,7 @@ protected function processSet( } } - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_SET] ?? ''; - - return str_replace('%1$s', implode(', ', $setSql), $specification); + return str_replace('%1$s', implode(', ', $setSql), $this->specificationString(static::SPECIFICATION_SET)); } protected function processWhere( @@ -255,13 +249,10 @@ protected function processWhere( return null; } - /** @var string $specification */ - $specification = $this->specifications[static::SPECIFICATION_WHERE] ?? ''; - return str_replace( '%1$s', $this->processExpression($this->where, $platform, $driver, $parameterContainer, 'where'), - $specification + $this->specificationString(static::SPECIFICATION_WHERE) ); } From dd8f15e4281d2937a601266f52ef327f8b71f18f Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 9 Jul 2026 08:39:24 +1000 Subject: [PATCH 08/22] Type Combine and Sql; accept aliased tables in statement constructors - Type Combine's combine-spec list, combine input, and narrow the mixed getRawState() column results in alignColumns() - Type Sql's table member/accessors, drop the dead null branches from getAdapter()/getSqlPlatform(), and render the table safely in the single-table guard message - Widen the Insert/Update/Delete constructors to accept the aliased-array table their into()/table()/from() already accept Signed-off-by: Simon Mundy --- src/Sql/Combine.php | 30 ++++++++++++++++++++++-------- src/Sql/Delete.php | 4 +++- src/Sql/Insert.php | 4 +++- src/Sql/Sql.php | 22 ++++++++++++++++------ src/Sql/Update.php | 4 +++- 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/Sql/Combine.php b/src/Sql/Combine.php index aafb7fac..e21d4966 100644 --- a/src/Sql/Combine.php +++ b/src/Sql/Combine.php @@ -19,6 +19,9 @@ /** * Combine SQL statement - allows combining multiple select statements into one + * + * @phpstan-import-type SpecificationArray from AbstractSql + * @phpstan-type CombineInput Select|array */ class Combine extends AbstractPreparableSql { @@ -32,14 +35,17 @@ class Combine extends AbstractPreparableSql final public const COMBINE_INTERSECT = 'intersect'; - /** @var string[] */ + /** @var array */ protected array $specifications = [ self::COMBINE => '%1$s (%2$s) ', ]; - /** @var array */ + /** @var list */ private array $combine = []; + /** + * @param CombineInput|null $select + */ public function __construct( Select|array|null $select = null, string $type = self::COMBINE_UNION, @@ -53,6 +59,7 @@ public function __construct( /** * Create combine clause * + * @param CombineInput $select * @throws Exception\InvalidArgumentException */ public function combine(Select|array $select, string $type = self::COMBINE_UNION, string $modifier = ''): static @@ -60,7 +67,8 @@ public function combine(Select|array $select, string $type = self::COMBINE_UNION if (is_array($select)) { foreach ($select as $combine) { if ($combine instanceof Select) { - $combine = [$combine]; + $this->combine($combine, $type, $modifier); + continue; } $this->combine( @@ -83,6 +91,8 @@ public function combine(Select|array $select, string $type = self::COMBINE_UNION /** * Create union clause + * + * @param CombineInput $select */ public function union(Select|array $select, string $modifier = ''): static { @@ -91,6 +101,8 @@ public function union(Select|array $select, string $modifier = ''): static /** * Create except clause + * + * @param CombineInput $select */ public function except(Select|array $select, string $modifier = ''): static { @@ -99,6 +111,8 @@ public function except(Select|array $select, string $modifier = ''): static /** * Create intersect clause + * + * @param CombineInput $select */ public function intersect(Select|array $select, string $modifier = ''): static { @@ -131,7 +145,7 @@ protected function buildSqlString( $sql .= str_replace( ['%1$s', '%2$s'], [$type, $select], - $this->specifications[self::COMBINE] + $this->specificationString(self::COMBINE) ); } @@ -146,13 +160,13 @@ public function alignColumns(): static $allColumns = []; foreach ($this->combine as $combine) { - $allColumns = array_merge( - $allColumns, - $combine['select']->getRawState(self::COLUMNS) - ); + /** @var array $columns */ + $columns = $combine['select']->getRawState(self::COLUMNS); + $allColumns = array_merge($allColumns, $columns); } foreach ($this->combine as $combine) { + /** @var array $combineColumns */ $combineColumns = $combine['select']->getRawState(self::COLUMNS); $aligned = []; foreach (array_keys($allColumns) as $alias) { diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index f022ee25..1438841d 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -45,8 +45,10 @@ class Delete extends AbstractPreparableSql /** * Constructor + * + * @param TableIdentifier|string|array|null $table */ - public function __construct(string|TableIdentifier|null $table = null) + public function __construct(string|TableIdentifier|array|null $table = null) { if ($table) { $this->from($table); diff --git a/src/Sql/Insert.php b/src/Sql/Insert.php index d484e300..9a8aca4e 100644 --- a/src/Sql/Insert.php +++ b/src/Sql/Insert.php @@ -57,8 +57,10 @@ class Insert extends AbstractPreparableSql /** * Constructor + * + * @param TableIdentifier|string|array|null $table */ - public function __construct(string|TableIdentifier|null $table = null) + public function __construct(string|TableIdentifier|array|null $table = null) { if ($table) { $this->into($table); diff --git a/src/Sql/Sql.php b/src/Sql/Sql.php index 525c4efb..fa7913d9 100644 --- a/src/Sql/Sql.php +++ b/src/Sql/Sql.php @@ -7,16 +7,22 @@ use PhpDb\Adapter\AdapterInterface; use PhpDb\Adapter\Driver\StatementInterface; +use function get_debug_type; +use function is_string; use function sprintf; class Sql { protected AdapterInterface $adapter; + /** @var TableIdentifier|string|array|null */ protected TableIdentifier|string|array|null $table; protected Platform\PlatformDecoratorInterface $sqlPlatform; + /** + * @param TableIdentifier|string|array|null $table + */ public function __construct( AdapterInterface $adapter, array|string|TableIdentifier|null $table = null @@ -26,7 +32,7 @@ public function __construct( $this->sqlPlatform = $adapter->getPlatform()->getSqlPlatformDecorator(); } - public function getAdapter(): ?AdapterInterface + public function getAdapter(): AdapterInterface { return $this->adapter; } @@ -37,6 +43,7 @@ public function hasTable(): bool } /** + * @param TableIdentifier|string|array $table * @throws Exception\InvalidArgumentException */ public function setTable(array|string|TableIdentifier $table): self @@ -46,12 +53,15 @@ public function setTable(array|string|TableIdentifier $table): self return $this; } + /** + * @return TableIdentifier|string|array|null + */ public function getTable(): array|string|TableIdentifier|null { return $this->table; } - public function getSqlPlatform(): ?Platform\PlatformDecoratorInterface + public function getSqlPlatform(): Platform\PlatformDecoratorInterface { return $this->sqlPlatform; } @@ -61,7 +71,7 @@ public function select(string|TableIdentifier|null $table = null): Select if ($this->table !== null && $table !== null) { throw new Exception\InvalidArgumentException(sprintf( 'This Sql object is intended to work with only the table "%s" provided at construction time.', - $this->table + is_string($this->table) ? $this->table : get_debug_type($this->table) )); } @@ -73,7 +83,7 @@ public function insert(string|null|TableIdentifier $table = null): Insert if ($this->table !== null && $table !== null) { throw new Exception\InvalidArgumentException(sprintf( 'This Sql object is intended to work with only the table "%s" provided at construction time.', - $this->table + is_string($this->table) ? $this->table : get_debug_type($this->table) )); } @@ -85,7 +95,7 @@ public function update(null|string|TableIdentifier $table = null): Update if ($this->table !== null && $table !== null) { throw new Exception\InvalidArgumentException(sprintf( 'This Sql object is intended to work with only the table "%s" provided at construction time.', - $this->table + is_string($this->table) ? $this->table : get_debug_type($this->table) )); } @@ -97,7 +107,7 @@ public function delete(null|string|TableIdentifier $table = null): Delete if ($this->table !== null && $table !== null) { throw new Exception\InvalidArgumentException(sprintf( 'This Sql object is intended to work with only the table "%s" provided at construction time.', - $this->table + is_string($this->table) ? $this->table : get_debug_type($this->table) )); } diff --git a/src/Sql/Update.php b/src/Sql/Update.php index a81cf110..9be68eb7 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -73,8 +73,10 @@ class Update extends AbstractPreparableSql /** * Constructor + * + * @param TableIdentifier|string|array|null $table */ - public function __construct(string|TableIdentifier|null $table = null) + public function __construct(string|TableIdentifier|array|null $table = null) { if ($table) { $this->table($table); From 69c9fdafa7fa4d221be0fdb2bca2207e3d778dc1 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 9 Jul 2026 09:16:00 +1000 Subject: [PATCH 09/22] Type Expression parameters and AbstractPlatform decorators - Type the Expression parameter shape (ExpressionParameter) and count placeholders with substr_count() instead of the str_replace out-param - Widen Argument\Values to accept any-keyed scalar arrays (it reindexes) - Make AbstractPlatform's subject nullable, type the decorator map, and guard the resolved decorator before prepareStatement()/getSqlString() Signed-off-by: Simon Mundy --- src/Sql/Argument/Values.php | 2 +- src/Sql/Expression.php | 14 ++++++++++++-- src/Sql/Platform/AbstractPlatform.php | 23 +++++++++++++++++++---- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/Sql/Argument/Values.php b/src/Sql/Argument/Values.php index 6a0c3406..30e1aa25 100644 --- a/src/Sql/Argument/Values.php +++ b/src/Sql/Argument/Values.php @@ -24,7 +24,7 @@ private array $values; /** - * @param list $values + * @param array $values */ public function __construct(array $values) { diff --git a/src/Sql/Expression.php b/src/Sql/Expression.php index 2778e0e7..dfe30f38 100644 --- a/src/Sql/Expression.php +++ b/src/Sql/Expression.php @@ -17,7 +17,11 @@ use function is_array; use function preg_match_all; use function str_replace; +use function substr_count; +/** + * @phpstan-type ExpressionParameter scalar|null|array|ArgumentInterface|ExpressionInterface + */ class Expression extends AbstractExpression { /** @@ -31,6 +35,7 @@ class Expression extends AbstractExpression protected array $parameters = []; /** + * @param ExpressionParameter|array $parameters * @todo Update documentation to show how parameters can be specifically typed */ public function __construct( @@ -46,6 +51,8 @@ public function __construct( * @deprecated * * @todo Make notes in documentation + * + * @var list $parameters */ $parameters = array_slice(func_get_args(), 1); } @@ -72,6 +79,7 @@ public function getExpression(): string } /** + * @param ExpressionParameter|array $parameters * @throws Exception\InvalidArgumentException */ public function setParameters( @@ -121,13 +129,15 @@ public function getExpressionData(): array } // assign locally, escaping % signs - $specification = str_replace(self::PLACEHOLDER, '%s', $specification, $count); + $count = substr_count($specification, self::PLACEHOLDER); + $specification = str_replace(self::PLACEHOLDER, '%s', $specification); // test number of replacements without considering same variable begin used many times first, which is // faster, if the test fails then resort to regex which are slow and used rarely if ($count !== $parametersCount) { + $matches = []; preg_match_all('/:\w*/', $specification, $matches); - if ($parametersCount !== count(array_unique($matches[0]))) { + if ($parametersCount !== count(array_unique($matches[0] ?? []))) { throw new Exception\RuntimeException( 'The number of replacements in the expression does not match the number of parameters' ); diff --git a/src/Sql/Platform/AbstractPlatform.php b/src/Sql/Platform/AbstractPlatform.php index cc8d5d67..6f24a8d7 100644 --- a/src/Sql/Platform/AbstractPlatform.php +++ b/src/Sql/Platform/AbstractPlatform.php @@ -13,8 +13,9 @@ class AbstractPlatform implements PlatformDecoratorInterface, PreparableSqlInterface, SqlInterface { - protected SqlInterface|PreparableSqlInterface $subject; + protected SqlInterface|PreparableSqlInterface|null $subject = null; + /** @var array */ protected array $decorators = []; /** @@ -47,7 +48,7 @@ public function getTypeDecorator( } /** - * @return array|PlatformDecoratorInterface[] + * @return array */ public function getDecorators(): array { @@ -68,7 +69,14 @@ public function prepareStatement( ); } - $this->getTypeDecorator($this->subject)->prepareStatement($adapter, $statementContainer); + $decorator = $this->getTypeDecorator($this->subject); + if (! $decorator instanceof PreparableSqlInterface) { + throw new Exception\RuntimeException( + 'The resolved type decorator does not implement PhpDb\Sql\PreparableSqlInterface' + ); + } + + $decorator->prepareStatement($adapter, $statementContainer); return $statementContainer; } @@ -87,6 +95,13 @@ public function getSqlString(?PlatformInterface $adapterPlatform = null): string ); } - return $this->getTypeDecorator($this->subject)->getSqlString($adapterPlatform); + $decorator = $this->getTypeDecorator($this->subject); + if (! $decorator instanceof SqlInterface) { + throw new Exception\RuntimeException( + 'The resolved type decorator does not implement PhpDb\Sql\SqlInterface' + ); + } + + return $decorator->getSqlString($adapterPlatform); } } From eeed2425e4738ca4fb21ac48d4a4788859dc2745 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 9 Jul 2026 14:38:38 +1000 Subject: [PATCH 10/22] Type the Predicate builders and align identifier params - Narrow the fluent Predicate identifier params to the real contract (string|ArgumentInterface) the concrete predicates accept; value operands keep bool|float|int|string so value escaping is unchanged - Type PredicateSet's predicate-entry list, the addPredicates spec input, and In's value set - Drop PredicateSet::getExpressionData's redundant single-predicate branch (the general loop yields the same result) and fix a latent bug where a non-predicate Expression was added instead of a Predicate\Expression Signed-off-by: Simon Mundy --- src/Sql/Predicate/In.php | 4 +++ src/Sql/Predicate/Predicate.php | 43 +++++++++++++++++------------- src/Sql/Predicate/PredicateSet.php | 32 +++++++++++----------- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/Sql/Predicate/In.php b/src/Sql/Predicate/In.php index e3984af2..f3e78996 100644 --- a/src/Sql/Predicate/In.php +++ b/src/Sql/Predicate/In.php @@ -21,6 +21,8 @@ class In extends AbstractExpression implements PredicateInterface /** * Constructor + * + * @param null|array|Select|ArgumentInterface $valueSet */ public function __construct( null|string|ArgumentInterface $identifier = null, @@ -57,6 +59,8 @@ public function getIdentifier(): ?ArgumentInterface /** * Set set of values for IN comparison + * + * @param array|Select|ArgumentInterface $valueSet */ public function setValueSet(array|Select|ArgumentInterface $valueSet): static { diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php index 2b3a320a..389de018 100644 --- a/src/Sql/Predicate/Predicate.php +++ b/src/Sql/Predicate/Predicate.php @@ -17,6 +17,8 @@ * @property Predicate $unnest * @property Predicate $NEST * @property Predicate $UNNEST + * + * @phpstan-import-type ExpressionParameter from \PhpDb\Sql\Expression */ class Predicate extends PredicateSet { @@ -75,7 +77,7 @@ public function unnest(): Predicate * Utilizes Operator predicate */ public function equalTo( - null|float|int|string|ArgumentInterface $left, + null|string|ArgumentInterface $left, null|float|int|string|ArgumentInterface $right, ): static { $this->addPredicate( @@ -91,7 +93,7 @@ public function equalTo( * Utilizes Operator predicate */ public function notEqualTo( - null|float|int|string|ArgumentInterface $left, + null|string|ArgumentInterface $left, null|float|int|string|ArgumentInterface $right ): static { $this->addPredicate( @@ -107,7 +109,7 @@ public function notEqualTo( * Utilizes Operator predicate */ public function lessThan( - null|float|int|string|ArgumentInterface $left, + null|string|ArgumentInterface $left, null|float|int|string|ArgumentInterface $right ): static { $this->addPredicate( @@ -125,7 +127,7 @@ public function lessThan( * @return $this Provides a fluent interface */ public function greaterThan( - null|float|int|string|ArgumentInterface $left, + null|string|ArgumentInterface $left, null|float|int|string|ArgumentInterface $right ): static { $this->addPredicate( @@ -143,7 +145,7 @@ public function greaterThan( * @return $this Provides a fluent interface */ public function lessThanOrEqualTo( - null|float|int|string|ArgumentInterface $left, + null|string|ArgumentInterface $left, null|float|int|string|ArgumentInterface $right ): static { $this->addPredicate( @@ -161,7 +163,7 @@ public function lessThanOrEqualTo( * @return $this Provides a fluent interface */ public function greaterThanOrEqualTo( - null|float|int|string|ArgumentInterface $left, + null|string|ArgumentInterface $left, null|float|int|string|ArgumentInterface $right ): static { $this->addPredicate( @@ -179,7 +181,7 @@ public function greaterThanOrEqualTo( * @return $this Provides a fluent interface */ public function like( - null|float|int|string|ArgumentInterface $identifier, + null|string|ArgumentInterface $identifier, null|float|int|string|ArgumentInterface $like ): static { $this->addPredicate( @@ -197,7 +199,7 @@ public function like( * @return $this Provides a fluent interface */ public function notLike( - null|float|int|string|ArgumentInterface $identifier, + null|string|ArgumentInterface $identifier, null|float|int|string|ArgumentInterface $notLike ): static { $this->addPredicate( @@ -211,11 +213,12 @@ public function notLike( /** * Create an expression, with parameter placeholders * + * @param ExpressionParameter|array $parameters * @return $this Provides a fluent interface */ public function expression( string $expression, - null|string|float|int|array|ArgumentInterface|ExpressionInterface $parameters = [] + null|bool|string|float|int|array|ArgumentInterface|ExpressionInterface $parameters = [] ): static { if ($parameters !== []) { $this->addPredicate( @@ -254,7 +257,7 @@ public function literal(string $literal): static * * @return $this Provides a fluent interface */ - public function isNull(float|int|string|ArgumentInterface $identifier): static + public function isNull(string|ArgumentInterface $identifier): static { $this->addPredicate( new IsNull($identifier), @@ -270,7 +273,7 @@ public function isNull(float|int|string|ArgumentInterface $identifier): static * * @return $this Provides a fluent interface */ - public function isNotNull(float|int|string|ArgumentInterface $identifier): static + public function isNotNull(string|ArgumentInterface $identifier): static { $this->addPredicate( new IsNotNull($identifier), @@ -284,9 +287,10 @@ public function isNotNull(float|int|string|ArgumentInterface $identifier): stati * Create "IN" predicate * Utilizes In predicate * + * @param array|ArgumentInterface $valueSet * @return $this Provides a fluent interface */ - public function in(float|int|string|ArgumentInterface $identifier, array|ArgumentInterface $valueSet): static + public function in(string|ArgumentInterface $identifier, array|ArgumentInterface $valueSet): static { $this->addPredicate( new In($identifier, $valueSet), @@ -300,9 +304,10 @@ public function in(float|int|string|ArgumentInterface $identifier, array|Argumen * Create "NOT IN" predicate * Utilizes NotIn predicate * + * @param array|ArgumentInterface $valueSet * @return $this Provides a fluent interface */ - public function notIn(float|int|string|ArgumentInterface $identifier, array|ArgumentInterface $valueSet): static + public function notIn(string|ArgumentInterface $identifier, array|ArgumentInterface $valueSet): static { $this->addPredicate( new NotIn($identifier, $valueSet), @@ -319,9 +324,9 @@ public function notIn(float|int|string|ArgumentInterface $identifier, array|Argu * @return $this Provides a fluent interface */ public function between( - null|float|int|string|array|ArgumentInterface $identifier, - null|float|int|string|array|ArgumentInterface $minValue, - null|float|int|string|array|ArgumentInterface $maxValue + null|string|ArgumentInterface $identifier, + null|float|int|string|ArgumentInterface $minValue, + null|float|int|string|ArgumentInterface $maxValue ): static { $this->addPredicate( new Between($identifier, $minValue, $maxValue), @@ -338,9 +343,9 @@ public function between( * @return $this Provides a fluent interface */ public function notBetween( - null|float|int|string|array|ArgumentInterface $identifier, - null|float|int|string|array|ArgumentInterface $minValue, - null|float|int|string|array|ArgumentInterface $maxValue + null|string|ArgumentInterface $identifier, + null|float|int|string|ArgumentInterface $minValue, + null|float|int|string|ArgumentInterface $maxValue ): static { $this->addPredicate( new NotBetween($identifier, $minValue, $maxValue), diff --git a/src/Sql/Predicate/PredicateSet.php b/src/Sql/Predicate/PredicateSet.php index a172ddf8..2c29f525 100644 --- a/src/Sql/Predicate/PredicateSet.php +++ b/src/Sql/Predicate/PredicateSet.php @@ -20,6 +20,10 @@ // phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse +/** + * @phpstan-type PredicateEntry array{0: string, 1: PredicateInterface} + * @phpstan-type PredicateSpecValue PredicateInterface|Expression|scalar|null|array + */ class PredicateSet implements PredicateInterface, Countable { final public const OP_AND = 'AND'; @@ -34,10 +38,13 @@ class PredicateSet implements PredicateInterface, Countable protected string $defaultCombination = self::OP_AND; + /** @var list */ protected array $predicates = []; /** * Constructor + * + * @param array|null $predicates */ public function __construct(?array $predicates = null, string $defaultCombination = self::OP_AND) { @@ -71,6 +78,7 @@ public function addPredicate(PredicateInterface $predicate, ?string $combination /** * Add predicates to set * + * @param PredicateInterface|Closure|string|array $predicates * @throws Exception\InvalidArgumentException */ public function addPredicates( @@ -119,9 +127,13 @@ public function addPredicates( $pvalue->getExpression(), $pvalue->getParameters() ); - } else { + } elseif (is_string($pvalue)) { $predicate = str_contains($pvalue, Expression::PLACEHOLDER) - ? new Expression($pvalue) : new Literal($pvalue); + ? new PredicateExpression($pvalue) : new Literal($pvalue); + } else { + throw new Exception\InvalidArgumentException( + 'Invalid predicate supplied; expected a string, Expression, or PredicateInterface' + ); } $this->addPredicate($predicate, $combination); @@ -132,6 +144,8 @@ public function addPredicates( /** * Return the predicates + * + * @return list */ public function getPredicates(): array { @@ -168,20 +182,6 @@ public function getExpressionData(): array return ['spec' => '', 'values' => []]; } - if ($predicateCount === 1) { - [$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; From ec4d9e6a6f45cd86d43080313260edf151e1bfdb Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 9 Jul 2026 19:19:18 +1000 Subject: [PATCH 11/22] Type the Ddl column, constraint, index and table builders - Type the column option maps, constraint/index column lists (list) and the create/alter member collections - Align Column subclass constructors' default to the setDefault() contract; type Check's expression as string (the ExpressionInterface form was declared but unimplemented) - Make the Ddl process methods take a non-null platform, drop the psalm list{0?:...} return tags mago cannot parse, and read plain-string specs through specificationString() - Replace index-based column loops with foreach and guard array truthiness Signed-off-by: Simon Mundy --- src/Sql/Ddl/AlterTable.php | 51 ++++++++++++------- src/Sql/Ddl/Column/AbstractLengthColumn.php | 6 ++- .../Ddl/Column/AbstractPrecisionColumn.php | 9 +++- src/Sql/Ddl/Column/Column.php | 16 ++++-- src/Sql/Ddl/Column/ColumnInterface.php | 3 ++ src/Sql/Ddl/Column/Integer.php | 2 +- src/Sql/Ddl/Constraint/AbstractConstraint.php | 14 ++++- src/Sql/Ddl/Constraint/Check.php | 8 +-- .../Ddl/Constraint/ConstraintInterface.php | 3 ++ src/Sql/Ddl/Constraint/ForeignKey.php | 12 +++-- src/Sql/Ddl/CreateTable.php | 47 ++++++++++------- src/Sql/Ddl/DropTable.php | 8 ++- src/Sql/Ddl/Index/Index.php | 11 ++-- 13 files changed, 128 insertions(+), 62 deletions(-) diff --git a/src/Sql/Ddl/AlterTable.php b/src/Sql/Ddl/AlterTable.php index f85849f4..6c359bca 100644 --- a/src/Sql/Ddl/AlterTable.php +++ b/src/Sql/Ddl/AlterTable.php @@ -14,6 +14,9 @@ use function is_int; use function strtoupper; +/** + * @phpstan-import-type SpecificationArray from AbstractSql + */ class AlterTable extends AbstractSql { final public const ADD_COLUMNS = 'addColumns'; @@ -32,22 +35,31 @@ class AlterTable extends AbstractSql final public const TABLE_OPTIONS = 'tableOptions'; + /** @var list */ protected array $addColumns = []; + /** @var list */ protected array $addConstraints = []; + /** @var array */ protected array $changeColumns = []; + /** @var list */ protected array $dropColumns = []; + /** @var list */ protected array $dropConstraints = []; + /** @var list */ protected array $dropIndexes = []; + /** @var array */ protected array $options = []; /** * Specifications for Sql String generation + * + * @var array */ protected array $specifications = [ self::TABLE => "ALTER TABLE %1\$s\n", @@ -155,18 +167,27 @@ public function setOption(string $name, Literal|bool|int|string $value): static return $this; } + /** + * @param array $options + */ public function setOptions(array $options): static { $this->options = $options; return $this; } + /** + * @return array + */ public function getOptions(): array { return $this->options; } - public function getRawState(?string $key = null): array|string + /** + * @return TableIdentifier|string|array + */ + public function getRawState(?string $key = null): array|string|TableIdentifier { $rawState = [ self::TABLE => $this->table, @@ -183,16 +204,15 @@ public function getRawState(?string $key = null): array|string } /** @return string[] */ - protected function processTable(?PlatformInterface $adapterPlatform = null): array + protected function processTable(PlatformInterface $adapterPlatform): array { - return [$this->resolveTable($this->table, $adapterPlatform)]; + return [$this->resolveTable($this->table, $adapterPlatform) ?? '']; } /** * @return string[][] - * @psalm-return list{list{0?: string,...}} */ - protected function processAddColumns(?PlatformInterface $adapterPlatform = null): array + protected function processAddColumns(PlatformInterface $adapterPlatform): array { $sqls = []; foreach ($this->addColumns as $column) { @@ -204,9 +224,8 @@ protected function processAddColumns(?PlatformInterface $adapterPlatform = null) /** * @return string[][][] - * @psalm-return list{list{0?: list{string, string},...}} */ - protected function processChangeColumns(?PlatformInterface $adapterPlatform = null): array + protected function processChangeColumns(PlatformInterface $adapterPlatform): array { $sqls = []; foreach ($this->changeColumns as $name => $column) { @@ -221,9 +240,8 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu /** * @return string[][] - * @psalm-return list{list{0?: string,...}} */ - protected function processDropColumns(?PlatformInterface $adapterPlatform = null): array + protected function processDropColumns(PlatformInterface $adapterPlatform): array { $sqls = []; foreach ($this->dropColumns as $column) { @@ -235,9 +253,8 @@ protected function processDropColumns(?PlatformInterface $adapterPlatform = null /** * @return string[][] - * @psalm-return list{list{0?: string,...}} */ - protected function processAddConstraints(?PlatformInterface $adapterPlatform = null): array + protected function processAddConstraints(PlatformInterface $adapterPlatform): array { $sqls = []; foreach ($this->addConstraints as $constraint) { @@ -249,9 +266,8 @@ protected function processAddConstraints(?PlatformInterface $adapterPlatform = n /** * @return string[][] - * @psalm-return list{list{0?: string,...}} */ - protected function processDropConstraints(?PlatformInterface $adapterPlatform = null): array + protected function processDropConstraints(PlatformInterface $adapterPlatform): array { $sqls = []; foreach ($this->dropConstraints as $constraint) { @@ -263,9 +279,8 @@ protected function processDropConstraints(?PlatformInterface $adapterPlatform = /** * @return string[][] - * @psalm-return list{list{0?: string,...}} */ - protected function processDropIndexes(?PlatformInterface $adapterPlatform = null): array + protected function processDropIndexes(PlatformInterface $adapterPlatform): array { $sqls = []; foreach ($this->dropIndexes as $index) { @@ -278,9 +293,9 @@ protected function processDropIndexes(?PlatformInterface $adapterPlatform = null /** * @return string[][]|null */ - protected function processTableOptions(?PlatformInterface $adapterPlatform = null): ?array + protected function processTableOptions(PlatformInterface $adapterPlatform): ?array { - if (! $this->options) { + if ($this->options === []) { return null; } @@ -294,7 +309,7 @@ protected function processTableOptions(?PlatformInterface $adapterPlatform = nul } elseif (is_int($value)) { $value = (string) $value; } else { - $value = $adapterPlatform->quoteTrustedValue($value); + $value = $adapterPlatform->quoteTrustedValue($value) ?? ''; } $sqls[] = $key . ' = ' . $value; } diff --git a/src/Sql/Ddl/Column/AbstractLengthColumn.php b/src/Sql/Ddl/Column/AbstractLengthColumn.php index b140712a..34c053b0 100644 --- a/src/Sql/Ddl/Column/AbstractLengthColumn.php +++ b/src/Sql/Ddl/Column/AbstractLengthColumn.php @@ -6,6 +6,7 @@ use Override; use PhpDb\Sql\Argument\Literal; +use PhpDb\Sql\Argument\Value; use function array_splice; @@ -15,11 +16,14 @@ abstract class AbstractLengthColumn extends Column protected ?int $length = null; + /** + * @param array $options + */ public function __construct( string $name, ?int $length = null, bool $nullable = false, - mixed $default = null, + string|int|float|bool|Literal|Value|null $default = null, array $options = [] ) { $this->setLength($length); diff --git a/src/Sql/Ddl/Column/AbstractPrecisionColumn.php b/src/Sql/Ddl/Column/AbstractPrecisionColumn.php index e8d4bee0..e3297aba 100644 --- a/src/Sql/Ddl/Column/AbstractPrecisionColumn.php +++ b/src/Sql/Ddl/Column/AbstractPrecisionColumn.php @@ -5,17 +5,22 @@ namespace PhpDb\Sql\Ddl\Column; use Override; +use PhpDb\Sql\Argument\Literal; +use PhpDb\Sql\Argument\Value; abstract class AbstractPrecisionColumn extends AbstractLengthColumn { protected ?int $decimal; + /** + * @param array $options + */ public function __construct( string $name, ?int $digits = null, ?int $decimal = null, bool $nullable = false, - mixed $default = null, + string|int|float|bool|Literal|Value|null $default = null, array $options = [] ) { $this->setDecimal($decimal); @@ -49,7 +54,7 @@ public function getDecimal(): ?int protected function getLengthExpression(): string { if ($this->decimal !== null) { - return $this->length . ',' . $this->decimal; + return (string) $this->length . ',' . $this->decimal; } return (string) $this->length; diff --git a/src/Sql/Ddl/Column/Column.php b/src/Sql/Ddl/Column/Column.php index 42c42770..3b22fec5 100644 --- a/src/Sql/Ddl/Column/Column.php +++ b/src/Sql/Ddl/Column/Column.php @@ -15,12 +15,13 @@ class Column implements ColumnInterface { - protected string|int|float|bool|Literal|Value|null $default; + protected string|int|float|bool|Literal|Value|null $default = null; protected bool $isNullable = false; protected string $name = ''; + /** @var array */ protected array $options = []; /** @var ConstraintInterface[] */ @@ -30,10 +31,13 @@ class Column implements ColumnInterface protected string $type = 'INTEGER'; + /** + * @param array $options + */ public function __construct( string $name = '', bool $nullable = false, - mixed $default = null, + string|int|float|bool|Literal|Value|null $default = null, array $options = [] ) { $this->setName($name); @@ -78,6 +82,9 @@ public function getDefault(): string|int|float|bool|Literal|Value|null return $this->default; } + /** + * @param array $options + */ public function setOptions(array $options): static { $this->options = $options; @@ -90,6 +97,9 @@ public function setOption(string $name, bool|string $value): static return $this; } + /** + * @return array + */ #[Override] public function getOptions(): array { @@ -113,7 +123,7 @@ public function getExpressionData(): array new Literal($this->type), ]; - if ($this->isNullable === false) { + if (! $this->isNullable) { $specParts[] = 'NOT NULL'; } else { $specParts[] = 'NULL'; diff --git a/src/Sql/Ddl/Column/ColumnInterface.php b/src/Sql/Ddl/Column/ColumnInterface.php index b9665c5d..215de764 100644 --- a/src/Sql/Ddl/Column/ColumnInterface.php +++ b/src/Sql/Ddl/Column/ColumnInterface.php @@ -19,5 +19,8 @@ public function isNullable(): bool; public function getDefault(): string|int|float|bool|Literal|Value|null; + /** + * @return array + */ public function getOptions(): array; } diff --git a/src/Sql/Ddl/Column/Integer.php b/src/Sql/Ddl/Column/Integer.php index 84ee8f37..d856d715 100644 --- a/src/Sql/Ddl/Column/Integer.php +++ b/src/Sql/Ddl/Column/Integer.php @@ -16,7 +16,7 @@ public function getExpressionData(): array $options = $this->getOptions(); if (isset($options['length'])) { - $expressionData['spec'] .= ' (' . $options['length'] . ')'; + $expressionData['spec'] .= ' (' . (string) $options['length'] . ')'; } return $expressionData; diff --git a/src/Sql/Ddl/Constraint/AbstractConstraint.php b/src/Sql/Ddl/Constraint/AbstractConstraint.php index aee749a6..04c1f5ad 100644 --- a/src/Sql/Ddl/Constraint/AbstractConstraint.php +++ b/src/Sql/Ddl/Constraint/AbstractConstraint.php @@ -22,8 +22,12 @@ abstract class AbstractConstraint implements ConstraintInterface protected string $name = ''; + /** @var list */ protected array $columns = []; + /** + * @param null|list|string $columns + */ public function __construct(null|array|string $columns = null, ?string $name = null) { if ($columns !== null) { @@ -46,6 +50,9 @@ public function getName(): string return $this->name; } + /** + * @param string|list $columns + */ public function setColumns(string|array $columns): static { $this->columns = (array) $columns; @@ -59,6 +66,9 @@ public function addColumn(string $column): static return $this; } + /** + * @return list + */ #[Override] public function getColumns(): array { return $this->columns; @@ -84,8 +94,8 @@ public function getExpressionData(): array if ($columnCount !== 0) { $columnSpec = array_fill(0, $columnCount, '%s'); $specParts[] = str_replace('%s', implode(', ', $columnSpec), $this->columnSpecification); - for ($i = 0; $i < $columnCount; $i++) { - $values[] = new Identifier($this->columns[$i]); + foreach ($this->columns as $column) { + $values[] = new Identifier($column); } } diff --git a/src/Sql/Ddl/Constraint/Check.php b/src/Sql/Ddl/Constraint/Check.php index 86413b07..49a0edd9 100644 --- a/src/Sql/Ddl/Constraint/Check.php +++ b/src/Sql/Ddl/Constraint/Check.php @@ -7,20 +7,16 @@ use Override; use PhpDb\Sql\Argument\Identifier; use PhpDb\Sql\Argument\Literal; -use PhpDb\Sql\ExpressionInterface; use function implode; class Check extends AbstractConstraint { - protected string|ExpressionInterface $expression; + protected string $expression; protected string $specification = 'CHECK (%s)'; - /** - * @param string|ExpressionInterface $expression - */ - public function __construct($expression, ?string $name) + public function __construct(string $expression, ?string $name) { parent::__construct(null, $name); diff --git a/src/Sql/Ddl/Constraint/ConstraintInterface.php b/src/Sql/Ddl/Constraint/ConstraintInterface.php index 9ca18cb3..27a90581 100644 --- a/src/Sql/Ddl/Constraint/ConstraintInterface.php +++ b/src/Sql/Ddl/Constraint/ConstraintInterface.php @@ -8,5 +8,8 @@ interface ConstraintInterface extends ExpressionInterface { + /** + * @return list + */ public function getColumns(): array; } diff --git a/src/Sql/Ddl/Constraint/ForeignKey.php b/src/Sql/Ddl/Constraint/ForeignKey.php index bcbad0f3..17dfdbee 100644 --- a/src/Sql/Ddl/Constraint/ForeignKey.php +++ b/src/Sql/Ddl/Constraint/ForeignKey.php @@ -22,17 +22,18 @@ class ForeignKey extends AbstractConstraint protected string $columnSpecification = 'FOREIGN KEY (%s)'; - /** @var string[] */ + /** @var list */ protected array $referenceColumn = []; - /** @var string[] */ + /** @var array{0: string, 1: string} */ protected array $referenceSpecification = [ 'REFERENCES %s', 'ON DELETE %s ON UPDATE %s', ]; /** - * @param string[]|string|null $referenceColumn + * @param string|list $columns + * @param list|string|null $referenceColumn */ public function __construct( string $name, @@ -71,13 +72,16 @@ public function setReferenceTable(string $referenceTable): static return $this; } + /** + * @return list + */ public function getReferenceColumn(): array { return $this->referenceColumn; } /** - * @param string[]|string $referenceColumn + * @param string|list $referenceColumn */ public function setReferenceColumn(array|string $referenceColumn): static { diff --git a/src/Sql/Ddl/CreateTable.php b/src/Sql/Ddl/CreateTable.php index 6d0ddb2b..2f290eeb 100644 --- a/src/Sql/Ddl/CreateTable.php +++ b/src/Sql/Ddl/CreateTable.php @@ -15,6 +15,9 @@ use function is_int; use function strtoupper; +/** + * @phpstan-import-type SpecificationArray from AbstractSql + */ class CreateTable extends AbstractSql { final public const COLUMNS = 'columns'; @@ -25,19 +28,20 @@ class CreateTable extends AbstractSql final public const TABLE_OPTIONS = 'tableOptions'; + /** @var list */ protected array $columns = []; + /** @var list */ protected array $constraints = []; protected bool $ifNotExists = false; protected bool $isTemporary = false; + /** @var array */ protected array $options = []; - /** - * {@inheritDoc} - */ + /** @var array */ protected array $specifications = [ self::TABLE => 'CREATE %1$sTABLE %2$s%3$s (', self::COLUMNS => [ @@ -109,22 +113,27 @@ public function setOption(string $name, Literal|bool|int|string $value): static return $this; } + /** + * @param array $options + */ public function setOptions(array $options): static { $this->options = $options; return $this; } + /** + * @return array + */ public function getOptions(): array { return $this->options; } /** - * @return ((Column\ColumnInterface|string)[]|Column\ColumnInterface|string)[]|string - * @psalm-return array|string>|string + * @return TableIdentifier|string|array */ - public function getRawState(?string $key = null): array|string + public function getRawState(?string $key = null): array|string|TableIdentifier { $rawState = [ self::COLUMNS => $this->columns, @@ -139,21 +148,21 @@ public function getRawState(?string $key = null): array|string /** * @return string[] */ - protected function processTable(?PlatformInterface $adapterPlatform = null): array + protected function processTable(PlatformInterface $adapterPlatform): array { return [ $this->isTemporary ? 'TEMPORARY ' : '', $this->ifNotExists ? 'IF NOT EXISTS ' : '', - $this->resolveTable($this->table, $adapterPlatform), + $this->resolveTable($this->table, $adapterPlatform) ?? '', ]; } /** * @return string[][]|null */ - protected function processColumns(?PlatformInterface $adapterPlatform = null): ?array + protected function processColumns(PlatformInterface $adapterPlatform): ?array { - if (! $this->columns) { + if ($this->columns === []) { return null; } @@ -166,10 +175,10 @@ protected function processColumns(?PlatformInterface $adapterPlatform = null): ? return [$sqls]; } - protected function processCombinedby(?PlatformInterface $adapterPlatform = null): string|null + protected function processCombinedby(): string|null { - if ($this->constraints && $this->columns) { - return $this->specifications['combinedBy']; + if ($this->constraints !== [] && $this->columns !== []) { + return $this->specificationString('combinedBy'); } return null; @@ -178,9 +187,9 @@ protected function processCombinedby(?PlatformInterface $adapterPlatform = null) /** * @return string[][]|null */ - protected function processConstraints(?PlatformInterface $adapterPlatform = null): ?array + protected function processConstraints(PlatformInterface $adapterPlatform): ?array { - if (! $this->constraints) { + if ($this->constraints === []) { return null; } @@ -196,7 +205,7 @@ protected function processConstraints(?PlatformInterface $adapterPlatform = null /** * @return string[] */ - protected function processStatementEnd(?PlatformInterface $adapterPlatform = null): array + protected function processStatementEnd(): array { return ["\n)"]; } @@ -204,9 +213,9 @@ protected function processStatementEnd(?PlatformInterface $adapterPlatform = nul /** * @return string[]|null */ - protected function processTableOptions(?PlatformInterface $adapterPlatform = null): ?array + protected function processTableOptions(PlatformInterface $adapterPlatform): ?array { - if (! $this->options) { + if ($this->options === []) { return null; } @@ -220,7 +229,7 @@ protected function processTableOptions(?PlatformInterface $adapterPlatform = nul } elseif (is_int($value)) { $value = (string) $value; } else { - $value = $adapterPlatform->quoteTrustedValue($value); + $value = $adapterPlatform->quoteTrustedValue($value) ?? ''; } $parts[] = $key . ' = ' . $value; } diff --git a/src/Sql/Ddl/DropTable.php b/src/Sql/Ddl/DropTable.php index dd0d85a5..631fe75a 100644 --- a/src/Sql/Ddl/DropTable.php +++ b/src/Sql/Ddl/DropTable.php @@ -8,12 +8,16 @@ use PhpDb\Sql\AbstractSql; use PhpDb\Sql\TableIdentifier; +/** + * @phpstan-import-type SpecificationArray from AbstractSql + */ class DropTable extends AbstractSql { final public const TABLE = 'table'; protected bool $ifExists = false; + /** @var array */ protected array $specifications = [ self::TABLE => 'DROP TABLE %1$s%2$s', ]; @@ -37,11 +41,11 @@ public function getIfExists(): bool } /** @return string[] */ - protected function processTable(?PlatformInterface $adapterPlatform = null): array + protected function processTable(PlatformInterface $adapterPlatform): array { return [ $this->ifExists ? 'IF EXISTS ' : '', - $this->resolveTable($this->table, $adapterPlatform), + $this->resolveTable($this->table, $adapterPlatform) ?? '', ]; } } diff --git a/src/Sql/Ddl/Index/Index.php b/src/Sql/Ddl/Index/Index.php index a3802782..a49ea17e 100644 --- a/src/Sql/Ddl/Index/Index.php +++ b/src/Sql/Ddl/Index/Index.php @@ -8,7 +8,6 @@ use PhpDb\Sql\Argument\Identifier; use PhpDb\Sql\Argument\Literal; -use function count; use function implode; use function str_replace; @@ -16,10 +15,15 @@ class Index extends AbstractIndex { protected string $specification = 'INDEX %s(...)'; + /** @var array */ protected array $lengths; protected ?string $type = null; + /** + * @param null|list|string $columns + * @param array $lengths + */ public function __construct(null|array|string $columns, ?string $name = null, array $lengths = []) { parent::__construct($columns, $name); @@ -42,13 +46,12 @@ public function getType(): ?string #[Override] public function getExpressionData(): array { - $colCount = count($this->columns); $values = [new Identifier($this->name)]; $specParts = []; - for ($i = 0; $i < $colCount; $i++) { + foreach ($this->columns as $i => $column) { $specPart = '%s'; - $values[] = new Identifier($this->columns[$i]); + $values[] = new Identifier($column); if (isset($this->lengths[$i])) { $specPart .= '(' . $this->lengths[$i] . ')'; From b6d922401bb37565f015f3ad4f14dd71f074da76 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 9 Jul 2026 20:06:27 +1000 Subject: [PATCH 12/22] Align where()/argument value array shapes with their delegates - Narrow the where()/having() array docblocks to the PredicateSpecValue contract they hand to PredicateSet::addPredicates, importing the shape rather than passing the wider array - Type Argument::values() and ArgumentInterface::getValue() array shapes to the scalar-or-null values they actually carry Signed-off-by: Simon Mundy --- src/Sql/Argument.php | 3 +++ src/Sql/ArgumentInterface.php | 3 +++ src/Sql/Delete.php | 3 ++- src/Sql/Select.php | 5 +++-- src/Sql/Update.php | 3 ++- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Sql/Argument.php b/src/Sql/Argument.php index c2d97067..cd98ba99 100644 --- a/src/Sql/Argument.php +++ b/src/Sql/Argument.php @@ -21,6 +21,9 @@ public static function value(null|string|int|float|bool $value): Value return new Value($value); } + /** + * @param array $values + */ public static function values(array $values): Values { return new Values($values); diff --git a/src/Sql/ArgumentInterface.php b/src/Sql/ArgumentInterface.php index 9a9b356b..5183caba 100644 --- a/src/Sql/ArgumentInterface.php +++ b/src/Sql/ArgumentInterface.php @@ -8,6 +8,9 @@ interface ArgumentInterface { public function getType(): ArgumentType; + /** + * @return ExpressionInterface|SqlInterface|string|int|float|bool|array|null + */ public function getValue(): ExpressionInterface|SqlInterface|string|int|float|bool|array|null; public function getSpecification(): string; diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index 1438841d..fe26d1ca 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -20,6 +20,7 @@ * @property Where $where * * @phpstan-import-type SpecificationArray from AbstractSql + * @phpstan-import-type PredicateSpecValue from Predicate\PredicateSet */ class Delete extends AbstractPreparableSql { @@ -84,7 +85,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/Select.php b/src/Sql/Select.php index c74923c6..918c708b 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -40,6 +40,7 @@ * @phpstan-import-type ColumnDescriptor from AbstractSql * @phpstan-import-type SpecResult from AbstractSql * @phpstan-import-type JoinName from Join + * @phpstan-import-type PredicateSpecValue from Predicate\PredicateSet */ class Select extends AbstractPreparableSql { @@ -276,7 +277,7 @@ public function join( /** * Create where clause * - * @param PredicateInterface|array|string|Closure $predicate + * @param PredicateInterface|array|string|Closure $predicate * @throws Exception\InvalidArgumentException */ public function where( @@ -307,7 +308,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( diff --git a/src/Sql/Update.php b/src/Sql/Update.php index 9be68eb7..01153df6 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -28,6 +28,7 @@ * @phpstan-import-type ColumnDescriptor from AbstractSql * @phpstan-import-type SpecificationArray from AbstractSql * @phpstan-import-type JoinName from Join + * @phpstan-import-type PredicateSpecValue from Predicate\PredicateSet */ class Update extends AbstractPreparableSql { @@ -142,7 +143,7 @@ public function set(array $values, string|int $flag = self::VALUES_SET): static /** * Create where clause * - * @param PredicateInterface|array|Closure|string|Where $predicate + * @param PredicateInterface|array|Closure|string|Where $predicate * @throws Exception\InvalidArgumentException */ public function where( From 74ba74af7206af5ae647be7b6e46191843668278 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 10:30:15 +1000 Subject: [PATCH 13/22] Accept subquery values in predicate and expression parameters A Select subquery is a valid where() value, so widen PredicateSpecValue (and Predicate::expression()/Expression parameters) to include SqlInterface. Route SqlInterface through SelectArgument in Expression::setParameters, which the wrapper already declared it accepts, closing a latent TypeError where a subquery placeholder parameter fell through to Value. Signed-off-by: Simon Mundy --- src/Sql/Expression.php | 8 ++++---- src/Sql/Predicate/Predicate.php | 3 ++- src/Sql/Predicate/PredicateSet.php | 5 ++++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Sql/Expression.php b/src/Sql/Expression.php index dfe30f38..a73e6fd7 100644 --- a/src/Sql/Expression.php +++ b/src/Sql/Expression.php @@ -20,7 +20,7 @@ use function substr_count; /** - * @phpstan-type ExpressionParameter scalar|null|array|ArgumentInterface|ExpressionInterface + * @phpstan-type ExpressionParameter scalar|null|array|ArgumentInterface|ExpressionInterface|SqlInterface */ class Expression extends AbstractExpression { @@ -40,7 +40,7 @@ class Expression extends AbstractExpression */ public function __construct( string $expression = '', - null|bool|string|float|int|array|ArgumentInterface|ExpressionInterface $parameters = [] + null|bool|string|float|int|array|ArgumentInterface|ExpressionInterface|SqlInterface $parameters = [] ) { if ($expression !== '') { $this->setExpression($expression); @@ -83,7 +83,7 @@ public function getExpression(): string * @throws Exception\InvalidArgumentException */ public function setParameters( - null|bool|string|float|int|array|ExpressionInterface|ArgumentInterface $parameters = [] + null|bool|string|float|int|array|ExpressionInterface|ArgumentInterface|SqlInterface $parameters = [] ): self { if (! is_array($parameters)) { $parameters = [$parameters]; @@ -92,7 +92,7 @@ public function setParameters( foreach ($parameters as $parameter) { if (is_array($parameter)) { $parameter = new Values($parameter); - } elseif ($parameter instanceof ExpressionInterface) { + } elseif ($parameter instanceof ExpressionInterface || $parameter instanceof SqlInterface) { $parameter = new SelectArgument($parameter); } elseif (! $parameter instanceof ArgumentInterface) { $parameter = new Value($parameter); diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php index 389de018..442a490b 100644 --- a/src/Sql/Predicate/Predicate.php +++ b/src/Sql/Predicate/Predicate.php @@ -7,6 +7,7 @@ use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Exception\RuntimeException; use PhpDb\Sql\ExpressionInterface; +use PhpDb\Sql\SqlInterface; /** * @property Predicate $and @@ -218,7 +219,7 @@ public function notLike( */ public function expression( string $expression, - null|bool|string|float|int|array|ArgumentInterface|ExpressionInterface $parameters = [] + null|bool|string|float|int|array|ArgumentInterface|ExpressionInterface|SqlInterface $parameters = [] ): static { if ($parameters !== []) { $this->addPredicate( diff --git a/src/Sql/Predicate/PredicateSet.php b/src/Sql/Predicate/PredicateSet.php index 2c29f525..0b52d45e 100644 --- a/src/Sql/Predicate/PredicateSet.php +++ b/src/Sql/Predicate/PredicateSet.php @@ -7,9 +7,12 @@ use Closure; use Countable; use Override; +use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Exception; use PhpDb\Sql\Expression; +use PhpDb\Sql\ExpressionInterface; use PhpDb\Sql\Predicate\Expression as PredicateExpression; +use PhpDb\Sql\SqlInterface; use ReturnTypeWillChange; use function count; @@ -22,7 +25,7 @@ /** * @phpstan-type PredicateEntry array{0: string, 1: PredicateInterface} - * @phpstan-type PredicateSpecValue PredicateInterface|Expression|scalar|null|array + * @phpstan-type PredicateSpecValue PredicateInterface|ExpressionInterface|SqlInterface|ArgumentInterface|scalar|null|array */ class PredicateSet implements PredicateInterface, Countable { From 0a8cd6ca95d367c849376bdba41aa85406f715e0 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 11:02:58 +1000 Subject: [PATCH 14/22] Type the Sql functional test base, Delete test and shared readers - Add readArrayAttribute() so reflection reads narrow to array at one boundary instead of a mixed cast at every call site - Give AbstractSqlFunctionalTestCase phpstan-type shapes for its data providers, dropping the per-iteration mixed assignments and suppressions - Convert DeleteTest predicate assertions to the predicateEntryAt extractor - Narrow Delete::getRawState() off mixed to its real union Signed-off-by: Simon Mundy --- src/Sql/Delete.php | 5 +- test/unit/DeprecatedAssertionsTrait.php | 16 +++++ .../Sql/AbstractSqlFunctionalTestCase.php | 56 +++++++++++------ test/unit/Sql/DeleteTest.php | 61 ++++++++++++------- 4 files changed, 98 insertions(+), 40 deletions(-) diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index fe26d1ca..b9cccfdc 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -72,7 +72,10 @@ public function from(TableIdentifier|string|array $table): static return $this; } - public function getRawState(?string $key = null): mixed + /** + * @return array|bool|string|TableIdentifier|Where + */ + public function getRawState(?string $key = null): array|bool|string|TableIdentifier|Where { $rawState = [ 'emptyWhereProtection' => $this->emptyWhereProtection, diff --git a/test/unit/DeprecatedAssertionsTrait.php b/test/unit/DeprecatedAssertionsTrait.php index fbec2b19..cdc99071 100644 --- a/test/unit/DeprecatedAssertionsTrait.php +++ b/test/unit/DeprecatedAssertionsTrait.php @@ -35,4 +35,20 @@ public function readAttribute(object $instance, string $attribute): mixed $r = new ReflectionProperty($instance, $attribute); return $r->getValue($instance); } + + /** + * Read an attribute known to hold an array, narrowing the reflected + * value to an array at this single boundary. + * + * @return array + * @throws ReflectionException + */ + public function readArrayAttribute(object $instance, string $attribute): array + { + /** @mago-expect analysis:mixed-assignment */ + $value = $this->readAttribute($instance, $attribute); + Assert::assertIsArray($value); + + return $value; + } } diff --git a/test/unit/Sql/AbstractSqlFunctionalTestCase.php b/test/unit/Sql/AbstractSqlFunctionalTestCase.php index 5a416439..efa33443 100644 --- a/test/unit/Sql/AbstractSqlFunctionalTestCase.php +++ b/test/unit/Sql/AbstractSqlFunctionalTestCase.php @@ -36,9 +36,29 @@ * @method Insert insert(TableIdentifier|null|string $sqlString) * @method CreateTable createTable(null|string|TableIdentifier $sqlString) * @method Column createColumn(null|string $sqlString) + * + * @phpstan-type DecoratorSpec PlatformDecoratorInterface|array{0: class-string, 1: string} + * @phpstan-type PlatformExpectation string|array{ + * 'string'?: string, + * prepare?: string, + * parameters?: array, + * decorators?: array, + * } + * @phpstan-type SqlTestCase array{ + * sqlObject: PreparableSqlInterface|SqlInterface, + * expected: array, + * } + * @phpstan-type ProvidedCase array{ + * sqlObject: PreparableSqlInterface|SqlInterface, + * platform: string, + * expected: PlatformExpectation, + * } */ abstract class AbstractSqlFunctionalTestCase extends TestCase { + /** + * @return array + */ protected static function dataProviderCommonProcessMethods(): array { // phpcs:disable Generic.Files.LineLength.TooLong @@ -194,6 +214,9 @@ protected static function dataProviderCommonProcessMethods(): array // phpcs:enable Generic.Files.LineLength.TooLong } + /** + * @return array + */ protected static function dataProviderDecorators(): array { return [ @@ -226,6 +249,9 @@ protected static function dataProviderDecorators(): array ]; } + /** + * @return array + */ public static function dataProvider(): array { $data = array_merge( @@ -235,13 +261,9 @@ public static function dataProvider(): array $res = []; foreach ($data as $index => $test) { - self::assertIsArray($test); - $testExpected = $test['expected'] ?? []; - self::assertIsArray($testExpected); - /** @psalm-suppress MixedAssignment */ - foreach ($testExpected as $platform => $expected) { + foreach ($test['expected'] as $platform => $expected) { $res[$index . '->' . $platform] = [ - 'sqlObject' => $test['sqlObject'] ?? null, + 'sqlObject' => $test['sqlObject'], 'platform' => $platform, 'expected' => $expected, ]; @@ -251,15 +273,16 @@ public static function dataProvider(): array return $res; } + /** + * @param PlatformExpectation $expected + */ #[DataProvider('dataProvider')] public function test(PreparableSqlInterface|SqlInterface $sqlObject, string $platform, string|array $expected): void { $sql = new Sql\Sql($this->resolveAdapter($platform)); if (is_array($expected) && isset($expected['decorators'])) { - /** @var PlatformDecoratorInterface|array $decorator */ foreach ($expected['decorators'] as $type => $decorator) { - self::assertIsString($type); $decorator = $this->resolveDecorator($decorator); $this->assertInstanceOf(PlatformDecoratorInterface::class, $decorator); @@ -270,7 +293,7 @@ public function test(PreparableSqlInterface|SqlInterface $sqlObject, string $pla } } - $expectedString = is_string($expected) ? $expected : (string) ($expected['string'] ?? ''); + $expectedString = is_string($expected) ? $expected : ($expected['string'] ?? ''); if ($expectedString !== '') { self::assertInstanceOf(SqlInterface::class, $sqlObject); $actual = $sql->buildSqlString($sqlObject); @@ -290,15 +313,14 @@ public function test(PreparableSqlInterface|SqlInterface $sqlObject, string $pla } } + /** + * @param DecoratorSpec $decorator + */ protected function resolveDecorator( PlatformDecoratorInterface|array $decorator - ): PlatformDecoratorInterface|MockObject|null { + ): PlatformDecoratorInterface|MockObject { if (is_array($decorator)) { - self::assertTrue(isset($decorator[0], $decorator[1])); - - /** @var class-string $classString */ - $classString = $decorator[0]; - $decoratorMock = $this->getMockBuilder($classString) + $decoratorMock = $this->getMockBuilder($decorator[0]) ->onlyMethods(['buildSqlString']) ->setConstructorArgs([null]) ->getMock(); @@ -310,7 +332,7 @@ protected function resolveDecorator( protected function resolveAdapter(string $platformName): Adapter\Adapter { - // Only sql92 platform is supported after abstraction + self::assertSame('sql92', $platformName, 'Only the sql92 platform is supported after abstraction'); $platform = new TestAsset\TrustingSql92Platform(); $mockDriver = $this->getMockBuilder(DriverInterface::class)->getMock(); @@ -325,7 +347,7 @@ protected function resolveAdapter(string $platformName): Adapter\Adapter $mockStatement = $this->createMock(StatementInterface::class); $mockStatement->expects($this->any()) ->method('setSql') - ->willReturnCallback(function ($sql) use ($container, $mockStatement): MockObject { + ->willReturnCallback(function (string $sql) use ($container, $mockStatement): MockObject { $container->setSql($sql); return $mockStatement; }); diff --git a/test/unit/Sql/DeleteTest.php b/test/unit/Sql/DeleteTest.php index 2d1ce488..6881c04f 100644 --- a/test/unit/Sql/DeleteTest.php +++ b/test/unit/Sql/DeleteTest.php @@ -44,6 +44,7 @@ final class DeleteTest extends TestCase { use AdapterTestTrait; use DeprecatedAssertionsTrait; + use ExpressionDataAssertionsTrait; protected Delete $delete; @@ -96,34 +97,45 @@ public function testWhere(): void $this->delete->where(['one' => 1, 'two' => 2]); $where = $this->delete->where; + self::assertInstanceOf(Where::class, $where); + + $predicates = $this->readArrayAttribute($where, 'predicates'); - $predicates = $this->readAttribute($where, 'predicates'); - self::assertEquals('AND', $predicates[0][0]); - self::assertInstanceOf(Literal::class, $predicates[0][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 0); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Literal::class, $predicate); - self::assertEquals('AND', $predicates[1][0]); - self::assertInstanceOf(Expression::class, $predicates[1][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 1); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Expression::class, $predicate); - self::assertEquals('AND', $predicates[2][0]); - self::assertInstanceOf(Operator::class, $predicates[2][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 2); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Operator::class, $predicate); - self::assertEquals('OR', $predicates[3][0]); - self::assertInstanceOf(Literal::class, $predicates[3][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 3); + self::assertEquals('OR', $operator); + self::assertInstanceOf(Literal::class, $predicate); - self::assertEquals('AND', $predicates[4][0]); - self::assertInstanceOf(IsNull::class, $predicates[4][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 4); + self::assertEquals('AND', $operator); + self::assertInstanceOf(IsNull::class, $predicate); - self::assertEquals('AND', $predicates[5][0]); - self::assertInstanceOf(In::class, $predicates[5][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 5); + self::assertEquals('AND', $operator); + self::assertInstanceOf(In::class, $predicate); - self::assertEquals('AND', $predicates[6][0]); - self::assertInstanceOf(IsNotNull::class, $predicates[6][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 6); + self::assertEquals('AND', $operator); + self::assertInstanceOf(IsNotNull::class, $predicate); - self::assertEquals('AND', $predicates[7][0]); - self::assertInstanceOf(Operator::class, $predicates[7][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 7); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Operator::class, $predicate); - self::assertEquals('AND', $predicates[8][0]); - self::assertInstanceOf(Operator::class, $predicates[8][1]); + [$operator, $predicate] = self::predicateEntryAt($predicates, 8); + self::assertEquals('AND', $operator); + self::assertInstanceOf(Operator::class, $predicate); $where = new Where(); $this->delete->where($where); @@ -264,9 +276,14 @@ public function testMagicGetReturnsWhereClause(): void public function testMagicGetReturnsNullForUnknownProperty(): void { - /** @noinspection PhpUndefinedFieldInspection */ - self::assertNull($this->delete->unknown); // @phpstan-ignore-line - self::assertNull($this->delete->table); // @phpstan-ignore-line + /** @mago-expect analysis:non-documented-property */ + self::assertNull($this->delete->unknown); + + /** + * @mago-expect analysis:invalid-property-read + * @mago-expect analysis:no-value + */ + self::assertNull($this->delete->table); } public function testConstructorWithTable(): void From fd41a6514edbe9b89ac12306cf648de4975b1042 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 12:05:52 +1000 Subject: [PATCH 15/22] Type SelectTest raw-state reads and magic property access - Read raw-state objects through /** @var Type */ or the ?Type magic property, narrowing lazy where/having/joins before use - Narrow list-index and raw-state key access; read the specifications attribute through readArrayAttribute - @mago-expect the deliberate invalid-argument/magic-property exception cases; scope unused shared data-provider parameters - Widen Select::quantifier()'s @param to ExpressionInterface to match its native signature (the docblock was narrower than the accepted type) Signed-off-by: Simon Mundy --- src/Sql/Select.php | 2 +- test/unit/Sql/SelectTest.php | 155 ++++++++++++++++++++++------------- 2 files changed, 98 insertions(+), 59 deletions(-) diff --git a/src/Sql/Select.php b/src/Sql/Select.php index 918c708b..f8dd3014 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -227,7 +227,7 @@ public function from(array|string|TableIdentifier $table): static } /** - * @param string|Expression $quantifier DISTINCT|ALL + * @param string|ExpressionInterface $quantifier DISTINCT|ALL * @throws Exception\InvalidArgumentException */ public function quantifier(ExpressionInterface|string $quantifier): static diff --git a/test/unit/Sql/SelectTest.php b/test/unit/Sql/SelectTest.php index 07cd4b94..abdd5503 100644 --- a/test/unit/Sql/SelectTest.php +++ b/test/unit/Sql/SelectTest.php @@ -25,6 +25,7 @@ use PhpDb\Sql\TableIdentifier; use PhpDb\Sql\Where; use PhpDbTest\AdapterTestTrait; +use PhpDbTest\DeprecatedAssertionsTrait; use PhpDbTest\TestAsset\TrustingSql92Platform; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\DataProvider; @@ -74,6 +75,7 @@ final class SelectTest extends TestCase { use AdapterTestTrait; + use DeprecatedAssertionsTrait; use ExpressionDataAssertionsTrait; public function testConstruct(): void @@ -127,9 +129,8 @@ public function testQuantifier(): void #[TestDox('unit test: Test quantifier() accepts expression')] public function testQuantifierParameterExpressionInterface(): void { - $expr = $this->getMockBuilder(ExpressionInterface::class)->onlyMethods([])->getMock(); + $expr = $this->createMock(ExpressionInterface::class); $select = new Select(); - /** @psalm-suppress InvalidArgument */ $select->quantifier($expr); self::assertSame( $expr, @@ -180,8 +181,8 @@ public function testJoin(): void self::assertSame($select, $result); // Verify the first mutation occurred + /** @var Join $joins */ $joins = $select->getRawState('joins'); - self::assertInstanceOf(Join::class, $joins); self::assertEquals( [ [ @@ -198,9 +199,13 @@ public function testJoin(): void $select->join('bar', 'a = b'); // Verify the instance was actually mutated - $joins2 = $select->getRawState('joins'); - self::assertCount(2, $joins2->getJoins()); - self::assertEquals('bar', $joins2->getJoins()[1]['name']); + /** @var Join $joins2 */ + $joins2 = $select->getRawState('joins'); + $joinSpecs = $joins2->getJoins(); + self::assertCount(2, $joinSpecs); + $secondJoin = $joinSpecs[1] ?? null; + self::assertNotNull($secondJoin); + self::assertEquals('bar', $secondJoin['name']); } #[TestDox('unit test: Test join() exception with bad join')] @@ -209,6 +214,7 @@ public function testBadJoin(): void $select = new Select(); $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage("expects 'foo' as"); + /** @mago-expect analysis:invalid-argument */ $select->join(['foo'], 'x = y'); } @@ -238,8 +244,8 @@ public function testWhereArgument1IsString(): void $select->where('x = y'); /** @var Where $where */ - $where = $select->getRawState('where'); - $predicates = $where->getPredicates(); + $where = $select->getRawState('where'); + $predicates = $where->getPredicates(); [, $literal] = self::predicateEntryAt($predicates, 0); self::assertInstanceOf(Literal::class, $literal); } @@ -296,8 +302,8 @@ public function testWhereArgument1IsAssociativeArrayNotContainingReplacementChar $select->where(['x = y']); /** @var Where $where */ - $where = $select->getRawState('where'); - $predicates = $where->getPredicates(); + $where = $select->getRawState('where'); + $predicates = $where->getPredicates(); [, $literal] = self::predicateEntryAt($predicates, 0); self::assertInstanceOf(Literal::class, $literal); } @@ -373,8 +379,8 @@ public function testWhereArgument1IsPredicate(): void $select->where($predicate); /** @var Where $where */ - $where = $select->getRawState('where'); - $predicates = $where->getPredicates(); + $where = $select->getRawState('where'); + $predicates = $where->getPredicates(); [, $actualPredicate] = self::predicateEntryAt($predicates, 0); self::assertSame($predicate, $actualPredicate); } @@ -456,9 +462,8 @@ public function testLimit(): void self::assertSame($select, $result); // Verify the first mutation occurred - $limit = $select->getRawState(Select::LIMIT); - self::assertIsNumeric($limit); - self::assertEquals(5, $limit); + self::assertIsNumeric($select->getRawState(Select::LIMIT)); + self::assertEquals(5, $select->getRawState(Select::LIMIT)); // Second mutation to verify mutability $select->limit(10); @@ -488,9 +493,8 @@ public function testOffset(): void self::assertSame($select, $result); // Verify the first mutation occurred - $offset = $select->getRawState(Select::OFFSET); - self::assertIsNumeric($offset); - self::assertEquals(10, $offset); + self::assertIsNumeric($select->getRawState(Select::OFFSET)); + self::assertEquals(10, $select->getRawState(Select::OFFSET)); // Second mutation to verify mutability $select->offset(20); @@ -541,15 +545,17 @@ public function testHaving(): void self::assertSame($select, $result); // Verify the first mutation occurred + /** @var Having $having */ $having = $select->getRawState('having'); - self::assertInstanceOf(Having::class, $having); self::assertEquals(1, $having->count()); // Second mutation to verify mutability (having predicates accumulate) $select->having(['y = ?' => 10]); // Verify the instance was actually mutated - self::assertEquals(2, $select->getRawState('having')->count()); + /** @var Having $having2 */ + $having2 = $select->getRawState('having'); + self::assertEquals(2, $having2->count()); } #[TestDox('unit test: Test having() returns same Select object (is chainable)')] @@ -571,8 +577,8 @@ public function testWhereAcceptsExpressionInterface(): void new Expression('COUNT(?) > ?', [new Identifier('id'), Argument::value(5)]), ]); + /** @var Where $where */ $where = $select->getRawState('where'); - self::assertInstanceOf(Where::class, $where); self::assertEquals(1, $where->count()); } @@ -586,8 +592,8 @@ public function testHavingAcceptsExpressionInterface(): void new Expression('SUM(?) > ?', [Argument::identifier('amount'), Argument::value(100)]), ]); + /** @var Having $having */ $having = $select->getRawState('having'); - self::assertInstanceOf(Having::class, $having); self::assertEquals(1, $having->count()); } @@ -604,8 +610,8 @@ public function testCombine(): void self::assertSame($select, $result); // Verify the first mutation occurred + /** @var array{select: Select, type: string, modifier: string} $state */ $state = $select->getRawState('combine'); - self::assertInstanceOf(Select::class, $state['select']); self::assertNotSame($select, $state['select']); self::assertEquals(Select::COMBINE_UNION, $state['type']); self::assertEquals('ALL', $state['modifier']); @@ -616,6 +622,7 @@ public function testCombine(): void $select2->combine($combine2, Select::COMBINE_INTERSECT, 'DISTINCT'); // Verify the instance was actually mutated + /** @var array{select: Select, type: string, modifier: string} $state2 */ $state2 = $select2->getRawState('combine'); self::assertEquals(Select::COMBINE_INTERSECT, $state2['type']); self::assertEquals('DISTINCT', $state2['modifier']); @@ -640,15 +647,15 @@ public function testReset(): void // joins $select->join('foo', 'id = boo'); + /** @var Join $joins */ $joins = $select->getRawState(Select::JOINS); - self::assertInstanceOf(Join::class, $joins); self::assertEquals( [['name' => 'foo', 'on' => 'id = boo', 'columns' => ['*'], 'type' => 'inner']], $joins->getJoins() ); $select->reset(Select::JOINS); + /** @var Join $emptyJoins */ $emptyJoins = $select->getRawState(Select::JOINS); - self::assertInstanceOf(Join::class, $emptyJoins); self::assertEmpty($emptyJoins->getJoins()); // where @@ -698,7 +705,10 @@ public function testReset(): void self::assertEmpty($select->getRawState(Select::ORDER)); } - /** @noinspection PhpUnusedParameterInspection */ + /** + * @param array $expectedParameters + * @mago-expect analysis:unused-parameter + */ #[DataProvider('providerData')] #[TestDox('unit test: Test prepareStatement() will produce expected sql and parameters based on a variety of provided arguments [uses data provider]')] @@ -745,7 +755,9 @@ public function testSelectUsingTableIdentifierWithEmptyScheme(): void ); } - /** @noinspection PhpUnusedParameterInspection */ + /** + * @mago-expect analysis:unused-parameter + */ #[DataProvider('providerData')] #[TestDox('unit test: Test getSqlString() will produce expected sql and parameters based on a variety of provided arguments [uses data provider]')] @@ -770,16 +782,25 @@ public function testCloning(): void $select1->where('id = foo'); $select1->having('id = foo'); - self::assertEquals(0, $select->where->count()); - self::assertEquals(1, $select1->where->count()); - - self::assertEquals(0, $select->having->count()); - self::assertEquals(1, $select1->having->count()); + $selectWhere = $select->where; + $select1Where = $select1->where; + self::assertInstanceOf(Where::class, $selectWhere); + self::assertInstanceOf(Where::class, $select1Where); + self::assertEquals(0, $selectWhere->count()); + self::assertEquals(1, $select1Where->count()); + + $selectHaving = $select->having; + $select1Having = $select1->having; + self::assertInstanceOf(Having::class, $selectHaving); + self::assertInstanceOf(Having::class, $select1Having); + self::assertEquals(0, $selectHaving->count()); + self::assertEquals(1, $select1Having->count()); } /** + * @param array> $internalTests * @throws ReflectionException - * @noinspection PhpUnusedParameterInspection + * @mago-expect analysis:unused-parameter */ #[DataProvider('providerData')] #[TestDox('unit test: Text process*() methods will return proper array when internally called, @@ -802,13 +823,9 @@ public function testProcessMethods( $sr = new ReflectionObject($select); - /** - * @var string $method - * @var array $expected - */ foreach ($internalTests as $method => $expected) { $mr = $sr->getMethod($method); - /** @psalm-suppress MixedAssignment */ + /** @mago-expect analysis:mixed-assignment */ $return = $mr->invokeArgs($select, [new Sql92(), $mockDriver, $parameterContainer]); self::assertEquals($expected, $return); } @@ -1156,7 +1173,7 @@ public static function providerData(): array ]; $select32subselect = new Select(); - $select32subselect->from('bar')->where->like('y', '%Foo%'); + $select32subselect->from('bar')->where?->like('y', '%Foo%'); $select32 = new Select(); $select32->from(['x' => $select32subselect]); @@ -1245,7 +1262,7 @@ public static function providerData(): array // subselect in join $select39subselect = new Select(); - $select39subselect->from('bar')->where->like('y', '%Foo%'); + $select39subselect->from('bar')->where?->like('y', '%Foo%'); $select39 = new Select(); $select39->from('foo')->join(['z' => $select39subselect], 'z.foo = bar.id'); $sqlPrep39 = 'SELECT "foo".*, "z".* FROM "foo" INNER JOIN (SELECT "bar".* FROM "bar" WHERE "y" LIKE ?) AS "z" ON "z"."foo" = "bar"."id"'; @@ -1378,7 +1395,7 @@ public static function providerData(): array $select50 = new Select(); $select50->from(new TableIdentifier('foo')) ->where - ->nest + ?->nest ->isNull('bar') ->and ->predicate(new Predicate\Literal('1=1')); @@ -1390,7 +1407,7 @@ public static function providerData(): array $select51 = new Select(); $select51->from(new TableIdentifier('foo')) ->where - ->nest + ?->nest ->isNull('bar') ->or ->predicate(new Predicate\Literal('1=1')); @@ -1523,7 +1540,7 @@ public function testFromThrowsExceptionForInvalidTableType(): void $select = new Select(); $this->expectException(TypeError::class); - /** @noinspection ALL */ + /** @mago-expect analysis:invalid-argument */ $select->from(123); } @@ -1533,7 +1550,8 @@ public function testFromThrowsExceptionForInvalidArrayFormat(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('from() expects $table as an array is a single element associative array'); - $select->from(['foo', 'bar']); // Numeric array instead of associative + /** @mago-expect analysis:invalid-argument */ + $select->from(['foo', 'bar']); } public function testSetSpecificationThrowsExceptionForInvalidName(): void @@ -1551,8 +1569,11 @@ public function testGetThrowsExceptionForInvalidProperty(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Not a valid magic property for this object'); - /** @noinspection ALL */ - $value = $select->invalidProperty; /** @phpstan-ignore-line */ + /** + * @mago-expect analysis:non-documented-property + * @mago-expect analysis:mixed-assignment + */ + $value = $select->invalidProperty; } public function testCombineWrapsStatementInParentheses(): void @@ -1613,12 +1634,13 @@ public function testColumnsWithPrefixDisabled(): void public function testGetRawStateInitializesLazyProperties(): void { - $select = new Select(); + $select = new Select(); + /** @var array $rawState */ $rawState = $select->getRawState(); - self::assertInstanceOf(Where::class, $rawState[Select::WHERE]); - self::assertInstanceOf(Join::class, $rawState[Select::JOINS]); - self::assertInstanceOf(Having::class, $rawState[Select::HAVING]); + self::assertInstanceOf(Where::class, $rawState[Select::WHERE] ?? null); + self::assertInstanceOf(Join::class, $rawState[Select::JOINS] ?? null); + self::assertInstanceOf(Having::class, $rawState[Select::HAVING] ?? null); } public function testCombineThrowsWhenAlreadyCombined(): void @@ -1659,13 +1681,16 @@ public function testResetCombine(): void self::assertEmpty($select->getRawState(Select::COMBINE)); } + /** + * @throws ReflectionException + */ public function testSetSpecificationStoresValidSpecification(): void { $select = new Select(); $select->setSpecification('Select', 'CUSTOM %1$s FROM %2$s'); - $rawState = (new ReflectionObject($select))->getProperty('specifications'); - self::assertSame('CUSTOM %1$s FROM %2$s', $rawState->getValue($select)['Select']); + $specifications = $this->readArrayAttribute($select, 'specifications'); + self::assertSame('CUSTOM %1$s FROM %2$s', $specifications['Select'] ?? null); } public function testMagicGetJoinsReturnsJoinInstance(): void @@ -1688,12 +1713,26 @@ public function testCloneDeepCopiesAllSubObjects(): void $clone->having('cnt > 1'); $clone->join('baz', 'foo.id = baz.foo_id'); - self::assertCount(1, $select->where); - self::assertCount(2, $clone->where); - self::assertCount(1, $select->having); - self::assertCount(2, $clone->having); - self::assertCount(1, $select->joins); - self::assertCount(2, $clone->joins); + $selectWhere = $select->where; + $cloneWhere = $clone->where; + self::assertInstanceOf(Where::class, $selectWhere); + self::assertInstanceOf(Where::class, $cloneWhere); + self::assertCount(1, $selectWhere); + self::assertCount(2, $cloneWhere); + + $selectHaving = $select->having; + $cloneHaving = $clone->having; + self::assertInstanceOf(Having::class, $selectHaving); + self::assertInstanceOf(Having::class, $cloneHaving); + self::assertCount(1, $selectHaving); + self::assertCount(2, $cloneHaving); + + $selectJoins = $select->joins; + $cloneJoins = $clone->joins; + self::assertInstanceOf(Join::class, $selectJoins); + self::assertInstanceOf(Join::class, $cloneJoins); + self::assertCount(1, $selectJoins); + self::assertCount(2, $cloneJoins); } public function testOrderWithAssociativeArray(): void From dc7f7565e952552cc11995d19a67d86e6f437632 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 12:27:10 +1000 Subject: [PATCH 16/22] Type the Insert, Combine, Ddl and Platform tests Complete the mago cleanup of the remaining Sql test files: read raw state and reflected attributes through typed @var/?? null guards, narrow list and string-key access, type closure and helper signatures, and replace psalm/inspection suppressions with @mago-expect on the deliberately invalid expectException calls and unmodellable magic-property reads. Signed-off-by: Simon Mundy --- test/unit/Sql/AbstractSqlTest.php | 1 + test/unit/Sql/ArgumentTest.php | 9 +++-- test/unit/Sql/CombineTest.php | 22 ++++++----- test/unit/Sql/Ddl/AlterTableTest.php | 24 +++++++----- .../Ddl/Constraint/AbstractConstraintTest.php | 2 +- test/unit/Sql/Ddl/CreateTableTest.php | 34 +++++++++++------ test/unit/Sql/InsertIgnoreTest.php | 12 +++++- test/unit/Sql/InsertTest.php | 22 +++++++---- test/unit/Sql/JoinTest.php | 6 ++- .../Sql/Platform/AbstractPlatformTest.php | 21 +++++++++-- test/unit/Sql/Platform/PlatformTest.php | 37 +++++++++++++------ test/unit/Sql/SqlTest.php | 5 ++- test/unit/Sql/TableIdentifierTest.php | 4 +- 13 files changed, 137 insertions(+), 62 deletions(-) diff --git a/test/unit/Sql/AbstractSqlTest.php b/test/unit/Sql/AbstractSqlTest.php index a925b210..b79a8990 100644 --- a/test/unit/Sql/AbstractSqlTest.php +++ b/test/unit/Sql/AbstractSqlTest.php @@ -101,6 +101,7 @@ public function testProcessExpressionWithParameterContainerAndParameterizationTy self::assertMatchesRegularExpression('#"x" > :expr\d+Param1 AND y < :expr\d+Param2#', $sqlAndParams); // Verify parameter names and values + $matches = []; preg_match('#expr(\d+)Param1#', (string) key($parameters), $matches); self::assertTrue(isset($matches[1])); $expressionNumber = $matches[1]; diff --git a/test/unit/Sql/ArgumentTest.php b/test/unit/Sql/ArgumentTest.php index 1d94c9f2..f27b07a4 100644 --- a/test/unit/Sql/ArgumentTest.php +++ b/test/unit/Sql/ArgumentTest.php @@ -57,9 +57,12 @@ public function testConstructorWithSqlInterface(): void public function testConstructorThrowsExceptionForInvalidSelectType(): void { $this->expectException(TypeError::class); - /** @noinspection PhpParamsInspection */ - /** @noinspection PhpExpressionResultUnusedInspection */ - new ArgumentSelect('simple_value'); /** @phpstan-ignore-line */ + /** + * @noinspection PhpParamsInspection + * @noinspection PhpExpressionResultUnusedInspection + * @mago-expect analysis:invalid-argument + */ + new ArgumentSelect('simple_value'); } public function testConstructorWithArrayContainingArgumentType(): void diff --git a/test/unit/Sql/CombineTest.php b/test/unit/Sql/CombineTest.php index bbe77ff7..1049480e 100644 --- a/test/unit/Sql/CombineTest.php +++ b/test/unit/Sql/CombineTest.php @@ -16,7 +16,6 @@ use PhpDb\Sql\Select; use PhpDbTest\AdapterTestTrait; use PHPUnit\Framework\Attributes\CoversMethod; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use TypeError; @@ -48,7 +47,7 @@ public function testRejectsInvalidStatement(): void { $this->expectException(TypeError::class); - /** @noinspection PhpParamsInspection */ + /** @mago-expect analysis:invalid-argument */ $this->combine->combine('foo'); } @@ -216,9 +215,10 @@ public function testAlignColumnsAppendsNullExpressionsForMissingColumns(): void $this->combine->union([$select1, $select2])->alignColumns(); + /** @var array $columns1 */ $columns1 = $select1->getRawState('columns'); self::assertArrayHasKey('b', $columns1); - self::assertInstanceOf(Expression::class, $columns1['b']); + self::assertInstanceOf(Expression::class, $columns1['b'] ?? null); } public function testConstructorWithSelectDelegatesToCombine(): void @@ -226,10 +226,12 @@ public function testConstructorWithSelectDelegatesToCombine(): void $select = new Select('foo'); $combine = new Combine($select, Combine::COMBINE_EXCEPT, 'ALL'); - $rawState = $combine->getRawState(); - self::assertCount(1, $rawState['combine']); - self::assertSame('except', $rawState['combine'][0]['type']); - self::assertSame('ALL', $rawState['combine'][0]['modifier']); + /** @var array{combine: array{0: array{type: string, modifier: string}}} $rawState */ + $rawState = $combine->getRawState(); + $combineState = $rawState['combine']; + self::assertCount(1, $combineState); + self::assertSame('except', $combineState[0]['type']); + self::assertSame('ALL', $combineState[0]['modifier']); } public function testAlignColumnsReturnsEarlyWhenEmpty(): void @@ -240,7 +242,7 @@ public function testAlignColumnsReturnsEarlyWhenEmpty(): void self::assertSame($combine, $result); } - protected function getMockAdapter(): Adapter|MockObject + protected function getMockAdapter(): Adapter { $parameterContainer = new ParameterContainer(); @@ -248,9 +250,9 @@ protected function getMockAdapter(): Adapter|MockObject $mockStatement->expects($this->any())->method('getParameterContainer') ->willReturn($parameterContainer); - $setGetSqlFunction = function ($sql = null) use ($mockStatement) { + $setGetSqlFunction = function (?string $sql = null) use ($mockStatement) { static $sqlValue; - if ($sql) { + if ($sql !== null) { $sqlValue = $sql; return $mockStatement; } diff --git a/test/unit/Sql/Ddl/AlterTableTest.php b/test/unit/Sql/Ddl/AlterTableTest.php index a5bf50a4..f8e3cb96 100644 --- a/test/unit/Sql/Ddl/AlterTableTest.php +++ b/test/unit/Sql/Ddl/AlterTableTest.php @@ -145,7 +145,8 @@ public function testConstructorWithTableIdentifier(): void // Get full raw state to avoid type issue with getRawState('table') $rawState = $at->getRawState(); - self::assertSame($tableId, $rawState['table']); + self::assertIsArray($rawState); + self::assertSame($tableId, $rawState['table'] ?? null); } public function testConstructorWithEmptyTable(): void @@ -178,13 +179,13 @@ public function testGetRawStateReturnsAllState(): void self::assertArrayHasKey(AlterTable::DROP_CONSTRAINTS, $rawState); self::assertArrayHasKey(AlterTable::DROP_INDEXES, $rawState); - self::assertEquals('test', $rawState[AlterTable::TABLE]); - self::assertEquals([$colMock], $rawState[AlterTable::ADD_COLUMNS]); - self::assertEquals(['old_col' => $colMock], $rawState[AlterTable::CHANGE_COLUMNS]); - self::assertEquals(['drop_col'], $rawState[AlterTable::DROP_COLUMNS]); - self::assertEquals([$conMock], $rawState[AlterTable::ADD_CONSTRAINTS]); - self::assertEquals(['drop_con'], $rawState[AlterTable::DROP_CONSTRAINTS]); - self::assertEquals(['drop_idx'], $rawState[AlterTable::DROP_INDEXES]); + self::assertEquals('test', $rawState[AlterTable::TABLE] ?? null); + self::assertEquals([$colMock], $rawState[AlterTable::ADD_COLUMNS] ?? null); + self::assertEquals(['old_col' => $colMock], $rawState[AlterTable::CHANGE_COLUMNS] ?? null); + self::assertEquals(['drop_col'], $rawState[AlterTable::DROP_COLUMNS] ?? null); + self::assertEquals([$conMock], $rawState[AlterTable::ADD_CONSTRAINTS] ?? null); + self::assertEquals(['drop_con'], $rawState[AlterTable::DROP_CONSTRAINTS] ?? null); + self::assertEquals(['drop_idx'], $rawState[AlterTable::DROP_INDEXES] ?? null); } public function testGetRawStateWithSpecificKey(): void @@ -210,7 +211,9 @@ public function testMultipleColumnsAndConstraints(): void $at->addColumn($col2); $at->addColumn($col3); - self::assertCount(3, $at->getRawState(AlterTable::ADD_COLUMNS)); + $addColumns = $at->getRawState(AlterTable::ADD_COLUMNS); + self::assertIsArray($addColumns); + self::assertCount(3, $addColumns); $sql = $at->getSqlString(); self::assertStringContainsString('ADD COLUMN "email"', $sql); @@ -384,8 +387,9 @@ public function testGetRawStateIncludesTableOptions(): void $rawState = $at->getRawState(); + self::assertIsArray($rawState); self::assertArrayHasKey(AlterTable::TABLE_OPTIONS, $rawState); - self::assertEquals(['engine' => new Literal('InnoDB')], $rawState[AlterTable::TABLE_OPTIONS]); + self::assertEquals(['engine' => new Literal('InnoDB')], $rawState[AlterTable::TABLE_OPTIONS] ?? null); } public function testGetSqlStringWithEngineOption(): void diff --git a/test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php b/test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php index 4dd670d1..354dec7a 100644 --- a/test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php +++ b/test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php @@ -16,7 +16,7 @@ #[CoversMethod(AbstractConstraint::class, 'getColumns')] final class AbstractConstraintTest extends TestCase { - protected MockObject $ac; + protected AbstractConstraint&MockObject $ac; /** * @throws Exception diff --git a/test/unit/Sql/Ddl/CreateTableTest.php b/test/unit/Sql/Ddl/CreateTableTest.php index 34c5a75a..efd571db 100644 --- a/test/unit/Sql/Ddl/CreateTableTest.php +++ b/test/unit/Sql/Ddl/CreateTableTest.php @@ -104,7 +104,7 @@ public function testAddColumn(): void $state = $ct->getRawState('columns'); self::assertIsArray($state); self::assertCount(1, $state); - self::assertInstanceOf(ColumnInterface::class, $state[0]); + self::assertInstanceOf(ColumnInterface::class, $state[0] ?? null); // Second mutation to verify mutability (columns accumulate) $column2 = $this->getMockBuilder(ColumnInterface::class)->getMock(); @@ -112,8 +112,9 @@ public function testAddColumn(): void // Verify the instance was actually mutated $state2 = $ct->getRawState('columns'); + self::assertIsArray($state2); self::assertCount(2, $state2); - self::assertInstanceOf(ColumnInterface::class, $state2[1]); + self::assertInstanceOf(ColumnInterface::class, $state2[1] ?? null); } public function testAddConstraint(): void @@ -131,7 +132,7 @@ public function testAddConstraint(): void $state = $ct->getRawState('constraints'); self::assertIsArray($state); self::assertCount(1, $state); - self::assertInstanceOf(ConstraintInterface::class, $state[0]); + self::assertInstanceOf(ConstraintInterface::class, $state[0] ?? null); // Second mutation to verify mutability (constraints accumulate) $constraint2 = $this->getMockBuilder(ConstraintInterface::class)->getMock(); @@ -139,8 +140,9 @@ public function testAddConstraint(): void // Verify the instance was actually mutated $state2 = $ct->getRawState('constraints'); + self::assertIsArray($state2); self::assertCount(2, $state2); - self::assertInstanceOf(ConstraintInterface::class, $state2[1]); + self::assertInstanceOf(ConstraintInterface::class, $state2[1] ?? null); } public function testGetSqlString(): void @@ -198,7 +200,8 @@ public function testConstructorWithTableIdentifier(): void $ct = new CreateTable($tableId); $rawState = $ct->getRawState(); - self::assertSame($tableId, $rawState[CreateTable::TABLE]); + self::assertIsArray($rawState); + self::assertSame($tableId, $rawState[CreateTable::TABLE] ?? null); } public function testConstructorWithTemporaryFlag(): void @@ -227,9 +230,9 @@ public function testGetRawStateReturnsAllState(): void self::assertArrayHasKey(CreateTable::COLUMNS, $rawState); self::assertArrayHasKey(CreateTable::CONSTRAINTS, $rawState); - self::assertEquals('users', $rawState[CreateTable::TABLE]); - self::assertEquals([$col], $rawState[CreateTable::COLUMNS]); - self::assertEquals([$con], $rawState[CreateTable::CONSTRAINTS]); + self::assertEquals('users', $rawState[CreateTable::TABLE] ?? null); + self::assertEquals([$col], $rawState[CreateTable::COLUMNS] ?? null); + self::assertEquals([$con], $rawState[CreateTable::CONSTRAINTS] ?? null); } public function testGetRawStateWithInvalidKey(): void @@ -259,8 +262,14 @@ public function testChainedOperations(): void self::assertSame($ct, $result); self::assertEquals('products', $ct->getRawState(CreateTable::TABLE)); self::assertTrue($ct->isTemporary()); - self::assertCount(2, $ct->getRawState(CreateTable::COLUMNS)); - self::assertCount(1, $ct->getRawState(CreateTable::CONSTRAINTS)); + + $columns = $ct->getRawState(CreateTable::COLUMNS); + self::assertIsArray($columns); + self::assertCount(2, $columns); + + $constraints = $ct->getRawState(CreateTable::CONSTRAINTS); + self::assertIsArray($constraints); + self::assertCount(1, $constraints); } public function testMultipleColumns(): void @@ -271,6 +280,7 @@ public function testMultipleColumns(): void $ct->addColumn(new Column('email')); $columns = $ct->getRawState(CreateTable::COLUMNS); + self::assertIsArray($columns); self::assertCount(3, $columns); $sql = $ct->getSqlString(); @@ -286,6 +296,7 @@ public function testMultipleConstraints(): void $ct->addConstraint(new Constraint\UniqueKey('order_number')); $constraints = $ct->getRawState(CreateTable::CONSTRAINTS); + self::assertIsArray($constraints); self::assertCount(2, $constraints); $sql = $ct->getSqlString(); @@ -348,8 +359,9 @@ public function testGetRawStateIncludesTableOptions(): void $rawState = $ct->getRawState(); + self::assertIsArray($rawState); self::assertArrayHasKey(CreateTable::TABLE_OPTIONS, $rawState); - self::assertEquals(['engine' => new Literal('InnoDB')], $rawState[CreateTable::TABLE_OPTIONS]); + self::assertEquals(['engine' => new Literal('InnoDB')], $rawState[CreateTable::TABLE_OPTIONS] ?? null); } public function testGetSqlStringWithLiteralOption(): void diff --git a/test/unit/Sql/InsertIgnoreTest.php b/test/unit/Sql/InsertIgnoreTest.php index e1c851ed..a6b16662 100644 --- a/test/unit/Sql/InsertIgnoreTest.php +++ b/test/unit/Sql/InsertIgnoreTest.php @@ -82,6 +82,7 @@ public function testValues(): void public function testValuesThrowsExceptionWhenNotArrayOrSelect(): void { $this->expectException(TypeError::class); + /** @mago-expect analysis:invalid-argument */ $this->insert->values(5); } @@ -176,7 +177,9 @@ public function testPrepareStatementWithSelect(): void 'INSERT IGNORE INTO "foo" ("col1") SELECT "bar".* FROM "bar" WHERE "x" = ?', $mockStatement->getSql() ); - $parameters = $mockStatement->getParameterContainer()->getNamedArray(); + $parameterContainer = $mockStatement->getParameterContainer(); + self::assertInstanceOf(ParameterContainer::class, $parameterContainer); + $parameters = $parameterContainer->getNamedArray(); self::assertSame(['subselect1where1' => 5], $parameters); } @@ -235,6 +238,7 @@ public function testGetSqlStringUsingColumnsAndValuesMethods(): void public function test__set(): void { // @codingStandardsIgnoreEnd + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertEquals(['foo'], $this->insert->getRawState('columns')); self::assertEquals(['bar'], $this->insert->getRawState('values')); @@ -244,6 +248,7 @@ public function test__set(): void public function test__unset(): void { // @codingStandardsIgnoreEnd + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertEquals(['foo'], $this->insert->getRawState('columns')); self::assertEquals(['bar'], $this->insert->getRawState('values')); @@ -251,6 +256,7 @@ public function test__unset(): void self::assertEquals([], $this->insert->getRawState('columns')); self::assertEquals([], $this->insert->getRawState('values')); + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = null; self::assertEquals(['foo'], $this->insert->getRawState('columns')); self::assertEquals([null], $this->insert->getRawState('values')); @@ -264,9 +270,11 @@ public function test__unset(): void public function test__isset(): void { // @codingStandardsIgnoreEnd + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertTrue(isset($this->insert->foo)); + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = null; self::assertTrue(isset($this->insert->foo)); } @@ -275,9 +283,11 @@ public function test__isset(): void public function test__get(): void { // @codingStandardsIgnoreEnd + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertEquals('bar', $this->insert->foo); + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = null; self::assertNull($this->insert->foo); } diff --git a/test/unit/Sql/InsertTest.php b/test/unit/Sql/InsertTest.php index cbc4d745..19bd3416 100644 --- a/test/unit/Sql/InsertTest.php +++ b/test/unit/Sql/InsertTest.php @@ -98,7 +98,7 @@ public function testValues(): void public function testValuesThrowsExceptionWhenNotArrayOrSelect(): void { $this->expectException(TypeError::class); - /** @psalm-suppress InvalidArgument */ + /** @mago-expect analysis:invalid-argument */ $this->insert->values(5); } @@ -255,7 +255,7 @@ public function testGetSqlStringUsingColumnsAndValuesMethods(): void public function test__set(): void { // @codingStandardsIgnoreEnd - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertEquals(['foo'], $this->insert->getRawState('columns')); self::assertEquals(['bar'], $this->insert->getRawState('values')); @@ -265,7 +265,7 @@ public function test__set(): void public function test__unset(): void { // @codingStandardsIgnoreEnd - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertEquals(['foo'], $this->insert->getRawState('columns')); self::assertEquals(['bar'], $this->insert->getRawState('values')); @@ -273,7 +273,7 @@ public function test__unset(): void self::assertEquals([], $this->insert->getRawState('columns')); self::assertEquals([], $this->insert->getRawState('values')); - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = null; self::assertEquals(['foo'], $this->insert->getRawState('columns')); self::assertEquals([null], $this->insert->getRawState('values')); @@ -287,12 +287,12 @@ public function test__unset(): void public function test__isset(): void { // @codingStandardsIgnoreEnd - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; /** @psalm-suppress RedundantCondition */ self::assertTrue(isset($this->insert->foo)); - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = null; /** @psalm-suppress TypeDoesNotContainType */ self::assertTrue(isset($this->insert->foo)); @@ -302,11 +302,11 @@ public function test__isset(): void public function test__get(): void { // @codingStandardsIgnoreEnd - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = 'bar'; self::assertEquals('bar', $this->insert->foo); - /** @psalm-suppress UndefinedMagicPropertyAssignment */ + /** @mago-expect analysis:non-documented-property */ $this->insert->foo = null; self::assertNull($this->insert->foo); } @@ -338,6 +338,7 @@ public function testUnsetThrowsExceptionForNonExistentColumn(): void { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('The key nonexistent was not found in this objects column list'); + /** @mago-expect analysis:non-documented-property */ unset($this->insert->nonexistent); } @@ -345,6 +346,10 @@ public function testGetThrowsExceptionForNonExistentColumn(): void { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('The key nonexistent was not found in this objects column list'); + /** + * @mago-expect analysis:non-documented-property + * @mago-expect analysis:mixed-assignment + */ $value = $this->insert->nonexistent; } @@ -459,6 +464,7 @@ public function testProcessInsertWithPdoDriverUsesDriverColumnQuoting(): void $this->insert->prepareStatement($mockAdapter, $mockStatement); $sql = $mockStatement->getSql(); + self::assertIsString($sql); self::assertStringContainsString(':c_0', $sql); self::assertStringContainsString(':c_1', $sql); } diff --git a/test/unit/Sql/JoinTest.php b/test/unit/Sql/JoinTest.php index 4faaa59b..6fb7afb3 100644 --- a/test/unit/Sql/JoinTest.php +++ b/test/unit/Sql/JoinTest.php @@ -129,7 +129,11 @@ public function testJoinWillThrowAnExceptionIfNameIsNoValid(): void $join = new Join(); $this->expectException(TypeError::class); - /** @noinspection PhpArgumentWithoutNamedIdentifierInspection */ + /** + * @noinspection PhpArgumentWithoutNamedIdentifierInspection + * @mago-expect analysis:possibly-invalid-argument + * @mago-expect analysis:false-argument + */ $join->join([], false); } diff --git a/test/unit/Sql/Platform/AbstractPlatformTest.php b/test/unit/Sql/Platform/AbstractPlatformTest.php index de30d259..2acb828f 100644 --- a/test/unit/Sql/Platform/AbstractPlatformTest.php +++ b/test/unit/Sql/Platform/AbstractPlatformTest.php @@ -75,6 +75,9 @@ public function testGetTypeDecoratorLoopMatchesByInstanceof(): void self::assertSame($decorator, $result); } + /** + * @throws RuntimeException + */ public function testPrepareStatementThrowsWhenSubjectNotPreparable(): void { $subject = $this->createMock(SqlInterface::class); @@ -88,6 +91,9 @@ public function testPrepareStatementThrowsWhenSubjectNotPreparable(): void $this->platform->prepareStatement($adapter, $statement); } + /** + * @throws RuntimeException + */ public function testGetSqlStringThrowsWhenSubjectNotSqlInterface(): void { $subject = $this->createMock(PreparableSqlInterface::class); @@ -98,6 +104,9 @@ public function testGetSqlStringThrowsWhenSubjectNotSqlInterface(): void $this->platform->getSqlString(); } + /** + * @throws RuntimeException + */ public function testGetSqlStringDelegatesToDecoratorSubject(): void { $select = new Select('foo'); @@ -109,6 +118,9 @@ public function testGetSqlStringDelegatesToDecoratorSubject(): void self::assertStringContainsString('"foo"', $sql); } + /** + * @throws RuntimeException + */ public function testPrepareStatementDelegatesToDecoratorSubject(): void { $select = new Select('foo'); @@ -116,8 +128,9 @@ public function testPrepareStatementDelegatesToDecoratorSubject(): void $this->platform->setSubject($select); $mockPlatform = $this->createMock(PlatformInterface::class); - $mockPlatform->method('quoteIdentifier')->willReturnCallback(fn($v) => '"' . $v . '"'); - $mockPlatform->method('quoteIdentifierInFragment')->willReturnCallback(fn($v) => '"' . $v . '"'); + $mockPlatform->method('quoteIdentifier')->willReturnCallback(fn(string $v): string => '"' . $v . '"'); + $mockPlatform->method('quoteIdentifierInFragment') + ->willReturnCallback(fn(string $v): string => '"' . $v . '"'); $mockPlatform->method('getIdentifierSeparator')->willReturn('.'); $mockPlatform->method('getSqlPlatformDecorator')->willReturn($this->platform); @@ -132,6 +145,8 @@ public function testPrepareStatementDelegatesToDecoratorSubject(): void $result = $this->platform->prepareStatement($adapter, $statement); self::assertSame($statement, $result); - self::assertStringContainsString('SELECT', $statement->getSql()); + $sql = $statement->getSql(); + self::assertNotNull($sql); + self::assertStringContainsString('SELECT', $sql); } } diff --git a/test/unit/Sql/Platform/PlatformTest.php b/test/unit/Sql/Platform/PlatformTest.php index 6bde4a47..8eb650a0 100644 --- a/test/unit/Sql/Platform/PlatformTest.php +++ b/test/unit/Sql/Platform/PlatformTest.php @@ -22,7 +22,6 @@ use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use ReflectionException; use ReflectionMethod; @@ -90,7 +89,6 @@ protected function resolveAdapter(string $platformName): Adapter break; } - /** @var DriverInterface&MockObject $mockDriver */ $mockDriver = $this->getMockBuilder(DriverInterface::class)->getMock(); $mockDriver->expects($this->any()) @@ -100,6 +98,8 @@ protected function resolveAdapter(string $platformName): Adapter ->method('createStatement') ->willReturnCallback(fn(): StatementContainer => new StatementContainer()); + self::assertNotNull($platform); + return new Adapter($mockDriver, $platform, new ResultSet()); } @@ -127,15 +127,17 @@ public function testGetTypeDecoratorReturnsSubjectWhenNoDecoratorRegistered(): v self::assertSame($select, $result); } + /** + * @throws ReflectionException + */ public function testGetDefaultPlatformReturnsInstance(): void { $adapterPlatform = new TestAsset\TrustingSql92Platform(); $platform = new Platform($adapterPlatform); $reflectionMethod = new ReflectionMethod($platform, 'getDefaultPlatform'); - $result = $reflectionMethod->invoke($platform); - self::assertSame($adapterPlatform, $result); + self::assertSame($adapterPlatform, $reflectionMethod->invoke($platform)); } /** @@ -148,13 +150,16 @@ public function testResolvePlatformNameCachesResult(): void $reflectionMethod = new ReflectionMethod($platform, 'resolvePlatformName'); - $first = $reflectionMethod->invoke($platform, null); - $second = $reflectionMethod->invoke($platform, null); + /** @var string $first */ + $first = $reflectionMethod->invoke($platform, null); - self::assertEquals($first, $second); + self::assertEquals($first, $reflectionMethod->invoke($platform, null)); self::assertEquals('sql92', $first); } + /** + * @throws ReflectionException + */ public function testResolvePlatformWithAdapterInterface(): void { $adapterPlatform = new TestAsset\TrustingSql92Platform(); @@ -167,11 +172,13 @@ public function testResolvePlatformWithAdapterInterface(): void $mockAdapter->expects($this->once())->method('getPlatform')->willReturn($mockPlatform); $reflectionMethod = new ReflectionMethod($platform, 'resolvePlatform'); - $result = $reflectionMethod->invoke($platform, $mockAdapter); - self::assertSame($mockPlatform, $result); + self::assertSame($mockPlatform, $reflectionMethod->invoke($platform, $mockAdapter)); } + /** + * @throws ReflectionException + */ public function testResolvePlatformWithPlatformInterface(): void { $adapterPlatform = new TestAsset\TrustingSql92Platform(); @@ -180,11 +187,13 @@ public function testResolvePlatformWithPlatformInterface(): void $mockPlatform = $this->createMock(PlatformInterface::class); $reflectionMethod = new ReflectionMethod($platform, 'resolvePlatform'); - $result = $reflectionMethod->invoke($platform, $mockPlatform); - self::assertSame($mockPlatform, $result); + self::assertSame($mockPlatform, $reflectionMethod->invoke($platform, $mockPlatform)); } + /** + * @throws RuntimeException + */ public function testPrepareStatementThrowsWhenSubjectNotPreparable(): void { $adapterPlatform = new TestAsset\TrustingSql92Platform(); @@ -200,6 +209,9 @@ public function testPrepareStatementThrowsWhenSubjectNotPreparable(): void $platform->prepareStatement($adapter, $statement); } + /** + * @throws RuntimeException + */ public function testGetSqlStringThrowsWhenSubjectNotSqlInterface(): void { $adapterPlatform = new TestAsset\TrustingSql92Platform(); @@ -212,6 +224,9 @@ public function testGetSqlStringThrowsWhenSubjectNotSqlInterface(): void $platform->getSqlString($adapterPlatform); } + /** + * @throws RuntimeException + */ public function testGetSqlStringDelegatesToTypeDecorator(): void { $adapterPlatform = new TestAsset\TrustingSql92Platform(); diff --git a/test/unit/Sql/SqlTest.php b/test/unit/Sql/SqlTest.php index c83b8a3e..ce2bb40b 100644 --- a/test/unit/Sql/SqlTest.php +++ b/test/unit/Sql/SqlTest.php @@ -92,7 +92,10 @@ public function test__construct(): void self::assertSame('foo', $sql->getTable()); $this->expectException(TypeError::class); - /** @noinspection PhpStrictTypeCheckingInspection */ + /** + * @noinspection PhpStrictTypeCheckingInspection + * @mago-expect analysis:null-argument + */ $sql->setTable(null); } diff --git a/test/unit/Sql/TableIdentifierTest.php b/test/unit/Sql/TableIdentifierTest.php index 2e6c717e..f4744f19 100644 --- a/test/unit/Sql/TableIdentifierTest.php +++ b/test/unit/Sql/TableIdentifierTest.php @@ -67,7 +67,7 @@ public function testGetSchemaFromObjectStringCast(): void public function testRejectsInvalidTable(mixed $invalidTable): void { $this->expectException($invalidTable === '' ? InvalidArgumentException::class : TypeError::class); - /** @psalm-suppress MixedArgument */ + /** @mago-expect analysis:mixed-argument */ new TableIdentifier($invalidTable); } @@ -75,7 +75,7 @@ public function testRejectsInvalidTable(mixed $invalidTable): void public function testRejectsInvalidSchema(mixed $invalidSchema): void { $this->expectException($invalidSchema === '' ? InvalidArgumentException::class : TypeError::class); - /** @psalm-suppress MixedArgument */ + /** @mago-expect analysis:mixed-argument */ new TableIdentifier('foo', $invalidSchema); } From 6255d80f7bc6073840209e1ebbdd51cc4a1ef19d Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 12:49:27 +1000 Subject: [PATCH 17/22] Type the Update, Expression and Predicate set/expression/in tests - Read raw state and reflected predicates through readArrayAttribute and /** @var Type */, narrowing lazy where and nullable getSql before use - Type the falsy-parameter data provider off mixed - Replace psalm/inspection suppressions with @mago-expect on the deliberately invalid and deprecated-variadic constructor calls Signed-off-by: Simon Mundy --- test/unit/Sql/ExpressionTest.php | 6 ++++-- test/unit/Sql/Predicate/ExpressionTest.php | 4 ++-- test/unit/Sql/Predicate/InTest.php | 1 + test/unit/Sql/Predicate/PredicateSetTest.php | 8 ++++---- test/unit/Sql/UpdateTest.php | 18 ++++++++---------- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/test/unit/Sql/ExpressionTest.php b/test/unit/Sql/ExpressionTest.php index 99ed8cbd..cc83c447 100644 --- a/test/unit/Sql/ExpressionTest.php +++ b/test/unit/Sql/ExpressionTest.php @@ -55,7 +55,7 @@ public function testSetExpressionException(): void { $expression = new Expression(); $this->expectException(TypeError::class); - /** @noinspection PhpStrictTypeCheckingInspection */ + /** @mago-expect analysis:null-argument */ $expression->setExpression(null); $expression = new Expression(); @@ -144,7 +144,7 @@ public function testNumberOfReplacementsConsidersWhenSameVariableIsUsedManyTimes } #[DataProvider('falsyExpressionParametersProvider')] - public function testConstructorWithFalsyValidParameters(mixed $falsyParameter): void + public function testConstructorWithFalsyValidParameters(bool|float|int|string $falsyParameter): void { $expression = new Expression('?', $falsyParameter); $falsyValue = Argument::value($falsyParameter); @@ -157,6 +157,7 @@ public function testConstructorWithFalsyValidParameters(mixed $falsyParameter): public function testConstructorWithInvalidParameter(): void { $this->expectException(TypeError::class); + /** @mago-expect analysis:possibly-invalid-argument */ new Expression('?', (object) []); } @@ -197,6 +198,7 @@ public function testGetExpressionDataThrowsExceptionWhenParameterCountMismatch() public function testConstructorWithMultipleArguments(): void { + /** @mago-expect analysis:too-many-arguments */ $expression = new Expression('? + ? - ?', 1, 2, 3); $expressionData = $expression->getExpressionData(); diff --git a/test/unit/Sql/Predicate/ExpressionTest.php b/test/unit/Sql/Predicate/ExpressionTest.php index 2c02f471..673e2cbe 100644 --- a/test/unit/Sql/Predicate/ExpressionTest.php +++ b/test/unit/Sql/Predicate/ExpressionTest.php @@ -69,7 +69,7 @@ public function testCanPassSinglePredicateParameterToConstructor(): void #[Group('6849')] public function testCanPassMultiScalarParametersToConstructor(): void { - /** @psalm-suppress TooManyArguments */ + /** @mago-expect analysis:too-many-arguments */ $expression = new Expression('? OR ?', 'foo', 'bar'); $foo = new Value('foo'); $bar = new Value('bar'); @@ -80,7 +80,7 @@ public function testCanPassMultiScalarParametersToConstructor(): void #[Group('6849')] public function testCanPassMultiNullParametersToConstructor(): void { - /** @psalm-suppress TooManyArguments */ + /** @mago-expect analysis:too-many-arguments */ $expression = new Expression('? OR ?', null, null); $null = new Value(null); diff --git a/test/unit/Sql/Predicate/InTest.php b/test/unit/Sql/Predicate/InTest.php index a7e98298..1c73fdcc 100644 --- a/test/unit/Sql/Predicate/InTest.php +++ b/test/unit/Sql/Predicate/InTest.php @@ -147,6 +147,7 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd self::assertEquals(ArgumentType::Values, $value1->getType()); // Test with typed value sets + /** @mago-expect analysis:invalid-argument */ $in->setIdentifier('foo.bar') ->setValueSet([ [1 => ArgumentType::Literal], diff --git a/test/unit/Sql/Predicate/PredicateSetTest.php b/test/unit/Sql/Predicate/PredicateSetTest.php index 4bd53385..c91e1d77 100644 --- a/test/unit/Sql/Predicate/PredicateSetTest.php +++ b/test/unit/Sql/Predicate/PredicateSetTest.php @@ -128,7 +128,7 @@ public function testAddPredicates(): void $predicateSet->addPredicates(['c2' => [1, 2, 3]]); $predicateSet->addPredicates([new IsNotNull('c3')]); - $predicates = (array) $this->readAttribute($predicateSet, 'predicates'); + $predicates = $this->readArrayAttribute($predicateSet, 'predicates'); self::assertCount(7, $predicates); [$operator, $predicate] = self::predicateEntryAt($predicates, 0); @@ -164,7 +164,7 @@ public function testAddPredicates(): void }); $this->expectException(TypeError::class); - /** @noinspection PhpStrictTypeCheckingInspection */ + /** @mago-expect analysis:null-argument */ $predicateSet->addPredicates(null); } @@ -182,7 +182,7 @@ public function testAddPredicatesWithExpression(): void new SqlExpression('COUNT(?) > ?', [Argument::identifier('id'), Argument::value(5)]), ]); - $predicates = (array) $this->readAttribute($predicateSet, 'predicates'); + $predicates = $this->readArrayAttribute($predicateSet, 'predicates'); self::assertCount(1, $predicates); [$operator, $predicate] = self::predicateEntryAt($predicates, 0); @@ -209,7 +209,7 @@ public function testAddPredicatesWithMultipleExpressions(): void new SqlExpression('AVG(?) < ?', [Argument::identifier('price'), Argument::value(50)]), ]); - $predicates = (array) $this->readAttribute($predicateSet, 'predicates'); + $predicates = $this->readArrayAttribute($predicateSet, 'predicates'); self::assertCount(2, $predicates); self::assertInstanceOf(Expression::class, self::predicateEntryAt($predicates, 0)[1]); diff --git a/test/unit/Sql/UpdateTest.php b/test/unit/Sql/UpdateTest.php index 3009e315..11876ba7 100644 --- a/test/unit/Sql/UpdateTest.php +++ b/test/unit/Sql/UpdateTest.php @@ -133,10 +133,9 @@ public function testWhere(): void $this->update->where([new IsNotNull('c3')]); $where = $this->update->where; + self::assertInstanceOf(Where::class, $where); - $predicates = $this->readAttribute($where, 'predicates'); - - self::assertIsArray($predicates); + $predicates = $this->readArrayAttribute($where, 'predicates'); self::assertEquals('AND', $predicates[0][0] ?? ''); self::assertInstanceOf(Literal::class, $predicates[0][1] ?? null); @@ -168,7 +167,7 @@ public function testWhere(): void }); $this->expectException(TypeError::class); - /** @noinspection PhpStrictTypeCheckingInspection */ + /** @mago-expect analysis:null-argument */ $this->update->where(null); } @@ -430,8 +429,7 @@ public function testSetWithMergeFlag(): void $this->update->set(['foo' => 'bar']); $this->update->set(['baz' => 'qux'], Update::VALUES_MERGE); - $set = $this->update->getRawState('set'); - self::assertEquals(['foo' => 'bar', 'baz' => 'qux'], $set); + self::assertEquals(['foo' => 'bar', 'baz' => 'qux'], $this->update->getRawState('set')); } public function testSetWithNumericPriority(): void @@ -440,8 +438,7 @@ public function testSetWithNumericPriority(): void $this->update->set(['one' => 'a'], 10); $this->update->set(['two' => 'b'], 20); - $set = $this->update->getRawState('set'); - self::assertEquals(['one' => 'a', 'two' => 'b', 'three' => 'c'], $set); + self::assertEquals(['one' => 'a', 'two' => 'b', 'three' => 'c'], $this->update->getRawState('set')); } public function testConstructWithTableIdentifier(): void @@ -469,9 +466,9 @@ public function testGetRawStateReturnsAllState(): void ->set(['bar' => 'baz']) ->where('x = y'); + /** @var array $rawState */ $rawState = $this->update->getRawState(); - self::assertIsArray($rawState); self::assertArrayHasKey('table', $rawState); self::assertArrayHasKey('set', $rawState); self::assertArrayHasKey('where', $rawState); @@ -504,8 +501,8 @@ public function testWhereAcceptsExpressionInterface(): void new Expression('COUNT(?) > ?', [new Identifier('id'), new Value(5)]), ]); + /** @var Where $where */ $where = $this->update->getRawState('where'); - self::assertInstanceOf(Where::class, $where); self::assertEquals(1, $where->count()); } @@ -526,6 +523,7 @@ public function testProcessSetWithPdoDriverUsesDriverColumnQuoting(): void $this->update->prepareStatement($mockAdapter, $mockStatement); $sql = $mockStatement->getSql(); + self::assertNotNull($sql); self::assertStringContainsString(':c_0', $sql); self::assertStringContainsString(':c_1', $sql); } From 3fcc81a2aa087a351affb26b63068dac45ae4b6c Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 13:32:34 +1000 Subject: [PATCH 18/22] Restore phpcs compliance after the type-safety cleanup - Define PredicateSpecValue as PredicateInterface|ExpressionParameter, importing the shared alias instead of restating (and overrunning the line limit with) the full union - Use the array short form for the Expression parameter alias - Regroup the docblock annotations and restore end-of-file newlines the analyzer widening had disturbed Signed-off-by: Simon Mundy --- src/Sql/Delete.php | 1 - src/Sql/Expression.php | 2 +- src/Sql/Predicate/Predicate.php | 1 - src/Sql/Predicate/PredicateSet.php | 6 ++---- test/unit/Sql/AbstractSqlFunctionalTestCase.php | 1 - test/unit/Sql/ExpressionDataAssertionsTrait.php | 2 +- test/unit/TestAsset/ConcreteSql.php | 2 +- 7 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index b9cccfdc..2d3c6b6d 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -18,7 +18,6 @@ /** * @property Where $where - * * @phpstan-import-type SpecificationArray from AbstractSql * @phpstan-import-type PredicateSpecValue from Predicate\PredicateSet */ diff --git a/src/Sql/Expression.php b/src/Sql/Expression.php index a73e6fd7..8321cacd 100644 --- a/src/Sql/Expression.php +++ b/src/Sql/Expression.php @@ -20,7 +20,7 @@ use function substr_count; /** - * @phpstan-type ExpressionParameter scalar|null|array|ArgumentInterface|ExpressionInterface|SqlInterface + * @phpstan-type ExpressionParameter scalar|null|array|ArgumentInterface|ExpressionInterface|SqlInterface */ class Expression extends AbstractExpression { diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php index 442a490b..560630cc 100644 --- a/src/Sql/Predicate/Predicate.php +++ b/src/Sql/Predicate/Predicate.php @@ -18,7 +18,6 @@ * @property Predicate $unnest * @property Predicate $NEST * @property Predicate $UNNEST - * * @phpstan-import-type ExpressionParameter from \PhpDb\Sql\Expression */ class Predicate extends PredicateSet diff --git a/src/Sql/Predicate/PredicateSet.php b/src/Sql/Predicate/PredicateSet.php index 0b52d45e..feca5aa5 100644 --- a/src/Sql/Predicate/PredicateSet.php +++ b/src/Sql/Predicate/PredicateSet.php @@ -7,12 +7,9 @@ use Closure; use Countable; use Override; -use PhpDb\Sql\ArgumentInterface; use PhpDb\Sql\Exception; use PhpDb\Sql\Expression; -use PhpDb\Sql\ExpressionInterface; use PhpDb\Sql\Predicate\Expression as PredicateExpression; -use PhpDb\Sql\SqlInterface; use ReturnTypeWillChange; use function count; @@ -24,8 +21,9 @@ // phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse /** + * @phpstan-import-type ExpressionParameter from Expression * @phpstan-type PredicateEntry array{0: string, 1: PredicateInterface} - * @phpstan-type PredicateSpecValue PredicateInterface|ExpressionInterface|SqlInterface|ArgumentInterface|scalar|null|array + * @phpstan-type PredicateSpecValue PredicateInterface|ExpressionParameter */ class PredicateSet implements PredicateInterface, Countable { diff --git a/test/unit/Sql/AbstractSqlFunctionalTestCase.php b/test/unit/Sql/AbstractSqlFunctionalTestCase.php index efa33443..b1682e97 100644 --- a/test/unit/Sql/AbstractSqlFunctionalTestCase.php +++ b/test/unit/Sql/AbstractSqlFunctionalTestCase.php @@ -36,7 +36,6 @@ * @method Insert insert(TableIdentifier|null|string $sqlString) * @method CreateTable createTable(null|string|TableIdentifier $sqlString) * @method Column createColumn(null|string $sqlString) - * * @phpstan-type DecoratorSpec PlatformDecoratorInterface|array{0: class-string, 1: string} * @phpstan-type PlatformExpectation string|array{ * 'string'?: string, diff --git a/test/unit/Sql/ExpressionDataAssertionsTrait.php b/test/unit/Sql/ExpressionDataAssertionsTrait.php index 02928be7..a62614c9 100644 --- a/test/unit/Sql/ExpressionDataAssertionsTrait.php +++ b/test/unit/Sql/ExpressionDataAssertionsTrait.php @@ -44,4 +44,4 @@ private static function predicateEntryAt(array $predicates, int $index): array return [$entry[0], $entry[1]]; } -} \ No newline at end of file +} diff --git a/test/unit/TestAsset/ConcreteSql.php b/test/unit/TestAsset/ConcreteSql.php index 21a0291a..ae8f2e07 100644 --- a/test/unit/TestAsset/ConcreteSql.php +++ b/test/unit/TestAsset/ConcreteSql.php @@ -93,4 +93,4 @@ public function callRenderTable(string $table, ?string $alias = null): string { return $this->renderTable($table, $alias); } -} \ No newline at end of file +} From 19f3a228e5707eefd1faadd6251d1aa6c614cc46 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 13:59:15 +1000 Subject: [PATCH 19/22] Stop enforcing class finality and @api/@internal annotations These are project-wide API-surface policies (179 sites across src and test) the fork has not adopted; disable the two checks so the analyzer reports only actionable issues rather than the whole legacy surface. Signed-off-by: Simon Mundy --- mago.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mago.toml b/mago.toml index b9fe4f5f..67a4d248 100644 --- a/mago.toml +++ b/mago.toml @@ -119,8 +119,8 @@ class-initializers = [ ] check-use-statements = true check-name-casing = true -enforce-class-finality = true -require-api-or-internal = true +enforce-class-finality = false +require-api-or-internal = false ignore = [ { code = "invalid-enum-case-value", in = "src/Sql/" }, ] \ No newline at end of file From 522fac042debbadbb07f003e4fc6070416d2189d Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 14:09:36 +1000 Subject: [PATCH 20/22] Drop the non-functional typed value-set assertions from InTest In::setValueSet's [value => ArgumentType] form was never interpreted: Argument\Values is scalar-only, stores the entries opaquely and would TypeError if rendered to SQL. Remove the speculative block (the scalar value-set case already covers the real behaviour) and rename the test. Signed-off-by: Simon Mundy --- test/unit/Sql/Predicate/InTest.php | 34 +----------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/test/unit/Sql/Predicate/InTest.php b/test/unit/Sql/Predicate/InTest.php index 1c73fdcc..0e644c0a 100644 --- a/test/unit/Sql/Predicate/InTest.php +++ b/test/unit/Sql/Predicate/InTest.php @@ -121,7 +121,7 @@ public function testValueSetIsMutable(): void self::assertEquals(ArgumentType::Values, $valueSet2->getType()); } - public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAndValuesAndArrayOfTypes(): void + public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAndValues(): void { $in = new In(); $in->setIdentifier('foo.bar') @@ -145,38 +145,6 @@ public function testRetrievingWherePartsReturnsSpecificationArrayOfIdentifierAnd $value1 = self::argumentAt($values, 1); self::assertEquals([1, 2, 3], $value1->getValue()); self::assertEquals(ArgumentType::Values, $value1->getType()); - - // Test with typed value sets - /** @mago-expect analysis:invalid-argument */ - $in->setIdentifier('foo.bar') - ->setValueSet([ - [1 => ArgumentType::Literal], - [2 => ArgumentType::Value], - [3 => ArgumentType::Literal], - ]); - - $expressionData = $in->getExpressionData(); - - // Verify specification - self::assertEquals('%s IN (%s, %s, %s)', $expressionData['spec']); - - // Verify expression values - $values = $expressionData['values']; - self::assertCount(2, $values); - - // Verify identifier argument - $value0 = self::argumentAt($values, 0); - self::assertEquals('foo.bar', $value0->getValue()); - self::assertEquals(ArgumentType::Identifier, $value0->getType()); - - // Verify value set argument with types - $value1 = self::argumentAt($values, 1); - self::assertEquals([ - [1 => ArgumentType::Literal], - [2 => ArgumentType::Value], - [3 => ArgumentType::Literal], - ], $value1->getValue()); - self::assertEquals(ArgumentType::Values, $value1->getType()); } public function testGetExpressionDataWithSubselect(): void From ce64bdcae672805c58a1222833d07301a1f5fdef Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 14:36:48 +1000 Subject: [PATCH 21/22] Restore phpstan compliance after the type-safety cleanup - Match InsertIgnore::$specifications to Insert's type (property types are invariant), drop the now-redundant getSqlPlatform() null assertion, and remove a stale inline @phpstan-ignore that the improved typing made moot - Regenerate the baseline: clear entries the cleanup fixed, and record the remaining mago/phpstan disagreements (defensive ??/?- forms mago requires), the deliberate __get/TypeError tests, and the deferred Platform issues Signed-off-by: Simon Mundy --- phpstan-baseline.neon | 64 +++++++++++++++---- src/Sql/InsertIgnore.php | 5 +- src/Sql/Platform/AbstractPlatform.php | 1 - .../Sql/AbstractSqlFunctionalTestCase.php | 1 - 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index d78b460f..ac79c147 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,10 +1,22 @@ parameters: ignoreErrors: - - message: '#^Variable \$paramSpecs might not be defined\.$#' - identifier: variable.undefined + message: '#^Offset 0 on array\{list\\} on left side of \?\? always exists and is not nullable\.$#' + identifier: nullCoalesce.offset count: 1 - path: src/Sql/AbstractSql.php + path: src/Sql/Expression.php + + - + message: '#^Argument of an invalid type PhpDb\\Sql\\Platform\\PlatformDecoratorInterface supplied for foreach, only iterables are supported\.$#' + identifier: foreach.nonIterable + count: 1 + path: src/Sql/Platform/Platform.php + + - + message: '#^Method PhpDb\\Sql\\Platform\\Platform\:\:getDecorators\(\) should return array\ but returns PhpDb\\Sql\\Platform\\PlatformDecoratorInterface\.$#' + identifier: return.type + count: 1 + path: src/Sql/Platform/Platform.php - message: '#^Parameter \#1 \$attribute \(string\) of method PhpDbTest\\Adapter\\Driver\\TestAsset\\PdoMock\:\:getAttribute\(\) should be compatible with parameter \$attribute \(int\) of method PDO\:\:getAttribute\(\)$#' @@ -78,6 +90,12 @@ parameters: count: 3 path: test/unit/Sql/AbstractSqlFunctionalTestCase.php + - + message: '#^Call to new PhpDb\\Sql\\Argument\\Select\(\) on a separate line has no effect\.$#' + identifier: new.resultUnused + count: 1 + path: test/unit/Sql/ArgumentTest.php + - message: '#^PHPDoc tag @var with type PhpDb\\Sql\\Ddl\\Column\\ColumnInterface is not subtype of native type PHPUnit\\Framework\\MockObject\\MockObject\.$#' identifier: varTag.nativeType @@ -91,22 +109,22 @@ parameters: path: test/unit/Sql/Ddl/AlterTableTest.php - - message: '#^Call to an undefined method PHPUnit\\Framework\\MockObject\\MockObject\:\:addColumn\(\)\.$#' - identifier: method.notFound + message: '#^Access to an undefined property PhpDb\\Sql\\Delete\:\:\$unknown\.$#' + identifier: property.notFound count: 1 - path: test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php + path: test/unit/Sql/DeleteTest.php - - message: '#^Call to an undefined method PHPUnit\\Framework\\MockObject\\MockObject\:\:getColumns\(\)\.$#' - identifier: method.notFound - count: 3 - path: test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php + message: '#^Access to protected property PhpDb\\Sql\\Delete\:\:\$table\.$#' + identifier: property.protected + count: 1 + path: test/unit/Sql/DeleteTest.php - - message: '#^Call to an undefined method PHPUnit\\Framework\\MockObject\\MockObject\:\:setColumns\(\)\.$#' - identifier: method.notFound - count: 2 - path: test/unit/Sql/Ddl/Constraint/AbstractConstraintTest.php + message: '#^Call to static method PHPUnit\\Framework\\Assert\:\:assertNull\(\) with array\|PhpDb\\Sql\\TableIdentifier\|string will always evaluate to false\.$#' + identifier: staticMethod.impossibleType + count: 1 + path: test/unit/Sql/DeleteTest.php - message: '#^Access to an undefined property PhpDb\\Sql\\InsertIgnore\:\:\$foo\.$#' @@ -125,3 +143,21 @@ parameters: identifier: property.notFound count: 1 path: test/unit/Sql/InsertTest.php + + - + message: '#^Access to an undefined property PhpDb\\Sql\\Select\:\:\$invalidProperty\.$#' + identifier: property.notFound + count: 1 + path: test/unit/Sql/SelectTest.php + + - + message: '#^Using nullsafe method call on non\-nullable type PhpDb\\Sql\\Where\. Use \-\> instead\.$#' + identifier: nullsafe.neverNull + count: 2 + path: test/unit/Sql/SelectTest.php + + - + message: '#^Using nullsafe property access on non\-nullable type PhpDb\\Sql\\Where\. Use \-\> instead\.$#' + identifier: nullsafe.neverNull + count: 2 + path: test/unit/Sql/SelectTest.php diff --git a/src/Sql/InsertIgnore.php b/src/Sql/InsertIgnore.php index e8b2e9fe..d57b1b5e 100644 --- a/src/Sql/InsertIgnore.php +++ b/src/Sql/InsertIgnore.php @@ -4,9 +4,12 @@ namespace PhpDb\Sql; +/** + * @phpstan-import-type SpecificationArray from AbstractSql + */ class InsertIgnore extends Insert { - /** @var string[]|array[] $specifications */ + /** @var array */ protected array $specifications = [ self::SPECIFICATION_INSERT => 'INSERT IGNORE INTO %1$s (%2$s) VALUES (%3$s)', self::SPECIFICATION_SELECT => 'INSERT IGNORE INTO %1$s %2$s %3$s', diff --git a/src/Sql/Platform/AbstractPlatform.php b/src/Sql/Platform/AbstractPlatform.php index 6f24a8d7..cb77647b 100644 --- a/src/Sql/Platform/AbstractPlatform.php +++ b/src/Sql/Platform/AbstractPlatform.php @@ -37,7 +37,6 @@ public function getTypeDecorator( PreparableSqlInterface|SqlInterface $subject ): PlatformDecoratorInterface|PreparableSqlInterface|SqlInterface { foreach ($this->decorators as $type => $decorator) { - /** @phpstan-ignore-next-line instanceof with string class name is valid */ if ($subject instanceof $type) { $decorator->setSubject($subject); return $decorator; diff --git a/test/unit/Sql/AbstractSqlFunctionalTestCase.php b/test/unit/Sql/AbstractSqlFunctionalTestCase.php index b1682e97..c66222fe 100644 --- a/test/unit/Sql/AbstractSqlFunctionalTestCase.php +++ b/test/unit/Sql/AbstractSqlFunctionalTestCase.php @@ -286,7 +286,6 @@ public function test(PreparableSqlInterface|SqlInterface $sqlObject, string $pla $this->assertInstanceOf(PlatformDecoratorInterface::class, $decorator); $platform = $sql->getSqlPlatform(); - $this->assertNotNull($platform); $this->assertInstanceOf(Sql\Platform\AbstractPlatform::class, $platform); $platform->setTypeDecorator($type, $decorator); } From 3c7ff50db607a21b84506852f5a1191338b4258b Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 10 Jul 2026 14:50:20 +1000 Subject: [PATCH 22/22] Build SelectTest where fixtures via closures, not the magic property phpstan resolves $select->where inconsistently across versions (local via __get's non-null union, CI via the possibly-null @property), so the nullsafe form produced a version-sensitive baseline entry that failed on CI's latest phpstan. Route the predicates through where(Closure), whose argument is an unambiguously non-null Where, and drop the fragile nullsafe baseline entries. Signed-off-by: Simon Mundy --- phpstan-baseline.neon | 12 ------------ test/unit/Sql/SelectTest.php | 30 ++++++++++++++++++------------ 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ac79c147..f57dd033 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -149,15 +149,3 @@ parameters: identifier: property.notFound count: 1 path: test/unit/Sql/SelectTest.php - - - - message: '#^Using nullsafe method call on non\-nullable type PhpDb\\Sql\\Where\. Use \-\> instead\.$#' - identifier: nullsafe.neverNull - count: 2 - path: test/unit/Sql/SelectTest.php - - - - message: '#^Using nullsafe property access on non\-nullable type PhpDb\\Sql\\Where\. Use \-\> instead\.$#' - identifier: nullsafe.neverNull - count: 2 - path: test/unit/Sql/SelectTest.php diff --git a/test/unit/Sql/SelectTest.php b/test/unit/Sql/SelectTest.php index abdd5503..925ac6ca 100644 --- a/test/unit/Sql/SelectTest.php +++ b/test/unit/Sql/SelectTest.php @@ -1173,7 +1173,9 @@ public static function providerData(): array ]; $select32subselect = new Select(); - $select32subselect->from('bar')->where?->like('y', '%Foo%'); + $select32subselect->from('bar')->where(static function (Where $where): void { + $where->like('y', '%Foo%'); + }); $select32 = new Select(); $select32->from(['x' => $select32subselect]); @@ -1262,7 +1264,9 @@ public static function providerData(): array // subselect in join $select39subselect = new Select(); - $select39subselect->from('bar')->where?->like('y', '%Foo%'); + $select39subselect->from('bar')->where(static function (Where $where): void { + $where->like('y', '%Foo%'); + }); $select39 = new Select(); $select39->from('foo')->join(['z' => $select39subselect], 'z.foo = bar.id'); $sqlPrep39 = 'SELECT "foo".*, "z".* FROM "foo" INNER JOIN (SELECT "bar".* FROM "bar" WHERE "y" LIKE ?) AS "z" ON "z"."foo" = "bar"."id"'; @@ -1394,11 +1398,12 @@ public static function providerData(): array // Test generic predicate is appended with AND $select50 = new Select(); $select50->from(new TableIdentifier('foo')) - ->where - ?->nest - ->isNull('bar') - ->and - ->predicate(new Predicate\Literal('1=1')); + ->where(static function (Where $where): void { + $where->nest + ->isNull('bar') + ->and + ->predicate(new Predicate\Literal('1=1')); + }); $sqlPrep50 = 'SELECT "foo".* FROM "foo" WHERE ("bar" IS NULL AND 1=1)'; $sqlStr50 = 'SELECT "foo".* FROM "foo" WHERE ("bar" IS NULL AND 1=1)'; $internalTests50 = []; @@ -1406,11 +1411,12 @@ public static function providerData(): array // Test generic predicate is appended with OR $select51 = new Select(); $select51->from(new TableIdentifier('foo')) - ->where - ?->nest - ->isNull('bar') - ->or - ->predicate(new Predicate\Literal('1=1')); + ->where(static function (Where $where): void { + $where->nest + ->isNull('bar') + ->or + ->predicate(new Predicate\Literal('1=1')); + }); $sqlPrep51 = 'SELECT "foo".* FROM "foo" WHERE ("bar" IS NULL OR 1=1)'; $sqlStr51 = 'SELECT "foo".* FROM "foo" WHERE ("bar" IS NULL OR 1=1)'; $internalTests51 = [];