Skip to content

Commit 2507352

Browse files
weaverryanweaverryan
committed
bug #755 fix(make:entity): handle new value if nullable in OneToOne association, close #195 (Kocal)
This PR was merged into the 1.0-dev branch. Discussion ---------- fix(make:entity): handle new value if nullable in OneToOne association, close #195 Hi 👋 This PR is a proposal for #195 for handling the new value (if nullable) in a OneToOne association. The following code was not valid, it fails at `$user->getUserProfile()` if we call `UserProfile#setUser(null)`: ```php class UserProfile { // ... public function setUser(?User $user): self { $this->user = $user; // set (or unset) the owning side of the relation if necessary $newUserProfile = null === $user ? null : $this; if ($user->getUserProfile() !== $newUserProfile) { $user->setUserProfile($newUserProfile); } return $this; } } ``` The following code takes care of: - unset the user profile of the previous User (`$this->user`) if we assign a nullable User - set the profile to the new user, and prevent an infinite loop with `user->getUserProfile() !== $this` ```php class UserProfile { public function setUser(?User $user): self { // unset the owning side of the relation if necessary if ($user === null && $this->user !== null) { $this->user->setUserProfile(null); } // set the owning side of the relation if necessary if ($user !== null && $user->getUserProfile() !== $this) { $user->setUserProfile($this); } $this->user = $user; return $this; } } ``` WDYT? Thanks! Commits ------- 6f8ebae fix(make:entity): handle new value if nullable in OneToOne association, close #195
2 parents d1dee90 + 6f8ebae commit 2507352

File tree

4 files changed

+80
-45
lines changed

4 files changed

+80
-45
lines changed

‎src/Util/ClassSourceManipulator.php‎

Lines changed: 60 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@ public function addGetter(string $propertyName, $returnType, bool $isReturnTypeN
232232
public function addSetter(string $propertyName, $type, bool $isNullable, array $commentLines = [])
233233
{
234234
$builder = $this->createSetterNodeBuilder($propertyName, $type, $isNullable, $commentLines);
235+
$builder->addStmt(
236+
new Node\Stmt\Expression(new Node\Expr\Assign(
237+
new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $propertyName),
238+
new Node\Expr\Variable($propertyName)
239+
))
240+
);
235241
$this->makeMethodFluent($builder);
236242
$this->addMethod($builder->getNode());
237243
}
@@ -411,13 +417,6 @@ private function createSetterNodeBuilder(string $propertyName, $type, bool $isNu
411417
}
412418
$setterNodeBuilder->addParam($paramBuilder->getNode());
413419

414-
$setterNodeBuilder->addStmt(
415-
new Node\Stmt\Expression(new Node\Expr\Assign(
416-
new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $propertyName),
417-
new Node\Expr\Variable($propertyName)
418-
))
419-
);
420-
421420
return $setterNodeBuilder;
422421
}
423422

@@ -538,11 +537,15 @@ private function addSingularRelation(BaseRelation $relation)
538537
// OneToOne is the only "singular" relation type that
539538
// may be the inverse side
540539
if ($relation instanceof RelationOneToOne && !$relation->isOwning()) {
541-
$setterNodeBuilder->addStmt($this->createBlankLineNode(self::CONTEXT_CLASS_METHOD));
542-
543540
$this->addNodesToSetOtherSideOfOneToOne($relation, $setterNodeBuilder);
544541
}
545542

543+
$setterNodeBuilder->addStmt(
544+
new Node\Stmt\Expression(new Node\Expr\Assign(
545+
new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $relation->getPropertyName()),
546+
new Node\Expr\Variable($relation->getPropertyName())
547+
))
548+
);
546549
$this->makeMethodFluent($setterNodeBuilder);
547550
$this->addMethod($setterNodeBuilder->getNode());
548551
}
@@ -1213,50 +1216,74 @@ private function addNodesToSetOtherSideOfOneToOne(RelationOneToOne $relation, Bu
12131216
)),
12141217
];
12151218
$setterNodeBuilder->addStmt($ifNode);
1219+
$setterNodeBuilder->addStmt($this->createBlankLineNode(self::CONTEXT_CLASS_METHOD));
12161220

12171221
return;
12181222
}
12191223

12201224
// at this point, we know the relation is nullable
12211225
$setterNodeBuilder->addStmt($this->createSingleLineCommentNode(
1222-
'set (or unset) the owning side of the relation if necessary',
1226+
'unset the owning side of the relation if necessary',
12231227
self::CONTEXT_CLASS_METHOD
12241228
));
12251229

