Skip to content

Commit 6c593a6

Browse files
hembergerhemberger
authored
DoctrineKeyValueStyleRule: properly handle unions (#575)
1 parent 184d6c6 commit 6c593a6

File tree

3 files changed

+96
-58
lines changed

3 files changed

+96
-58
lines changed

‎src/Rules/DoctrineKeyValueStyleRule.php‎

Lines changed: 63 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
use PHPStan\Rules\RuleError;
1515
use PHPStan\Rules\RuleErrorBuilder;
1616
use PHPStan\ShouldNotHappenException;
17-
use PHPStan\Type\Constant\ConstantArrayType;
18-
use PHPStan\Type\Constant\ConstantStringType;
1917
use PHPStan\Type\IntegerRangeType;
2018
use PHPStan\Type\IntegerType;
2119
use PHPStan\Type\ObjectType;
@@ -113,7 +111,8 @@ public function processNode(Node $callLike, Scope $scope): array
113111

114112
$tableExpr = $args[0]->value;
115113
$tableType = $scope->getType($tableExpr);
116-
if (! $tableType instanceof ConstantStringType) {
114+
$tableNames = $tableType->getConstantStrings();
115+
if (\count($tableNames) === 0) {
117116
return [
118117
RuleErrorBuilder::message('Argument #0 expects a constant string, got ' . $tableType->describe(VerbosityLevel::precise()))->line($callLike->getLine())->build(),
119118
];
@@ -126,74 +125,81 @@ public function processNode(Node $callLike, Scope $scope): array
126125

127126
$checkIntegerRanges = QueryReflection::getRuntimeConfiguration()->isParameterTypeValidationStrict();
128127

129-
// Table name may be escaped with backticks
130-
$argTableName = trim($tableType->getValue(), '`');
131-
$table = $schemaReflection->getTable($argTableName);
132-
if (null === $table) {
133-
return [
134-
RuleErrorBuilder::message('Query error: Table "' . $argTableName . '" does not exist')->line($callLike->getLine())->build(),
135-
];
136-
}
137-
138-
// All array arguments should have table columns as keys
139128
$errors = [];
140-
foreach ($arrayArgPositions as $arrayArgPosition) {
141-
// If the argument doesn't exist, just skip it since we don't want
142-
// to error in case it has a default value
143-
if (! \array_key_exists($arrayArgPosition, $args)) {
144-
continue;
145-
}
146-
147-
$argType = $scope->getType($args[$arrayArgPosition]->value);
148-
if (! $argType instanceof ConstantArrayType) {
149-
$errors[] = 'Argument #' . $arrayArgPosition . ' is not a constant array, got ' . $argType->describe(VerbosityLevel::precise());
129+
foreach ($tableNames as $tableName) {
130+
// Table name may be escaped with backticks
131+
$tableName = trim($tableName->getValue(), '`');
132+
$table = $schemaReflection->getTable($tableName);
133+
if (null === $table) {
134+
$errors[] = 'Table "' . $tableName . '" does not exist';
150135
continue;
151136
}
152137

153-
foreach ($argType->getKeyTypes() as $keyIndex => $keyType) {
154-
if (! $keyType instanceof ConstantStringType) {
155-
$errors[] = 'Element #' . $keyIndex . ' of argument #' . $arrayArgPosition . ' must have a string key, got ' . $keyType->describe(VerbosityLevel::precise());
138+
// All array arguments should have table columns as keys
139+
foreach ($arrayArgPositions as $arrayArgPosition) {
140+
// If the argument doesn't exist, just skip it since we don't want
141+
// to error in case it has a default value
142+
if (! \array_key_exists($arrayArgPosition, $args)) {
156143
continue;
157144
}
158145

159-
// Column name may be escaped with backticks
160-
$argColumnName = trim($keyType->getValue(), '`');
161-
162-
$argColumn = null;
163-
foreach ($table->getColumns() as $column) {
164-
if ($argColumnName === $column->getName()) {
165-
$argColumn = $column;
166-
}
167-
}
168-
if (null === $argColumn) {
169-
$errors[] = 'Column "' . $table->getName() . '.' . $argColumnName . '" does not exist';
146+
$argType = $scope->getType($args[$arrayArgPosition]->value);
147+
$argArrays = $argType->getConstantArrays();
148+
if (\count($argArrays) === 0) {
149+
$errors[] = 'Argument #' . $arrayArgPosition . ' is not a constant array, got ' . $argType->describe(VerbosityLevel::precise());
170150
continue;
171151
}
172152

173-
$argColumnType = $argColumn->getType();
174-
$valueType = $argType->getValueTypes()[$keyIndex];
175-
176-
if (false === $checkIntegerRanges) {
177-
// Convert IntegerRangeType column types into IntegerType so
178-
// that any integer value is accepted for integer columns,
179-
// since it is uncommon to check integer value ranges.
180-
if ($argColumnType instanceof IntegerRangeType) {
181-
$argColumnType = new IntegerType();
182-
} elseif ($argColumnType instanceof UnionType) {
183-
$newTypes = [];
184-
foreach ($argColumnType->getTypes() as $type) {
185-
if ($type instanceof IntegerRangeType) {
186-
$type = new IntegerType();
153+
foreach ($argArrays as $argArray) {
154+
foreach ($argArray->getKeyTypes() as $keyIndex => $keyType) {
155+
$keyNames = $keyType->getConstantStrings();
156+
if (\count($keyNames) === 0) {
157+
$errors[] = 'Element #' . $keyIndex . ' of argument #' . $arrayArgPosition . ' must have a string key, got ' . $keyType->describe(VerbosityLevel::precise());
158+
continue;
159+
}
160+
161+
foreach ($keyNames as $keyName) {
162+
// Column name may be escaped with backticks
163+
$argColumnName = trim($keyName->getValue(), '`');
164+
165+
$argColumn = null;
166+
foreach ($table->getColumns() as $column) {
167+
if ($argColumnName === $column->getName()) {
168+
$argColumn = $column;
169+
}
170+
}
171+
if (null === $argColumn) {
172+
$errors[] = 'Column "' . $table->getName() . '.' . $argColumnName . '" does not exist';
173+
continue;
174+
}
175+
176+
$argColumnType = $argColumn->getType();
177+
$valueType = $argArray->getValueTypes()[$keyIndex];
178+
179+
if (false === $checkIntegerRanges) {
180+
// Convert IntegerRangeType column types into IntegerType so
181+
// that any integer value is accepted for integer columns,
182+
// since it is uncommon to check integer value ranges.
183+
if ($argColumnType instanceof IntegerRangeType) {
184+
$argColumnType = new IntegerType();
185+
} elseif ($argColumnType instanceof UnionType) {
186+
$newTypes = [];
187+
foreach ($argColumnType->getTypes() as $type) {
188+
if ($type instanceof IntegerRangeType) {
189+
$type = new IntegerType();
190+
}
191+
$newTypes[] = $type;
192+
}
193+
$argColumnType = TypeCombinator::union(...$newTypes);
194+
}
195+
}
196+
197+
if (! $argColumnType->isSuperTypeOf($valueType)->yes()) {
198+
$errors[] = 'Column "' . $table->getName() . '.' . $argColumnName . '" expects value type ' . $argColumnType->describe(VerbosityLevel::precise()) . ', got type ' . $valueType->describe(VerbosityLevel::precise());
187199
}
188-
$newTypes[] = $type;
189200
}
190-
$argColumnType = TypeCombinator::union(...$newTypes);
191201
}
192202
}
193-
194-
if (! $argColumnType->isSuperTypeOf($valueType)->yes()) {
195-
$errors[] = 'Column "' . $table->getName() . '.' . $argColumnName . '" expects value type ' . $argColumnType->describe(VerbosityLevel::precise()) . ', got type ' . $valueType->describe(VerbosityLevel::precise());
196-
}
197203
}
198204
}
199205

‎tests/rules/DoctrineKeyValueStyleRuleTest.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,22 @@ public function testRule(): void
6868
'Query error: Column "ada.adaid" expects value type int, got type mixed',
6969
56,
7070
],
71+
[
72+
'Query error: Table "not_a_table1" does not exist',
73+
112,
74+
],
75+
[
76+
'Query error: Table "not_a_table2" does not exist',
77+
112,
78+
],
79+
[
80+
'Query error: Column "ada.not_a_column1" does not exist',
81+
120,
82+
],
83+
[
84+
'Query error: Column "ada.not_a_column2" does not exist',
85+
120,
86+
],
7187
];
7288

7389
$this->analyse([__DIR__ . '/data/doctrine-key-value-style.php'], $expectedErrors);

‎tests/rules/data/doctrine-key-value-style.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
class Foo
88
{
9-
public function errorIfTableIsNotLiteralString(Connection $conn, string $tableName)
9+
public function errorIfTableIsNotConstantString(Connection $conn, string $tableName)
1010
{
1111
$conn->assembleNoArrays($tableName);
1212
}
@@ -103,4 +103,20 @@ public function noErrorWithBackticks(Connection $conn)
103103
{
104104
$conn->assembleOneArray('`ada`', ['`adaid`' => 1]);
105105
}
106+
107+
/**
108+
* @param 'ada'|'not_a_table1'|'not_a_table2' $table
109+
*/
110+
public function errorInTableNameUnion(Connection $conn, string $table)
111+
{
112+
$conn->assembleNoArrays($table);
113+
}
114+
115+
/**
116+
* @param array{email: string}|array{not_a_column1: int}|array{not_a_column2: int} $params
117+
*/
118+
public function errorInParamArrayUnion(Connection $conn, array $params)
119+
{
120+
$conn->assembleOneArray('ada', $params);
121+
}
106122
}

0 commit comments

Comments
 (0)