Fixes #29, Fixes #30, Adds sorting (#49)

NielsdeBlaauw · Feb 6, 2019 · ee37a81 · ee37a81
1 parent ec6b006
commit ee37a81
Show file tree

Hide file tree

Showing 15 changed files with 98 additions and 28 deletions.
diff --git a/README.md b/README.md
@@ -61,6 +61,12 @@ Options:
   -g, --grouping-method <arg>               Allows different grouping of the
                                             results list [file, none, metric,
                                             severity, fileline] [default: file]
+  -s, --sorting-method <arg>                Sorting for the results. Natural
+                                            sorts by name for groups and line
+                                            for findings. Value uses the
+                                            cumulative group score, and finding
+                                            score as sorting value. [natural,
+                                            value] [default: natural]
   -i, --ignore-violations-on-exit           Will exit with a zero code, even if
                                             any violations are found
   -?, --help                                Show this help and quit

diff --git a/bin/php-doc-check b/bin/php-doc-check
@@ -54,7 +54,7 @@ $parser = (new \PhpParser\ParserFactory)->create(
 
 $analysisResults = array();
 $fileFinder = new \NdB\PhpDocCheck\FileFinder();
-$groupManager = new \NdB\PhpDocCheck\GroupManager($arguments->getOption('grouping-method'));
+$groupManager = new \NdB\PhpDocCheck\GroupManager($arguments->getOption('grouping-method'), $arguments->getOption('sorting-method'));
 foreach ($fileFinder->getFiles($arguments) as $name => $object) {
     $file = new \NdB\PhpDocCheck\AnalysableFile(new SplFileInfo($name), $parser, $arguments, $groupManager);
     $result = $file->analyse();

diff --git a/src/AnalysableFile.php b/src/AnalysableFile.php
@@ -34,7 +34,8 @@ public function analyse() : AnalysisResult
                 sprintf('Failed parsing: %s', $e->getRawMessage()),
                 new InvalidFileNode,
                 $this,
-                new \NdB\PhpDocCheck\Metrics\InvalidFile()
+                new \NdB\PhpDocCheck\Metrics\InvalidFile(),
+                0
             );
             $analysisResult->addProgress($finding);
             $this->groupManager->addFinding($finding);

diff --git a/src/ApplicationArgumentsProvider.php b/src/ApplicationArgumentsProvider.php
@@ -36,6 +36,13 @@ public function __construct()
                     '[file, none, metric, severity, fileline] [default: file]'
                 )
                 ->setDefaultValue('file'),
+            \GetOpt\Option::create('s', 'sorting-method', \GetOpt\GetOpt::REQUIRED_ARGUMENT)
+                ->setDescription(
+                    'Sorting for the results. Natural sorts by name for groups and line for findings. '.
+                    'Value uses the cumulative group score, and finding score as sorting value. '.
+                    '[natural, value] [default: natural]'
+                )
+                ->setDefaultValue('file'),
             \GetOpt\Option::create('i', 'ignore-violations-on-exit', \GetOpt\GetOpt::NO_ARGUMENT)
                 ->setDescription('Will exit with a zero code, even if any violations are found'),
             \GetOpt\Option::create('?', 'help', \GetOpt\GetOpt::NO_ARGUMENT)

diff --git a/src/Findings/Finding.php b/src/Findings/Finding.php
@@ -2,25 +2,31 @@
 
 namespace NdB\PhpDocCheck\Findings;
 
-abstract class Finding implements \JsonSerializable, Groupable
+abstract class Finding implements \JsonSerializable, Groupable, \NdB\PhpDocCheck\Sortable
 {
     public $message;
     public $node;
     public $sourceFile;
     public $metric;
+    public $value;
 
     public function __construct(
         string $message,
         \PhpParser\Node $node,
         \NdB\PhpDocCheck\AnalysableFile $sourceFile,
-        \NdB\PhpDocCheck\Metrics\Metric $metric
+        \NdB\PhpDocCheck\Metrics\Metric $metric,
+        int $value
     ) {
         $this->message    = $message;
         $this->node       = $node;
         $this->sourceFile = $sourceFile;
         $this->metric     = $metric;
+        $this->value      = $value;
     }
 
+    /**
+     * Based on the group key, new groups are made by the groupManager.
+     */
     public function getGroupKey(string $groupingMethod) : string
     {
         switch ($groupingMethod) {
@@ -55,11 +61,20 @@ public function getMessage():string
 
     abstract public function getType():string;
 
+    public function getSortValue($sortMethod): string
+    {
+        if ($sortMethod === 'value') {
+            return (string) $this->value;
+        }
+        return (string) $this->getLine();
+    }
+
     public function jsonSerialize() : array
     {
         return array(
             'message'=> $this->getMessage(),
             'type'=> $this->getType(),
+            'value'=> $this->value,
             'metric'=> $this->metric,
             'sourceFile'=> $this->sourceFile,
             'node'=> array(

diff --git a/src/GroupManager.php b/src/GroupManager.php
@@ -5,24 +5,36 @@
 class GroupManager implements GroupContainer
 {
     protected $groupingMethod = 'file';
+    protected $sortingMethod = 'natural';
     protected $groups = array();
-    public function __construct($groupingMethod)
+    public function __construct(string $groupingMethod, string $sortingMethod)
     {
         $this->groupingMethod = $groupingMethod;
+        $this->sortingMethod  = $sortingMethod;
     }
 
     public function addFinding(Findings\Groupable $finding)
     {
         if (!array_key_exists($finding->getGroupKey($this->groupingMethod), $this->groups)) {
             $this->groups[$finding->getGroupKey($this->groupingMethod)] = new ResultGroup(
-                $finding->getGroupKey($this->groupingMethod)
+                $finding->getGroupKey($this->groupingMethod),
+                $this->sortingMethod
             );
         }
         $this->groups[$finding->getGroupKey($this->groupingMethod)]->addFinding($finding);
     }
 
     public function getGroups(): array
     {
+        uasort($this->groups, function ($prev, $next) {
+            if ($prev->getSortValue($this->sortingMethod) == $next->getSortValue($this->sortingMethod)) {
+                return 0;
+            }
+            return ($prev->getSortValue($this->sortingMethod) < $next->getSortValue($this->sortingMethod)) ? -1 : 1;
+        });
+        if ($this->sortingMethod === 'value') {
+            $this->groups = array_reverse($this->groups, true);
+        }
         return $this->groups;
     }
 }
diff --git a/src/Metrics/CognitiveComplexity.php b/src/Metrics/CognitiveComplexity.php
@@ -24,7 +24,6 @@ final class CognitiveComplexity implements Metric
         'Expr_BinaryOp_Coalesce',
         'Expr_Ternary',
     );
-    public $value;
 
     public function getName():string
     {
@@ -33,10 +32,7 @@ public function getName():string
 
     public function getValue(\PhpParser\Node $node):int
     {
-        if (is_null($this->value)) {
-            $this->value = $this->calculateNodeValue($node, 0);
-        }
-        return $this->value;
+        return $this->calculateNodeValue($node, 0);
     }
 
     /**
@@ -74,8 +70,7 @@ protected function isComplexNode(\PhpParser\Node $node): bool
     public function jsonSerialize() : array
     {
         return array(
-            'name'=>$this->getName(),
-            'value'=>$this->value
+            'name'=>$this->getName()
         );
     }
 }
diff --git a/src/Metrics/CyclomaticComplexity.php b/src/Metrics/CyclomaticComplexity.php
@@ -21,7 +21,6 @@ final class CyclomaticComplexity implements Metric
         'Expr_Ternary',
     );
 
-    public $value;
 
     public function getName():string
     {
@@ -30,10 +29,7 @@ public function getName():string
 
     public function getValue(\PhpParser\Node $node):int
     {
-        if (is_null($this->value)) {
-            $this->value = $this->calculateNodeValue($node) + 1;
-        }
-        return $this->value;
+        return $this->calculateNodeValue($node) + 1;
     }
 
     /**
@@ -69,7 +65,6 @@ public function jsonSerialize() : array
     {
         return array(
             'name'=>$this->getName(),
-            'value'=>$this->value
         );
     }
 }
diff --git a/src/NodeVisitor.php b/src/NodeVisitor.php
@@ -45,7 +45,8 @@ public function leaveNode(\PhpParser\Node $node)
                         sprintf("%s has no documentation and a complexity of %d", $name, $metricValue),
                         $node,
                         $this->sourceFile,
-                        $this->metric
+                        $this->metric,
+                        $metricValue
                     );
                     $this->analysisResult->addProgress($finding);
                     $this->groupManager->addFinding($finding);
@@ -54,7 +55,8 @@ public function leaveNode(\PhpParser\Node $node)
                         sprintf("%s has no documentation and a complexity of %d", $name, $metricValue),
                         $node,
                         $this->sourceFile,
-                        $this->metric
+                        $this->metric,
+                        $metricValue
                     );
                     $this->analysisResult->addProgress($finding);
                     $this->groupManager->addFinding($finding);

diff --git a/src/Output/Formats/Text.php b/src/Output/Formats/Text.php
@@ -18,7 +18,7 @@ protected function getFileOutput(\NdB\PhpDocCheck\ResultGroup $resultGroup)
     {
         $output = '';
         $output .= "\n";
-        $output .= sprintf("Group: %s\n", $resultGroup->getName());
+        $output .= sprintf("Group: %s (score: %d)\n", $resultGroup->getName(), $resultGroup->getValue());
         $header = array(
             'Severity',
             'Message',

diff --git a/src/ResultGroup.php b/src/ResultGroup.php
@@ -2,14 +2,16 @@
 
 namespace NdB\PhpDocCheck;
 
-class ResultGroup implements \JsonSerializable
+class ResultGroup implements \JsonSerializable, Sortable
 {
     public $name;
+    public $sortingMethod;
     protected $findings = array();
 
-    public function __construct(string $name)
+    public function __construct(string $name, string $sortingMethod)
     {
         $this->name = $name;
+        $this->sortingMethod = $sortingMethod;
     }
 
     public function getName():string
@@ -24,6 +26,15 @@ public function addFinding(Findings\Finding $finding)
 
     public function getFindings(): array
     {
+        uasort($this->findings, function ($prev, $next) {
+            if ($prev->getSortValue($this->sortingMethod) == $next->getSortValue($this->sortingMethod)) {
+                return 0;
+            }
+            return ($prev->getSortValue($this->sortingMethod) < $next->getSortValue($this->sortingMethod)) ? -1 : 1;
+        });
+        if ($this->sortingMethod === 'value') {
+            $this->findings = array_reverse($this->findings, true);
+        }
         return $this->findings;
     }
 
@@ -32,6 +43,22 @@ public function jsonSerialize() : array
         return array(
             'groupName'=>$this->getName(),
             'findings'=>$this->getFindings(),
+            'value'=>$this->getValue()
         );
     }
+
+    public function getValue() : int
+    {
+        return array_reduce($this->getFindings(), function (int $carry, Findings\Finding $finding) {
+            return $carry + $finding->value;
+        }, 0);
+    }
+
+    public function getSortValue($sortMethod): string
+    {
+        if ($sortMethod === 'value') {
+            return (string) $this->getValue();
+        }
+        return $this->getName();
+    }
 }
diff --git a/src/Sortable.php b/src/Sortable.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace NdB\PhpDocCheck;
+
+interface Sortable
+{
+    public function getSortValue($sortMethod): string;
+}
diff --git a/tests/AnalysisResultTest.php b/tests/AnalysisResultTest.php
@@ -28,7 +28,8 @@ public function testChangesStateWhenAnWarningIsAdded($analysisResult)
             "Basic warning",
             $this->node,
             $this->analysableFile,
-            $this->metric
+            $this->metric,
+            0
         );
         $analysisResult->addProgress($finding);
         $this->assertEquals('W', $analysisResult->getProgressIndicator());
@@ -44,7 +45,8 @@ public function testChangesStateWhenAnErrorIsAdded($analysisResult)
             "Basic error",
             $this->node,
             $this->analysableFile,
-            $this->metric
+            $this->metric,
+            1
         );
         $analysisResult->addProgress($finding);
         $this->assertEquals('E', $analysisResult->getProgressIndicator());

diff --git a/tests/NodeVisitorTest.php b/tests/NodeVisitorTest.php
@@ -16,7 +16,7 @@ public function testCanAnalyseNodesForWarningFindings()
         $analysableFile->arguments = $arguments;
         $metric = $this->createMock(\NdB\PhpDocCheck\Metrics\Metric::class);
         $metric->method('getValue')->willReturn(4);
-        $groupManager = new \NdB\PhpDocCheck\GroupManager('none');
+        $groupManager = new \NdB\PhpDocCheck\GroupManager('none', 'natural');
         $nodeVisitor = new NodeVisitor($analysisResult, $analysableFile, $metric, $groupManager);
         $node = $this->createMock(\PhpParser\Node\Stmt\Function_::class);
         $nodeVisitor->leaveNode($node);

diff --git a/tests/Output/Formats/TextTest.php b/tests/Output/Formats/TextTest.php
@@ -27,7 +27,7 @@ public function testCanOutputSimpleResults()
         $analysableFile = $this->createMock('\NdB\PhpDocCheck\AnalysableFile');
         $node = $this->createMock('\PhpParser\Node');
         $results->method('getFindings')->willReturn(array(
-            new \NdB\PhpDocCheck\Findings\Warning("Basic warning", $node, $analysableFile, $metric)
+            new \NdB\PhpDocCheck\Findings\Warning("Basic warning", $node, $analysableFile, $metric, 0)
         ));
         $results->sourceFile = $this->createMock(\NdB\PhpDocCheck\AnalysableFile::class);
         $results->sourceFile->file = $this->createMock(\SplFileInfo::class);