Skip to content

Commit 769fe16

Browse files
authored
Fix duplicate count constants and globals (#38)
* Don't count undocumented globals as duplicates if only checking documented globals * Don't count constants in conditions as duplicates * globals without @var in comment are not documented Common false positive in WordPress where a global is annotated with @param since it's assigned the result of an apply_filters * update tests and add test for doc in constant * remove comments from undocumented globals
1 parent e87d28f commit 769fe16

8 files changed

Lines changed: 74 additions & 15 deletions

src/NodeVisitor.php

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,14 @@ private function needsNode(Node $node, string $namespace): bool
376376
$node->consts,
377377
function (\PhpParser\Node\Const_ $const) {
378378
$fullyQualifiedName = $const->name->name;
379-
return $this->count('constants', $fullyQualifiedName)
380-
&& !defined($fullyQualifiedName);
379+
380+
if (
381+
!$this->isInIf ||
382+
!isset($this->counts['constants'][$fullyQualifiedName])
383+
) {
384+
return $this->count('constants', $fullyQualifiedName)
385+
&& !defined($fullyQualifiedName);
386+
}
381387
}
382388
);
383389

@@ -392,8 +398,13 @@ function (\PhpParser\Node\Const_ $const) {
392398
) {
393399
$fullyQualifiedName = $node->expr->args[0]->value->value;
394400

395-
return $this->count('constants', $fullyQualifiedName)
396-
&& !defined($fullyQualifiedName);
401+
if (
402+
!$this->isInIf ||
403+
!isset($this->counts['constants'][$fullyQualifiedName])
404+
) {
405+
return $this->count('constants', $fullyQualifiedName)
406+
&& !defined($fullyQualifiedName);
407+
}
397408
}
398409
}
399410

@@ -404,12 +415,24 @@ function (\PhpParser\Node\Const_ $const) {
404415
&& $node->expr->var instanceof Variable
405416
&& is_string($node->expr->var->name)
406417
) {
407-
$this->count('globals', $node->expr->var->name);
408418
// We'll keep regular global variable declarations, depending on
409419
// whether or not they are documented.
410-
if ($node->getDocComment()) {
420+
// Only @var comments are actual, useful documentation for stubs
421+
$hasValidComment = $node->getDocComment() && preg_match('/@(?:[a-z]+-)?var\s/', $node->getDocComment()->getText());
422+
423+
if ($this->needsDocumentedGlobals && $hasValidComment) {
424+
$this->count('globals', $node->expr->var->name);
411425
return $this->needsDocumentedGlobals;
412-
} else {
426+
}
427+
428+
if ($this->needsUndocumentedGlobals && !$hasValidComment) {
429+
$this->count('globals', $node->expr->var->name);
430+
431+
// remove useless comments
432+
if ($node->getComments() !== []) {
433+
$node->setAttribute('comments', []);
434+
}
435+
413436
return $this->needsUndocumentedGlobals;
414437
}
415438
}

test/files/constants.in.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,13 @@
77
define('FIZ', 'BUZ');
88

99
const A = 1, B = 2;
10+
11+
if (!defined('ACME')) {
12+
define('ACME', 1);
13+
}
14+
15+
if (!defined('ACME')) {
16+
define('ACME', 2);
17+
}
18+
19+
\define( 'FOOBAR', /** @var int */ 0 );

test/files/constants.out.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,11 @@
77
\define('FIZ', 'BUZ');
88

99
const A = 1, B = 2;
10+
11+
\define('ACME', 1);
12+
13+
\define(
14+
'FOOBAR',
15+
/** @var int */
16+
0
17+
);

test/files/globals.all.out.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
<?php
2-
/** doc */
2+
/** @var string */
33
$a = 'a' . 'a';
44

5-
/** @doc */
5+
/** @var string $b */
66
$b = 'b' . 'b';
77

88
$c = 'c' . 'c';
99

1010
$d = 'd' . 'd';
11+
12+
$x = 'x' . 'x';
13+
14+
$y = 'y' . 'y';

test/files/globals.doc.out.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
2-
/** doc */
2+
/** @var string */
33
$a = 'a' . 'a';
44

5-
/** @doc */
5+
/** @var string $b */
66
$b = 'b' . 'b';

test/files/globals.in.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<?php
2-
/** doc */
2+
/** @var string */
33
$a = 'a' . 'a';
44

5-
/** @doc */
5+
/** @var string $b */
66
$GLOBALS['b'] = 'b' . 'b';
77

88
$c = 'c' . 'c';
@@ -25,3 +25,9 @@
2525

2626
function () {
2727
};
28+
29+
/** doc */
30+
$x = 'x' . 'x';
31+
32+
/** @doc */
33+
$GLOBALS['y'] = 'y' . 'y';

test/files/globals.no-doc.out.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,7 @@
22
$c = 'c' . 'c';
33

44
$d = 'd' . 'd';
5+
6+
$x = 'x' . 'x';
7+
8+
$y = 'y' . 'y';
Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
<?php
2-
/** doc */
2+
/** @var string */
33
$a = \null;
44

5-
/** @doc */
5+
/** @var string $b */
66
$b = \null;
77

88
$c = \null;
99

1010
$d = \null;
11+
12+
$x = \null;
13+
14+
$y = \null;

0 commit comments

Comments
 (0)