Skip to content
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

TASK: Phpstan level 2 for Flow 8.3 and ignore to be fixed things #3264

Merged
merged 4 commits into from
Jan 20, 2024

Conversation

mhsdesign
Copy link
Member

Flow 9 Pr #3217

Upgrade instructions

Review instructions

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

This method is used in the `PropertyConditionGenerator`. There we expect it to return a string.
Additionally, the tests prove that indeed a string is returned and not an array.

The `$subselectSql` at line 411 will hold this string value when running the `ContentSecurityTest` test:

> SELECT n0_.persistence_object_identifier AS persistence_object_identifier_0 FROM neos_flow_tests_functional_security_fixtures_testentityb n0_ WHERE n0_.stringvalue = ?
@mhsdesign mhsdesign force-pushed the task/phpStanLevel2-backport83 branch from c51d0d4 to b1ce5f0 Compare January 17, 2024 12:22
@mhsdesign mhsdesign force-pushed the task/phpStanLevel2-backport83 branch 2 times, most recently from c1646dd to 0145b9a Compare January 17, 2024 15:06
@mhsdesign mhsdesign force-pushed the task/phpStanLevel2-backport83 branch from 0145b9a to 883ad0b Compare January 17, 2024 15:07
Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

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

Looks good, but I left two questions.

Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

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

Hi @mhsdesign,

I did a full review and added a bunch of comments. Nothing deal-breaking I suppose, mostly karma stuff. The only truly irritating spot is the one that @dlubitz pointed out already.

Also, I can confirm that phpstan analysis on level 2 works locally. Thanks a lot for taking the time to create those fixes 💯

You may go ahead and address/resolve/dismiss my comments as you see fit and I'll give you a 👍 in return :)

Neos.Eel/Classes/CompilingEvaluator.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/CompilingEvaluator.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/Helper/MathHelper.php Show resolved Hide resolved
Neos.Eel/Classes/InterpretedEvaluator.php Outdated Show resolved Hide resolved
Neos.Eel/Classes/InterpretedEvaluator.php Outdated Show resolved Hide resolved
Neos.Flow/Classes/Aop/Advice/AdviceChain.php Outdated Show resolved Hide resolved
Neos.Flow/Classes/Cli/Response.php Outdated Show resolved Hide resolved
Neos.Flow/Classes/Cli/Response.php Outdated Show resolved Hide resolved
Neos.Flow/Classes/Command/ResourceCommandController.php Outdated Show resolved Hide resolved
Neos.Flow/Classes/Log/ThrowableStorage/FileStorage.php Outdated Show resolved Hide resolved
@mhsdesign mhsdesign force-pushed the task/phpStanLevel2-backport83 branch from 9b62592 to 9079261 Compare January 20, 2024 12:53
@mhsdesign mhsdesign requested review from grebaldi and dlubitz January 20, 2024 13:01
@mhsdesign
Copy link
Member Author

Thank you for your reveiws @dlubitz and @grebaldi. I adjusted everything ;)

@dlubitz
Copy link
Contributor

dlubitz commented Jan 20, 2024

Thanks for taking care!

@mhsdesign mhsdesign changed the title TASK: Backport phpstan level 2 to Flow 8.3 and ignore to be fixed things TASK: Phpstan level 2 for Flow 8.3 and ignore to be fixed things Jan 20, 2024
Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @mhsdesign! 👍

btw: Nice idea aliasing that InternalAopProxyInterface - much clearer now :)

@mhsdesign
Copy link
Member Author

Thank you for your quick re-reviews ;)

@mhsdesign mhsdesign merged commit 4acb253 into neos:8.3 Jan 20, 2024
10 checks passed
@mhsdesign mhsdesign deleted the task/phpStanLevel2-backport83 branch January 20, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants