[OffsetAccessAssignmentToScalarRule] Initial implementation#1307
[OffsetAccessAssignmentToScalarRule] Initial implementation#1307iluuu1994 wants to merge 1 commit into
Conversation
13caabc to
d54ddd4
Compare
|
There's more that we should cover for string types: https://3v4l.org/qJG1U But we'll do that in a separate rule. I'll create a new issue for that. |
d54ddd4 to
5ce3111
Compare
5ce3111 to
4e0e08f
Compare
|
Hi, this is already (at least partially) covered by returning ErrorType from This kind of rule also has to respect levels - level 3 for always incorrect values and level 6 for partially incorrect values (unions where at least one type is fine). |
This PR already supports |
|
@iluuu1994 It would be nicer to employ |
4e0e08f to
459044e
Compare
|
@ondrejmirtes One issue: Currently, Modifying the https://github.com/phpstan/phpstan/blob/master/src/Analyser/NodeScopeResolver.php#L819 by adding Any idea on how to fix this? |
|
Also, the rule is very redundant. The issue is that I need to filter the |
|
Can you show that by reproducing a problem on phpstan.org and posting an
expected output? 😊
On Fri, 10 Aug 2018 at 22:26, Ilija Tovilo ***@***.***> wrote:
Also, the rule is very redundant. The issue is that I need to filter the
maybeTypes for expr and dim. Any tips on how to improve that?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1307 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGZuKSduMI7UmWqh2HcQfE2chJlpoaaks5uPexogaJpZM4VlYHZ>
.
--
Ondřej Mirtes
|
|
You can select your PR from the branches selectbox.
On Fri, 10 Aug 2018 at 22:33, Ondřej Mirtes ***@***.***> wrote:
Can you show that by reproducing a problem on phpstan.org and posting an
expected output? 😊
On Fri, 10 Aug 2018 at 22:26, Ilija Tovilo ***@***.***>
wrote:
> Also, the rule is very redundant. The issue is that I need to filter the
> maybeTypes for expr and dim. Any tips on how to improve that?
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <#1307 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAGZuKSduMI7UmWqh2HcQfE2chJlpoaaks5uPexogaJpZM4VlYHZ>
> .
>
--
Ondřej Mirtes
--
Ondřej Mirtes
|
|
@ondrejmirtes I don't see my PR in the dropdown 🤔 |
|
Ok, do it on master if the point is that there’s actually no error in
practice, or write a failing test 😊
On Fri, 10 Aug 2018 at 22:36, Ilija Tovilo ***@***.***> wrote:
@ondrejmirtes <https://github.com/ondrejmirtes> I don't see my PR in the
dropdown 🤔
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1307 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGZuK_zlICTI0g-Vfp3B0oYIw-rb7Ftks5uPe7WgaJpZM4VlYHZ>
.
--
Ondřej Mirtes
|
|
@ondrejmirtes The reason the build is failing is because of this. The tests already exist. This one fails which is correct: $value = 'Foo';
$value['foo'] = null;This one should fail, but it doesn't: $value = 'Foo';
$value['foo'] += null;That is because |
|
Ok, I’ll try to look into it and make the tests pass and the rule written
elegantly, but there’s no rush since this can be added in 0.11.
On Fri, 10 Aug 2018 at 22:52, Ilija Tovilo ***@***.***> wrote:
@ondrejmirtes <https://github.com/ondrejmirtes> The reason the build is
failing is because of this. The tests already exist.
This one fails which is correct:
$value = 'Foo';$value['foo'] = null;
This one should fail, but it doesn't:
$value = 'Foo';$value['foo'] += null;
That is because $value['foo'] in $value['foo'] += null; is not recognized
as an assignment expression which means the rule will ignore it (since we
check isInExpressionAssign).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1307 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGZuDVQ3K-c57cREZpkk5Aec2v0sufnks5uPfJ3gaJpZM4VlYHZ>
.
--
Ondřej Mirtes
|
|
@ondrejmirtes Thanks 🙂 |
be23b86 to
c6a8cd1
Compare
5904f1c to
99bb3a9
Compare
|
I've taken over this PR and created 56be6db, thanks :) |
|
Cool 🙂 |
Closes #724
OffsetAccessAssignmentToScalarRule