analyse closures (#8)

* analyse closures * cleanup * analyser test * ate own dog food
seferov · Oct 24, 2019 · ca36ff8 · ca36ff8
1 parent c12fc20
commit ca36ff8
Show file tree

Hide file tree

Showing 15 changed files with 211 additions and 48 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,3 +1,4 @@
+build
 bin/typhp.phar
 vendor
 composer.lock
diff --git a/.travis.yml b/.travis.yml
@@ -53,3 +53,6 @@ jobs:
         - chmod +x phpstan.phar
       script:
         - ./phpstan.phar analyse src --level=5
+
+notifications:
+  email: false
diff --git a/README.md b/README.md
@@ -71,7 +71,7 @@ Example output
 
 ## Todo
 
-- [ ] Analyse closures
+- [x] Analyse closures
 
 - [ ] Check by PHP version. For example, don't suppress `@param object` for >= PHP 7.2
 

diff --git a/phpunit.xml b/phpunit.xml
@@ -1,9 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/7.5/phpunit.xsd"
+         xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
          bootstrap="vendor/autoload.php"
-         forceCoversAnnotation="true"
-         beStrictAboutCoversAnnotation="true"
          beStrictAboutOutputDuringTests="true"
          beStrictAboutTodoAnnotatedTests="true"
          verbose="true">

diff --git a/src/Analyser.php b/src/Analyser.php
@@ -18,10 +18,6 @@ class Analyser
      * @var string
      */
     private $code;
-    /**
-     * @var string
-     */
-    private $fileName;
     /**
      * @var IssueCollection
      */
@@ -35,10 +31,9 @@ class Analyser
      */
     private $docBlockAnalyser;
 
-    public function __construct(string $fileName, string $code)
+    public function __construct(string $code)
     {
         $this->code = $code;
-        $this->fileName = $fileName;
         $this->issueCollection = new IssueCollection();
         $this->docBlockFactory  = DocBlockFactory::createInstance();
         $this->docBlockAnalyser  = new DocBlockAnalyser();
@@ -62,6 +57,14 @@ private function analyseNode(Node $node): void
     {
         if ($node instanceof Node\FunctionLike) {
             $this->analyseFunctionLike($node);
+        } elseif ($node instanceof Node\Stmt\Expression) {
+            $this->analyseNode($node->expr);
+        } elseif ($node instanceof Node\Expr\FuncCall || $node instanceof Node\Expr\MethodCall) {
+            foreach ($node->args as $arg) {
+                $this->analyseNode($arg->value);
+            }
+        } elseif ($node instanceof Node\Expr\Assign) {
+            $this->analyseNode($node->expr);
         }
 
         if (isset($node->stmts)) {
@@ -73,13 +76,17 @@ private function analyseNode(Node $node): void
 
     private function analyseFunctionLike(Node\FunctionLike $functionLike): void
     {
-        if (!$functionLike instanceof Node\Stmt\ClassMethod && !$functionLike instanceof Node\Stmt\Function_) {
-            // todo: support closures and arrow functions
+        if ($functionLike instanceof Node\Stmt\ClassMethod || $functionLike instanceof Node\Stmt\Function_) {
+            $name = $functionLike->name->name;
+            $line = $functionLike->name->getStartLine();
+        } elseif ($functionLike instanceof Node\Expr\Closure) {
+            $name = 'n/a (closure)';
+            $line = $functionLike->getStartLine();
+        } else {
+            // todo: support arrow functions (PHP 7.4)
             return;
         }
 
-        $name = $functionLike->name;
-
         $docBlock = null;
         if ($functionLike->getDocComment()) {
             try {
@@ -93,19 +100,19 @@ private function analyseFunctionLike(Node\FunctionLike $functionLike): void
             return;
         }
 
-        if ($functionLike instanceof Node\Stmt\ClassMethod && in_array($name->name, ['__construct', '__destruct', '__call', '__callStatic', '__get', '__set', '__isset', '__unset', '__sleep', '__wakeup', '__toString', '__invoke', '__set_state', '__clone', '__debugInfo'])) {
+        if ($functionLike instanceof Node\Stmt\ClassMethod && in_array($name, ['__construct', '__destruct', '__call', '__callStatic', '__get', '__set', '__isset', '__unset', '__sleep', '__wakeup', '__toString', '__invoke', '__set_state', '__clone', '__debugInfo'])) {
             $this->analyseSpecialMagicMethods($functionLike, $docBlock);
             return;
         }
 
-        $this->analyseParams($functionLike->getParams(), $name, $docBlock);
-        $this->analyseReturn($functionLike->getReturnType(), $name, $docBlock);
+        $this->analyseParams($functionLike->getParams(), $name, $line, $docBlock);
+        $this->analyseReturn($functionLike->getReturnType(), $name, $line, $docBlock);
     }
 
     /**
      * @param Node\Param[] $params
      */
-    private function analyseParams(array $params, Node\Identifier $name, ?DocBlock $docBlock): void
+    private function analyseParams(array $params, string $name, int $line, ?DocBlock $docBlock): void
     {
         foreach ($params as $param) {
             if (null !== $param->type) {
@@ -116,14 +123,14 @@ private function analyseParams(array $params, Node\Identifier $name, ?DocBlock $
                 continue;
             }
 
-            $this->issueCollection->add(UntypedArgumentIssue::create($name->name, $name->getStartLine(), $param->var->name));
+            $this->issueCollection->add(UntypedArgumentIssue::create($name, $line, $param->var->name));
         }
     }
 
     /**
      * @param null|Identifier|Node\Name|Node\NullableType $returnType
      */
-    private function analyseReturn($returnType, Node\Identifier $name, ?DocBlock $docBlock): void
+    private function analyseReturn($returnType, string $name, int $line, ?DocBlock $docBlock): void
     {
         if (null !== $returnType) {
             return;
@@ -133,92 +140,91 @@ private function analyseReturn($returnType, Node\Identifier $name, ?DocBlock $do
             return;
         }
 
-        $this->issueCollection->add(UntypedReturnIssue::create($name->name, $name->getStartLine()));
+        $this->issueCollection->add(UntypedReturnIssue::create($name, $line));
     }
 
     private function analyseSpecialMagicMethods(Node\Stmt\ClassMethod $classMethod, ?DocBlock $docBlock): void
     {
-        $name = $classMethod->name;
-        switch ($name->name) {
+        $name = $classMethod->name->name;
+        $line = $classMethod->name->getStartLine();
+        switch ($name) {
             case '__construct':
             case '__invoke':
-                $this->analyseParams($classMethod->getParams(), $name, $docBlock);
+                $this->analyseParams($classMethod->getParams(), $name, $line, $docBlock);
                 break;
             case '__call':
             case '__callStatic':
-                // string $name, array $arguments. ANALYSE
-                // mixed return type. ANALYSE
                 $params = $classMethod->getParams();
                 $firstParam = array_shift($params);
                 if (null === $firstParam->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $firstParam->var->name, 'string'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $firstParam->var->name, 'string'));
                 }
                 $secondParam = array_shift($params);
                 if (null === $secondParam->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $secondParam->var->name, 'array'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $secondParam->var->name, 'array'));
                 }
-                $this->analyseReturn($classMethod->getReturnType(), $name, $docBlock);
+                $this->analyseReturn($classMethod->getReturnType(), $name, $line, $docBlock);
                 break;
             case '__get':
                 $params = $classMethod->getParams();
                 if (null === $params[0]->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $params[0]->var->name, 'string'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $params[0]->var->name, 'string'));
                 }
-                $this->analyseReturn($classMethod->getReturnType(), $name, $docBlock);
+                $this->analyseReturn($classMethod->getReturnType(), $name, $line, $docBlock);
                 break;
             case '__set':
                 $params = $classMethod->getParams();
                 $firstParam = array_shift($params);
                 if (null === $firstParam->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $firstParam->var->name, 'string'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $firstParam->var->name, 'string'));
                 }
-                $this->analyseParams($params, $name, $docBlock);
-                $this->analyseReturn($classMethod->getReturnType(), $name, $docBlock);
+                $this->analyseParams($params, $name, $line, $docBlock);
+                $this->analyseReturn($classMethod->getReturnType(), $name, $line, $docBlock);
                 break;
             case '__unset':
                 $params = $classMethod->getParams();
                 $firstParam = array_shift($params);
                 if (null === $firstParam->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $firstParam->var->name, 'string'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $firstParam->var->name, 'string'));
                 }
                 $return = $classMethod->getReturnType();
                 if (null === $return) {
-                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name->name, $name->getStartLine(), 'void'));
+                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name, $line, 'void'));
                 }
                 break;
             case '__sleep':
             case '__debugInfo':
                 $return = $classMethod->getReturnType();
                 if (null === $return) {
-                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name->name, $name->getStartLine(), 'array'));
+                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name, $line, 'array'));
                 }
                 break;
             case '__wakeup':
                 $return = $classMethod->getReturnType();
                 if (null === $return) {
-                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name->name, $name->getStartLine(), 'void'));
+                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name, $line, 'void'));
                 }
                 break;
             case '__toString':
                 $return = $classMethod->getReturnType();
                 if (null === $return) {
-                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name->name, $name->getStartLine(), 'string'));
+                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name, $line, 'string'));
                 }
                 break;
             case '__isset':
                 $params = $classMethod->getParams();
                 if (null === $params[0]->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $params[0]->var->name, 'string'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $params[0]->var->name, 'string'));
                 }
                 $return = $classMethod->getReturnType();
                 if (null === $return) {
-                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name->name, $name->getStartLine(), 'bool'));
+                    $this->issueCollection->add(UntypedKnownReturnIssue::create($name, $line, 'bool'));
                 }
                 break;
             case '__set_state':
                 $params = $classMethod->getParams();
                 if (null === $params[0]->type) {
-                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name->name, $name->getStartLine(), $params[0]->var->name, 'array'));
+                    $this->issueCollection->add(UntypedKnownArgumentIssue::create($name, $line, $params[0]->var->name, 'array'));
                 }
                 break;
             case '__destruct':

diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php
@@ -27,6 +27,7 @@ class AnalyseCommand extends Command
     protected function configure(): void
     {
         $this
+            ->setAliases(['analyze'])
             ->addArgument('path', InputArgument::OPTIONAL, 'Path to analyse. Ignores config file if provided.')
             ->addOption('config', 'c', InputOption::VALUE_OPTIONAL, 'config file', getcwd().'/.typhp.yml')
             ->addOption('format', null, InputOption::VALUE_REQUIRED, 'format can be either table or compact', 'table')
@@ -54,7 +55,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         }
 
         foreach ($files as $file) {
-            $analyser = new Analyser($file->getPathname(), $file->getContents());
+            $analyser = new Analyser($file->getContents());
             try {
                 $issueCollection = $analyser->analyse();
             } catch (Error $e) {

diff --git a/src/Issue/UntypedArgumentIssue.php b/src/Issue/UntypedArgumentIssue.php
@@ -21,7 +21,7 @@ public static function create(string $name, int $line, string $argumentName): se
 
     public function getIssueCompact(): string
     {
-        return implode(';', [$this->getLine(), $this->getName(), 'untyped-argument', $this->argumentName]);
+        return implode(';', [$this->line, $this->name, 'untyped-argument', $this->argumentName]);
     }
 
     public function getIssue(): string

diff --git a/src/Issue/UntypedKnownArgumentIssue.php b/src/Issue/UntypedKnownArgumentIssue.php
@@ -26,7 +26,7 @@ public static function create(string $name, int $line, string $argumentName, str
 
     public function getIssueCompact(): string
     {
-        return implode(';', [$this->getLine(), $this->getName(), 'untyped-known-argument', $this->argumentName]);
+        return implode(';', [$this->line, $this->name, 'untyped-known-argument', $this->argumentName]);
     }
 
     public function getIssue(): string

diff --git a/src/Issue/UntypedKnownReturnIssue.php b/src/Issue/UntypedKnownReturnIssue.php
@@ -21,7 +21,7 @@ public static function create(string $name, int $line, string $type): self
 
     public function getIssueCompact(): string
     {
-        return implode(';', [$this->getLine(), $this->getName(), 'untyped-known-return']);
+        return implode(';', [$this->line, $this->name, 'untyped-known-return']);
     }
 
     public function getIssue(): string

diff --git a/src/Issue/UntypedReturnIssue.php b/src/Issue/UntypedReturnIssue.php
@@ -15,7 +15,7 @@ public static function create(string $name, int $line): self
 
     public function getIssueCompact(): string
     {
-        return implode(';', [$this->getLine(), $this->getName(), 'untyped-return']);
+        return implode(';', [$this->line, $this->name, 'untyped-return']);
     }
 
     public function getIssue(): string

diff --git a/tests/Functional/Scenarios/ClosuresTest.php b/tests/Functional/Scenarios/ClosuresTest.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace Seferov\Typhp\Tests\Functional\Scenarios;
+
+use Seferov\Typhp\Tests\Functional\FunctionalTestCase;
+
+class ClosuresTest extends FunctionalTestCase
+{
+    public function testNoIssues(): void
+    {
+        $output = $this->process('tests/Functional/Scenarios/Fixtures/ClosuresNoIssues.php');
+        $outputLines = $output->getLines();
+        $this->assertCount(0, $outputLines);
+        $this->assertSame(0, $output->getExitCode());
+    }
+
+    public function testWithIssues(): void
+    {
+        $output = $this->process('tests/Functional/Scenarios/Fixtures/ClosuresWithIssues.php');
+        $outputLines = $output->getLines();
+        $this->assertCount(8, $outputLines);
+        $this->assertSame(4, $output->getExitCode());
+
+        $this->assertSame('5;n/a (closure);untyped-argument;foo', array_shift($outputLines));
+        $this->assertSame('5;n/a (closure);untyped-return', array_shift($outputLines));
+        $this->assertSame('11;n/a (closure);untyped-argument;a', array_shift($outputLines));
+        $this->assertSame('11;n/a (closure);untyped-return', array_shift($outputLines));
+        $this->assertSame('17;foo;untyped-argument;foo', array_shift($outputLines));
+        $this->assertSame('17;foo;untyped-return', array_shift($outputLines));
+        $this->assertSame('23;n/a (closure);untyped-argument;bar', array_shift($outputLines));
+        $this->assertSame('23;n/a (closure);untyped-return', array_shift($outputLines));
+    }
+}
diff --git a/tests/Functional/Scenarios/Fixtures/ClosuresNoIssues.php b/tests/Functional/Scenarios/Fixtures/ClosuresNoIssues.php
@@ -0,0 +1,25 @@
+<?php
+
+namespace Seferov\Typhp\Tests\Functional\Scenarios\Fixtures;
+
+$foo = function (int $foo): int {
+    return $foo;
+};
+
+array_map($foo, [0, 1, 2]);
+
+array_filter([0, 1, 2], function (int $a): bool {
+    return $a > 1;
+});
+
+class ClosuresNoIssues
+{
+    public function foo(callable $foo): void
+    {
+        $foo(1);
+    }
+}
+
+(new ClosuresNoIssues())->foo(function (int $bar): bool {
+    return $bar > 1;
+});
diff --git a/tests/Functional/Scenarios/Fixtures/ClosuresWithIssues.php b/tests/Functional/Scenarios/Fixtures/ClosuresWithIssues.php
@@ -0,0 +1,25 @@
+<?php
+
+namespace Seferov\Typhp\Tests\Functional\Scenarios\Fixtures;
+
+$foo = function ($foo) {
+    return $foo;
+};
+
+array_map($foo, [0, 1, 2]);
+
+array_filter([0, 1, 2], function ($a) {
+    return $a > 1;
+});
+
+class ClosuresWithIssues
+{
+    public function foo($foo)
+    {
+        $foo(1);
+    }
+}
+
+(new ClosuresWithIssues())->foo(function ($bar) {
+    return $bar > 1;
+});