Skip to content

Commit ac82998

Browse files
kschroederkschroeder
committed
Updated for code review responses, handle namespaces better, and handle some token edge cases
1 parent c593ef6 commit ac82998

File tree

5 files changed

+227
-21
lines changed

5 files changed

+227
-21
lines changed

‎setup/src/Magento/Setup/Module/Di/Code/Reader/ClassesScanner.php

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\Setup\Module\Di\Code\Reader;
77

88
use Magento\Framework\App\Filesystem\DirectoryList;
9+
use Magento\Framework\App\ObjectManager;
910
use Magento\Framework\Exception\FileSystemException;
1011

1112
class ClassesScanner implements ClassesScannerInterface
@@ -21,12 +22,29 @@ class ClassesScanner implements ClassesScannerInterface
2122

2223
protected $fileResults = [];
2324

25+
/**
26+
* @var string
27+
*/
28+
29+
protected $generationDirectory;
30+
2431
/**
2532
* @param array $excludePatterns
2633
*/
27-
public function __construct(array $excludePatterns = [])
34+
public function __construct(array $excludePatterns = [], $generationDirectory = false)
2835
{
2936
$this->excludePatterns = $excludePatterns;
37+
$this->generationDirectory = $generationDirectory;
38+
}
39+
40+
public function getGenerationDirectory()
41+
{
42+
if ($this->generationDirectory === false) {
43+
$directoryList = ObjectManager::getInstance()->get(DirectoryList::class);
44+
/* @var $directoryList DirectoryList */
45+
$this->generationDirectory = $directoryList->getPath(DirectoryList::GENERATION);
46+
}
47+
return $this->generationDirectory;
3048
}
3149

