diff --git a/.gitignore b/.gitignore index 6302db5..4f2dc80 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +build bin/typhp.phar vendor composer.lock diff --git a/.travis.yml b/.travis.yml index ecb041d..42e2404 100644 --- 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 index 3ae543d..b0eab09 100644 --- 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 index ad9b759..c287656 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,9 +1,7 @@ diff --git a/src/Analyser.php b/src/Analyser.php index 8c5e8e8..ca47e6d 100644 --- 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 index e501944..4a38819 100644 --- 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 index 0dd3c59..6098dd3 100644 --- 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 index 8e4f4b0..5c674a8 100644 --- 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 index 015e63c..ea9d3c0 100644 --- 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 index 3803757..47122a3 100644 --- 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 new file mode 100644 index 0000000..655f0ac --- /dev/null +++ b/tests/Functional/Scenarios/ClosuresTest.php @@ -0,0 +1,33 @@ +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 new file mode 100644 index 0000000..12c858d --- /dev/null +++ b/tests/Functional/Scenarios/Fixtures/ClosuresNoIssues.php @@ -0,0 +1,25 @@ + 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 new file mode 100644 index 0000000..e1493c1 --- /dev/null +++ b/tests/Functional/Scenarios/Fixtures/ClosuresWithIssues.php @@ -0,0 +1,25 @@ + 1; +}); + +class ClosuresWithIssues +{ + public function foo($foo) + { + $foo(1); + } +} + +(new ClosuresWithIssues())->foo(function ($bar) { + return $bar > 1; +}); diff --git a/tests/Unit/AnalyserTest.php b/tests/Unit/AnalyserTest.php new file mode 100644 index 0000000..67e8980 --- /dev/null +++ b/tests/Unit/AnalyserTest.php @@ -0,0 +1,61 @@ + 0; + } + + $foo = function ($foo) { + return $foo; + }; + + array_map($foo, [0, 1, 2]); + + class ClosuresWithIssues + { + public function foo($foo) + { + $foo(1); + } + + /** + * {@inheritDoc} + */ + public function inheritdoc() + { + return $this->foo(); + } + + /** + * @return object|array + */ + public function multiple() + { + return $this->foo(); + } + } + + (new ClosuresWithIssues())->foo(function ($bar) { + return $bar > 1; + }); + '; + + $analyser = new Analyser($code); + $issueCollection = $analyser->analyse(); + $this->assertInstanceOf(IssueCollection::class, $issueCollection); + $this->assertCount(8, $issueCollection); + + $this->assertInstanceOf(IssueInterface::class, $issueCollection->current()); + } +} diff --git a/tests/Unit/DocBlockAnalyserTest.php b/tests/Unit/DocBlockAnalyserTest.php index a97c292..51b807f 100644 --- a/tests/Unit/DocBlockAnalyserTest.php +++ b/tests/Unit/DocBlockAnalyserTest.php @@ -60,6 +60,12 @@ public function InheritParamSuppressedData(): array ]; } + public function testInheritParamNotSuppressed(): void + { + $docBlockAnalyser = new DocBlockAnalyser(); + $this->assertFalse($docBlockAnalyser->isSuppressedByInheritDoc($this->getDocBlock('/** @inheritDoc */'))); + } + public function testParamNotSuppressed(): void { $docComment = ' @@ -67,6 +73,7 @@ public function testParamNotSuppressed(): void * @param string $foo * @param DocBlock $bar * @param array|null $baz + * @param $variable */ '; @@ -75,6 +82,8 @@ public function testParamNotSuppressed(): void $this->assertFalse($docBlockAnalyser->isParamSuppressedByDocBlock('foo', $docBlock)); $this->assertFalse($docBlockAnalyser->isParamSuppressedByDocBlock('bar', $docBlock)); $this->assertFalse($docBlockAnalyser->isParamSuppressedByDocBlock('baz', $docBlock)); + $this->assertFalse($docBlockAnalyser->isParamSuppressedByDocBlock('variable', $docBlock)); + $this->assertFalse($docBlockAnalyser->isParamSuppressedByDocBlock('variableDoesNotExistInDoc', $docBlock)); } public function testReturnSuppressed(): void @@ -83,7 +92,7 @@ public function testReturnSuppressed(): void $this->assertTrue($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return mixed */'))); $this->assertTrue($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return object */'))); - $this->assertTrue($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return object|return */'))); + $this->assertTrue($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return bool|int */'))); } public function testReturnNotSuppressed(): void @@ -95,6 +104,7 @@ public function testReturnNotSuppressed(): void $this->assertFalse($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return DocBlock */'))); $this->assertFalse($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return array|null */'))); $this->assertFalse($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @return int|null */'))); + $this->assertFalse($docBlockAnalyser->isReturnSuppressedByDocBlock($this->getDocBlock('/** @param int $foo */'))); } private function getDocBlock(string $docComment): DocBlock