Skip to content

Bump PHPStan analysis to level 6 #2272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 30, 2018
Merged

Conversation

antograssiot
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

ref #2267 (comment)

Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move some casts on the variable assignation instead of where we're using it?

@@ -191,7 +191,7 @@ private function buildQuery(array $identifiers, array $context, QueryNameGenerat
foreach ($normalizedIdentifiers as $key => $value) {
$placeholder = $queryNameGenerator->generateParameterName($key);
$qb->andWhere("$alias.$key = :$placeholder");
$topQueryBuilder->setParameter($placeholder, $value, $classMetadata->getTypeOfField($key));
$topQueryBuilder->setParameter($placeholder, $value, (string) $classMetadata->getTypeOfField($key));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$topQueryBuilder->setParameter($placeholder, $value, (string) $classMetadata->getTypeOfField($key));
$topQueryBuilder->setParameter($placeholder, $value, (string) $classMetadata->getTypeOfField($key));

);
} else {
$output->writeln($content);
$output->writeln((string) $content);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have to cast here? Maybe that we should cast above in the variable declaration?

Copy link
Contributor Author

@antograssiot antograssiot Oct 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was because of the json_encode that could return false but I've corrected in a "cleaner" way I think

@@ -94,11 +94,11 @@ private function isRequiredFilterValid(string $name, Request $request): bool
if (\is_array($matches[$rootName])) {
$keyName = array_keys($matches[$rootName])[0];

$queryParameter = $request->query->get($rootName);
$queryParameter = $request->query->get((string) $rootName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same can't we cast above?

@antograssiot antograssiot changed the title Bump PHPStan analysis to level 6 WIP: Bump PHPStan analysis to level 6 Oct 24, 2018
@antograssiot antograssiot changed the title WIP: Bump PHPStan analysis to level 6 Bump PHPStan analysis to level 6 Oct 24, 2018
@antograssiot antograssiot force-pushed the phpstan-level6 branch 2 times, most recently from 3bc38f1 to a303384 Compare October 29, 2018 19:54
@antograssiot
Copy link
Contributor Author

I've solved the merge conflicts and rebased, anything left to be done here ?
ping @teohhanhui

@dunglas dunglas merged commit 66d6a98 into api-platform:2.3 Oct 30, 2018
@dunglas
Copy link
Member

dunglas commented Oct 30, 2018

Thanks @antograssiot!

@antograssiot antograssiot deleted the phpstan-level6 branch October 30, 2018 16:18
cr3a7ure added a commit to cr3a7ure/core that referenced this pull request Nov 5, 2018
* ApiPlatformExtension cleanup

* Remove a now useless composer hack

* remove unset attributes key in normalization context

* Add tests on doctrine collection purge

* Fix 2285 - force Yaml::dump to dump empty array as actual empty array

* improve graphiql CSS

* Prefix root resource route_prefix to sub-resources

Unit test also added

* Bump PHPStan analysis to level 6 (api-platform#2272)

* Bump PHPStan analysis to level 6

* Bump PHPStan analysis to level 6

* Bump PHPStan analysis to level 6

* Deprecate dead code in QueryJoinParser and remove internal usage

* Add a non regression test for api-platform#2285 api-platform#2286

* Ability to modify response headers (mainly cache related headers) (api-platform#2288)

* Allow user-defined cache headers

A user may wish to define response cache headers in a custom controller or per resource. E.g. a /entity/random endpoint should have a max-age of 0

* Override cache max-age and shared-max-age

Add annotation attributes for cache_headers which allows max_age and shared_max_age to be assigned per resource.

* Remove unused property

The property was added when I thought it may be good to have the option outside of 'attributes' - decided to put the option in attributes instead and failed to remove this property.

* Code style fixes

* Test code style fix

* Review changes applied

- Order of constructor arguments + default null value
- Remove 'throws' annotation
- setting $resourceCacheHeaders with default fallback to empty array
- Improved conditional statements

* Add cacheHeaders Attribute annotation

* Accidental typo

* Alphabetical order

Updated attribute annotation to alphabetical order - moved the property into alphabetical order as well

* Add data providers and data persisters matches to the debug panel (api-platform#2262)

* Add data providers and data persisters matches to the debug panel

* Add tests on traceable providers and persisters

* Use internal public properties

* Merge config file and add tests on Extension

* Avoid extra loop in data persister/providers

* remove duplicated code

* Fix after rebasing

* Allow an input and an output for a given resource class

* [Bugfix] Remove type Error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants