Skip to content

Commit

Permalink
minor #282 Phpstan fixes (sstok)
Browse files Browse the repository at this point in the history
This PR was merged into the 2.0-dev branch.

Discussion
----------

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Fixed tickets | 
| License       | MIT


Commits
-------

c68a600 Fix PHPStan reported problems
25318e7 Use a fixed version of phpqa image
  • Loading branch information
sstok committed Apr 13, 2020
2 parents cc8d895 + 25318e7 commit 4202b95
Show file tree
Hide file tree
Showing 14 changed files with 62 additions and 40 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,4 @@ jobs:
-
name: Run PHP-QA
run: |
make lint || exit 0
make lint
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
/.php_cs.cache
build/**
*.phar
var/
27 changes: 15 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
QA_DOCKER_IMAGE=jakzal/phpqa:alpine
QA_DOCKER_COMMAND=docker run -t --rm -v "$(shell pwd):/project" -w /project ${QA_DOCKER_IMAGE}
QA_DOCKER_IMAGE=jakzal/phpqa:1.34.1-php7.4-alpine
QA_DOCKER_COMMAND=docker run --init -t --rm --user "$(shell id -u):$(shell id -g)" --volume /tmp/tmp-phpqa-$(shell id -u):/tmp --volume "$(shell pwd):/project" --workdir /project ${QA_DOCKER_IMAGE}

dist: install cs-full phpstan test
ci: cs-full-check phpstan test
Expand All @@ -18,22 +18,22 @@ test: docker-up
docker-compose run --rm php make in-docker-test
@$(MAKE) docker-down

test-coverage: docker-up
test-coverage: ensure docker-up
mkdir -p build/logs build/cov
docker-compose run --rm php make in-docker-test-coverage
sh -c "${QA_DOCKER_COMMAND} phpdbg -qrr /usr/local/bin/phpcov merge --clover build/logs/clover.xml build/cov"
@$(MAKE) docker-down

phpstan:
sh -c "${QA_DOCKER_COMMAND} phpstan analyse --configuration phpstan.neon --level 5 ."
phpstan: ensure
sh -c "${QA_DOCKER_COMMAND} phpstan analyse"

cs:
cs: ensure
sh -c "${QA_DOCKER_COMMAND} php-cs-fixer fix -vvv --diff"

cs-full:
cs-full: ensure
sh -c "${QA_DOCKER_COMMAND} php-cs-fixer fix -vvv --using-cache=false --diff"

cs-full-check:
cs-full-check: ensure
sh -c "${QA_DOCKER_COMMAND} php-cs-fixer fix -vvv --using-cache=false --diff --dry-run"

##
Expand All @@ -48,6 +48,11 @@ docker-up:
docker-down:
docker-compose down

docs:
docker build docs/ -t rollerworks-search-docs
@echo 'The documentation is availbe at http://localhost:8000 (to end the process use Ctrl+C)'
docker run --rm -p 8000:80 rollerworks-search-docs:latest

##
# Private targets
##
Expand Down Expand Up @@ -78,10 +83,8 @@ in-docker-test-coverage:
SYMFONY_DEPRECATIONS_HELPER=weak phpdbg -qrr vendor/bin/phpunit --verbose --configuration phpunit/pgsql.xml --coverage-php build/cov/coverage-phpunit-pgsql.cov
SYMFONY_DEPRECATIONS_HELPER=weak phpdbg -qrr vendor/bin/phpunit --verbose --configuration phpunit/mysql.xml --coverage-php build/cov/coverage-phpunit-mysql.cov

docs:
docker build docs/ -t rollerworks-search-docs
@echo 'The documentation is availbe at http://localhost:8000 (to end the process use Ctrl+C)'
docker run --rm -p 8000:80 rollerworks-search-docs:latest
ensure:
mkdir -p ${HOME}/.composer /tmp/tmp-phpqa-$(shell id -u)

.PHONY: install install-dev install-lowest phpstan cs cs-full cs-full-checks docker-up down-down
.PHONY: in-docker-install in-docker-install-dev in-docker-install-lowest in-docker-test in-docker-test-coverage docs
10 changes: 8 additions & 2 deletions lib/ApiPlatform/Doctrine/Orm/Extension/SearchExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,14 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
{
$request = $this->requestStack->getCurrentRequest();

/** @var SearchCondition $condition */
if (!$request || null === $condition = $request->attributes->get('_api_search_condition')) {
if (!$request) {
return;
}

/** @var SearchCondition|null $condition */
$condition = $request->attributes->get('_api_search_condition');

if (null === $condition) {
return;
}

Expand Down
6 changes: 4 additions & 2 deletions lib/ApiPlatform/Elasticsearch/Extension/SearchExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
{
$request = $this->requestStack->getCurrentRequest();

/** @var SearchCondition $condition */
if (!$request || null === $condition = $request->attributes->get('_api_search_condition')) {
/** @var SearchCondition|null $condition */
$condition = $request->attributes->get('_api_search_condition');

if (null === $condition) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private function getNumberFormatter(): \NumberFormatter

if (!$formatter || $formatter->getLocale() !== \Locale::getDefault()) {
$formatter = new \NumberFormatter(\Locale::getDefault(), \NumberFormatter::TYPE_INT32);
$formatter->setAttribute(\NumberFormatter::GROUPING_USED, false);
$formatter->setAttribute(\NumberFormatter::GROUPING_USED, 0);
}

return $formatter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ public function it_transforms_age_to_integer()

self::assertEquals(18, $transformer->reverseTransform('18'));
self::assertEquals('18', $transformer->transform(18));

self::assertEquals(18000, $transformer->reverseTransform('18000'));
self::assertEquals('18000', $transformer->transform(18000));
}

/** @test */
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/Dbal/Query/QueryGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ private function getConversionHints(QueryField $mappingConfig, string $column =
private static function implodeWithValue(string $glue, array $values, array $wrap = []): string
{
// Remove the empty values
$values = array_filter($values, 'strlen');
$values = array_filter($values, function (string $val): bool {
return '' !== $val;
});

if (0 === \count($values)) {
return '';
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/Dbal/SqlConditionGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function __construct(Connection $connection, SearchCondition $searchCondi
$this->connection = $connection;
}

public function setField(string $fieldName, string $column, string $alias = null, string $type = 'string')
public function setField(string $fieldName, string $column, string $alias = null, string $type = 'string'): self
{
if ($this->whereClause) {
throw new BadMethodCallException(
Expand All @@ -98,6 +98,8 @@ public function setField(string $fieldName, string $column, string $alias = null
$column,
$alias
);

return $this;
}

public function getWhereClause(string $prependQuery = ''): string
Expand Down
16 changes: 5 additions & 11 deletions lib/Doctrine/Orm/DoctrineOrmFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function __construct(Cache $cacheDriver = null)
*
* @return NativeQueryConditionGenerator|DqlConditionGenerator
*/
public function createConditionGenerator($query, SearchCondition $searchCondition)
public function createConditionGenerator($query, SearchCondition $searchCondition): ConditionGenerator
{
if ($query instanceof NativeQuery) {
return new NativeQueryConditionGenerator($query, $searchCondition);
Expand All @@ -58,10 +58,6 @@ public function createConditionGenerator($query, SearchCondition $searchConditio
if ($query instanceof Query || $query instanceof QueryBuilder) {
return new DqlConditionGenerator($query, $searchCondition);
}

throw new \InvalidArgumentException(
sprintf('Query "%s" is not supported by the DoctrineOrmFactory.', \get_class($query))
);
}

/**
Expand All @@ -72,20 +68,18 @@ public function createConditionGenerator($query, SearchCondition $searchConditio
* the driver supports TTL then the library may set a default value
* for it or let the driver take care of that.
*/
public function createCachedConditionGenerator($conditionGenerator, $ttl = null): ConditionGenerator
public function createCachedConditionGenerator(ConditionGenerator $conditionGenerator, $ttl = null): ConditionGenerator
{
if (null === $this->cacheDriver) {
return $conditionGenerator;
}

if ($conditionGenerator instanceof DqlConditionGenerator) {
return new CachedDqlConditionGenerator($conditionGenerator, $this->cacheDriver, $ttl);
} elseif ($conditionGenerator instanceof NativeQueryConditionGenerator) {
return new CachedNativeQueryConditionGenerator($conditionGenerator, $this->cacheDriver, $ttl);
}

throw new \InvalidArgumentException(
sprintf('ConditionGenerator "%s" is not supported by the DoctrineOrmFactory.', \get_class($conditionGenerator))
);
if ($conditionGenerator instanceof NativeQueryConditionGenerator) {
return new CachedNativeQueryConditionGenerator($conditionGenerator, $this->cacheDriver, $ttl);
}
}
}
1 change: 1 addition & 0 deletions lib/Doctrine/Orm/FieldConfigBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ private function getEntityAndProperty($fieldName, string $entity, string $proper
private function getMappingType(string $fieldName, string $entity, string $propertyName, ?string $type = null): MappingType
{
if (!$type) {
/** @var object|string|null $type */
$type = $this->entityManager->getClassMetadata($entity)->getTypeOfField($propertyName);
}

Expand Down
1 change: 0 additions & 1 deletion lib/Elasticsearch/Extension/Conversion/DateConversion.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ public function convertQuery(string $propertyName, $value, QueryPreparationHints
case QueryPreparationHints::CONTEXT_SIMPLE_VALUES:
case QueryPreparationHints::CONTEXT_EXCLUDED_SIMPLE_VALUES:
// dates as single values, need to convert them to a date range
/** @var array $value */
foreach ($value as $singleValue) {
$dateRange = $this->generateDateRange($propertyName, new Range($singleValue, $singleValue));
$query[Generator::QUERY_BOOL][Generator::CONDITION_OR][] = $dateRange;
Expand Down
2 changes: 2 additions & 0 deletions lib/Elasticsearch/QueryConditionGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ public function registerField(string $fieldName, string $property, array $condit
}

$this->mappings[$fieldName] = new FieldMapping($fieldName, $this->injectParameters($property), $this->fieldSet->get($fieldName), $conditionMappings, $options);

return $this;
}

public function getQuery(): Query
Expand Down
23 changes: 15 additions & 8 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
includes:
- /tools/.composer/vendor-bin/phpstan/vendor/phpstan/phpstan-symfony/extension.neon
- /tools/.composer/vendor-bin/phpstan/vendor/phpstan/phpstan-doctrine/extension.neon
- /tools/.composer/vendor-bin/phpstan/vendor/phpstan/phpstan-phpunit/extension.neon

parameters:
#reportUnmatchedIgnoredErrors: false
#checkNullables: false

tmpDir: %currentWorkingDirectory%/var/phpstan
level: 5

autoload_files:
- vendor/autoload.php

paths:
- ./lib
excludes_analyse:
- */lib/*/Tests/Fixtures/*
- vendor/
- %currentWorkingDirectory%/lib/*/Tests/**
- %currentWorkingDirectory%/lib/*/*/Tests/**
ignoreErrors:
- '#__construct\(\) does not call parent constructor from .+#'

ignoreErrors:
# ValueHolder guard there own correctness. A ValuesBag never returns a wrong object (collection).
- '#expects Rollerworks\\Component\\Search\\Value\\[a-zA-Z]+, Rollerworks\\Component\\Search\\Value\\ValueHolder given#'
- '#Call to an undefined method Rollerworks\\Component\\Search\\Value\\ValueHolder\:\:#'
Expand All @@ -27,8 +39,3 @@ parameters:
- '#Call to an undefined method Doctrine\\DBAL\\Driver\\PDOConnection\:\:sqliteCreateFunction\(\)#'
- '#Parameter \#2 \$type of method Doctrine\\DBAL\\Connection\:\:quote\(\) expects ([^\s]+)#'
- '#Call to an undefined method Rollerworks\\Component\\Search\\Doctrine\\Orm\\ConditionGenerator\:\:get[a-zA-Z]+#'
- '#Parameter \#\d \$(firstResult|maxResults) of method Doctrine\\ORM\\QueryBuilder\:\:(setFirstResult|setMaxResults)\(\)#'
- '#Call to function method_exists\(\)#'

## Needs investigation
#- '#ChoiceView constructor expects string, false|string given#' # I don't know if there is good reason to allow bool here - @sstok

0 comments on commit 4202b95

Please sign in to comment.