From this class DefaultModuleHelper. Coder complains about this:

  protected function setFactory(IFactory $factory) {
    if (empty($factory)) {
      throw new \Exception('IFactory $factory must not be empty !');
    }
    if ($factory->getModuleName() != $this->getModule()) {
      throw new Exception(
      'The module machine name(' . $factory->getModuleName() .
      ') of the provided factory does match' .
      'The module machine name(' . $this->getModule() . ') of this !'
          );
    }
    $this->factory = $factory;
    return $this;
  }

Coder report:

ERROR   | String concat is not required here; use a single string
     |         | instead

I also tried this, it still complains:

    $error = 'The module machine name(' . $factory->getModuleName() .
      ') of the provided factory does match' .
      'The module machine name(' . $this->getModule() . ') of this !';
      throw new Exception($error);

I can't see anything wrong with either of these forms, they are perfectly acceptable PHP concatenations, I consider it a bug.

Comments

klausi’s picture

Version: 7.x-2.2 » 8.x-2.x-dev
Component: Review/Rules » Coder Sniffer

You are concatenated two string literals, which is not necessary, right? You are doing it for the sake of breaking the line I assume.

I guess we should improve the sniff for string literals concatenated on the same line. I think this applies to 8.x-2.x, too, so it should be fixed there first and then backported.

webel’s picture

thanks, the point is that although there are lots of ways of performing string concatenation in PHP, the one shown is valid, it should not trip Coder.

anoopjohn’s picture

@klausi - what should be the fix? The error should trigger only if the literals are on the same line?

glennmcewan’s picture

+1 for scrapping this sniff; it simply becomes a problem when attempting to make code more readable. Are there any benefits of requiring this sniff, other than helping to prevent redundancy? It makes sense to detect two adjacent string literals (although it still seems a bit superfluous), but not when there is a line break separating them.