Skip to content

Commit 04ab867

Browse files
committed
#632 #645 #646 ensured that unset() code generated by PublicScopeSimulator respects :void no-return-value semantics
Before this, generating `unset()` code while forgetting to pass all parameters to `PublicScopeSimulator` led to a crash due to `__unset(): void { ... }` containing `return $value` statements (invalid starting from PHP 7.3.0)
1 parent 7b57b2d commit 04ab867

File tree

3 files changed

+26
-8
lines changed

3 files changed

+26
-8
lines changed

src/ProxyManager/ProxyGenerator/Util/PublicScopeSimulator.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ public static function getPublicAccessSimulationCode(
5959
? 'new \\ReflectionClass(get_parent_class($this))'
6060
: 'new \\ReflectionClass(' . var_export($originalClass->getName(), true) . ')';
6161

62+
$returnPropertyName = $returnPropertyName
63+
?? ($operationType === self::OPERATION_UNSET ? 'unset' : $returnPropertyName);
64+
6265
return '$realInstanceReflection = ' . $originalClassReflection . ';' . "\n\n"
6366
. 'if (! $realInstanceReflection->hasProperty($' . $nameParameter . ')) {' . "\n"
6467
. ' $targetObject = ' . $target . ';' . "\n\n"
6568
. self::getUndefinedPropertyNotice($operationType, $nameParameter)
6669
. ' ' . self::getOperation($operationType, $nameParameter, $valueParameter) . "\n"
67-
. " return;\n"
6870
. '}' . "\n\n"
6971
. '$targetObject = ' . self::getTargetObject($valueHolder) . ";\n"
7072
. '$accessor = function ' . $byRef . '() use ($targetObject, $' . $nameParameter . $value . ') {' . "\n"
@@ -144,7 +146,9 @@ private static function getOperation(string $operationType, string $nameParamete
144146
return 'return isset($targetObject->$' . $nameParameter . ');';
145147

146148
case self::OPERATION_UNSET:
147-
return 'unset($targetObject->$' . $nameParameter . ');';
149+
return 'unset($targetObject->$' . $nameParameter . ');'
150+
. "\n\n"
151+
. ' return;';
148152
}
149153

150154
throw new InvalidArgumentException(sprintf('Invalid operation "%s" provided', $operationType));

tests/ProxyManagerTest/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicUnsetTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function testBodyStructure(): void
4141
"\$this->foo && \$this->foo->__invoke(\$this->bar, \$this, '__unset', array('name' => \$name)"
4242
. ", \$this->foo);\n\n"
4343
. "if (isset(self::\$bar[\$name])) {\n unset(\$this->bar->\$name);\n\n return;\n}"
44-
. '%areturn %s;',
44+
. '%a',
4545
$magicIsset->getBody()
4646
);
4747
}

tests/ProxyManagerTest/ProxyGenerator/Util/PublicScopeSimulatorTest.php

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ public function testSimpleGet(): void
4242
\E_USER_NOTICE
4343
);
4444
return $targetObject->$foo;
45-
return;
4645
}
4746
4847
$targetObject = $realInstanceReflection->newInstanceWithoutConstructor();
@@ -76,7 +75,6 @@ public function testSimpleSet(): void
7675
$targetObject = $this;
7776
7877
$targetObject->$foo = $baz; return $targetObject->$foo;
79-
return;
8078
}
8179
8280
$targetObject = $realInstanceReflection->newInstanceWithoutConstructor();
@@ -110,7 +108,6 @@ public function testSimpleIsset(): void
110108
$targetObject = $this;
111109
112110
return isset($targetObject->$foo);
113-
return;
114111
}
115112
116113
$targetObject = $realInstanceReflection->newInstanceWithoutConstructor();
@@ -144,12 +141,15 @@ public function testSimpleUnset(): void
144141
$targetObject = $this;
145142
146143
unset($targetObject->$foo);
144+
147145
return;
148146
}
149147
150148
$targetObject = $realInstanceReflection->newInstanceWithoutConstructor();
151149
$accessor = function () use ($targetObject, $foo) {
152150
unset($targetObject->$foo);
151+
152+
return;
153153
};
154154
$backtrace = debug_backtrace(true, 2);
155155
$scopeObject = isset($backtrace[1]['object']) ? $backtrace[1]['object'] : new \ProxyManager\Stub\EmptyClassStub();
@@ -169,6 +169,22 @@ public function testSimpleUnset(): void
169169
);
170170
}
171171

172+
/**
173+
* @group #632
174+
* @group #645
175+
* @group #646
176+
*/
177+
public function testUnsetCodeWillNotProduceReturnValueStatements(): void
178+
{
179+
self::assertStringNotContainsString(
180+
'return ',
181+
PublicScopeSimulator::getPublicAccessSimulationCode(
182+
PublicScopeSimulator::OPERATION_UNSET,
183+
'foo'
184+
)
185+
);
186+
}
187+
172188
public function testSetRequiresValueParameterName(): void
173189
{
174190
$this->expectException(InvalidArgumentException::class);
@@ -191,7 +207,6 @@ public function testDelegatesToValueHolderWhenAvailable(): void
191207
$targetObject = $this->valueHolder;
192208
193209
$targetObject->$foo = $baz; return $targetObject->$foo;
194-
return;
195210
}
196211
197212
$targetObject = $this->valueHolder;
@@ -243,7 +258,6 @@ public function testWillReturnDirectlyWithNoReturnParam(): void
243258
\E_USER_NOTICE
244259
);
245260
return $targetObject->$foo;
246-
return;
247261
}
248262
249263
$targetObject = $realInstanceReflection->newInstanceWithoutConstructor();

0 commit comments

Comments
 (0)