Skip to content

Commit 1aba650

Browse files
authored
Prevent ForbidMethodDeclaration sniff aborting on deprecated traits (#36)
is_subclass_of()/method_exists() autoload the analysed class. On PHP 8.5 the #[\Deprecated] attribute is honoured on traits, so loading a class that uses a deprecated trait emits a deprecation ("Trait X used by Y is deprecated"). PHP_CodeSniffer turns that into an exception, aborting the whole file check with an Internal.Exception. Silence deprecations around the reflection calls (they are a side effect of the analysed code, not of the sniff) and add a regression test that fails on the leaked deprecation.
1 parent e70ac2f commit 1aba650

6 files changed

Lines changed: 115 additions & 4 deletions

File tree

‎IxDFCodingStandard/Sniffs/Classes/ForbidMethodDeclarationSniff.php‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,8 @@ public function process(File $phpcsFile, $classPointer): void
3131
$fqcn = ClassHelper::getFullyQualifiedName($phpcsFile, $classPointer);
3232
foreach ($this->forbiddenMethods as $typeAndMethod => $replacement) {
3333
[$type, $method] = explode('::', $typeAndMethod);
34-
if (! is_subclass_of($fqcn, $type)) {
35-
continue;
36-
}
3734

38-
if (! method_exists($fqcn, $method)) {
35+
if (! $this->declaresForbiddenMethod($fqcn, $type, $method)) {
3936
continue;
4037
}
4138

@@ -46,4 +43,20 @@ public function process(File $phpcsFile, $classPointer): void
4643
);
4744
}
4845
}
46+
47+
/** @param class-string $fqcn */
48+
private function declaresForbiddenMethod(string $fqcn, string $type, string $method): bool
49+
{
50+
// is_subclass_of()/method_exists() autoload $fqcn. The analysed class may use a deprecated trait
51+
// or extend a deprecated class, which emits a deprecation on PHP 8.5+ (e.g. "Trait X used by Y is
52+
// deprecated"). That deprecation is a side effect of the analysed code, not of this sniff, and would
53+
// otherwise be turned into an exception that aborts the whole file check, so it is silenced here.
54+
set_error_handler(static fn(): bool => true, \E_DEPRECATED | \E_USER_DEPRECATED);
55+
56+
try {
57+
return is_subclass_of($fqcn, $type) && method_exists($fqcn, $method);
58+
} finally {
59+
restore_error_handler();
60+
}
61+
}
4962
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace IxDFCodingStandard\Sniffs\Classes;
4+
5+
use IxDFCodingStandard\Sniffs\Classes\data\ForbidMethodDeclarationSniff\ForbiddenParent;
6+
use IxDFCodingStandard\TestCase;
7+
use PHPUnit\Framework\Attributes\CoversClass;
8+
use PHPUnit\Framework\Attributes\RequiresPhp;
9+
use PHPUnit\Framework\Attributes\Test;
10+
11+
#[CoversClass(ForbidMethodDeclarationSniff::class)]
12+
final class ForbidMethodDeclarationSniffTest extends TestCase
13+
{
14+
private const FORBIDDEN_METHODS = [
15+
ForbiddenParent::class.'::shouldSend' => '\App\Notifications\Support\ShouldCheckConditionsBeforeSendingOut::shouldBeSent',
16+
];
17+
18+
#[Test]
19+
#[RequiresPhp('>= 8.5.0')]
20+
public function it_reports_forbidden_method_even_when_the_class_uses_a_deprecated_trait(): void
21+
{
22+
// #[\Deprecated] is only allowed on traits since PHP 8.5 (earlier versions reject the fixture at compile time).
23+
// Resolving the class autoloads it; using a deprecated trait then emits a deprecation, and the sniff must
24+
// silence it instead of letting it abort the whole file check (Internal.Exception).
25+
$deprecations = [];
26+
set_error_handler(
27+
static function (int $errno, string $message) use (&$deprecations): bool {
28+
$deprecations[] = $message;
29+
30+
return true;
31+
},
32+
\E_DEPRECATED | \E_USER_DEPRECATED
33+
);
34+
35+
try {
36+
$report = self::checkFile(
37+
__DIR__.'/data/ForbidMethodDeclarationSniff/ChildDeclaringForbiddenMethod.php',
38+
['forbiddenMethods' => self::FORBIDDEN_METHODS]
39+
);
40+
} finally {
41+
restore_error_handler();
42+
}
43+
44+
self::assertSame([], $deprecations, 'The sniff must not leak deprecations triggered while autoloading the analysed class.');
45+
self::assertSame(1, $report->getErrorCount());
46+
self::assertSniffError($report, 5, ForbidMethodDeclarationSniff::FORBIDDEN_METHOD_DECLARATION);
47+
}
48+
49+
#[Test]
50+
public function it_does_not_report_when_the_subclass_does_not_declare_the_forbidden_method(): void
51+
{
52+
$report = self::checkFile(
53+
__DIR__.'/data/ForbidMethodDeclarationSniff/ChildWithoutForbiddenMethod.php',
54+
['forbiddenMethods' => self::FORBIDDEN_METHODS]
55+
);
56+
57+
self::assertNoSniffErrorInFile($report);
58+
}
59+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace IxDFCodingStandard\Sniffs\Classes\data\ForbidMethodDeclarationSniff;
4+
5+
final class ChildDeclaringForbiddenMethod extends ForbiddenParent
6+
{
7+
use DeprecatedTrait;
8+
9+
public function shouldSend(): bool
10+
{
11+
return true;
12+
}
13+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace IxDFCodingStandard\Sniffs\Classes\data\ForbidMethodDeclarationSniff;
4+
5+
final class ChildWithoutForbiddenMethod extends ForbiddenParent
6+
{
7+
public function send(): bool
8+
{
9+
return true;
10+
}
11+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace IxDFCodingStandard\Sniffs\Classes\data\ForbidMethodDeclarationSniff;
4+
5+
#[\Deprecated(message: 'Use composition instead.')]
6+
trait DeprecatedTrait
7+
{
8+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace IxDFCodingStandard\Sniffs\Classes\data\ForbidMethodDeclarationSniff;
4+
5+
class ForbiddenParent
6+
{
7+
}

0 commit comments

Comments
 (0)