3250
/**
@@ -40,6 +58,19 @@ public function addExcludePatterns(array $excludePatterns)
4058
$this->excludePatterns = array_merge($this->excludePatterns, $excludePatterns);
4159
}
4260

61+
/**
62+
* Determines if the path provided is in the var/generation folder
63+
*
64+
* @param $path
65+
* @return bool
66+
*/
67+
68+
public function isGeneration($path)
69+
{
70+
$generation = $this->getGenerationDirectory();
71+
return strpos($path, $generation) === 0;
72+
}
73+
4374
/**
4475
* Retrieves list of classes for given path
4576
*
@@ -51,7 +82,9 @@ public function getList($path)
5182
{
5283

5384
$realPath = realpath($path);
54-
$isGeneration = strpos($realPath, DIRECTORY_SEPARATOR . 'generation' . DIRECTORY_SEPARATOR) !== false;
85+
$isGeneration = $this->isGeneration($realPath);
86+
87+
// Generation folders should not have their results cached since they may actually change during compile
5588
if (!$isGeneration) {
5689
if (isset($this->fileResults[$realPath])) {
5790
return $this->fileResults[$realPath];
@@ -97,22 +130,18 @@ private function extract(\RecursiveIteratorIterator $recursiveIterator)
97130
$classNames = $fileScanner->getClassNames();
98131
$this->includeClasses($classNames, $fileItemPath);
99132
$classes = array_merge($classes, $classNames);
100-
101133
}
102134
return $classes;
103135
}
104136

105137
protected function includeClasses(array $classNames, $fileItemPath)
106138
{
107-
$classExists = false;
108139
foreach ($classNames as $className) {
109-
if (class_exists($className)) {
110-
$classExists = true;
140+
if (!class_exists($className)) {
141+
require_once $fileItemPath;
142+
return;
111143
}
112144
}
113-
if (!$classExists) {
114-
require_once $fileItemPath;
115-
}
116145
}
117146

118147
/**

‎setup/src/Magento/Setup/Module/Di/Code/Reader/FileClassScanner.php

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ class FileClassScanner
2525

2626
protected $classNames = false;
2727

28+
/**
29+
* @var array
30+
*/
31+
32+
protected $tokens;
33+
2834
/**
2935
* Constructor for the file class scanner. Requires the filename
3036
*
@@ -65,17 +71,27 @@ public function getFileContents()
6571

6672
protected function extract()
6773
{
74+
$allowedOpenBraces = [T_CURLY_OPEN, T_DOLLAR_OPEN_CURLY_BRACES, T_STRING_VARNAME];
6875
$classes = [];
69-
$tokens = token_get_all($this->getFileContents());
7076
$namespace = '';
7177
$class = '';
7278
$triggerClass = false;
7379
$triggerNamespace = false;
74-
foreach ($tokens as $token) {
75-
80+
$braceLevel = 0;
81+
$bracedNamespace = false;
82+
83+
$this->tokens = token_get_all($this->getFileContents());
84+
foreach ($this->tokens as $index => $token) {
85+
// Is either a literal brace or an interpolated brace
86+
if ($token == '{' || (is_array($token) && in_array($token[0], $allowedOpenBraces))) {
87+
$braceLevel++;
88+
} else if ($token == '}') {
89+
$braceLevel--;
90+
}
7691
// The namespace keyword was found in the last loop
7792
if ($triggerNamespace) {
78-
if (!is_array($token)) {
93+
// A string ; or a discovered namespace that looks like "namespace name { }"
94+
if (!is_array($token) || ($namespace && $token[0] == T_WHITESPACE)) {
7995
$triggerNamespace = false;
8096
$namespace .= '\\';
8197
continue;
@@ -92,10 +108,14 @@ protected function extract()
92108
case T_NAMESPACE:
93109
// Current loop contains the namespace keyword. Between this and the semicolon is the namespace
94110
$triggerNamespace = true;
111+
$namespace = '';
112+
$bracedNamespace = $this->isBracedNamespace($index);
95113
break;
96114
case T_CLASS:
97115
// Current loop contains the class keyword. Next loop will have the class name itself.
98-
$triggerClass = true;
116+
if ($braceLevel == 0 || ($bracedNamespace && $braceLevel == 1)) {
117+
$triggerClass = true;
118+
}
99119
break;
100120
}
101121

@@ -104,10 +124,37 @@ protected function extract()
104124
$namespace = trim($namespace);
105125
$fqClassName = $namespace . trim($class);
106126
$classes[] = $fqClassName;
107-
return $classes;
127+
$class = '';
128+
}
129+
}
130+
return $classes;;
131+
}
132+
133+
/**
134+
* Looks forward from the current index to determine if the namespace is nested in {} or terminated with ;
135+
*
136+
* @param $index
137+
* @return bool
138+
*/
139+
140+
protected function isBracedNamespace($index)
141+
{
142+
$len = count($this->tokens);
143+
while ($index++ < $len) {
144+
if (!is_array($this->tokens[$index])) {
145+
if ($this->tokens[$index] == ';') {
146+
return false;
147+
} else if ($this->tokens[$index] == '{') {
148+
return true;
149+
}
150+
continue;
151+
}
152+
153+
if (!in_array($this->tokens[$index][0], [T_WHITESPACE, T_STRING, T_NS_SEPARATOR])) {
154+
throw new InvalidFileException('Namespace not defined properly');
108155
}
109156
}
110-
return [];
157+
throw new InvalidFileException('Could not find namespace termination');
111158
}
112159

113160
/**

‎setup/src/Magento/Setup/Test/Unit/Module/Di/Code/Reader/ClassesScannerTest.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,18 @@ class ClassesScannerTest extends \PHPUnit_Framework_TestCase
1212
*/
1313
private $model;
1414

15+
/**
16+
* the /var/generation directory realpath
17+
*
18+
* @var string
19+
*/
20+
21+
private $generation;
22+
1523
protected function setUp()
1624
{
17-
$this->model = new \Magento\Setup\Module\Di\Code\Reader\ClassesScanner();
25+
$this->generation = realpath(__DIR__ . '/../../_files/var/generation');
26+
$this->model = new \Magento\Setup\Module\Di\Code\Reader\ClassesScanner([], $this->generation);
1827
}
1928

2029
public function testGetList()
@@ -24,4 +33,15 @@ public function testGetList()
2433
$this->assertTrue(is_array($actual));
2534
$this->assertCount(5, $actual);
2635
}
36+
37+
public function testIsGenerationIgnoresRegularPath()
38+
{
39+
self::assertFalse($this->model->isGeneration(__DIR__));
40+
}
41+
42+
public function testIsGenerationNotesGenerationPath()
43+
{
44+
self::assertTrue($this->model->isGeneration($this->generation));
45+
}
46+
2747
}

‎setup/src/Magento/Setup/Test/Unit/Module/Di/Code/Reader/FileClassScannerTest.php

