Skip to content

Commit 5511280

Browse files
simPodsimPod
committed
Test cover and refactor
1 parent 71672c3 commit 5511280

File tree

3 files changed

+179
-83
lines changed

3 files changed

+179
-83
lines changed

‎src/Provider/PhpBenchUsageProvider.php‎

Lines changed: 74 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@
1616
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
1717
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage;
1818
use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin;
19-
use function array_map;
2019
use function array_merge;
2120
use function explode;
2221
use function in_array;
2322
use function is_array;
2423
use function is_string;
24+
use function preg_match;
2525
use function sprintf;
2626
use function strpos;
2727
use function substr;
@@ -69,7 +69,7 @@ public function getUsages(
6969
$methodName = $method->getName();
7070

7171
$paramProviderMethods = array_merge(
72-
$this->getParamProvidersFromAnnotations($method->getDocComment()),
72+
$this->getMethodNamesFromAnnotation($method->getDocComment(), '@ParamProviders'),
7373
$this->getParamProvidersFromAttributes($method),
7474
);
7575

@@ -81,15 +81,26 @@ public function getUsages(
8181
);
8282
}
8383

84+
$beforeAfterMethodsFromAttributes = array_merge(
85+
$this->getMethodNamesFromAttribute($method, BeforeMethods::class),
86+
$this->getMethodNamesFromAttribute($method, AfterMethods::class),
87+
);
88+
89+
foreach ($beforeAfterMethodsFromAttributes as $beforeAfterMethod) {
90+
$usages[] = $this->createUsage(
91+
$className,
92+
$beforeAfterMethod,
93+
sprintf('Before/After method, used by %s', $methodName),
94+
);
95+
}
96+
8497
if ($this->isBenchmarkMethod($method)) {
8598
$usages[] = $this->createUsage($className, $methodName, 'Benchmark method');
8699
}
87100

88-
if (!$this->isBeforeOrAfterMethod($method, $className)) {
89-
continue;
101+
if ($this->isBeforeOrAfterMethod($method)) {
102+
$usages[] = $this->createUsage($className, $methodName, 'Before/After method');
90103
}
91-
92-
$usages[] = $this->createUsage($className, $methodName, 'Before/After method');
93104
}
94105

95106
return $usages;
@@ -100,26 +111,49 @@ private function isBenchmarkMethod(ReflectionMethod $method): bool
100111
return strpos($method->getName(), 'bench') === 0;
101112
}
102113

103-
private function isBeforeOrAfterMethod(
114+
/**
115+
* @return list<string>
116+
*/
117+
private function getMethodNamesFromAttribute(
104118
ReflectionMethod $method,
105-
string $className
106-
): bool
119+
string $attributeClass
120+
): array
121+
{
122+
$result = [];
123+
124+
foreach ($method->getAttributes($attributeClass) as $attribute) {
125+
$methods = $attribute->getArguments()[0] ?? $attribute->getArguments()['methods'] ?? [];
126+
if (!is_array($methods)) {
127+
$methods = [$methods];
128+
}
129+
130+
foreach ($methods as $methodName) {
131+
if (is_string($methodName)) {
132+
$result[] = $methodName;
133+
}
134+
}
135+
}
136+
137+
return $result;
138+
}
139+
140+
private function isBeforeOrAfterMethod(ReflectionMethod $method): bool
107141
{
108142
$classReflection = $method->getDeclaringClass();
109143
$methodName = $method->getName();
110144

111-
// Check annotations
145+
// Check class-level annotations
112146
$docComment = $classReflection->getDocComment();
113147
if ($docComment !== false) {
114-
$beforeMethodsFromAnnotations = $this->getBeforeMethodsFromAnnotations($docComment);
115-
$afterMethodsFromAnnotations = $this->getAfterMethodsFromAnnotations($docComment);
148+
$beforeMethodsFromAnnotations = $this->getMethodNamesFromAnnotation($docComment, '@BeforeMethods');
149+
$afterMethodsFromAnnotations = $this->getMethodNamesFromAnnotation($docComment, '@AfterMethods');
116150

117151
if (in_array($methodName, $beforeMethodsFromAnnotations, true) || in_array($methodName, $afterMethodsFromAnnotations, true)) {
118152
return true;
119153
}
120154
}
121155

122-
// Check attributes
156+
// Check class-level attributes
123157
foreach ($classReflection->getAttributes(BeforeMethods::class) as $attribute) {
124158
$methods = $attribute->getArguments()[0] ?? $attribute->getArguments()['methods'] ?? [];
125159
if (!is_array($methods)) {
@@ -153,9 +187,12 @@ private function isBeforeOrAfterMethod(
153187
* @param false|string $rawPhpDoc
154188
* @return list<string>
155189
*/
156-
private function getParamProvidersFromAnnotations($rawPhpDoc): array
190+
private function getMethodNamesFromAnnotation(
191+
$rawPhpDoc,
192+
string $annotationName
193+
): array
157194
{
158-
if ($rawPhpDoc === false || strpos($rawPhpDoc, '@ParamProviders') === false) {
195+
if ($rawPhpDoc === false || strpos($rawPhpDoc, $annotationName) === false) {
159196
return [];
160197
}
161198

@@ -164,15 +201,30 @@ private function getParamProvidersFromAnnotations($rawPhpDoc): array
164201

165202
$result = [];
166203

167-
foreach ($phpDoc->getTagsByName('@ParamProviders') as $tag) {
204+
foreach ($phpDoc->getTagsByName($annotationName) as $tag) {
168205
$value = (string) $tag->value;
169-
// Parse the value which could be like "provideData" or {"provideData", "provideMore"}
170-
// For simplicity, we'll extract method names from the string
171-
// This is a basic implementation - PhpBench uses simple format like @ParamProviders({"provideData"})
172-
$value = trim($value, '{}()');
173-
$methods = array_map('trim', explode(',', $value));
206+
207+
// Extract content from parentheses: @BeforeMethods("setUp") -> "setUp"
208+
// or @BeforeMethods({"setUp", "tearDown"}) -> {"setUp", "tearDown"}
209+
if (preg_match('~\((.+)\)\s*$~', $value, $matches) === 1) {
210+
$value = $matches[1];
211+
}
212+
213+
$value = trim($value);
214+
$value = trim($value, '"\'');
215+
216+
// If it's a single method name, add it directly
217+
if (strpos($value, ',') === false && strpos($value, '{') === false) {
218+
$result[] = $value;
219+
continue;
220+
}
221+
222+
// Handle array format: {"method1", "method2"}
223+
$value = trim($value, '{}');
224+
$methods = explode(',', $value);
174225
foreach ($methods as $method) {
175-
$method = trim($method, '\'" ');
226+
$method = trim($method);
227+
$method = trim($method, '"\'');
176228
if ($method !== '') {
177229
$result[] = $method;
178230
}
@@ -208,66 +260,6 @@ private function getParamProvidersFromAttributes(ReflectionMethod $method): arra
208260
return $result;
209261
}
210262

211-
/**
212-
* @param false|string $rawPhpDoc
213-
* @return list<string>
214-
*/
215-
private function getBeforeMethodsFromAnnotations($rawPhpDoc): array
216-
{
217-
if ($rawPhpDoc === false || strpos($rawPhpDoc, '@BeforeMethods') === false) {
218-
return [];
219-
}
220-
221-
$tokens = new TokenIterator($this->lexer->tokenize($rawPhpDoc));
222-
$phpDoc = $this->phpDocParser->parse($tokens);
223-
224-
$result = [];
225-
226-
foreach ($phpDoc->getTagsByName('@BeforeMethods') as $tag) {
227-
$value = (string) $tag->value;
228-
$value = trim($value, '{}()');
229-
$methods = array_map('trim', explode(',', $value));
230-
foreach ($methods as $method) {
231-
$method = trim($method, '\'" ');
232-
if ($method !== '') {
233-
$result[] = $method;
234-
}
235-
}
236-
}
237-
238-
return $result;
239-
}
240-
241-
/**
242-
* @param false|string $rawPhpDoc
243-
* @return list<string>
244-
*/
245-
private function getAfterMethodsFromAnnotations($rawPhpDoc): array
246-
{
247-
if ($rawPhpDoc === false || strpos($rawPhpDoc, '@AfterMethods') === false) {
248-
return [];
249-
}
250-
251-
$tokens = new TokenIterator($this->lexer->tokenize($rawPhpDoc));
252-
$phpDoc = $this->phpDocParser->parse($tokens);
253-
254-
$result = [];
255-
256-
foreach ($phpDoc->getTagsByName('@AfterMethods') as $tag) {
257-
$value = (string) $tag->value;
258-
$value = trim($value, '{}()');
259-
$methods = array_map('trim', explode(',', $value));
260-
foreach ($methods as $method) {
261-
$method = trim($method, '\'" ');
262-
if ($method !== '') {
263-
$result[] = $method;
264-
}
265-
}
266-
}
267-
268-
return $result;
269-
}
270-
271263
private function createUsage(
272264
string $className,
273265
string $methodName,

‎tests/Rule/DeadCodeRuleTest.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use ShipMonk\PHPStan\DeadCode\Provider\EnumUsageProvider;
4343
use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider;
4444
use ShipMonk\PHPStan\DeadCode\Provider\NetteUsageProvider;
45+
use ShipMonk\PHPStan\DeadCode\Provider\PhpBenchUsageProvider;
4546
use ShipMonk\PHPStan\DeadCode\Provider\PhpStanUsageProvider;
4647
use ShipMonk\PHPStan\DeadCode\Provider\PhpUnitUsageProvider;
4748
use ShipMonk\PHPStan\DeadCode\Provider\ReflectionBasedMemberUsageProvider;
@@ -766,7 +767,7 @@ public static function provideGroupingFiles(): Traversable
766767
}
767768

768769
/**
769-
* @return Traversable<string, array{0: string|list<string>, 1?: bool}>
770+
* @return Traversable<string, array{0: string|non-empty-list<string>, 1?: bool}>
770771
*/
771772
public static function provideFiles(): Traversable
772773
{
@@ -856,6 +857,7 @@ public static function provideFiles(): Traversable
856857
yield 'provider-symfony-7.1' => [__DIR__ . '/data/providers/symfony-gte71.php', self::requiresPhp(8_00_00) && self::requiresPackage('symfony/dependency-injection', '>= 7.1')];
857858
yield 'provider-twig' => [__DIR__ . '/data/providers/twig.php', self::requiresPhp(8_00_00)];
858859
yield 'provider-phpunit' => [__DIR__ . '/data/providers/phpunit.php', self::requiresPhp(8_00_00)];
860+
yield 'provider-phpbench' => [__DIR__ . '/data/providers/phpbench.php', self::requiresPhp(8_00_00)];
859861
yield 'provider-doctrine' => [__DIR__ . '/data/providers/doctrine.php', self::requiresPhp(8_01_00)];
860862
yield 'provider-phpstan' => [__DIR__ . '/data/providers/phpstan.php'];
861863
yield 'provider-nette' => [__DIR__ . '/data/providers/nette.php'];
@@ -991,6 +993,11 @@ private function getMemberUsageProviders(): array
991993
self::getContainer()->getByType(PhpDocParser::class),
992994
self::getContainer()->getByType(Lexer::class),
993995
),
996+
new PhpBenchUsageProvider(
997+
$this->providersEnabled,
998+
self::getContainer()->getByType(PhpDocParser::class),
999+
self::getContainer()->getByType(Lexer::class),
1000+
),
9941001
new DoctrineUsageProvider(
9951002
$this->providersEnabled,
9961003
),

‎tests/Rule/data/providers/phpbench.php‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PhpBench;
4+
5+
use PhpBench\Attributes\AfterMethods;
6+
use PhpBench\Attributes\BeforeMethods;
7+
use PhpBench\Attributes\ParamProviders;
8+
9+
#[BeforeMethods('setUp')]
10+
#[AfterMethods('tearDown')]
11+
class SimpleBench
12+
{
13+
14+
public function setUp(): void
15+
{
16+
}
17+
18+
public function tearDown(): void
19+
{
20+
}
21+
22+
public function benchSimpleMethod(): void
23+
{
24+
}
25+
26+
#[ParamProviders(['provideParams'])]
27+
public function benchWithParams(array $params): void
28+
{
29+
}
30+
31+
public function provideParams(): array
32+
{
33+
return [];
34+
}
35+
36+
}
37+
38+
class MethodLevelAttributesBench
39+
{
40+
41+
public function beforeMethod(): void
42+
{
43+
}
44+
45+
public function afterMethod(): void
46+
{
47+
}
48+
49+
#[BeforeMethods('beforeMethod')]
50+
#[AfterMethods('afterMethod')]
51+
public function benchSomething(): void
52+
{
53+
}
54+
55+
}
56+
57+
/**
58+
* @BeforeMethods("setUpAnnotation")
59+
* @AfterMethods({"tearDownAnnotation"})
60+
*/
61+
class AnnotationBench
62+
{
63+
64+
public function setUpAnnotation(): void
65+
{
66+
}
67+
68+
public function tearDownAnnotation(): void
69+
{
70+
}
71+
72+
public function benchAnnotationStyle(): void
73+
{
74+
}
75+
76+
/**
77+
* @ParamProviders({"provideDataAnnotation"})
78+
*/
79+
public function benchWithAnnotationParams(array $params): void
80+
{
81+
}
82+
83+
public function provideDataAnnotation(): array
84+
{
85+
return [];
86+
}
87+
88+
}
89+
90+
class NotBenchmark
91+
{
92+
93+
public function benchSomething(): void // error: Unused PhpBench\NotBenchmark::benchSomething
94+
{
95+
}
96+
97+
}

0 commit comments

Comments
 (0)