-
-
Notifications
You must be signed in to change notification settings - Fork 410
[code-quality] Improve InlineArrayReturnAssignRector to include implicit array instantiation and nested arrays #7271
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
Conversation
It looks similar to rector-src/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php Line 29 in 77b0f25
see https://getrector.com/rule-detail/inline-array-return-assign-rector |
Thanks 👍 I could not find this one. I'll extend it |
8826594
to
94924c8
Compare
...ity/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc
Show resolved
Hide resolved
cd8970e
to
68fb88b
Compare
fb7b68e
to
ee3da33
Compare
ef52b3b
to
7d6a5b5
Compare
7d6a5b5
to
d74c298
Compare
…cit variable init and nested keys
d74c298
to
c644034
Compare
final class IncludeEmptyAssign | ||
{ | ||
public function getPerson() | ||
public function run($person2) |
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.
when param named $person
sshould be skipped
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.
Indeed 👍 Could you handle it? I'm off soon
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.
I will check if any regression if conflict with existing params
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.
There are many regression on this, eg: this should be skipped public function some(SomeObject $someObject): array
{
- $message = [
- 'type' => 'type'
- 'id' => (string) $someObject->getId(),
- ];
- $message['link'] = $this->getSome($someObject);
- return $message;
+ return ['link' => $this->getSome($fuelOrder)];
} Also, if there is an if before: $someVar[$variable] = (float) $data;
if (rand(0, 1)) {
- $someVar[sprintf('%', $type)] = 0.0;
- $someVar[$type] = 0.0;
-
- return $someVar;
+ return ['' => 0.0];
} also cause crash on UnaryMinus:
I will look into those :) |
The reason for complex append is invalid is because we don't know the variable is available before first variable as this PR cause not rely on initialization, that cause this invalid change: $someVar[$variable] = (float) $data;
if (rand(0, 1)) {
- $someVar[sprintf('%', $type)] = 0.0;
- $someVar[$type] = 0.0;
-
- return $someVar;
+ return ['' => 0.0];
} |
PR for skip init before if |
This dynamic key should also be skipped: public function create($variable, $type, $data)
{
- $someVar[sprintf('%', $type)] = 0.0;
- $someVar[$type] = 0.0;
-
- return $someVar;
+ return ['' => 0.0]; |
PR for dynamic key skip: |
PR for skip init non empty array: |
PR for handle crash on unary plus/minus: |
No description provided.