1226-
$varName = 'new'.ucfirst($relation->getTargetPropertyName());
1227-
// $newUserProfile = null === $user ? null : $this;
1228-
$setterNodeBuilder->addStmt(
1229-
new Node\Stmt\Expression(new Node\Expr\Assign(
1230-
new Node\Expr\Variable($varName),
1231-
new Node\Expr\Ternary(
1232-
new Node\Expr\BinaryOp\Identical(
1233-
$this->createNullConstant(),
1234-
new Node\Expr\Variable($relation->getPropertyName())
1235-
),
1236-
$this->createNullConstant(),
1237-
new Node\Expr\Variable('this')
1238-
)
1239-
))
1240-
);
1241-
1242-
// if ($user->getUserProfile() !== $newUserProfile) {
1243-
$ifNode = new Node\Stmt\If_(new Node\Expr\BinaryOp\NotIdentical(
1244-
new Node\Expr\MethodCall(
1230+
// if ($user !== null && $user->getUserProfile() !== $this)
1231+
$ifNode = new Node\Stmt\If_(new Node\Expr\BinaryOp\BooleanAnd(
1232+
new Node\Expr\BinaryOp\Identical(
12451233
new Node\Expr\Variable($relation->getPropertyName()),
1246-
$relation->getTargetGetterMethodName()
1234+
$this->createNullConstant()
12471235
),
1248-
new Node\Expr\Variable($varName)
1236+
new Node\Expr\BinaryOp\NotIdentical(
1237+
new Node\Expr\PropertyFetch(
1238+
new Node\Expr\Variable('this'),
1239+
$relation->getPropertyName()
1240+
),
1241+
$this->createNullConstant()
1242+
)
12491243
));
1244+
$ifNode->stmts = [
1245+
// $this->user->setUserProfile(null)
1246+
new Node\Stmt\Expression(new Node\Expr\MethodCall(
1247+
new Node\Expr\PropertyFetch(
1248+
new Node\Expr\Variable('this'),
1249+
$relation->getPropertyName()
1250+
),
1251+
$relation->getTargetSetterMethodName(),
1252+
[new Node\Arg($this->createNullConstant())]
1253+
)),
1254+
];
1255+
$setterNodeBuilder->addStmt($ifNode);
12501256

1251-
// $user->setUserProfile($newUserProfile);
1257+
$setterNodeBuilder->addStmt($this->createBlankLineNode(self::CONTEXT_CLASS_METHOD));
1258+
$setterNodeBuilder->addStmt($this->createSingleLineCommentNode(
1259+
'set the owning side of the relation if necessary',
1260+
self::CONTEXT_CLASS_METHOD
1261+
));
1262+
1263+
// if ($user === null && $this->user !== null)
1264+
$ifNode = new Node\Stmt\If_(new Node\Expr\BinaryOp\BooleanAnd(
1265+
new Node\Expr\BinaryOp\NotIdentical(
1266+
new Node\Expr\Variable($relation->getPropertyName()),
1267+
$this->createNullConstant()
1268+
),
1269+
new Node\Expr\BinaryOp\NotIdentical(
1270+
new Node\Expr\MethodCall(
1271+
new Node\Expr\Variable($relation->getPropertyName()),
1272+
$relation->getTargetGetterMethodName()
1273+
),
1274+
new Node\Expr\Variable('this')
1275+
)
1276+
));
12521277
$ifNode->stmts = [
12531278
new Node\Stmt\Expression(new Node\Expr\MethodCall(
12541279
new Node\Expr\Variable($relation->getPropertyName()),
12551280
$relation->getTargetSetterMethodName(),
1256-
[new Node\Arg(new Node\Expr\Variable($varName))]
1281+
[new Node\Arg(new Node\Expr\Variable('this'))]
12571282
)),
12581283
];
12591284
$setterNodeBuilder->addStmt($ifNode);
1285+
1286+
$setterNodeBuilder->addStmt($this->createBlankLineNode(self::CONTEXT_CLASS_METHOD));
12601287
}
12611288

12621289
private function methodExists(string $methodName): bool

‎tests/Doctrine/fixtures/expected_overwrite/src/Entity/User.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,18 @@ public function customMethod()
5151

5252
public function setUserProfile(?UserProfile $userProfile): self
5353
{
54-
$this->userProfile = $userProfile;
54+
// unset the owning side of the relation if necessary
55+
if ($userProfile === null && $this->userProfile !== null) {
56+
$this->userProfile->setUser(null);
57+
}
5558

56-
// set (or unset) the owning side of the relation if necessary
57-
$newUser = null === $userProfile ? null : $this;
58-
if ($userProfile->getUser() !== $newUser) {
59-
$userProfile->setUser($newUser);
59+
// set the owning side of the relation if necessary
60+
if ($userProfile !== null && $userProfile->getUser() !== $this) {
61+
$userProfile->setUser($this);
6062
}
6163

64+
$this->userProfile = $userProfile;
65+
6266
return $this;
6367
}
6468

‎tests/Util/fixtures/add_one_to_one_relation/UserProfile_simple_inverse.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,18 @@ public function getUser(): ?User
3333

3434
public function setUser(?User $user): self
3535
{
36-
$this->user = $user;
36+
// unset the owning side of the relation if necessary
37+
if ($user === null && $this->user !== null) {
38+
$this->user->setUserProfile(null);
39+
}
3740

38-
// set (or unset) the owning side of the relation if necessary
39-
$newUserProfile = null === $user ? null : $this;
40-
if ($user->getUserProfile() !== $newUserProfile) {
41-
$user->setUserProfile($newUserProfile);
41+
// set the owning side of the relation if necessary
42+
if ($user !== null && $user->getUserProfile() !== $this) {
43+
$user->setUserProfile($this);
4244
}
4345

46+
$this->user = $user;
47+
4448
return $this;
4549
}
4650
}

‎tests/Util/fixtures/add_one_to_one_relation/UserProfile_simple_inverse_not_nullable.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@ public function getUser(): ?User
3333

3434
public function setUser(User $user): self
3535
{
36-
$this->user = $user;
37-
3836
// set the owning side of the relation if necessary
3937
if ($user->getUserProfile() !== $this) {
4038
$user->setUserProfile($this);
4139
}
4240

41+
$this->user = $user;
42+
4343
return $this;
4444
}
4545
}

0 commit comments

Comments
 (0)