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.

Sign up for GitHub

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

DevKit updates for 4.x branch #8133

Merged
merged 8 commits into from
Dec 8, 2023
Merged

DevKit updates for 4.x branch #8133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b837a16
DevKit updates
SonataCI Dec 4, 2023
d24df59
Fix one psalm issue
jordisala1991 Dec 4, 2023
e2ebcab
Fix another psalm issue
jordisala1991 Dec 4, 2023
b7eae9b
Fix PHPStan
jordisala1991 Dec 4, 2023
5cb914c
DevKit updates
SonataCI Dec 7, 2023
e90fcfd
DevKit updates
SonataCI Dec 7, 2023
ee88214
Try
VincentLanglet Dec 8, 2023
701ab92
Add adapter
VincentLanglet Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions .github/workflows/test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,18 @@ jobs:
- php-version: '8.2'
dependencies: highest
allowed-to-fail: false
symfony-require: 6.2.*
variant: symfony/symfony:"6.2.*"
symfony-require: 6.3.*
variant: symfony/symfony:"6.3.*"
- php-version: '8.2'
dependencies: highest
allowed-to-fail: false
symfony-require: 6.3.*
variant: symfony/symfony:"6.3.*"
symfony-require: 6.4.*
variant: symfony/symfony:"6.4.*"
- php-version: '8.2'
dependencies: highest
allowed-to-fail: false
symfony-require: 7.0.*
variant: symfony/symfony:"7.0.*"

steps:
- name: Checkout
Expand Down
2 changes: 0 additions & 2 deletions rector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

use Rector\Config\RectorConfig;
use Rector\Php70\Rector\FunctionLike\ExceptionHandlerTypehintRector;
use Rector\Php71\Rector\FuncCall\CountOnNullRector;
use Rector\PHPUnit\CodeQuality\Rector\Class_\AddSeeTestAnnotationRector;
use Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector;
use Rector\PHPUnit\Set\PHPUnitLevelSetList;
Expand All @@ -41,7 +40,6 @@
$rectorConfig->importNames();
$rectorConfig->importShortClasses(false);
$rectorConfig->skip([
CountOnNullRector::class,
ExceptionHandlerTypehintRector::class,
AddSeeTestAnnotationRector::class,
PreferPHPUnitThisCallRector::class,
Expand Down
6 changes: 3 additions & 3 deletions src/ArgumentResolver/CompatibleValueResolverInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
use Symfony\Component\HttpKernel\Controller\ValueResolverInterface;

// TODO: Remove this interface when dropping support of Symfony < 6.2 and replace its usage with ValueResolverInterface
if (interface_exists(ValueResolverInterface::class)) {
if (interface_exists(ArgumentValueResolverInterface::class)) {
/** @internal */
interface CompatibleValueResolverInterface extends ValueResolverInterface
interface CompatibleValueResolverInterface extends ArgumentValueResolverInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

@VincentLanglet isn't it the other way around?

this code gives me deprecations right now

Copy link
Member

@VincentLanglet VincentLanglet Dec 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can you try

if (interface_exists(ArgumentValueResolverInterface::class) && !interface_exists(ValueResolverInterface::class)) {
    /** @internal */
    interface CompatibleValueResolverInterface extends ArgumentValueResolverInterface
    {
    }
} else {
    /** @internal */
    interface CompatibleValueResolverInterface extends ValueResolverInterface
    {
    }
}

?

If it works for you PR is welcome

Copy link
Contributor

@Hanmac Hanmac Dec 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The Question is more about what to do between the versions:

  • 3.1.0 - 6.2.0 there was only ArgumentValueResolverInterface
  • 6.2.0 - 7.0.0 there are both ArgumentValueResolverInterface and ValueResolverInterface, the first one is deprecated.
  • 7.0.0 - there is only ValueResolverInterface

what would be the right choice for 6.2.0 - 7.0.0 ? Which Interface should we extend?

there shouldn't be extra logic needed, its just ArgumentValueResolverInterface =>ValueResolverInterface

as it was before, using ValueResolverInterface if able was better?

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to open a pr with what you have in mind and try having a green build

Copy link
Contributor

Choose a reason for hiding this comment

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

this should work:
#8139

{
}
} else {
/** @internal */
interface CompatibleValueResolverInterface extends ArgumentValueResolverInterface
interface CompatibleValueResolverInterface extends ValueResolverInterface
{
}
}
2 changes: 1 addition & 1 deletion src/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
final class Configuration implements ConfigurationInterface
{
/**
* @psalm-suppress PossiblyNullReference, UndefinedInterfaceMethod
* @psalm-suppress UndefinedInterfaceMethod
*
* @see https://github.com/psalm/psalm-plugin-symfony/issues/174
*/
Expand Down
2 changes: 2 additions & 0 deletions src/Route/AdminPoolLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

/**
* @author Thomas Rabaix <thomas.rabaix@sonata-project.org>
*
* @psalm-suppress PropertyNotSetInConstructor
*/
final class AdminPoolLoader extends Loader
{
Expand Down
19 changes: 7 additions & 12 deletions tests/Form/Widget/FormChoiceWidgetTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
use Symfony\Component\Form\FormTypeInterface;
use Symfony\Component\HttpKernel\Kernel;

final class FormChoiceWidgetTest extends BaseWidgetTest
{
Expand All @@ -41,17 +40,13 @@ public function testLabelRendering(): void

$html = $this->renderWidget($choice->createView());

if (0 !== preg_match("/7\..\../", Kernel::VERSION)) {
static::assertStringContainsString(
'<li><div class="checkbox"><label><input type="checkbox" id="choice_0" name="choice[]" value="0"><span class="control-label__text">[trans]some[/trans]</span></label></div></li>',
$this->cleanHtmlWhitespace($html)
);
} else {
static::assertStringContainsString(
'<li><div class="checkbox"><label><input type="checkbox" id="choice_0" name="choice[]" value="0" /><span class="control-label__text">[trans]some[/trans]</span></label></div></li>',
$this->cleanHtmlWhitespace($html)
);
}
// TODO: Remove this adapter when dropping support for Symfony < 7.
$html = str_replace('value="0" />', 'value="0">', $html);

static::assertStringContainsString(
'<li><div class="checkbox"><label><input type="checkbox" id="choice_0" name="choice[]" value="0"><span class="control-label__text">[trans]some[/trans]</span></label></div></li>',
$this->cleanHtmlWhitespace($html)
);
}

public function testDefaultValueRendering(): void
Expand Down