リファクタリングのサンプル実装

src/Parse/VariableParser.php

@@ -5,125 +5,65 @@
 namespace Smeghead\PhpVariableHardUsage\Parse;
 
 use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Stmt;
-use PhpParser\Node\Stmt\ClassMethod;
-use PhpParser\Node\Stmt\Class_;
-use PhpParser\Node\Stmt\Function_;
-use PhpParser\NodeDumper;
-use PhpParser\NodeVisitor\FindingVisitor;
+use PhpParser\Node\FunctionLike;
+use PhpParser\NodeFinder;
+use PhpParser\NodeTraverser;
 use PhpParser\Parser;
 use Smeghead\PhpVariableHardUsage\Parse\Exception\ParseFailedException;
+use Smeghead\PhpVariableHardUsage\Parse\Visitor\FunctionLikeFindingVisitor;
 
 final class VariableParser
 {
     private Parser $parser;
 
+    private NodeFinder $nodeFinder;
+
     public function __construct()
     {
         $this->parser = (new \PhpParser\ParserFactory())->createForNewestSupportedVersion();
+        $this->nodeFinder = new NodeFinder();
     }
 
-    /**
-     * @param list<Stmt> $stmt
-     * @return list<Function_>
-     */
-    private function getFunctions(array $stmt): array
+    public function parse(string $content): ParseResult
     {
-        $functionVisitor = new FindingVisitor(function ($node) {
-            return $node instanceof Function_;
-        });
-        $traverser = new \PhpParser\NodeTraverser();
-        $traverser->addVisitor($functionVisitor);
-        $traverser->traverse($stmt);
+        $stmts = $this->parser->parse($content);
+        if ($stmts === null) {
+            throw new ParseFailedException();
+        }
 
-        return $functionVisitor->getFoundNodes(); // @phpstan-ignore-line
-    }
+        $traverser = new NodeTraverser();
+        $visitor = new FunctionLikeFindingVisitor(fn($node) => $node instanceof FunctionLike);
+        $traverser->addVisitor($visitor);
+        $traverser->traverse($stmts);
+        $functionLikes = $visitor->getFoundNodes();
 
-    /**
-     * @param Function_|ClassMethod $function
-     * @return list<Variable>
-     */
-    private function getVariables(Function_|ClassMethod $function): array
-    {
-        $variableVisitor = new FindingVisitor(function ($node) {
-            return $node instanceof Variable;
-        });
-        $traverser = new \PhpParser\NodeTraverser();
-        $traverser->addVisitor($variableVisitor);
-        $traverser->traverse([$function]);
+        $functions = $this->collectParseResultPerFunctionLike($functionLikes);
 
-        return $variableVisitor->getFoundNodes(); // @phpstan-ignore-line
+        return new ParseResult($functions);
     }
 
     /**
-     * @param list<Stmt> $stmts
+     * @param list<FunctionLike> $functionLikes
      * @return list<Func>
      */
-    private function parseFunctions(array $stmts): array
+    private function collectParseResultPerFunctionLike(array $functionLikes): array
     {
-        $foundFunctions = $this->getFunctions($stmts);
-
-        $functions = [];
-        foreach ($foundFunctions as $foundFunction) {
-            $variables = $this->getVariables($foundFunction);
-            $func = new Func($foundFunction->name->name);
+        return array_map(function (FunctionLike $function) {
+            $className = $function->getAttribute('className'); // Get the class name set in FunctionLikeFindingVisitor
+            $functionName = $function->name->name ?? $function->getType() . '@' . $function->getStartLine();
+            $functionIdentifier = sprintf(
+                '%s%s',
+                isset($className) ? $className . '::' : '',
+                $functionName
+            );
+
+            $func = new Func($functionIdentifier);
+
+            $variables = $this->nodeFinder->findInstanceOf($function, Variable::class);
             foreach ($variables as $variable) {
                 $func->addVariable(new VarReference($variable->name, $variable->getLine())); // @phpstan-ignore-line
             }
-            $functions[] = $func;
-        }
-        return $functions;
-    }
-
-    /**
-     * @param list<Stmt> $stmt
-     * @return list<Class_>
-     */
-    private function getClasses(array $stmt): array
-    {
-        $classVisitor = new FindingVisitor(function ($node) {
-            return $node instanceof \PhpParser\Node\Stmt\Class_;
-        });
-        $traverser = new \PhpParser\NodeTraverser();
-        $traverser->addVisitor($classVisitor);
-        $traverser->traverse($stmt);
-
-        return $classVisitor->getFoundNodes(); // @phpstan-ignore-line
-    }
-
-    /**
-     * @param list<Stmt> $stmts
-     * @return list<Func>
-     */
-    private function parseClasses(array $stmts): array
-    {
-        $foundClasses = $this->getClasses($stmts);
-
-        $methods = [];
-        foreach ($foundClasses as $foundClass) {
-            foreach ($foundClass->getMethods() as $method) {
-                $variables = $this->getVariables($method);
-                $func = new Func(sprintf('%s::%s', $foundClass->name, $method->name->name));
-                foreach ($variables as $variable) {
-                    $func->addVariable(new VarReference($variable->name, $variable->getLine())); // @phpstan-ignore-line
-                }
-                $methods[] = $func;
-            }
-        }
-        return $methods;
-    }
-
-    public function parse(string $content): ParseResult
-    {
-        $stmts = $this->parser->parse($content);
-        if ($stmts === null) {
-            throw new ParseFailedException();
-        }
-        $dumper = new NodeDumper();
-        // echo $dumper->dump($stmts) . PHP_EOL;
-
-        $functions = $this->parseFunctions($stmts) + $this->parseClasses($stmts);
-
-        return new ParseResult($functions);
+            return $func;
+        }, $functionLikes);
     }
 }

