Skip to content

Commit 1428287

Browse files
lisachenkolisachenko
authored
Fix handling of special self type hint, resolves #376 (#381)
* Transforms all `self` keywords in the source with resolved class name, resolves #376 * Silence all tests/ errors for nitpick
1 parent 34e6f62 commit 1428287

15 files changed

+642
-3
lines changed

‎nitpick.json

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"ignore": [
3+
"tests/*"
4+
]
5+
}

‎src/Core/AspectKernel.php

+2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Go\Instrument\ClassLoading\SourceTransformingLoader;
1616
use Go\Instrument\PathResolver;
1717
use Go\Instrument\Transformer\ConstructorExecutionTransformer;
18+
use Go\Instrument\Transformer\SelfValueTransformer;
1819
use Go\Instrument\Transformer\SourceTransformer;
1920
use Go\Instrument\Transformer\WeavingTransformer;
2021
use Go\Instrument\Transformer\CachingTransformer;
@@ -247,6 +248,7 @@ protected function registerTransformers()
247248
$transformers[] = $filterInjector;
248249
}
249250
$aspectContainer = $aspectKernel->getContainer();
251+
$transformers[] = new SelfValueTransformer($aspectKernel);
250252
$transformers[] = new WeavingTransformer(
251253
$aspectKernel,
252254
$aspectContainer->get('aspect.advice_matcher'),

‎src/Instrument/Transformer/SelfValueTransformer.php

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
/*
3+
* Go! AOP framework
4+
*
5+
* @copyright Copyright 2018, Lisachenko Alexander <[email protected]>
6+
*
7+
* This source file is subject to the license that is bundled
8+
* with this source code in the file LICENSE.
9+
*/
10+
11+
namespace Go\Instrument\Transformer;
12+
13+
use PhpParser\Node\Name\FullyQualified;
14+
use PhpParser\NodeTraverser;
15+
16+
/**
17+
* Transformer that replaces `self` constants in the source code, e.g. new self()
18+
*/
19+
class SelfValueTransformer extends BaseSourceTransformer
20+
{
21+
/**
22+
* This method may transform the supplied source and return a new replacement for it
23+
*
24+
* @param StreamMetaData $metadata Metadata for source
25+
* @return string See RESULT_XXX constants in the interface
26+
*/
27+
public function transform(StreamMetaData $metadata)
28+
{
29+
$selfValueVisitor = new SelfValueVisitor();
30+
$traverser = new NodeTraverser();
31+
$traverser->addVisitor($selfValueVisitor);
32+
$traverser->traverse($metadata->syntaxTree);
33+
34+
$this->adjustSelfTokens($metadata, $selfValueVisitor->getReplacedNodes());
35+
36+
// We should always vote abstain, because if there are only changes for self we can drop them
37+
return self::RESULT_ABSTAIN;
38+
}
39+
40+
/**
41+
* Adjusts tokens in the source code
42+
*
43+
* @param StreamMetaData $metadata
44+
* @param FullyQualified[] $replacedNodes Replaced nodes in the source code
45+
*/
46+
private function adjustSelfTokens(StreamMetaData $metadata, array $replacedNodes)
47+
{
48+
foreach ($replacedNodes as $replacedNode)
49+
{
50+
$position = $replacedNode->getAttribute('startTokenPos');
51+
$metadata->tokenStream[$position][1] = $replacedNode->toString();
52+
}
53+
}
54+
}

‎src/Instrument/Transformer/SelfValueVisitor.php

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
<?php
2+
/*
3+
* Go! AOP framework
4+
*
5+
* @copyright Copyright 2018, Lisachenko Alexander <[email protected]>
6+
*
7+
* This source file is subject to the license that is bundled
8+
* with this source code in the file LICENSE.
9+
*/
10+
11+
namespace Go\Instrument\Transformer;
12+
13+
use PhpParser\Node;
14+
use PhpParser\Node\Name;
15+
use PhpParser\Node\Name\FullyQualified;
16+
use PhpParser\Node\Stmt;
17+
use PhpParser\Node\Expr;
18+
use PhpParser\NodeVisitorAbstract;
19+
20+
/**
21+
* Node visitor that resolves class name for `self` nodes with FQN
22+
*/
23+
final class SelfValueVisitor extends NodeVisitorAbstract
24+
{
25+
/**
26+
* List of replaced nodes
27+
*
28+
* @var Node[]
29+
*/
30+
protected $replacedNodes = [];
31+
32+
/**
33+
* Current namespace
34+
*
35+
* @var null|Name|string
36+
*/
37+
protected $namespace;
38+
39+
/**
40+
* Current class name
41+
*
42+
* @var null|Name
43+
*/
44+
protected $className;
45+
46+
/**
47+
* Returns list of changed `self` nodes
48+
*
49+
* @return Node[]
50+
*/
51+
public function getReplacedNodes()
52+
{
53+
return $this->replacedNodes;
54+
}
55+
56+
/**
57+
* @inheritDoc
58+
*/
59+
public function beforeTraverse(array $nodes)
60+
{
61+
$this->namespace = null;
62+
$this->className = null;
63+
$this->replacedNodes = [];
64+
}
65+
66+
/**
67+
* @inheritDoc
68+
*/
69+
public function enterNode(Node $node)
70+
{
71+
if ($node instanceof Stmt\Namespace_) {
72+
$this->namespace = $node->name;
73+
} elseif ($node instanceof Stmt\Class_) {
74+
if ($node->name !== null) {
75+
$this->className = new Name($node->name);
76+
}
77+
} elseif ($node instanceof Stmt\ClassMethod || $node instanceof Expr\Closure) {
78+
$node->returnType = $this->resolveType($node->returnType);
79+
} elseif ($node instanceof Node\Param) {
80+
$node->type = $this->resolveType($node->type);
81+
} elseif (
82+
$node instanceof Expr\StaticCall
83+
|| $node instanceof Expr\ClassConstFetch
84+
|| $node instanceof Expr\New_
85+
|| $node instanceof Expr\Instanceof_
86+
) {
87+
if ($node->class instanceof Name) {
88+
$node->class = $this->resolveClassName($node->class);
89+
}
90+
} elseif ($node instanceof Stmt\Catch_) {
91+
foreach ($node->types as &$type) {
92+
$type = $this->resolveClassName($type);
93+
}
94+
}
95+
}
96+
97+
/**
98+
* Resolves `self` class name with value
99+
*
100+
* @param Name $name Instance of original node
101+
*
102+
* @return Name|FullyQualified
103+
*/
104+
protected function resolveClassName(Name $name)
105+
{
106+
// Skip all names except special `self`
107+
if (strtolower($name->toString()) !== 'self') {
108+
return $name;
109+
}
110+
111+
// Save the original name
112+
$originalName = $name;
113+
$name = clone $originalName;
114+
$name->setAttribute('originalName', $originalName);
115+
116+
$fullClassName = Name::concat($this->namespace, $this->className);
117+
$resolvedSelfName = new FullyQualified('\\' . $fullClassName->toString(), $name->getAttributes());
118+
119+
$this->replacedNodes[] = $resolvedSelfName;
120+
121+
return $resolvedSelfName;
122+
}
123+
124+
/**
125+
* Helper method for resolving type nodes
126+
*
127+
* @param Node|string|null $node Instance of node
128+
*
129+
* @return Node|Name|FullyQualified
130+
*/
131+
private function resolveType($node)
132+
{
133+
if ($node instanceof Node\NullableType) {
134+
$node->type = $this->resolveType($node->type);
135+
return $node;
136+
}
137+
if ($node instanceof Name) {
138+
return $this->resolveClassName($node);
139+
}
140+
141+
return $node;
142+
}
143+
}

‎src/Proxy/AbstractProxy.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,15 @@ protected function getParameterCode(ReflectionParameter $parameter)
108108
if ($reflectionType) {
109109
$nullablePrefix = (PHP_VERSION_ID >= 70100 && $reflectionType->allowsNull()) ? '?' : '';
110110
$nsPrefix = $reflectionType->isBuiltin() ? '' : '\\';
111-
$type = $nullablePrefix . $nsPrefix . (string) $reflectionType;
111+
$type = $nullablePrefix . $nsPrefix . ltrim((string) $reflectionType, '\\');
112112
}
113113
} else {
114114
if ($parameter->isArray()) {
115115
$type = 'array';
116116
} elseif ($parameter->isCallable()) {
117117
$type = 'callable';
118118
} elseif ($parameter->getClass()) {
119-
$type = '\\' . $parameter->getClass()->name;
119+
$type = '\\' . ltrim($parameter->getClass()->name, '\\');
120120
}
121121
}
122122
$defaultValue = null;
@@ -211,7 +211,7 @@ protected function getOverriddenFunction(ReflectionFunctionAbstract $functionLik
211211
$nullablePrefix = $reflectionReturnType->allowsNull() ? '?' : '';
212212
$nsPrefix = $reflectionReturnType->isBuiltin() ? '' : '\\';
213213

214-
$reflectionReturnType = $nullablePrefix . $nsPrefix . (string) $reflectionReturnType;
214+
$reflectionReturnType = $nullablePrefix . $nsPrefix . ltrim((string) $reflectionReturnType, '\\');
215215
}
216216
if ($functionLike instanceof ReflectionMethod) {
217217
$modifiersLine = implode(' ', Reflection::getModifierNames($functionLike->getModifiers()));

‎tests/Go/Instrument/Transformer/SelfValueTransformerTest.php

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
namespace Go\Instrument\Transformer;
4+
5+
use Go\Core\AspectKernel;
6+
7+
class SelfValueTransformerTest extends \PHPUnit_Framework_TestCase
8+
{
9+
/**
10+
* @var SelfValueTransformer
11+
*/
12+
protected $transformer;
13+
14+
/**
15+
* @var StreamMetaData|null
16+
*/
17+
protected $metadata;
18+
19+
/**
20+
* {@inheritDoc}
21+
*/
22+
public function setUp()
23+
{
24+
$this->transformer = new SelfValueTransformer(
25+
$this->getKernelMock([
26+
'cacheDir' => __DIR__,
27+
'appDir' => dirname(__DIR__),
28+
])
29+
);
30+
}
31+
32+
/**
33+
* Returns a mock for kernel
34+
*
35+
* @return \PHPUnit_Framework_MockObject_MockObject|\Go\Core\AspectKernel
36+
*/
37+
protected function getKernelMock($options)
38+
{
39+
$mock = $this->getMockForAbstractClass(
40+
AspectKernel::class,
41+
[],
42+
'',
43+
false,
44+
true,
45+
true,
46+
['getOptions']
47+
);
48+
$mock->expects($this->any())
49+
->method('getOptions')
50+
->will(
51+
$this->returnValue($options)
52+
);
53+
return $mock;
54+
}
55+
56+
public function testTransformerReplacesAllSelfPlaces()
57+
{
58+
$testFile = fopen(__DIR__ . '/_files/file-with-self.php', 'rb');
59+
$content = stream_get_contents($testFile);
60+
$metadata = new StreamMetaData($testFile, $content);
61+
$this->transformer->transform($metadata);
62+
$expected = file_get_contents(__DIR__ . '/_files/file-with-self-transformed.php');
63+
$this->assertSame($expected, (string) $metadata->source);
64+
}
65+
}

‎tests/Go/Instrument/Transformer/WeavingTransformerTest.php

+10
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ public function testWeaverForPhp7Class()
133133
$actual = $this->normalizeWhitespaces($metadata->source);
134134
$expected = $this->normalizeWhitespaces($this->loadTest('php7-class-woven')->source);
135135
$this->assertEquals($expected, $actual);
136+
if (preg_match("/AOP_CACHE_DIR . '(.+)';$/", $actual, $matches)) {
137+
$actualProxyContent = $this->normalizeWhitespaces(file_get_contents('vfs://' . $matches[1]));
138+
$expectedProxyContent = $this->normalizeWhitespaces($this->loadTest('php7-class-proxy')->source);
139+
$this->assertEquals($expectedProxyContent, $actualProxyContent);
140+
}
136141
}
137142

138143
/**
@@ -172,6 +177,11 @@ public function testTransformerWithIncludePaths()
172177
$actual = $this->normalizeWhitespaces($metadata->source);
173178
$expected = $this->normalizeWhitespaces($this->loadTest('class-woven')->source);
174179
$this->assertEquals($expected, $actual);
180+
if (preg_match("/AOP_CACHE_DIR . '(.+)';$/", $actual, $matches)) {
181+
$actualProxyContent = $this->normalizeWhitespaces(file_get_contents('vfs://' . $matches[1]));
182+
$expectedProxyContent = $this->normalizeWhitespaces($this->loadTest('class-proxy')->source);
183+
$this->assertEquals($expectedProxyContent, $actualProxyContent);
184+
}
175185
}
176186

177187
/**

0 commit comments

Comments
 (0)