Allow Symfony 7 #699

kylekatarnls · 2023-12-06T12:59:24Z

Type: bugfix
Issue: Fix #695
Breaking change: no

Add lazy configuration for Symfony (JIT loading of the implementation compatible with installed version of symfony/config)
Move test dependencies in the test folder (so it can have different Symfony packages version without conflict)
Test each Symfony version in GitHub Actions
Make composer test automatically installing test dependencies

codecov · 2023-12-06T13:00:09Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (8dfc0c4) 84.81% compared to head (a14bfb7) 84.82%.

Files	Patch %	Lines
...epend/DependencyInjection/Configuration.strong.php	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #699   +/-   ##
=========================================
  Coverage     84.81%   84.82%           
- Complexity     3460     3462    +2     
=========================================
  Files           155      157    +2     
  Lines          9418     9423    +5     
=========================================
+ Hits           7988     7993    +5     
  Misses         1430     1430

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

AJenbo

I think rather then checking the parent for a return type we can simply do it if it's php 7+ since it's always valid to narrow the return. Other then that I like this approach.

kylekatarnls · 2023-12-06T13:30:44Z

I think rather then checking the parent for a return type we can simply do it if it's php 7+ since it's always valid to narrow the return. Other then that I like this approach.

No, you can perfectly run PHP 8.3 and install Symfony 5 if you like. PHP level is irrelevant there.

AJenbo · 2023-12-06T13:33:37Z

I think rather then checking the parent for a return type we can simply do it if it's php 7+ since it's always valid to narrow the return. Other then that I like this approach.

No, you can perfectly run PHP 8.3 and install Symfony 5 if you like. PHP level is irrelevant there.

But, you can perfectly use Symfony 5 and still add return types. Symfony version is irrelevant here.

kylekatarnls · 2023-12-06T13:57:50Z

But, you can perfectly use Symfony 5 and still add return types. Symfony version is irrelevant here.

"you" will need to be refined in this story.

Because if PDepend add return types on a public method on a non-final class, it's a breaking change, so it might break whoever build on the top of it.

If we base on PHP level, we actually narrow typing when PHP >= 7 and Symfony < 7 ➡️ BC, and 3 mix to support.

If we simply strictly align our definition to current Symfony declaration, we keep on 2 mix to support and don't create any new divergence that can break in dependent projects (including user world but also PHPMD).

AJenbo · 2023-12-06T14:07:22Z

Because if PDepend add return types on a public method on a non-final class, it's a breaking change, so it might break whoever build on the top of it.

That's fair enough, I did not see this as an issue for classes we extend. But yeah if we are that strict about it your solution ins the better one.

kylekatarnls · 2023-12-07T18:41:46Z

For the record, here is the demonstration that will also solve the issue in PHPMD:
phpmd/phpmd#1049

src/test/composer.json

.github/workflows/symfony.yml

composer.json

src/main/php/Lazy/PDepend/DependencyInjection/Configuration.strong.php

src/main/php/Lazy/PDepend/DependencyInjection/Configuration.weak.php

src/test/composer.json

src/test/symfony-version.php

Co-authored-by: Marc Würth <ravage@bluewin.ch>

Add missing coverage annotations

Comments resolved