src/Parse/Visitor/FunctionLikeFindingVisitor.php

@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Smeghead\PhpVariableHardUsage\Parse\Visitor;
+
+use PhpParser\Node;
+use PhpParser\NodeVisitor\FindingVisitor;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
+
+/**
+ * FindingVisitor that searches for elements of FunctionLike
+ * 
+ * For ClassMethod, set the class name to ClassName as Attribute.
+ */
+class FunctionLikeFindingVisitor extends FindingVisitor {
+    protected array $foundNodes = [];
+    private ?string $currentClass = null;
+
+    public function enterNode(Node $node) {
+        if ($node instanceof Class_) {
+            // Record class name
+            $this->currentClass = $node->name ? $node->name->name : null;
+        }
+
+        if ($node instanceof ClassMethod) {
+            $node->setAttribute('className', $this->currentClass);
+        }
+        return parent::enterNode($node);
+    }
+
+    public function leaveNode(Node $node) {
+        if ($node instanceof Class_) {
+            // Reset because it leaves the class scope
+            $this->currentClass = null;
+        }
+    }
+}

test/VariableParserTest.php

@@ -51,4 +51,27 @@ public function testParseClass(): void
         $this->assertSame('num', $vars[3]->name);
         $this->assertSame(15, $vars[3]->lineNumber, 'second $num (15)');
     }
+
+    public function testParseAnonymousFunction(): void
+    {
+        $parser = new VariableParser();
+        $content = file_get_contents($this->fixtureDir . '/AnonymousFunction.php');
+        $result = $parser->parse($content);
+
+        $this->assertInstanceOf(ParseResult::class, $result);
+        $functions = $result->functions;
+        $this->assertCount(1, $functions);
+        $this->assertEquals('Expr_Closure@4', $functions[0]->name);
+        $this->assertCount(4, $functions[0]->getVariables());
+
+        $vars = $functions[0]->getVariables();
+        $this->assertSame('a', $vars[0]->name);
+        $this->assertSame(4, $vars[0]->lineNumber, 'first $a (4)');
+        $this->assertSame('b', $vars[1]->name);
+        $this->assertSame(4, $vars[1]->lineNumber, 'second $b (4)');
+        $this->assertSame('a', $vars[2]->name);
+        $this->assertSame(5, $vars[2]->lineNumber, 'second $a (5)');
+        $this->assertSame('b', $vars[3]->name);
+        $this->assertSame(5, $vars[3]->lineNumber, 'second $b (5)');
+    }
 }

test/fixtures/AnonymousFunction.php

@@ -0,0 +1,6 @@
+<?php
+
+$items = ['ca', 'aa', 'bc', 'ab', 'ac'];
+usort($items, function (string $a, string $b): int {
+    return $a <=> $b;
+});

test/fixtures/improved_my_average.php

@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * 最小値と最大値を除いて平均を返却します。
+ */
+function improved_my_avearge(array $items): float {
+    sort($items);
+    $newItems = array_slice($items, 1, count($items) - 2);
+    $sum = array_sum($newItems);
+    return $sum / count($newItems);
+}
+
+$items = [3, 99, 40, 45, 50, 52];
+print_r(improved_my_avearge($items) . PHP_EOL);

test/fixtures/ugly_my_average.php

@@ -0,0 +1,31 @@
+<?php
+
+/**
+ * 最小値と最大値を除いて平均を返却します。
+ */
+function ugly_my_average(array $items): float {
+    $minValue = PHP_INT_MAX;
+    $maxValue = PHP_INT_MIN;
+
+    for ($i = 0; $i < count($items); $i++) {
+        $v = $items[$i];
+        if ($v < $minValue) {
+            $minValue = $v;
+        }
+        if ($v > $maxValue) {
+            $maxValue = $v;
+        }
+    }
+    $sum = 0;
+    for ($i = 0; $i < count($items); $i++) {
+        $v = $items[$i];
+        if (in_array($v, [$minValue, $maxValue])) {
+            continue;
+        }
+        $sum += $v;
+    }
+    return $sum / (count($items) - 2);
+}
+
+$items = [3, 99, 40, 45, 50, 52];
+print_r(ugly_my_average($items) . PHP_EOL);