Phpinspectionsea: Can not iterate 'string' - ForeachSourceInspector

[funivan]

Probably this is false-psitive.

  $files = [
    '1.html',
    '2.html',
  ];
  foreach ($files as $file) { // Get and error
    echo $file;
  }

Plugin: 2.3.4

PhpStorm 2017.1 EAP
Build #PS-171.3691.14, built on March 1, 2017
PhpStorm EAP User
Expiration date: March 31, 2017
JRE: 1.8.0_112-release-724-b6 amd64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
Linux 4.4.0-64-generic

[rentalhost]

I am running same version than you and I don't get any error (even with master.jar). Are you really running 2.3.4? The ForeachSourceInspector seems to be modified on this version, but on a non-related issue #160.

[kalessil]

160 applies to function calls only, which are defined in stubs.

I convinced it's due to EAP-related changes, seems to be a good time to give a try to JB Toolbox =)

[rentalhost]

@kalessil Wow... It was very strange... I just closed and opened the PS and I receive the error. haha Still more strange: I closed PS again and no error found.

[kalessil]

o_O, O_O, x_x

[rentalhost]

I never seem that before. I am always running EAP versions, but nothing like that happen at anytime. @funivan Can you select all code, cut and paste on same place? Just to check if the second time will detect some error. On my case, it gones too. Now I don't know in which moment it can happen.

[kalessil]

@funivan : can you try reproducing the same behaviour please as @rentalhost ?

[funivan]

Yes. It still happen. I just cut/paste/cut etc =) Does not help(

[funivan]

I find a way how to reproduce it. This bug affect variables that are defined/used in the global scope.

1. Create empty php project
2. Create file with the following content

 $files = [
    '1.html',
    '2.html',
  ];
  foreach ($files as $file) { // Get and error
    echo $file;
  }

At this stage inspection does not fire. Variable $files is ok.

1. Create new php file with following content:

  $files = 'asdf';

1. Return to the first file. We get an error.

p.s. I know that variable in global scope are bad practice and i`m working on it =)

[rjhdby]

Plugin version 2.3.4

    /**
     * @param \SimpleXMLElement $cell
     * @param null|string|string[] $values
     */
    private function setValue(&$cell, $values) {
        if (!is_array($values)) {
            $values = explode(PHP_EOL, $values);
        }
        foreach ($values as $string) {                              //This string marked as wrong
            $cell->addChild('text:p', $string, OdsDocument::NS_TEXT);
        }
    }

Cannot iterate 'null'...

Cannot iterate 'string'

Inside foreach $values can be an array only

[funivan]

@rjhdby this code is not so good. In php 7 with declare strict types it would not work properly ($values == null then you will get error)
Here is some solutions how to resolve your issue:
1.

   /**
     * @param \SimpleXMLElement $cell
     * @param null|string|string[] $values
     */
    private function setValue(&$cell, $values) {
      $items = [];
      if (is_string($values)) {
        $items = explode(PHP_EOL, $values);
      }
      foreach ($items as $string) {                                 //This string marked as wrong
        $cell->addChild('text:p', $string, OdsDocument::NS_TEXT);
      }
    }

2.

   /**
     * @param \SimpleXMLElement $cell
     * @param null|string|string[] $values
     */
    private function setValue(&$cell, $values) {
      if (!is_array($values)) {
        $values = explode(PHP_EOL, $values);
      }
      foreach ((array) $values as $string) {                                 //This string marked as wrong
        $cell->addChild('text:p', $string, OdsDocument::NS_TEXT);
      }
    }

[Schrank]

I have the same Problem in Magento. But as in https://github.com/kalessil/phpinspectionsea/issues/172#issuecomment-285271131 described, I think this is intended behaviour. There is no (easy) way to decide wether a file is used in global or non-global context as in

class A {
    public function B() {
        require 'file.php';
    }
}

//file.php
<?php

// looks like global context

[kalessil]

Let me check at least why types are so strangely recognized.

[kalessil]

Currently, it looks like we have to restrict the inspection scope: functions and methods only.
Then I could reduce a number of false-positives and cover all cases in this report. What do you think guys?

In the worth case, we could drop the inspections - it makes too many problems for quite a while.

[funivan]

Agree. Skip check in global scope

On 29 Mar 2017 9:14 p.m., "Vladimir Reznichenko" [e-mail]
wrote:

Currently, it looks like we have to restrict the scope: functions and
methods only.
Then I could reduce a number of false-positives and cover all cases in
this report. What do you think guys?

In the worth case, we could drop the inspections - it makes too many
problems for quite a while.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kalessil/phpinspectionsea/issues/172#issuecomment-290176851,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZ8-Hl73NNyJZC7lD-uSGXyMW7vxgf8ks5rqp9pgaJpZM4MVcYn
.

[kalessil]

On this today, going ahead with checking foreach source only inside functions/methods.

[kalessil]

@funivan , @Schrank , @rjhdby : I pushed a fresh binary into the master with fixes and looking for feedback. Does anyone of you want to run the fixed version this and next week?

[kalessil]

Closing, will re-open in case of negative feedback.

[funivan]

I will try ;)

[kalessil]

Great, tests are green but a battlefield is always spots new issues =)

[kalessil]

So, it seems like the last changes are good enough, or?

[funivan]

@kalessil works for 6 days with latest PhpInspectionsEA binary and all is ok =)

[kalessil]

Super =)