src/main/php/Lazy/PDepend/DependencyInjection/Configuration.strong.php

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [phpmd/phpmd](https://phpmd.org/) ([source](https://togithub.com/phpmd/phpmd)) | `2.14.1` -> `2.15.0` | [![age](https://developer.mend.io/api/mc/badges/age/packagist/phpmd%2fphpmd/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/phpmd%2fphpmd/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/phpmd%2fphpmd/2.14.1/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/phpmd%2fphpmd/2.14.1/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>phpmd/phpmd (phpmd/phpmd)</summary> ### [`v2.15.0`](https://togithub.com/phpmd/phpmd/blob/HEAD/CHANGELOG#phpmd-2150-20231211) [Compare Source](https://togithub.com/phpmd/phpmd/compare/2.14.1...2.15.0) \======================== - Added [#1036](https://togithub.com/phpmd/phpmd/issues/1036) \[CLI] Allow option and value separated with equal sign - Require pdepend/pdepend 2.16.1 - Support PHP 8.3 [pdepend/pdepend#699](https://togithub.com/pdepend/pdepend/issues/699) - Support Symfony 7 [pdepend/pdepend#692](https://togithub.com/pdepend/pdepend/issues/692) - Fixed [pdepend/pdepend#691](https://togithub.com/pdepend/pdepend/issues/691) Float parsing for number starting with 0. - Fixed [pdepend/pdepend#689](https://togithub.com/pdepend/pdepend/issues/689) Handle conversion to/detection of UTF-8 encoding using either mbstring PHP extension or the polyfill provided by Symfony - Fixed [pdepend/pdepend#687](https://togithub.com/pdepend/pdepend/issues/687) Parsing the correct comment for method doc-block (Allow correct SuppressWarnings annotation handling on PHPMD) ([https://github.com/phpmd/phpmd/issues/914](https://togithub.com/phpmd/phpmd/issues/914)) - Fixed [pdepend/pdepend#694](https://togithub.com/pdepend/pdepend/issues/694) Handle yield termination depending on context ([https://github.com/phpmd/phpmd/issues/804](https://togithub.com/phpmd/phpmd/issues/804)) - Fixed [#1044](https://togithub.com/phpmd/phpmd/issues/1044) strict option on applyOnClassMethods - Documented [#1041](https://togithub.com/phpmd/phpmd/issues/1041) Mention public key used for signing the Phars - Documented [#1042](https://togithub.com/phpmd/phpmd/issues/1042) Document installation with PHIVE </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/line/line-bot-sdk-php).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

kylekatarnls added this to the 2.16.1 milestone Dec 6, 2023

kylekatarnls force-pushed the fix/symfony-compatibility branch 2 times, most recently from 545478f to 5cb54fc Compare December 6, 2023 13:12

kylekatarnls requested review from AJenbo, ravage84 and tvbeek December 6, 2023 13:21

kylekatarnls added the Compatiblity label Dec 6, 2023

AJenbo reviewed Dec 6, 2023

View reviewed changes

kylekatarnls requested a review from AJenbo December 6, 2023 13:31

Add lazy configuration for Symfony

6afd2c0

kylekatarnls force-pushed the fix/symfony-compatibility branch from 5cb54fc to 6afd2c0 Compare December 7, 2023 18:12

AJenbo previously approved these changes Dec 7, 2023

View reviewed changes

ravage84 reviewed Dec 7, 2023

View reviewed changes

src/test/composer.json Show resolved Hide resolved

kylekatarnls mentioned this pull request Dec 7, 2023

Allow Symfony 7 phpmd/phpmd#1049

Merged

ravage84 reviewed Dec 7, 2023

View reviewed changes

.github/workflows/symfony.yml Show resolved Hide resolved

ravage84 reviewed Dec 7, 2023

View reviewed changes

composer.json Show resolved Hide resolved

ravage84 previously requested changes Dec 7, 2023

View reviewed changes

CS

3e7ceaf

ravage84 dismissed AJenbo’s stale review via 3e7ceaf December 7, 2023 19:09

kylekatarnls and others added 4 commits December 7, 2023 21:15

Add description in src/test/composer.json

6b94e4f

Co-authored-by: Marc Würth <ravage@bluewin.ch>

Add description for test workflows

743f2e1

Add missing coverage annotations

Fix composer test script

18ec098

Add comments to make replacements already done detectable

0cd7836

kylekatarnls force-pushed the fix/symfony-compatibility branch from 195a3df to 0cd7836 Compare December 7, 2023 22:09

Disable track_errors for test

9d32808

kylekatarnls requested a review from ravage84 December 7, 2023 22:18

Add unit test for TreeBuilder

a18f373

kylekatarnls force-pushed the fix/symfony-compatibility branch from e7c3b8c to a18f373 Compare December 7, 2023 22:55

AJenbo previously approved these changes Dec 8, 2023

View reviewed changes

kylekatarnls dismissed AJenbo’s stale review via 43f72c4 December 8, 2023 07:52

Use local composer.phar if available for test dependencies

ee08d65

kylekatarnls force-pushed the fix/symfony-compatibility branch from 43f72c4 to ee08d65 Compare December 8, 2023 07:58

AJenbo previously approved these changes Dec 8, 2023

View reviewed changes

kylekatarnls changed the title ~~Add lazy configuration for Symfony~~ Allow Symfony 7 Dec 8, 2023

kylekatarnls dismissed AJenbo’s stale review via 9b5dafd December 8, 2023 23:11

Build phar with Configuration weak types

a14bfb7

kylekatarnls force-pushed the fix/symfony-compatibility branch from 9b5dafd to a14bfb7 Compare December 8, 2023 23:23

AJenbo reviewed Dec 8, 2023

View reviewed changes

src/main/php/Lazy/PDepend/DependencyInjection/Configuration.strong.php Show resolved Hide resolved

AJenbo approved these changes Dec 10, 2023

View reviewed changes

kylekatarnls merged commit 71602df into master Dec 10, 2023
79 checks passed

kylekatarnls deleted the fix/symfony-compatibility branch December 10, 2023 17:01

kylekatarnls mentioned this pull request Dec 10, 2023

Fix: Restrain symfony/config to ^6.0 to fix #695 #697

Closed

This was referenced Dec 10, 2023

Fix Declaration of Configuration::getConfigTreeBuilder() #696

Closed

Broken Interface Type for Symfony\Component\Config\Definition\ConfigurationInterface::getConfigTreeBuilder #698

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Symfony 7 #699

Allow Symfony 7 #699

kylekatarnls commented Dec 6, 2023 •

edited

Loading

codecov bot commented Dec 6, 2023 •

edited

Loading

AJenbo left a comment

kylekatarnls commented Dec 6, 2023

AJenbo commented Dec 6, 2023

kylekatarnls commented Dec 6, 2023 •

edited

Loading

AJenbo commented Dec 6, 2023

kylekatarnls commented Dec 7, 2023

Allow Symfony 7 #699

Allow Symfony 7 #699

Conversation

kylekatarnls commented Dec 6, 2023 • edited Loading

codecov bot commented Dec 6, 2023 • edited Loading

Codecov Report

AJenbo left a comment

Choose a reason for hiding this comment

kylekatarnls commented Dec 6, 2023

AJenbo commented Dec 6, 2023

kylekatarnls commented Dec 6, 2023 • edited Loading

AJenbo commented Dec 6, 2023

kylekatarnls commented Dec 7, 2023

kylekatarnls commented Dec 6, 2023 •

edited

Loading

codecov bot commented Dec 6, 2023 •

edited

Loading

kylekatarnls commented Dec 6, 2023 •

edited

Loading