Skip to content

Commit 739393a

Browse files
sirbrilligsirbrillig
authored
Merge pull request #18 from sirbrillig/fix/ignored-unused-import-symbols
Refactor imported symbol records
2 parents a7acd32 + 7cbc088 commit 739393a

File tree

8 files changed

+211
-68
lines changed

8 files changed

+211
-68
lines changed

‎ImportDetection/FileSymbolRecord.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,8 @@ public function addImportedClasses($names) {
2020
public function addImportedConsts($names) {
2121
$this->importedConsts = array_merge($this->importedConsts, $names);
2222
}
23+
24+
public function addImportedSymbolRecords($names) {
25+
$this->importedSymbolRecords = array_merge($this->importedSymbolRecords, $names);
26+
}
2327
}

‎ImportDetection/ImportedSymbol.php‎

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎ImportDetection/SniffHelpers.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,37 @@ private function getImportNamesFromGroup(File $phpcsFile, int $stackPtr): array
108108
return $collectedSymbols;
109109
}
110110

111+
private function getImportedSymbolsFromGroupStatement(File $phpcsFile, int $stackPtr): array {
112+
$tokens = $phpcsFile->getTokens();
113+
$endBracketPtr = $phpcsFile->findNext([T_CLOSE_USE_GROUP], $stackPtr + 1);
114+
$startBracketPtr = $phpcsFile->findNext([T_OPEN_USE_GROUP], $stackPtr + 1);
115+
if (! $endBracketPtr || ! $startBracketPtr) {
116+
throw new \Exception('Invalid group import statement starting at token ' . $stackPtr . ': ' . $tokens[$stackPtr]['content']);
117+
}
118+
119+
// Get the namespace for the import first, so we can attach it to each Symbol
120+
$importNamespace = $this->getFullSymbol($phpcsFile, $startBracketPtr - 1);
121+
122+
$lastImportPtr = $stackPtr;
123+
$collectedSymbols = [];
124+
$isLastImport = false;
125+
while (! $isLastImport) {
126+
$nextEndOfImportPtr = $phpcsFile->findNext([T_COMMA], $lastImportPtr + 1, $endBracketPtr);
127+
if (! $nextEndOfImportPtr) {
128+
$isLastImport = true;
129+
$nextEndOfImportPtr = $endBracketPtr;
130+
}
131+
$lastStringPtr = $phpcsFile->findPrevious([T_STRING], $nextEndOfImportPtr - 1, $stackPtr);
132+
if (! $lastStringPtr || ! isset($tokens[$lastStringPtr])) {
133+
break;
134+
}
135+
$fullSymbolParts = array_merge($importNamespace->getTokens(), [Symbol::getTokenWithPosition($tokens[$lastStringPtr], $lastStringPtr)]);
136+
$collectedSymbols[] = new Symbol($fullSymbolParts);
137+
$lastImportPtr = $nextEndOfImportPtr;
138+
}
139+
return $collectedSymbols;
140+
}
141+
111142
public function getImportNames(File $phpcsFile, $stackPtr): array {
112143
$tokens = $phpcsFile->getTokens();
113144

@@ -138,6 +169,36 @@ public function getImportNames(File $phpcsFile, $stackPtr): array {
138169
return [$tokens[$lastStringPtr]['content']];
139170
}
140171

172+
public function getImportedSymbolsFromImportStatement(File $phpcsFile, $stackPtr): array {
173+
$tokens = $phpcsFile->getTokens();
174+
175+
$endOfStatementPtr = $phpcsFile->findNext([T_SEMICOLON], $stackPtr + 1);
176+
if (! $endOfStatementPtr) {
177+
return [];
178+
}
179+
180+
// Process grouped imports differently
181+
$nextBracketPtr = $phpcsFile->findNext([T_OPEN_USE_GROUP], $stackPtr + 1, $endOfStatementPtr);
182+
if ($nextBracketPtr) {
183+
return $this->getImportedSymbolsFromGroupStatement($phpcsFile, $stackPtr);
184+
}
185+
186+
// Get the last string before the last semicolon, comma, or closing curly bracket
187+
$endOfImportPtr = $phpcsFile->findPrevious(
188+
[T_COMMA, T_CLOSE_USE_GROUP],
189+
$stackPtr + 1,
190+
$endOfStatementPtr
191+
);
192+
if (! $endOfImportPtr) {
193+
$endOfImportPtr = $endOfStatementPtr;
194+
}
195+
$lastStringPtr = $phpcsFile->findPrevious([T_STRING], $endOfImportPtr - 1, $stackPtr);
196+
if (! $lastStringPtr || ! isset($tokens[$lastStringPtr])) {
197+
return [];
198+
}
199+
return [$this->getFullSymbol($phpcsFile, $lastStringPtr)];
200+
}
201+
141202
public function getPreviousStatementPtr(File $phpcsFile, int $stackPtr): int {
142203
return $phpcsFile->findPrevious([T_SEMICOLON, T_CLOSE_CURLY_BRACKET], $stackPtr - 1) ?: 1;
143204
}

‎ImportDetection/Sniffs/Imports/RequireImportsSniff.php‎

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace ImportDetection\Sniffs\Imports;
44

55
use ImportDetection\Symbol;
6-
use ImportDetection\ImportedSymbol;
76
use ImportDetection\SniffHelpers;
87
use ImportDetection\FileSymbolRecord;
98
use PHP_CodeSniffer\Sniffs\Sniff;
@@ -22,67 +21,71 @@ public function process(File $phpcsFile, $stackPtr) {
2221
$helper = new SniffHelpers();
2322
$tokens = $phpcsFile->getTokens();
2423
$token = $tokens[$stackPtr];
24+
// Keep one set of symbol records per file
2525
$this->symbolRecordsByFile[$phpcsFile->path] = $this->symbolRecordsByFile[$phpcsFile->path] ?? new FileSymbolRecord;
2626
if ($token['type'] === 'T_WHITESPACE') {
2727
$this->debug('found whitespace');
2828
return $this->processEndOfFile($phpcsFile, $stackPtr);
2929
}
3030
if ($token['type'] === 'T_USE') {
3131
$this->debug('found import');
32+
if (in_array($helper->getImportType($phpcsFile, $stackPtr), ['function', 'const', 'class'])) {
33+
$this->recordImportedSymbols($phpcsFile, $stackPtr);
34+
}
3235
return $this->processUse($phpcsFile, $stackPtr);
3336
}
3437
$symbol = $helper->getFullSymbol($phpcsFile, $stackPtr);
3538
// If the symbol has been seen before (if this is a duplicate), ignore it
3639
if (in_array($symbol, $this->symbolRecordsByFile[$phpcsFile->path]->seenSymbols)) {
37-
$this->debug('found duplicate symbol ' . $symbol->getName());
40+
$this->debug('found duplicate symbol: ' . $symbol->getName());
3841
return;
3942
}
4043
$this->symbolRecordsByFile[$phpcsFile->path]->seenSymbols[] = $symbol;
4144
// If the symbol is in the ignore list, ignore it
4245
if ($this->isSymbolIgnored($symbol)) {
43-
$this->debug('found ignored symbol ' . $symbol->getName());
46+
$this->debug('found ignored symbol: ' . $symbol->getName());
4447
$this->markSymbolUsed($phpcsFile, $symbol);
4548
return;
4649
}
4750
// If the symbol is a fully-qualified namespace, ignore it
4851
if ($symbol->isAbsoluteNamespace()) {
49-
$this->debug('found absolute namespaced symbol ' . $symbol->getName());
52+
$this->debug('found absolute namespaced symbol: ' . $symbol->getName());
5053
return;
5154
}
5255
// If this symbol is a definition, ignore it
5356
if ($helper->isSymbolADefinition($phpcsFile, $symbol)) {
54-
$this->debug('found definition symbol ' . $symbol->getName());
57+
$this->debug('found definition symbol: ' . $symbol->getName());
5558
return;
5659
}
5760
// If this symbol is a static reference or an object reference, ignore it
5861
if ($helper->isStaticReference($phpcsFile, $stackPtr) || $helper->isObjectReference($phpcsFile, $stackPtr)) {
59-
$this->debug('found static symbol ' . $symbol->getName());
62+
$this->debug('found static symbol: ' . $symbol->getName());
6063
return;
6164
}
6265
// If this symbol is a namespace definition, ignore it
6366
if ($helper->isWithinNamespaceStatement($phpcsFile, $symbol->getSymbolPosition())) {
64-
$this->debug('found namespace definition symbol ' . $symbol->getName());
67+
$this->debug('found namespace definition symbol: ' . $symbol->getName());
6568
return;
6669
}
6770
// If this symbol is an import, ignore it
6871
if ($helper->isWithinImportStatement($phpcsFile, $symbol->getSymbolPosition())) {
69-
$this->debug('found symbol inside an import ' . $symbol->getName());
72+
$this->debug('found symbol inside an import: ' . $symbol->getName());
7073
return;
7174
}
7275
// If the symbol is predefined, ignore it
7376
if ($helper->isPredefinedConstant($phpcsFile, $stackPtr) || $helper->isBuiltInFunction($phpcsFile, $stackPtr)) {
74-
$this->debug('found predefined symbol ' . $symbol->getName());
77+
$this->debug('found predefined symbol: ' . $symbol->getName());
7578
return;
7679
}
7780
// If this symbol is a predefined typehint, ignore it
7881
if ($helper->isPredefinedTypehint($phpcsFile, $stackPtr)) {
79-
$this->debug('found typehint symbol ' . $symbol->getName());
82+
$this->debug('found typehint symbol: ' . $symbol->getName());
8083
return;
8184
}
8285
// If the symbol's namespace is imported or defined, ignore it
8386
// If the symbol has no namespace and is itself is imported or defined, ignore it
8487
if ($this->isSymbolDefined($phpcsFile, $symbol)) {
85-
$this->debug('found defined symbol ' . $symbol->getName());
88+
$this->debug('found defined symbol: ' . $symbol->getName());
8689
$this->markSymbolUsed($phpcsFile, $symbol);
8790
return;
8891
}
@@ -150,30 +153,37 @@ private function processUse(File $phpcsFile, $stackPtr) {
150153
}
151154
}
152155

153-
private function recordImportedSymbols(File $phpcsFile, int $stackPtr, array $importNames) {
154-
foreach ($importNames as $symbol) {
155-
$this->symbolRecordsByFile[$phpcsFile->path]->importedSymbolRecords[] = new ImportedSymbol($stackPtr, $symbol);
156-
}
156+
private function recordImportedSymbols(File $phpcsFile, int $stackPtr) {
157+
$helper = new SniffHelpers();
158+
$symbols = $helper->getImportedSymbolsFromImportStatement($phpcsFile, $stackPtr);
159+
$this->debug('recording imported symbols: ' . implode(', ', array_map(function (Symbol $symbol): string {
160+
return $symbol->getName();
161+
}, $symbols)));
162+
$symbols = array_map(function ($symbol) {
163+
if ($this->isSymbolIgnored($symbol)) {
164+
$this->debug('found ignored imported symbol: ' . $symbol->getName());
165+
$symbol->markUsed();
166+
}
167+
return $symbol;
168+
}, $symbols);
169+
$this->symbolRecordsByFile[$phpcsFile->path]->addImportedSymbolRecords($symbols);
157170
}
158171

159172
private function saveFunctionImport(File $phpcsFile, $stackPtr) {
160173
$helper = new SniffHelpers();
161174
$importNames = $helper->getImportNames($phpcsFile, $stackPtr);
162-
$this->recordImportedSymbols($phpcsFile, $stackPtr, $importNames);
163175
$this->symbolRecordsByFile[$phpcsFile->path]->addImportedFunctions($importNames);
164176
}
165177

166178
private function saveConstImport(File $phpcsFile, $stackPtr) {
167179
$helper = new SniffHelpers();
168180
$importNames = $helper->getImportNames($phpcsFile, $stackPtr);
169-
$this->recordImportedSymbols($phpcsFile, $stackPtr, $importNames);
170181
$this->symbolRecordsByFile[$phpcsFile->path]->addImportedConsts($importNames);
171182
}
172183

173184
private function saveClassImport(File $phpcsFile, $stackPtr) {
174185
$helper = new SniffHelpers();
175186
$importNames = $helper->getImportNames($phpcsFile, $stackPtr);
176-
$this->recordImportedSymbols($phpcsFile, $stackPtr, $importNames);
177187
$this->symbolRecordsByFile[$phpcsFile->path]->addImportedClasses($importNames);
178188
}
179189

@@ -228,16 +238,20 @@ private function isConstDefined(File $phpcsFile, string $functionName): bool {
228238
}
229239

230240
private function markSymbolUsed(File $phpcsFile, Symbol $symbol) {
231-
$record = $this->getSymbolRecord($phpcsFile, $symbol);
241+
$record = $this->getRecordedImportedSymbolMatchingSymbol($phpcsFile, $symbol);
232242
if (! $record) {
243+
// Symbol records only exist for imported symbols, so if a used symbol
244+
// has not been imported we don't need to mark anything.
245+
$this->debug("ignoring marking symbol used since it was never imported: {$symbol->getName()}");
233246
return;
234247
}
235248
$record->markUsed();
236249
}
237250

238-
private function getSymbolRecord(File $phpcsFile, Symbol $symbol) {
251+
private function getRecordedImportedSymbolMatchingSymbol(File $phpcsFile, Symbol $symbol) {
239252
foreach ($this->symbolRecordsByFile[$phpcsFile->path]->importedSymbolRecords as $record) {
240-
if ($record->getName() === $symbol->getTopLevelNamespace()) {
253+
$this->debug("comparing symbol {$symbol->getTopLevelNamespace()} to alias {$record->getAlias()}");
254+
if ($record->getAlias() === $symbol->getTopLevelNamespace()) {
241255
return $record;
242256
}
243257
}
@@ -253,8 +267,9 @@ private function processEndOfFile(File $phpcsFile, int $stackPtr) {
253267
// For each import, if the Symbol was not used, mark a warning
254268
foreach ($this->symbolRecordsByFile[$phpcsFile->path]->importedSymbolRecords as $record) {
255269
if (! $record->isUsed()) {
270+
$this->debug("found unused symbol: {$record->getName()}");
256271
$error = "Found unused symbol '{$record->getName()}'.";
257-
$phpcsFile->addWarning($error, $record->getPtr(), 'Import');
272+
$phpcsFile->addWarning($error, $record->getSymbolPosition(), 'Import');
258273
}
259274
}
260275
}

‎ImportDetection/Symbol.php‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@
55

66
class Symbol {
77
private $tokens;
8+
private $isUsed;
89

910
public function __construct(array $tokens) {
1011
if (empty($tokens)) {
11-
throw new \Exception('Symbols cannot be empty');
12+
throw new \Exception('Cannot construct Symbol with no tokens');
13+
}
14+
foreach ($tokens as $token) {
15+
if (empty($token) || ! is_array($token)) {
16+
throw new \Exception('Cannot construct Symbol with invalid token: ' . var_export($token, true));
17+
}
1218
}
1319
$this->tokens = $tokens;
20+
$this->isUsed = false;
1421
}
1522

1623
public static function getTokenWithPosition(array $token, int $stackPtr): array {
@@ -26,6 +33,10 @@ public function getName(): string {
2633
return $this->joinSymbolParts($this->tokens);
2734
}
2835

36+
public function getAlias(): string {
37+
return $this->tokens[count($this->tokens) - 1]['content'];
38+
}
39+
2940
public function isAbsoluteNamespace(): bool {
3041
$type = $this->tokens[0]['type'] ?? '';
3142
return $type === 'T_NS_SEPARATOR';
@@ -42,6 +53,14 @@ public function getSymbolPosition(): int {
4253
return $this->tokens[0]['tokenPosition'] ?? 1;
4354
}
4455

56+
public function markUsed() {
57+
$this->isUsed = true;
58+
}
59+
60+
public function isUsed(): bool {
61+
return $this->isUsed;
62+
}
63+
4564
private function joinSymbolParts(array $tokens): string {
4665
$symbolStrings = array_map(function (array $token): string {
4766
return $token['content'] ?? '';

‎tests/SniffTestHelper.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
// phpcs:disable PSR1.Classes.ClassDeclaration.MultipleClasses
23
declare(strict_types=1);
34

45
namespace ImportDetectionTest;
@@ -8,6 +9,13 @@
89
use PHP_CodeSniffer\Ruleset;
910
use PHP_CodeSniffer\Config;
1011

12+
class MessageRecord {
13+
public $rowNumber;
14+
public $columnNumber;
15+
public $message;
16+
public $source;
17+
}
18+
1119
class SniffTestHelper {
1220
public function prepareLocalFileForSniffs($sniffFiles, string $fixtureFile): LocalFile {
1321
$config = new Config();
@@ -47,6 +55,23 @@ public function processFiles($sniffFiles) {
4755
}
4856
}
4957

58+
public function getWarningMessageRecords(array $messages) {
59+
$messageRecords = [];
60+
foreach ($messages as $rowNumber => $messageRow) {
61+
foreach ($messageRow as $columnNumber => $messageArrays) {
62+
foreach ($messageArrays as $messageArray) {
63+
$messageRecord = new MessageRecord();
64+
$messageRecord->rowNumber = $rowNumber;
65+
$messageRecord->columnNumber = $columnNumber;
66+
$messageRecord->message = $messageArray['message'];
67+
$messageRecord->source = $messageArray['source'];
68+
$messageRecords[] = $messageRecord;
69+
}
70+
}
71+
}
72+
return $messageRecords;
73+
}
74+
5075
public function getLineNumbersFromMessages(array $messages): array {
5176
$lines = array_keys($messages);
5277
sort($lines);

0 commit comments

Comments
 (0)