Lines changed: 114 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ public function testGetClassNameAndMultiNamespace()
9696
<<<PHP
9797
<?php
9898
99-
namespace This\Is\My\Namespace;
99+
namespace This\Is\My\Ns;
100100
101101
class ThisIsMyTest {
102102
103103
public function __construct()
104104
{
105-
\This\Is\Another\Namespace::class;
105+
\This\Is\Another\Ns::class;
106106
}
107107
108108
public function test()
@@ -117,10 +117,97 @@ public function test()
117117
$result = $scanner->getClassNames();
118118

119119
self::assertCount(1, $result);
120-
self::assertContains('This\Is\My\Namespace\ThisIsMyTest', $result);
120+
self::assertContains('This\Is\My\Ns\ThisIsMyTest', $result);
121121
}
122122

123-
public function testTheWeirdExceptionCaseThatHappens()
123+
124+
public function testGetMultiClassNameAndMultiNamespace()
125+
{
126+
$scanner = $this->getMockBuilder(FileClassScanner::class)->disableOriginalConstructor()->setMethods([
127+
'getFileContents'
128+
])->getMock();
129+
$scanner->expects(self::once())->method('getFileContents')->willReturn(
130+
<<<PHP
131+
<?php
132+
133+
namespace This\Is\My\Ns;
134+
135+
class ThisIsMyTest {
136+
137+
public function __construct()
138+
{
139+
\$this->get(\This\Is\Another\Ns::class)->method();
140+
self:: class;
141+
}
142+
143+
public function test()
144+
{
145+
146+
}
147+
}
148+
149+
class ThisIsForBreaking {
150+
151+
}
152+
153+
PHP
154+
);
155+
/* @var $scanner FileClassScanner */
156+
157+
$result = $scanner->getClassNames();
158+
159+
self::assertCount(2, $result);
160+
self::assertContains('This\Is\My\Ns\ThisIsMyTest', $result);
161+
self::assertContains('This\Is\My\Ns\ThisIsForBreaking', $result);
162+
}
163+
164+
public function testBracketedNamespacesAndClasses()
165+
{
166+
$scanner = $this->getMockBuilder(FileClassScanner::class)->disableOriginalConstructor()->setMethods([
167+
'getFileContents'
168+
])->getMock();
169+
$scanner->expects(self::once())->method('getFileContents')->willReturn(
170+
<<<PHP
171+
<?php
172+
173+
namespace This\Is\My\Ns {
174+
175+
class ThisIsMyTest
176+
{
177+
178+
public function __construct()
179+
{
180+
\This\Is\Another\Ns::class;
181+
self:: class;
182+
}
183+
184+
}
185+
186+
class ThisIsForBreaking
187+
{
188+
}
189+
}
190+
191+
namespace This\Is\Not\My\Ns {
192+
193+
class ThisIsNotMyTest
194+
{
195+
}
196+
}
197+
198+
PHP
199+
);
200+
/* @var $scanner FileClassScanner */
201+
202+
$result = $scanner->getClassNames();
203+
204+
self::assertCount(3, $result);
205+
self::assertContains('This\Is\My\Ns\ThisIsMyTest', $result);
206+
self::assertContains('This\Is\My\Ns\ThisIsForBreaking', $result);
207+
self::assertContains('This\Is\Not\My\Ns\ThisIsNotMyTest', $result);
208+
}
209+
210+
public function testClassKeywordInMiddleOfFile()
124211
{
125212
$filename = __DIR__
126213
. '/../../../../../../../../../..'
@@ -131,4 +218,27 @@ public function testTheWeirdExceptionCaseThatHappens()
131218

132219
self::assertCount(1, $result);
133220
}
221+
222+
public function testInvalidPHPCodeThrowsExceptionWhenCannotDetermineBraceOrSemiColon()
223+
{
224+
$this->setExpectedException(InvalidFileException::class);
225+
$scanner = $this->getMockBuilder(FileClassScanner::class)->disableOriginalConstructor()->setMethods([
226+
'getFileContents'
227+
])->getMock();
228+
$scanner->expects(self::once())->method('getFileContents')->willReturn(
229+
<<<PHP
230+
<?php
231+
232+
namespace This\Is\My\Ns
233+
234+
class ThisIsMyTest
235+
{
236+
}
237+
238+
PHP
239+
);
240+
/* @var $scanner FileClassScanner */
241+
242+
$scanner->getClassNames();
243+
}
134244
}

‎setup/src/Magento/Setup/Test/Unit/Module/Di/_files/var/generation/.keep

Whitespace-only changes.

0 commit comments

Comments
 (0)