From d88eb6e81ac550ace6d6787ff646915db2d1e88b Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Wed, 1 Dec 2021 00:14:24 +0100 Subject: [PATCH] phpstan: introduce literal-string (BC break!) --- composer.json | 2 +- src/Collection/DbalCollection.php | 4 +- .../Functions/BaseAggregateFunction.php | 10 +++-- .../Functions/CompareEqualsFunction.php | 6 +-- .../Functions/CompareNotEqualsFunction.php | 6 +-- .../Functions/ConjunctionOperatorFunction.php | 5 ++- .../Functions/DisjunctionOperatorFunction.php | 5 ++- src/Collection/Helpers/DbalAnyAggregator.php | 5 ++- .../Helpers/DbalExpressionResult.php | 45 ++++++++++++++++--- src/Collection/Helpers/DbalJoinEntry.php | 18 ++++++-- src/Collection/Helpers/DbalNoneAggregator.php | 5 ++- .../Helpers/DbalQueryBuilderHelper.php | 36 ++++++++------- src/Mapper/Dbal/DbalMapper.php | 13 +++++- .../Dbal/RelationshipMapperManyHasMany.php | 2 + .../Dbal/DbalValueOperatorFunctionTest.phpt | 14 +++--- 15 files changed, 121 insertions(+), 55 deletions(-) diff --git a/composer.json b/composer.json index 3409d559..32663c57 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,7 @@ "marc-mabe/php-enum": "~3.0", "mockery/mockery": "~1.2", "phpstan/extension-installer": "1.1.0", - "phpstan/phpstan": "1.0.2", + "phpstan/phpstan": "1.2.0", "phpstan/phpstan-deprecation-rules": "1.0.0", "phpstan/phpstan-nette": "1.0.0", "phpstan/phpstan-mockery": "1.0.0", diff --git a/src/Collection/DbalCollection.php b/src/Collection/DbalCollection.php index 6a6abf54..a607a60e 100644 --- a/src/Collection/DbalCollection.php +++ b/src/Collection/DbalCollection.php @@ -125,9 +125,9 @@ public function findBy(array $conds): ICollection } if ($expression->isHavingClause) { - $collection->queryBuilder->andHaving(...$expression->args); + $collection->queryBuilder->andHaving($expression->expression, ...$expression->args); } else { - $collection->queryBuilder->andWhere(...$expression->args); + $collection->queryBuilder->andWhere($expression->expression, ...$expression->args); } return $collection; } diff --git a/src/Collection/Functions/BaseAggregateFunction.php b/src/Collection/Functions/BaseAggregateFunction.php index bc4ee365..ebbc81dd 100644 --- a/src/Collection/Functions/BaseAggregateFunction.php +++ b/src/Collection/Functions/BaseAggregateFunction.php @@ -20,10 +20,13 @@ abstract class BaseAggregateFunction implements IArrayFunction, IQueryBuilderFunction { - /** @var string */ + /** @var literal-string */ private $sqlFunction; + /** + * @param literal-string $sqlFunction + */ protected function __construct(string $sqlFunction) { $this->sqlFunction = $sqlFunction; @@ -70,7 +73,7 @@ public function processQueryBuilderExpression( } $aggregator = new class implements IDbalAggregator { - /** @var string */ + /** @var literal-string */ public $sqlFunction; @@ -80,7 +83,8 @@ public function aggregate( ): DbalExpressionResult { return new DbalExpressionResult( - ["{$this->sqlFunction}(%ex)", $expression->args], + "{$this->sqlFunction}($expression->expression)", + $expression->args, $expression->joins, null, true, diff --git a/src/Collection/Functions/CompareEqualsFunction.php b/src/Collection/Functions/CompareEqualsFunction.php index af0efae0..298b2784 100644 --- a/src/Collection/Functions/CompareEqualsFunction.php +++ b/src/Collection/Functions/CompareEqualsFunction.php @@ -32,7 +32,7 @@ protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalE if (count($value) > 0) { // extract column names for multiOr simplification // array{%column, array} - $args = $expression->args; + $args = $expression->getExpansionArguments(); if (count($args) === 2 && $args[0] === '%column' && is_array($args[1])) { $columns = $args[1]; $value = array_map(function ($value) use ($columns): array { @@ -45,12 +45,12 @@ protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalE } return $combined; }, $value); - return $expression->withArgs(['%multiOr', $value]); + return $expression->withArgs('%multiOr', [$value]); } else { return $expression->append('IN %any', $value); } } else { - return $expression->withArgs(['1=0']); + return $expression->withArgs('1=0', []); } } elseif ($value === null) { return $expression->append('IS NULL'); diff --git a/src/Collection/Functions/CompareNotEqualsFunction.php b/src/Collection/Functions/CompareNotEqualsFunction.php index 7c6640b2..afc5fbcd 100644 --- a/src/Collection/Functions/CompareNotEqualsFunction.php +++ b/src/Collection/Functions/CompareNotEqualsFunction.php @@ -32,7 +32,7 @@ protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalE if (count($value) > 0) { // extract column names for multiOr simplification // array{%column, array} - $args = $expression->args; + $args = $expression->getExpansionArguments(); if (count($args) === 2 && $args[0] === '%column' && is_array($args[1])) { $columns = $args[1]; $value = array_map(function ($value) use ($columns): array { @@ -45,12 +45,12 @@ protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalE } return $combined; }, $value); - return $expression->withArgs(['NOT (%multiOr)', $value]); + return $expression->withArgs('NOT (%multiOr)', [$value]); } else { return $expression->append('NOT IN %any', $value); } } else { - return $expression->withArgs(['1=1']); + return $expression->withArgs('1=1', []); } } elseif ($value === null) { return $expression->append('IS NOT NULL'); diff --git a/src/Collection/Functions/ConjunctionOperatorFunction.php b/src/Collection/Functions/ConjunctionOperatorFunction.php index 51b9bf85..347d27d7 100644 --- a/src/Collection/Functions/ConjunctionOperatorFunction.php +++ b/src/Collection/Functions/ConjunctionOperatorFunction.php @@ -56,13 +56,14 @@ public function processQueryBuilderExpression( foreach ($this->normalizeFunctions($args) as $collectionFunctionArgs) { $expression = $helper->processFilterFunction($builder, $collectionFunctionArgs, $aggregator); $expression = $expression->applyAggregator($builder); - $processedArgs[] = $expression->args; + $processedArgs[] = $expression->getExpansionArguments(); $joins = array_merge($joins, $expression->joins); $isHavingClause = $isHavingClause || $expression->isHavingClause; } return new DbalExpressionResult( - ['%and', $processedArgs], + '%and', + [$processedArgs], $joins, null, $isHavingClause, diff --git a/src/Collection/Functions/DisjunctionOperatorFunction.php b/src/Collection/Functions/DisjunctionOperatorFunction.php index 1c456fe5..e9161a9f 100644 --- a/src/Collection/Functions/DisjunctionOperatorFunction.php +++ b/src/Collection/Functions/DisjunctionOperatorFunction.php @@ -56,13 +56,14 @@ public function processQueryBuilderExpression( foreach ($this->normalizeFunctions($args) as $collectionFunctionArgs) { $expression = $helper->processFilterFunction($builder, $collectionFunctionArgs, $aggregator); $expression = $expression->applyAggregator($builder); - $processedArgs[] = $expression->args; + $processedArgs[] = $expression->getExpansionArguments(); $joins = array_merge($joins, $expression->joins); $isHavingClause = $isHavingClause || $expression->isHavingClause; } return new DbalExpressionResult( - ['%or', $processedArgs], + '%or', + [$processedArgs], $joins, null, $isHavingClause, diff --git a/src/Collection/Helpers/DbalAnyAggregator.php b/src/Collection/Helpers/DbalAnyAggregator.php index 175e7451..51f48475 100644 --- a/src/Collection/Helpers/DbalAnyAggregator.php +++ b/src/Collection/Helpers/DbalAnyAggregator.php @@ -17,7 +17,7 @@ public function aggregate( DbalExpressionResult $expression ): DbalExpressionResult { - $joinExpression = array_shift($expression->args); + $joinExpression = $expression->expression; $joinArgs = $expression->args; $joins = $expression->joins; @@ -38,7 +38,8 @@ public function aggregate( $queryBuilder->addGroupBy('%table.%column', $join->alias, $primaryKey); return new DbalExpressionResult( - ['COUNT(%table.%column) > 0', $join->alias, $primaryKey], + 'COUNT(%table.%column) > 0', + [$join->alias, $primaryKey], $joins, null, true, diff --git a/src/Collection/Helpers/DbalExpressionResult.php b/src/Collection/Helpers/DbalExpressionResult.php index 8355e46c..acd87a34 100644 --- a/src/Collection/Helpers/DbalExpressionResult.php +++ b/src/Collection/Helpers/DbalExpressionResult.php @@ -3,7 +3,6 @@ namespace Nextras\Orm\Collection\Helpers; -use MongoDB\Driver\Query; use Nextras\Dbal\QueryBuilder\QueryBuilder; use Nextras\Orm\Entity\Reflection\PropertyMetadata; use Nextras\Orm\Exception\InvalidArgumentException; @@ -18,7 +17,14 @@ class DbalExpressionResult { /** - * Holds expression as the first argument and then all its arguments. + * Holds expression separately from its arguments. + * @var string + * @phpstan-var literal-string + */ + public $expression; + + /** + * Expression's arguments. * @var mixed[] * @phpstan-var list */ @@ -56,12 +62,14 @@ class DbalExpressionResult /** + * @phpstan-param literal-string $expression * @param mixed[] $args * @param DbalJoinEntry[] $joins * @phpstan-param list $args * @param bool $isHavingClause */ public function __construct( + string $expression, array $args, array $joins = [], ?IDbalAggregator $aggregator = null, @@ -70,6 +78,7 @@ public function __construct( ?callable $valueNormalizer = null ) { + $this->expression = $expression; $this->args = $args; $this->aggregator = $aggregator; $this->joins = $joins; @@ -86,24 +95,46 @@ public function __construct( /** * Appends SQL expression to the original expression. * If you need prepend or other complex expression, create new instance of DbalExpressionResult. + * @phpstan-param literal-string $expression * @phpstan-param list $args */ public function append(string $expression, ...$args): DbalExpressionResult { - array_unshift($args, $this->args); - array_unshift($args, "%ex $expression"); - return $this->withArgs($args); + $args = array_merge($this->args, $args); + return $this->withArgs("{$this->expression} $expression", $args); + } + + + /** + * Returns all arguments including the expression. + * Suitable as an `%ex` modifier argument. + * @return array + */ + public function getExpansionArguments(): array + { + $args = $this->args; + array_unshift($args, $this->expression); + return $args; } /** * Creates a new DbalExpression from the passed $args and keeps the original expression * properties (joins, aggregator, ...). + * @phpstan-param literal-string $expression * @param array $args */ - public function withArgs(array $args): DbalExpressionResult + public function withArgs(string $expression, array $args): DbalExpressionResult { - return new DbalExpressionResult($args, $this->joins, $this->aggregator, $this->isHavingClause, null, null); + return new DbalExpressionResult( + $expression, + $args, + $this->joins, + $this->aggregator, + $this->isHavingClause, + null, + null + ); } diff --git a/src/Collection/Helpers/DbalJoinEntry.php b/src/Collection/Helpers/DbalJoinEntry.php index e7271441..55a038e9 100644 --- a/src/Collection/Helpers/DbalJoinEntry.php +++ b/src/Collection/Helpers/DbalJoinEntry.php @@ -12,13 +12,22 @@ */ class DbalJoinEntry { - /** @var string */ + /** + * @var string + * @phpstan-var literal-string + */ public $toExpression; - /** @var string */ + /** + * @var string + * @phpstan-var literal-string + */ public $alias; - /** @var string */ + /** + * @var string + * @phpstan-var literal-string + */ public $onExpression; /** @var array */ @@ -29,6 +38,9 @@ class DbalJoinEntry /** + * @phpstan-param literal-string $toExpression + * @phpstan-param literal-string $toAlias + * @phpstan-param literal-string $onExpression * @param array $args */ public function __construct( diff --git a/src/Collection/Helpers/DbalNoneAggregator.php b/src/Collection/Helpers/DbalNoneAggregator.php index 568a9389..f071b36e 100644 --- a/src/Collection/Helpers/DbalNoneAggregator.php +++ b/src/Collection/Helpers/DbalNoneAggregator.php @@ -17,7 +17,7 @@ public function aggregate( DbalExpressionResult $expression ): DbalExpressionResult { - $joinExpression = array_shift($expression->args); + $joinExpression = $expression->expression; $joinArgs = $expression->args; $joins = $expression->joins; @@ -38,7 +38,8 @@ public function aggregate( $queryBuilder->addGroupBy('%table.%column', $join->alias, $primaryKey); return new DbalExpressionResult( - ['COUNT(%table.%column) = 0', $join->alias, $primaryKey], + 'COUNT(%table.%column) = 0', + [$join->alias, $primaryKey], $joins, null, true, diff --git a/src/Collection/Helpers/DbalQueryBuilderHelper.php b/src/Collection/Helpers/DbalQueryBuilderHelper.php index 80322ed1..f3076871 100644 --- a/src/Collection/Helpers/DbalQueryBuilderHelper.php +++ b/src/Collection/Helpers/DbalQueryBuilderHelper.php @@ -143,35 +143,36 @@ public function processOrder(QueryBuilder $builder, $expr, string $direction): v */ private function processOrderDirection(DbalExpressionResult $expression, string $direction): array { + $args = $expression->getExpansionArguments(); if ($this->platformName === 'mysql') { if ($direction === ICollection::ASC || $direction === ICollection::ASC_NULLS_FIRST) { - return ['%ex ASC', $expression->args]; + return ['%ex ASC', $args]; } elseif ($direction === ICollection::DESC || $direction === ICollection::DESC_NULLS_LAST) { - return ['%ex DESC', $expression->args]; + return ['%ex DESC', $args]; } elseif ($direction === ICollection::ASC_NULLS_LAST) { - return ['%ex IS NULL, %ex ASC', $expression->args, $expression->args]; + return ['%ex IS NULL, %ex ASC', $args, $args]; } elseif ($direction === ICollection::DESC_NULLS_FIRST) { - return ['%ex IS NOT NULL, %ex DESC', $expression->args, $expression->args]; + return ['%ex IS NOT NULL, %ex DESC', $args, $args]; } } elseif ($this->platformName === 'mssql') { if ($direction === ICollection::ASC || $direction === ICollection::ASC_NULLS_FIRST) { - return ['%ex ASC', $expression->args]; + return ['%ex ASC', $args]; } elseif ($direction === ICollection::DESC || $direction === ICollection::DESC_NULLS_LAST) { - return ['%ex DESC', $expression->args]; + return ['%ex DESC', $args]; } elseif ($direction === ICollection::ASC_NULLS_LAST) { - return ['CASE WHEN %ex IS NULL THEN 1 ELSE 0 END, %ex ASC', $expression->args, $expression->args]; + return ['CASE WHEN %ex IS NULL THEN 1 ELSE 0 END, %ex ASC', $args, $args]; } elseif ($direction === ICollection::DESC_NULLS_FIRST) { - return ['CASE WHEN %ex IS NOT NULL THEN 1 ELSE 0 END, %ex DESC', $expression->args, $expression->args]; + return ['CASE WHEN %ex IS NOT NULL THEN 1 ELSE 0 END, %ex DESC', $args, $args]; } } elseif ($this->platformName === 'pgsql') { if ($direction === ICollection::ASC || $direction === ICollection::ASC_NULLS_LAST) { - return ['%ex ASC', $expression->args]; + return ['%ex ASC', $args]; } elseif ($direction === ICollection::DESC || $direction === ICollection::DESC_NULLS_FIRST) { - return ['%ex DESC', $expression->args]; + return ['%ex DESC', $args]; } elseif ($direction === ICollection::ASC_NULLS_FIRST) { - return ['%ex ASC NULLS FIRST', $expression->args]; + return ['%ex ASC NULLS FIRST', $args]; } elseif ($direction === ICollection::DESC_NULLS_LAST) { - return ['%ex DESC NULLS LAST', $expression->args]; + return ['%ex DESC NULLS LAST', $args]; } } @@ -301,7 +302,8 @@ private function processTokens( ); return new DbalExpressionResult( - ['%column', $column], + '%column', + [$column], $joins, $makeDistinct ? ($aggregator ?? new DbalAnyAggregator()) : null, $makeDistinct, @@ -373,11 +375,12 @@ private function processRelationship( $targetMapper->getManyHasManyParameters($sourceProperty, $currentMapper); } + /** @phpstan-var literal-string $joinAlias */ $joinAlias = self::getAlias($joinTable, array_slice($tokens, 0, $tokenIndex)); $joins[] = new DbalJoinEntry( - "[$joinTable]", + "[$joinTable]", // @phpstan-ignore-line TODO: fix after JOINs refactoring $joinAlias, - "[$currentAlias.$fromColumn] = %table.[$inColumn]", + "[$currentAlias.$fromColumn] = %table.[$inColumn]", // @phpstan-ignore-line TODO: fix after JOINs refactoring [], $currentConventions ); @@ -390,11 +393,12 @@ private function processRelationship( } $targetTable = $targetMapper->getTableName(); + /** @phpstan-var literal-string $targetAlias */ $targetAlias = self::getAlias($tokens[$tokenIndex], array_slice($tokens, 0, $tokenIndex)); $joins[] = new DbalJoinEntry( "[$targetTable]", $targetAlias, - "[$currentAlias.$fromColumn] = %table.[$toColumn]", + "[$currentAlias.$fromColumn] = %table.[$toColumn]", // @phpstan-ignore-line TODO: fix after JOINs refactoring [], $targetConventions ); diff --git a/src/Mapper/Dbal/DbalMapper.php b/src/Mapper/Dbal/DbalMapper.php index 47c908db..7abe6f56 100644 --- a/src/Mapper/Dbal/DbalMapper.php +++ b/src/Mapper/Dbal/DbalMapper.php @@ -38,7 +38,10 @@ abstract class DbalMapper implements IMapper /** @var IConnection */ protected $connection; - /** @var string|null */ + /** + * @var string|null + * @phpstan-var literal-string|null + */ protected $tableName; /** @var Cache */ @@ -103,6 +106,7 @@ public function findAll(): ICollection public function builder(): QueryBuilder { $tableName = $this->getTableName(); + /** @phpstan-var literal-string $alias */ $alias = DbalQueryBuilderHelper::getAlias($tableName); $builder = $this->connection->createQueryBuilder(); $builder->from("[$tableName]", $alias); @@ -117,13 +121,18 @@ public function getDatabasePlatform(): IPlatform } + /** + * @phpstan-return literal-string + */ public function getTableName(): string { if ($this->tableName === null) { $className = preg_replace('~^.+\\\\~', '', get_class($this)); assert($className !== null); $tableName = str_replace('Mapper', '', $className); - $this->tableName = StringHelper::underscore($tableName); + /** @var literal-string $tableName */ + $tableName = StringHelper::underscore($tableName); + $this->tableName = $tableName; } return $this->tableName; diff --git a/src/Mapper/Dbal/RelationshipMapperManyHasMany.php b/src/Mapper/Dbal/RelationshipMapperManyHasMany.php index 7db7cc63..9b985fd6 100644 --- a/src/Mapper/Dbal/RelationshipMapperManyHasMany.php +++ b/src/Mapper/Dbal/RelationshipMapperManyHasMany.php @@ -141,6 +141,7 @@ protected function execute(DbalCollection $collection, IEntity $parent): MultiEn private function fetchByTwoPassStrategy(QueryBuilder $builder, array $values): MultiEntityIterator { $sourceTable = $builder->getFromAlias(); + /** @phpstan-var literal-string $targetTable */ $targetTable = DbalQueryBuilderHelper::getAlias($this->joinTable); $builder = clone $builder; @@ -234,6 +235,7 @@ protected function executeCounts(DbalCollection $collection, IEntity $parent): a private function fetchCounts(QueryBuilder $builder, array $values): array { $sourceTable = $builder->getFromAlias(); + /** @phpstan-var literal-string $targetTable */ $targetTable = DbalQueryBuilderHelper::getAlias($this->joinTable); $builder = clone $builder; diff --git a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt index 67b99efa..96ad7a55 100644 --- a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt +++ b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt @@ -30,7 +30,7 @@ class DbalValueOperatorFunctionTest extends TestCase */ public function testOperators(BaseCompareFunction $function, array $expected, array $expr): void { - $expressionResult = new DbalExpressionResult(['%column', 'books.id']); + $expressionResult = new DbalExpressionResult('%column', ['books.id']); $helper = Mockery::mock(DbalQueryBuilderHelper::class); $helper->shouldReceive('processPropertyExpr')->once()->andReturn($expressionResult); @@ -40,7 +40,7 @@ class DbalValueOperatorFunctionTest extends TestCase Assert::same( $expected, - $function->processQueryBuilderExpression($helper, $builder, $expr)->args + $function->processQueryBuilderExpression($helper, $builder, $expr)->getExpansionArguments() ); } @@ -51,11 +51,11 @@ class DbalValueOperatorFunctionTest extends TestCase protected function operatorTestProvider(): array { return [ - [new CompareEqualsFunction(), ['%ex = %any', ['%column', 'books.id'], 1], ['id', 1]], - [new CompareNotEqualsFunction(), ['%ex != %any', ['%column', 'books.id'], 1], ['id', 1]], - [new CompareEqualsFunction(), ['%ex IN %any', ['%column', 'books.id'], [1, 2]], ['id', [1, 2]]], - [new CompareNotEqualsFunction(), ['%ex NOT IN %any', ['%column', 'books.id'], [1, 2]], ['id', [1, 2]]], - [new CompareNotEqualsFunction(), ['%ex IS NOT NULL', ['%column', 'books.id']], ['id', null]], + [new CompareEqualsFunction(), ['%column = %any', 'books.id', 1], ['id', 1]], + [new CompareNotEqualsFunction(), ['%column != %any', 'books.id', 1], ['id', 1]], + [new CompareEqualsFunction(), ['%column IN %any', 'books.id', [1, 2]], ['id', [1, 2]]], + [new CompareNotEqualsFunction(), ['%column NOT IN %any', 'books.id', [1, 2]], ['id', [1, 2]]], + [new CompareNotEqualsFunction(), ['%column IS NOT NULL', 'books.id'], ['id', null]], ]; } }