Repository: tightenco/tlint (php)

Issue:
Add lint for spaces-around-concatenators
Needs opinion:

```php
$thing = '';
        
echo "a $thing";
echo "a " . $thing;
echo "a {$thing}";
```

Pull request description:
Correctly Formatted Concatenations Lint
- Drops unnecessary `LinterInterface`
- Adds `CorrectlyFormattedConcatenations` lint

closes #13

Patch:
diff --git a/src/BaseLinter.php b/src/BaseLinter.php
index 54fb22b..e453a83 100644
--- a/src/BaseLinter.php
+++ b/src/BaseLinter.php
@@ -4,9 +4,10 @@ namespace Tighten;
 
 use Illuminate\Filesystem\Filesystem;
 use Illuminate\View\Compilers\BladeCompiler;
+use PhpParser\Node;
 use PhpParser\Parser;
 
-class BaseLinter implements LinterInterface
+class BaseLinter
 {
     protected $description = 'No Description for Linter.';
     protected $extension;
@@ -57,6 +58,17 @@ class BaseLinter implements LinterInterface
         return $this->getCodeLines()[$line - 1];
     }
 
+    public function getCodeLinesFromNode(Node $node)
+    {
+        return array_reduce(
+            range($node->getStartLine(), $node->getEndLine()),
+            function ($carry, $line) {
+                return $carry . $this->getCodeLine($line);
+            },
+            ''
+        );
+    }
+
     public function getCodeLines()
     {
         return $this->codeLines;
diff --git a/src/Commands/LintCommand.php b/src/Commands/LintCommand.php
index b4564cc..b0f8278 100644
--- a/src/Commands/LintCommand.php
+++ b/src/Commands/LintCommand.php
@@ -15,8 +15,8 @@ use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Process\Exception\ProcessFailedException;
 use Symfony\Component\Process\Process;
+use Tighten\BaseLinter;
 use Tighten\Lint;
-use Tighten\LinterInterface;
 use Tighten\Linters\AlphabeticalImports;
 use Tighten\Linters\ApplyMiddlewareInRoutes;
 use Tighten\Linters\ClassThingsOrder;
@@ -37,6 +37,7 @@ use Tighten\Linters\QualifiedNamesOnlyForClassName;
 use Tighten\Linters\RemoveLeadingSlashNamespaces;
 use Tighten\Linters\SpaceAfterSoleNotOperator;
 use Tighten\Linters\SpacesAroundBladeRenderContent;
+use Tighten\Linters\ConcatenationSpacing;
 use Tighten\Linters\TrailingCommasOnArrays;
 use Tighten\Linters\UseAuthHelperOverFacade;
 use Tighten\Linters\ViewWithOverArrayParamaters;
@@ -79,7 +80,7 @@ class LintCommand extends Command
         $lints = [];
 
         foreach ($linters as $linterClass => $parseAs) {
-            /** @var LinterInterface $linterInstance */
+            /** @var BaseLinter $linterInstance */
             $linterInstance = new $linterClass(
                 file_get_contents($file),
                 $parseAs
@@ -302,6 +303,7 @@ class LintCommand extends Command
                 SpaceAfterSoleNotOperator::class => '.php',
                 OneLineBetweenClassVisibilityChanges::class => '.php',
                 NoStringInterpolationWithoutBraces::class => '.php',
+                ConcatenationSpacing::class => '.php',
             ],
             $this->getRoutesFilesLinters($path),
             $this->getControllerFilesLinters($path),
diff --git a/src/Lint.php b/src/Lint.php
index 1942364..315158f 100644
--- a/src/Lint.php
+++ b/src/Lint.php
@@ -9,7 +9,7 @@ class Lint
     private $linter;
     private $node;
 
-    public function __construct(LinterInterface $linter, Node $node)
+    public function __construct(BaseLinter $linter, Node $node)
     {
         $this->linter = $linter;
         $this->node = $node;
@@ -19,7 +19,7 @@ class Lint
     {
         if ($this->linter->getExtension() === '.php') {
             return '! ' . $this->linter->getLintDescription() . PHP_EOL
-                . $this->node->getLine() . ' : `' . $this->linter->getCodeLine($this->node->getLine()) . '`';
+                    . $this->node->getLine() . ' : `' . $this->linter->getCodeLine($this->node->getLine()) . '`';
         }
 
         if ($this->linter->getExtension() === '.blade.php') {
@@ -29,10 +29,7 @@ class Lint
         return 'Something is wrong.';
     }
 
-    /**
-     * @return LinterInterface
-     */
-    public function getLinter(): LinterInterface
+    public function getLinter(): BaseLinter
     {
         return $this->linter;
     }
diff --git a/src/LinterInterface.php b/src/LinterInterface.php
deleted file mode 100644
index 07c182b..0000000
--- a/src/LinterInterface.php
+++ /dev/null
@@ -1,18 +0,0 @@
-<?php
-
-namespace Tighten;
-
-use PhpParser\Parser;
-
-interface LinterInterface
-{
-    /**
-     * @param Parser $parser
-     * @return @return Lint[]
-     */
-    public function lint(Parser $parser);
-    public function getCodeLine(int $line);
-    public function getLintDescription();
-    public function setLintDescription(string $description);
-    public function getExtension();
-}
diff --git a/src/Linters/ConcatenationSpacing.php b/src/Linters/ConcatenationSpacing.php
new file mode 100644
index 0000000..0b5e257
--- /dev/null
+++ b/src/Linters/ConcatenationSpacing.php
@@ -0,0 +1,102 @@
+<?php
+
+namespace Tighten\Linters;
+
+use PhpParser\Node;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Expr\BinaryOp\Concat;
+use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitor\FindingVisitor;
+use PhpParser\Parser;
+use Tighten\BaseLinter;
+
+class ConcatenationSpacing extends BaseLinter
+{
+    protected $description = 'There should be 1 space around `.` concatenations, and additional lines should'
+        . ' always start with a `.`';
+
+    public function lint(Parser $parser)
+    {
+        $traverser = new NodeTraverser;
+
+        $visitor = new FindingVisitor(function (Node $node) {
+            static $startLine;
+
+            if ($node instanceof Concat) {
+                /**
+                 * Stop multiple lints triggers for a single concat
+                 */
+                if ($startLine === $node->getStartLine()) {
+                    return false;
+                }
+
+                $startLine = $node->getStartLine();
+                $stringLiteralCorrectlyFormattedConcatCount = 0;
+                $concatCount = 1;
+                $left = $node->left;
+
+                /**
+                 * Count correctly formatted raw string concats
+                 */
+                if ($left instanceof String_) {
+                    $stringLiteralCorrectlyFormattedConcatCount += $this->countCorrectlyFormattedConcats($left->value);
+                }
+
+                /**
+                 * Count concat operations via parser tokens
+                 */
+                while ($left instanceof Concat) {
+                    $concatCount += 1;
+
+                    $left = $left->left;
+                }
+
+                $totalCodeLine = $node->getEndLine() > $node->getStartLine()
+                    ? $this->getCodeLinesFromNode($node)
+                    : $this->getCodeLine($node->getLine());
+
+                $correctlyFormattedCodeLineConcatCount = $this->countCorrectlyFormattedConcats($totalCodeLine);
+
+                $concatCount -= $this->countAdditionalLinesThatStartWithConcat($node);
+
+                /**
+                 * Compare the parsed vs raw string (correct) count
+                 */
+                return $correctlyFormattedCodeLineConcatCount - $stringLiteralCorrectlyFormattedConcatCount
+                    < $concatCount;
+            }
+
+            return false;
+        });
+
+        $traverser->addVisitor($visitor);
+
+        $traverser->traverse($parser->parse($this->code));
+
+        return $visitor->getFoundNodes();
+    }
+
+    private function countCorrectlyFormattedConcats(string $string)
+    {
+        preg_match_all('/(?<= )(?<!  |^)\.(?= )(?!  |$)/', $string, $matches);
+
+        return count($matches[0] ?? []);
+    }
+
+    /**
+     * Count non-first line start of line concats (with space after)
+     */
+    private function countAdditionalLinesThatStartWithConcat(Node $node)
+    {
+        $concatCount = 0;
+
+        if ($node->getEndLine() > $node->getStartLine()) {
+            foreach (range($node->getStartLine() + 1, $node->getEndLine()) as $lineNumber) {
+                preg_match_all('/^\s*\.(?= )(?!  |$)/', $this->getCodeLine($lineNumber), $matches);
+                $concatCount += count($matches[0] ?? []) > 0 ? 1 : 0;
+            }
+        }
+
+        return $concatCount;
+    }
+}
diff --git a/src/TLint.php b/src/TLint.php
index 49bda7b..01b168e 100644
--- a/src/TLint.php
+++ b/src/TLint.php
@@ -3,6 +3,7 @@
 namespace Tighten;
 
 use PhpParser\Error;
+use PhpParser\Lexer;
 use PhpParser\Node;
 use PhpParser\NodeAbstract;
 use PhpParser\ParserFactory;
@@ -13,14 +14,10 @@ class TLint
 
     public function __construct()
     {
-        $this->parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7);
+        $this->parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7, new Lexer);
     }
 
-    /**
-     * @param LinterInterface $linter
-     * @return Lint[]
-     */
-    public function lint(LinterInterface $linter)
+    public function lint(BaseLinter $linter)
     {
         return array_map(function (Node $node) use ($linter) {
             return new Lint($linter, $node);


Test patch:
diff --git a/tests/Linters/ConcatenationSpacingTest.php b/tests/Linters/ConcatenationSpacingTest.php
new file mode 100644
index 0000000..fa31b60
--- /dev/null
+++ b/tests/Linters/ConcatenationSpacingTest.php
@@ -0,0 +1,135 @@
+<?php
+
+use PHPUnit\Framework\TestCase;
+use Tighten\Linters\ConcatenationSpacing;
+use Tighten\TLint;
+
+class ConcatenationSpacingTest extends TestCase
+{
+    /** @test */
+    public function catches_concat_without_spaces()
+    {
+        $file = <<<file
+<?php
+
+echo "foo bar . ".\$baz;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function catches_concat_with_too_many_spaces()
+    {
+        $file = <<<file
+<?php
+
+echo "foo bar . "  . \$baz;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function catches_concat_with_space_on_one_side()
+    {
+        $file = <<<file
+<?php
+
+echo "foo bar . ". \$baz;
+echo "foo bar . " .\$baz;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+        $this->assertEquals(4, $lints[1]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function does_not_trigger_on_concat_with_spaces()
+    {
+        $file = <<<file
+<?php
+
+echo "foo bar . " . \$baz;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function handles_valid_multiline_concat()
+    {
+        $file = <<<file
+<?php
+
+echo '! ' . \$this->linter->getLintDescription() . PHP_EOL
+                . \$this->node->getLine() . ' : `' . \$this->linter->getCodeLine(\$this->node->getLine()) . '`';
+
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function triggers_on_multiline_concat()
+    {
+        $file = <<<file
+<?php
+
+echo getcwd() . '/'
+    .\$a;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function triggers_on_multiline_concat_where_lines_do_not_start_with_concat()
+    {
+        $file = <<<file
+<?php
+
+       \$directory = 'images/articles' . DIRECTORY_SEPARATOR .
+            date('Y') . DIRECTORY_SEPARATOR .
+            date('m') . DIRECTORY_SEPARATOR .
+            date('d') . DIRECTORY_SEPARATOR;
+            
+file;
+
+        $lints = (new TLint)->lint(
+            new ConcatenationSpacing($file)
+        );
+
+        $this->assertEquals(1, count($lints));
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+    }
+}