-
Notifications
You must be signed in to change notification settings - Fork 14
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
IBX-8138: [Rector] Applied rules from Symfony 5 Rector set lists #385
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alongosz
force-pushed
the
ibx-8400-fix-redundant-repository-factory
branch
from
June 18, 2024 12:52
e3e913f
to
139b872
Compare
Base automatically changed from
ibx-8400-fix-redundant-repository-factory
to
main
June 18, 2024 14:27
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
from
June 18, 2024 14:34
876de38
to
f6c613d
Compare
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
2 times, most recently
from
June 21, 2024 12:36
fb6aea3
to
2cd6d9b
Compare
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
5 times, most recently
from
July 4, 2024 15:56
62ca525
to
9c0b13c
Compare
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
2 times, most recently
from
July 11, 2024 12:14
e347340
to
16e0239
Compare
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
2 times, most recently
from
July 15, 2024 10:31
a7f52b2
to
bb3fa84
Compare
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
from
July 15, 2024 13:18
bb3fa84
to
b42a789
Compare
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
from
July 15, 2024 13:24
b42a789
to
bcdc124
Compare
adamwojs
approved these changes
Jul 17, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done! 🚀
src/lib/Persistence/Legacy/Content/FieldValue/Converter/AuthorConverter.php
Outdated
Show resolved
Hide resolved
konradoboza
reviewed
Jul 17, 2024
src/bundle/RepositoryInstaller/Command/ValidatePasswordHashesCommand.php
Show resolved
Hide resolved
src/lib/MVC/Symfony/Component/Serializer/AbstractPropertyWhitelistNormalizer.php
Outdated
Show resolved
Hide resolved
src/lib/MVC/Symfony/Component/Serializer/CompoundMatcherNormalizer.php
Outdated
Show resolved
Hide resolved
src/lib/MVC/Symfony/Component/Serializer/SimplifiedRequestNormalizer.php
Outdated
Show resolved
Hide resolved
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
from
August 26, 2024 11:27
aae16c6
to
e2a12d6
Compare
This was referenced Aug 26, 2024
Applied rules: * LiteralGetToRequestClassConstantRector * ResponseStatusCodeRector * MakeCommandLazyRector
Applied rules: * ReturnTypeFromStrictNativeCallRector * ReturnTypeFromStrictScalarReturnExprRector
Co-authored-by: Konrad Oboza <[email protected]>
alongosz
force-pushed
the
ibx-8138-symfony-5-rule-sets
branch
from
August 28, 2024 13:47
e2a12d6
to
0584976
Compare
Quality Gate failedFailed conditions |
barw4
pushed a commit
that referenced
this pull request
Oct 17, 2024
For more details see https://issues.ibexa.co/browse/IBX-8138 and #385 Key changes: * [Rector] Applied Symfony 5.1 CommandConstantReturnCodeRector * [Rector] Applied Symfony 5.2 RenameMethodRector * [Rector] Applied Symfony 5.3 Rector sets * [Rector] Applied Symfony code quality Rector sets Applied rules: * LiteralGetToRequestClassConstantRector * ResponseStatusCodeRector * MakeCommandLazyRector * [Rector] Applied Return type rectors Applied rules: * ReturnTypeFromStrictNativeCallRector * ReturnTypeFromStrictScalarReturnExprRector * [Rector] Applied Symfony 6.0 AddReturnTypeDeclarationRector * [Rector] Added Symfony Bundle::getContainerExtension return type * Added strict types for InstallPlatformCommand consts * Made XML serialization exception more verbose in Author and DateTime converters * Implemented `\Ibexa\Core\MVC\Symfony\Security\User::getUserIdentifier` --------- Co-Authored-By: Paweł Niedzielski <[email protected]> Co-Authored-By: Konrad Oboza <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Caution
Related PRs:
Follow-ups to ibexa/core:
IBX-8138 rector changes for the other packages:
Description:
Applied rules from the Symfony 5 Rector set lists.
This is just one milestone on the way towards Symfony 6. Some Rectors address code quality. Not all deprecations are covered by the Symfony-provided Rectors. Additionally, not all deprecations are related to PHP code, which is out of scope for Rector. Moreover, some deprecations can be resolved via Symfony 6 Rectors, but only some can be applied without upgrading to Symfony 6.
This PR also includes necessary fixes to align changes with the PHPStan baseline and to address return type bugs.
The applied Rectors are as follows:
SF 5.0
AddParamTypeDeclarationRector
adding parameter types for chosen set of Symfony methods, if used by 1st party,SF 5.1
CommandConstantReturnCodeRector
- e.g.:return self::SUCCESS
instead of a literalint
,SF 5.2
RenameMethodRector
renaming calls to\Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken::getProviderKey
togetFirewallName
(however there's no rector for e.g. deprecatedgetCredentials
method of the same class),SF 5.3
RenameMethodRector
:RequestStack::getMasterRequest()
->RequestStack::getMainRequest()
,SF 5.3
RenameClassConstFetchRector
:HttpKernelInterface::MASTER_REQUEST
->HttpKernelInterface::MAIN_REQUEST
,SF 5.3
RenameClassRector
:Symfony\Component\Security\Core\Exception\UsernameNotFoundException
->Symfony\Component\Security\Core\Exception\UserNotFoundException
,SF 5.4
AddReturnTypeDeclarationRector
- adding return type?\Symfony\Component\DependencyInjection\Extension\ExtensionInterface
for classes implementing\Symfony\Component\HttpKernel\Bundle\BundleInterface::getContainerExtension()
methodSF 6.0
AddReturnTypeDeclarationRector
- adding return types added by Symfony in 6. It was mostly possible to do this on SF 5 for our extension points as those type didn't change, they were just hinted in PHPDoc. Still, required some manual fixes afterwards (see also a few points below - theReturnTypeFrom*Rector
remarks,general code quality rector sets:
MakeCommandLazyRector
- replacingsetName()
andsetDescription()
with$defaultName
and$defaultDescription
properties in Commands. Note that this rule however is deprecated, to be replaced with attributes in SF 6.1. For now this is just a "dead middle step" as Symfony itself states in the deprecation message,LiteralGetToRequestClassConstantRector
replacing'GET'
withRequest::GET
constant,ResponseStatusCodeRector
replacing e.g. 302 withResponse::HTTP_FOUND
constant in relavant places,ReturnTypeFrom*Rector
andAddReturnTypeDeclarationBasedOnParentClassMethodRector
- a set of rectors applying return types based on either inferred method body or parent declaration, if feasible - this one has been proved to be quite risky as the returned type didn't always strictly follow what it should return.CI Status
I've spent quite some time trying to rectify code duplication issues. Sadly this is not doable fully in a reasonable time as the entire codebase contains copy-pasted code since 2012. Since the Rector touched many places, all of them pop up now as new code, which makes quality gate fail. I have a set of changes fixing some of those WET code parts, however the scope of this PR is too big. I'll make follow ups.
For QA:
No QA needed. Regression tests should be enough.
Regression run: ibexa/commerce#930
Documentation:
This is code refactoring without behavior change. No documentation changes needed.