-
Notifications
You must be signed in to change notification settings - Fork 0
[27]: add RemoveArrayItem visitor #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 5 commits
be330a9
899f64f
88a6196
ae48f69
04ec384
dc28d1b
4a1faf5
d4bf6d3
c1d492f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,94 @@ | ||||||||||||||||
| <?php | ||||||||||||||||
|
|
||||||||||||||||
| namespace RonasIT\Larabuilder\Visitors; | ||||||||||||||||
|
|
||||||||||||||||
| use PhpParser\Node; | ||||||||||||||||
| use PhpParser\Node\Expr\Array_; | ||||||||||||||||
| use PhpParser\Node\Expr\ConstFetch; | ||||||||||||||||
| use PhpParser\Node\Scalar; | ||||||||||||||||
| use PhpParser\Node\Stmt\Property; | ||||||||||||||||
| use RonasIT\Larabuilder\Exceptions\UnexpectedPropertyTypeException; | ||||||||||||||||
|
|
||||||||||||||||
| class RemoveArrayPropertyItem extends SetPropertyValue | ||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be better to implement new abstract class |
||||||||||||||||
| { | ||||||||||||||||
| protected array $valuesToRemove; | ||||||||||||||||
|
|
||||||||||||||||
| public function __construct( | ||||||||||||||||
| protected string $name, | ||||||||||||||||
| array $valuesToRemove, | ||||||||||||||||
| ) { | ||||||||||||||||
| $this->valuesToRemove = array_map( | ||||||||||||||||
| callback: fn ($value) => $this->getPropertyValue($value)[0], | ||||||||||||||||
| array: $valuesToRemove, | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| parent::__construct($name, []); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+17
to
+27
|
||||||||||||||||
|
|
||||||||||||||||
| /** @param Property $node */ | ||||||||||||||||
| protected function updateNode(Node $node): void | ||||||||||||||||
| { | ||||||||||||||||
| $arrayProperty = $node->props[0]->default; | ||||||||||||||||
|
|
||||||||||||||||
| if (!$arrayProperty instanceof Array_) { | ||||||||||||||||
| throw new UnexpectedPropertyTypeException( | ||||||||||||||||
| property: $this->name, | ||||||||||||||||
| expectedType: 'array', | ||||||||||||||||
| actualType: (is_null($node->type)) ? 'null' : (string) $node->type, | ||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be better to allow to receive null in exception
Suggested change
|
||||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| $newItems = []; | ||||||||||||||||
|
|
||||||||||||||||
| foreach ($arrayProperty->items as $item) { | ||||||||||||||||
| if (!$this->shouldRemoveItem($item)) { | ||||||||||||||||
| $newItems[] = $item; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| $arrayProperty->items = $newItems; | ||||||||||||||||
DenTray marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| protected function areNodesEqual(Node $expected, Node $actual): bool | ||||||||||||||||
| { | ||||||||||||||||
| return match (true) { | ||||||||||||||||
| $expected instanceof Scalar && $actual instanceof Scalar => $expected->value === $actual->value, | ||||||||||||||||
| $expected instanceof ConstFetch && $actual instanceof ConstFetch => $expected->name->name === $actual->name->name, | ||||||||||||||||
| $expected instanceof Array_ && $actual instanceof Array_ => $this->areArrayNodesEqual($expected, $actual), | ||||||||||||||||
| default => false, | ||||||||||||||||
| }; | ||||||||||||||||
|
Comment on lines
+50
to
+55
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we use something like this?
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| protected function areArrayNodesEqual(Array_ $expected, Array_ $actual): bool | ||||||||||||||||
| { | ||||||||||||||||
| if (count($expected->items) !== count($actual->items)) { | ||||||||||||||||
| return false; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| foreach ($expected->items as $index => $expectedItem) { | ||||||||||||||||
| $actualItem = $actual->items[$index]; | ||||||||||||||||
|
|
||||||||||||||||
| if (!$this->areNodesEqual($expectedItem->value, $actualItem->value)) { | ||||||||||||||||
| return false; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
DenTray marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||
|
|
||||||||||||||||
| return true; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| protected function shouldRemoveItem(Node $item): bool | ||||||||||||||||
| { | ||||||||||||||||
| foreach ($this->valuesToRemove as $removeValue) { | ||||||||||||||||
| if ($this->areNodesEqual($item->value, $removeValue)) { | ||||||||||||||||
| return true; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| return false; | ||||||||||||||||
DenTray marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| protected function insertNode(Node $node): Node | ||||||||||||||||
| { | ||||||||||||||||
| return $node; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| <?php | ||
|
|
||
| namespace RonasIT\Larabuilder\Tests\Support; | ||
|
|
||
| use Some; | ||
|
|
||
| class SomeClass | ||
| { | ||
| public string $stringProperty = 'some value'; | ||
| protected array $tags = ['one', 78.4]; | ||
| public bool $notArray = false; | ||
| protected array $fillable = [ | ||
| 'email', | ||
| ]; | ||
| public array $newMultiArrayProperty = [ | ||
| 'arrayProperty2' => [1, 2, 3], | ||
| ]; | ||
|
|
||
| public function __construct() | ||
| { | ||
| } | ||
|
|
||
| public function someMethod() | ||
| { | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
|
|
||
| namespace RonasIT\Larabuilder\Tests\Support; | ||
|
|
||
| use Some; | ||
| use Test; | ||
| use Some\SomeTrait; | ||
|
|
||
| /** | ||
| * Test | ||
| */ | ||
| class SomeClass implements Test, Some | ||
| { | ||
| use SomeTrait; | ||
|
|
||
| public function __construct() | ||
| { | ||
| } | ||
|
|
||
| public function someMethod() | ||
| { | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation for
removeArrayPropertyItemis incomplete. It should clarify that if the specified property doesn't exist, the operation is silently skipped (no error is thrown and no property is created). Consider adding: "Remove items from thearrayclass property. If the property does not exist, no action is taken."