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

[Tentative] Drop PHP < 8 #1054

Merged
merged 20 commits into from Apr 29, 2024
Merged

[Tentative] Drop PHP < 8 #1054

merged 20 commits into from Apr 29, 2024

Conversation

kylekatarnls
Copy link
Member

@kylekatarnls kylekatarnls commented Dec 11, 2023

Type: refactoring
Breaking change: yes

This an exploration PR to see what it would take to migrate the stack for the project and verify consequences.

@kylekatarnls kylekatarnls added this to the 3.0.0 milestone Dec 11, 2023
@AJenbo
Copy link
Member

AJenbo commented Dec 11, 2023

I have a few followups for this one :D

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Attention: Patch coverage is 95.34050% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 92.66%. Comparing base (8dae244) to head (412a07d).
Report is 27 commits behind head on 3.x.

❗ Current head 412a07d differs from pull request most recent head a076eae. Consider uploading reports for the commit a076eae to get more accurate results

Files Patch % Lines
src/main/php/PHPMD/AbstractRule.php 78.57% 6 Missing ⚠️
...hp/PHPMD/Rule/Controversial/CamelCaseClassName.php 0.00% 2 Missing ⚠️
...main/php/PHPMD/Rule/Controversial/Superglobals.php 0.00% 2 Missing ⚠️
...rc/main/php/PHPMD/Cache/Model/ResultCacheState.php 88.88% 1 Missing ⚠️
src/main/php/PHPMD/Renderer/HTMLRenderer.php 87.50% 1 Missing ⚠️
src/main/php/PHPMD/Rule/UnusedLocalVariable.php 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                3.x    #1054      +/-   ##
============================================
+ Coverage     92.54%   92.66%   +0.11%     
+ Complexity     1239     1238       -1     
============================================
  Files           111      111              
  Lines          3405     3447      +42     
============================================
+ Hits           3151     3194      +43     
+ Misses          254      253       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kylekatarnls kylekatarnls changed the title Drop PHP < 8 [Tentative] Drop PHP < 8 Dec 11, 2023
@tvbeek tvbeek mentioned this pull request Dec 15, 2023
10 tasks
@AJenbo
Copy link
Member

AJenbo commented Dec 24, 2023

Once we settle on a version and merge we can use php-cs-fixer to make migrations of old style php docs to native types and other code modinizatios automatically in follow up, that way we can heep the version deprication PR as simple to review as possible

@kylekatarnls
Copy link
Member Author

Yes, but sadly many of them are incomplete, for instance, many of our PHPDoc types miss a nullable flag: getReport() which had @return Report but actually @return Report|null so it will still need manual attention.

@kylekatarnls
Copy link
Member Author

kylekatarnls commented Dec 24, 2023

And why the test fail is actually for the same reason. PHPDoc said @param int for getIntProperty but it's actually int|null and it rely on the null to throw or not the OutOfBoundException.

@kylekatarnls kylekatarnls force-pushed the feature/3.x-experiment branch 2 times, most recently from 486914f to 4a065a8 Compare December 24, 2023 15:57
@AJenbo
Copy link
Member

AJenbo commented Dec 24, 2023

Well I feel that this is even more of a reason to split it in multiple PRs

@kylekatarnls
Copy link
Member Author

Oh, I have already few other branches soon to be pushed. 😄

kylekatarnls and others added 5 commits January 10, 2024 22:04
# Conflicts:
#	.travis.yml
#	build.properties
#	composer.json
#	src/main/php/PHPMD/ParserFactory.php
#	src/test/php/PHPMD/AbstractTest.php
#	src/test/php/PHPMD/Node/ASTNodeTest.php
#	src/test/php/PHPMD/Node/ClassNodeTest.php
#	src/test/php/PHPMD/Node/FunctionTest.php
#	src/test/php/PHPMD/Node/MethodNodeTest.php
#	src/test/php/PHPMD/ParserFactoryTest.php
#	src/test/php/PHPMD/ParserTest.php
#	src/test/php/PHPMD/Regression/AcceptsFilesAndDirectoriesAsInputTicket001Test.php
#	src/test/php/PHPMD/Regression/MaximumNestingLevelTicket24975295Test.php
#	src/test/php/PHPMD/Renderer/HTMLRendererTest.php
#	src/test/php/PHPMD/RuleTest.php
#	src/test/php/PHPMD/TextUI/CommandLineOptionsTest.php
Reset 3.x to current master state
@kylekatarnls kylekatarnls changed the base branch from master to 3.x January 17, 2024 11:34
@frankdekker
Copy link
Contributor

Cool! will make development much more pleasant :)

.github/workflows/tests.yml Outdated Show resolved Hide resolved
@AJenbo
Copy link
Member

AJenbo commented Apr 27, 2024

Yes, but sadly many of them are incomplete, for instance, many of our PHPDoc types miss a nullable flag: getReport() which had @return Report but actually @return Report|null so it will still need manual attention.

I prefer to first get a project to pass PHPStan level 9 before doing the conversion for this reason.

Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

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

Ok, finished the review of this juggernaut. Looking forward to when we can merge this :)

kylekatarnls and others added 3 commits April 28, 2024 09:29
Co-authored-by: Anders Jenbo <anders@jenbo.dk>
Co-authored-by: Anders Jenbo <anders@jenbo.dk>
Co-authored-by: Anders Jenbo <anders@jenbo.dk>
@AJenbo AJenbo marked this pull request as ready for review April 29, 2024 14:58
AJenbo
AJenbo previously approved these changes Apr 29, 2024
Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me, the few remaining comments I had can easily be addressed/checked in a follow up so I'll do that as the next thing on my list.

@AJenbo AJenbo merged commit f184a7c into 3.x Apr 29, 2024
34 of 36 checks passed
@AJenbo AJenbo deleted the feature/3.x-experiment branch April 29, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants