Skip to content

[Form][PhpUnitBridge] Remove usage of noop ReflectionProperty::setAccessible() #61207

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

Merged
merged 1 commit into from
Jul 24, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 0 additions & 2 deletions src/Symfony/Bridge/PhpUnit/CoverageListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ public function startTest(Test $test): void
private function addCoversForClassToAnnotationCache(Test $test, array $covers): void
{
$r = new \ReflectionProperty(TestUtil::class, 'annotationCache');
$r->setAccessible(true);
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to revert the changes made to the PHPUnit bridge as it supports PHP 7.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why does/would the PhpUnit bridge in Symfony 6.4, which requires PHP 8.1, need to support PHP 7?

Copy link
Member

Choose a reason for hiding this comment

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

We have to indeed, my bad for missing that!
The bridge on 7.4 is the one that's bumped to PHP >= 8.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So back to where we been yesterday? I target 7.4 for the full changes, and 6.4 for everything but bridge?

Copy link
Member

Choose a reason for hiding this comment

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

I added these calls back in 0270060 and removed them again when merging branches up in 72ccba7.


$cache = $r->getValue();
$cache = array_replace_recursive($cache, [
Expand All @@ -103,7 +102,6 @@ private function addCoversForDocBlockInsideRegistry(Test $test, array $covers):
$docBlock = Registry::getInstance()->forClassName(\get_class($test));

$symbolAnnotations = new \ReflectionProperty($docBlock, 'symbolAnnotations');
$symbolAnnotations->setAccessible(true);

// Exclude internal classes; PHPUnit 9.1+ is picky about tests covering, say, a \RuntimeException
$covers = array_filter($covers, function (string $class) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,6 @@ public function toString()
{
$exception = new \Exception($this->message);
$reflection = new \ReflectionProperty($exception, 'trace');
$reflection->setAccessible(true);
$reflection->setValue($exception, $this->trace);

return ($this->originatesFromAnObject() ? 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().":\n" : '')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ private function willBeIsolated(TestCase $test): bool
}

$r = new \ReflectionProperty($test, 'runTestInSeparateProcess');
$r->setAccessible(true);

return $r->getValue($test) ?? false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ public static function setUpBeforeClass(): void
$loader = require $v.'/autoload.php';
$reflection = new \ReflectionClass($loader);
$prop = $reflection->getProperty('prefixDirsPsr4');
$prop->setAccessible(true);
$currentValue = $prop->getValue($loader);
self::$prefixDirsPsr4[] = [$prop, $loader, $currentValue];
$currentValue['Symfony\\Bridge\\PhpUnit\\'] = [realpath(__DIR__.'/../..')];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,6 @@ public function testGetEnvDoesNotAutoCastNullWithDefaultEnvVarProcessor()
$container->compile();

$r = new \ReflectionMethod($container, 'getEnv');
$r->setAccessible(true);
$this->assertNull($r->invoke($container, 'FOO'));
}

Expand All @@ -436,7 +435,6 @@ public function testGetEnvDoesNotAutoCastNullWithEnvVarProcessorsLocatorReturnin
$container->compile();

$r = new \ReflectionMethod($container, 'getEnv');
$r->setAccessible(true);
$this->assertNull($r->invoke($container, 'FOO'));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class ErrorHandlerTest extends TestCase
protected function tearDown(): void
{
$r = new \ReflectionProperty(ErrorHandler::class, 'exitCode');
$r->setAccessible(true);
$r->setValue(null, 0);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,6 @@ public function testRoundMethodKeepsIntegersAsIntegers()
// Use reflection to test the private round() method directly
$reflection = new \ReflectionClass($transformer);
$roundMethod = $reflection->getMethod('round');
$roundMethod->setAccessible(true);

$int = \PHP_INT_MAX - 1;
$result = $roundMethod->invoke($transformer, $int);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ public function testThrowsTransportExceptionOnFailure()
$sendmailTransport->send($mail, $envelope);

$streamProperty = new \ReflectionProperty(SendmailTransport::class, 'stream');
$streamProperty->setAccessible(true);
$stream = $streamProperty->getValue($sendmailTransport);
$this->assertNull($stream->stream);
}
Expand All @@ -112,10 +111,8 @@ public function testStreamIsClearedOnFailure()
}

$streamProperty = new \ReflectionProperty(SendmailTransport::class, 'stream');
$streamProperty->setAccessible(true);
$stream = $streamProperty->getValue($sendmailTransport);
$innerStreamProperty = new \ReflectionProperty(ProcessStream::class, 'stream');
$innerStreamProperty->setAccessible(true);
$this->assertNull($innerStreamProperty->getValue($stream));
}

Expand All @@ -127,7 +124,6 @@ public function testDoesNotThrowWhenInteractive()

$sendmailTransport = new SendmailTransport(self::FAKE_INTERACTIVE_SENDMAIL);
$transportProperty = new \ReflectionProperty(SendmailTransport::class, 'transport');
$transportProperty->setAccessible(true);

// Replace the transport with an anonymous consumer that trigger the stream methods
$transportProperty->setValue($sendmailTransport, new class($transportProperty->getValue($sendmailTransport)->getStream()) extends SmtpTransport {
Expand Down
Loading