Phpinspectionsea: Potential to be a bit more clever about `foreach` type checks

[asgrim]

Because we do the is_array check (so we know it's an array type now), you could potentially be a bit more clever about figuring out the type of $response['externalAttachmentsList'] here and not flag this code.

[kalessil]

I'll implement a workaround (we are dealing with PS type resolver issues in this case),
but it'll be pretty silly and adding more statements between foreach and the if will trigger the same warning.

[asgrim]

Surely the type resolver figures stuff like that out already? I'm sure I've seen similar with $x instanceof Foo then following that, PS knows that $x can use methods from Foo etc.?

[kalessil]

Autocompletion and type resolver probably are not connected, I don;t know.

[asgrim]

Interesting; so actually, this is probably something that PS should improve on perhaps

[kalessil]

Give them a push, perhaps you can attract enough attention to this problem than me =)

[asgrim]

Ah - interesting, this works and correctly identifies the type:

/** @var array $a */
if (is_array($a['item'])) {
    foreach ($a['item'] as $b) {
        $a['a'] += $b;
    }
}

But this does not (i.e. adding the isset check breaks it):

/** @var array $a */
if (isset($a['item']) && is_array($a['item'])) {
    foreach ($a['item'] as $b) {
        $a['a'] += $b;
    }
}

[kalessil]

In global context or inside a function/method?

[asgrim]

This is inside a method, I wouldn't have thought it'd make a difference though?

[asgrim]

Checked in a separate file, no it does not make any difference, same issue. I'm reporting it to PS anyway too

[kalessil]

Super, can you post here the bug-report link as well? I still think we need a workaround, as fixing can take a while.

[asgrim]

It's in a support request at the moment (so private) but if it is escalated to their YouTrack I'll post a link

[kalessil]

Ok, can you ping me back if we don;t need any workarounds an the support gives an idea how to deal with similar cases, please?

[asgrim]

@kalessil ah, the PS folks have responded and say the issue to vote for is here: https://youtrack.jetbrains.com/issue/WI-15715 :)

[kalessil]

Wow, it's even older than I might think xD Thank you, James!

On Mon, 7 Aug 2017 at 21:08, James Titcumb [e-mail] wrote:

@kalessil https://github.com/kalessil ah, the PS folks have responded
and say the issue to vote for is here:
https://youtrack.jetbrains.com/issue/WI-15715 :)

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/kalessil/phpinspectionsea/issues/441#issuecomment-320673645,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABgQ4YRrzP4kXG5v67gUfBavBUbu-paKks5sVxpQgaJpZM4OsHSP
.

>

Regards, Vladimir A. Reznichenko

[kalessil]

I gave a try for a workaround - it messes up the inspection.
BTW I'm considering to drop the inspection in Php Inspections (EA Ultimate), does the inspection bring real value?

[asgrim]

Well, you can iterate over things that are not arrays or iterable, right? Like a string I think... Maybe warn if it's definitely a not iterable type instead of maybe a not iterable type?

[kalessil]

When type-info is incomplete it difficult to guess is it maybe or definitely case.

What I see is that big part of similar foreach sources inspection reports is about array accesses, so skipping array accesses while PS types resolving getting improved is a viable option.

[kalessil]

And you response helped, thank you!

[kalessil]

The plugin will respect this pattern, while PS improves types analysis.