Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[DependencyInjection] More bullet-proof expression evaluation#59976

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Wirone wants to merge4 commits intosymfony:6.4
base:6.4
Choose a base branch
Loading
fromWirone:codito/full-bulletproof-for-evaluating-expression
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
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
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -231,7 +231,7 @@ private function checkType(Definition $checkedDefinition, mixed $value, \Reflect
} elseif ($value instanceof Expression) {
try {
$value = $this->getExpressionLanguage()->evaluate($value, ['container' => $this->container]);
} catch (\Exception) {
} catch (\Throwable) {
// If a service from the expression cannot be fetched from the container, we skip the validation.
return;
}
Expand All@@ -246,7 +246,7 @@ private function checkType(Definition $checkedDefinition, mixed $value, \Reflect
if ('' === preg_replace('/'.$envPlaceholderUniquePrefix.'_\w+_[a-f0-9]{32}/U', '', $value, -1, $c) && 1 === $c) {
try {
$value = $this->container->resolveEnvPlaceholders($value, true);
} catch (\Exception) {
} catch (\Throwable) {
// If an env placeholder cannot be resolved, we skip the validation.
return;
}
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeException;
use Symfony\Component\DependencyInjection\Parameter;
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Bar;
Expand All@@ -28,6 +29,8 @@
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\BarOptionalArgument;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\BarOptionalArgumentNotNull;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Deprecated;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\ExpectsIntegerArgument;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\ExpectsIntegerArgumentWrapper;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Foo;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\FooObject;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\IntersectionConstructor;
Expand DownExpand Up@@ -642,6 +645,27 @@ public function testProcessSuccessWhenExpressionReturnsObject()
$this->addToAssertionCount(1);
}

public function testErrorOnEvaluatingExpressionDoesNotStopTheProcess()
{
$container = new ContainerBuilder(new EnvPlaceholderParameterBag([
'some_int' => '%env(SOME_INT)%',
]));

$container
->register('eia', ExpectsIntegerArgument::class)
->setPublic(true)
->setShared(false)
->setFactory([ExpectsIntegerArgument::class, 'create'])
->addArgument(new Parameter('some_int'));
$container
->register('eia_wrapper', ExpectsIntegerArgumentWrapper::class)
->addArgument(new Expression('service("eia").instance()'));

(new CheckTypeDeclarationsPass(true))->process($container);

$this->addToAssertionCount(1);
}

public function testProcessHandleMixedEnvPlaceholder()
{
$container = new ContainerBuilder(new EnvPlaceholderParameterBag([
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

Choose a reason for hiding this comment

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

to be replaced by the licence header (same below)

Copy link
Member

Choose a reason for hiding this comment

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

also in fixture file?
if so, we could run the Fixer to apply this for other fixture files

Choose a reason for hiding this comment

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

the thing is we'd like the licence header in fixtures (because why not), but then, no need to enforce other rules I'd say (at least we'll get many false-positives if we do)

Copy link
Member

Choose a reason for hiding this comment

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

and PHP-CS-Fixer only allows to exclude files entirely, not to override the config being applied to some files to apply only some of the rules.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yeah, we definitely need to introduce skipping rules like ECS has, I like it a lot.

But you can also have a separate ruleset for fixtures and run Fixer twice, pointing to the config file.


namespace Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass;

final class ExpectsIntegerArgument
{
public function __construct(public readonly int $foo) {}

public static function create(int $foo): self
{
return new self($foo);
}

public function instance(): self
{
return $this;
}
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass;

final class ExpectsIntegerArgumentWrapper
{
public function __construct(public readonly ExpectsIntegerArgument $b) {}
}
Loading
[8]ページ先頭
©2009-2025 Movatter.jp