Fix phparkitect#126 improve error messages including classname when p…

…ossible Added ViolationDescription, as input for a Violation Added a description to the ViolationDescription - Only for Expressions where it made sense
annervisser · Aug 16, 2022 · b791c37 · b791c37
1 parent c76a3cf
commit b791c37
Show file tree

Hide file tree

Showing 29 changed files with 197 additions and 100 deletions.
diff --git a/src/Expression/Expression.php b/src/Expression/Expression.php
@@ -18,7 +18,7 @@ interface Expression
      * Returns a human readable description of the expression
      * $theClass can be used to add contextual information.
      */
-    public function describe(ClassDescription $theClass, string $because): Description;
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription;
 
     /**
      * Evaluates the expression for the class passed as parameter.

diff --git a/src/Expression/Description.php → src/Expression/ExpressionDescription.php b/src/Expression/Description.php → src/Expression/ExpressionDescription.php
@@ -4,7 +4,7 @@
 
 namespace Arkitect\Expression;
 
-class Description
+class ExpressionDescription
 {
     /** @var string */
     private $description;

diff --git a/src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php b/src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php
@@ -6,8 +6,9 @@
 
 use Arkitect\Analyzer\ClassDependency;
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -21,11 +22,11 @@ public function __construct(string ...$namespace)
         $this->namespaces = $namespace;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
         $desc = implode(', ', $this->namespaces);
 
-        return new Description("should depend only on classes in one of these namespaces: $desc", $because);
+        return new ExpressionDescription("should depend only on classes in one of these namespaces: $desc", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -44,7 +45,10 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
             if (!$dependency->matchesOneOf(...$this->namespaces)) {
                 $violation = Violation::createWithErrorLine(
                     $theClass->getFQCN(),
-                    $this->describe($theClass, $because)->toString(),
+                    ViolationDescription::withDescription(
+                        $this->describe($theClass, $because),
+                        "depends on {$dependency->getFQCN()->toString()}"
+                    ),
                     $dependency->getLine()
                 );
 

diff --git a/src/Expression/ForClasses/DocBlockContains.php b/src/Expression/ForClasses/DocBlockContains.php
@@ -5,8 +5,9 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -20,17 +21,17 @@ public function __construct(string $docBlock)
         $this->docBlock = $docBlock;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("should have a doc block that contains {$this->docBlock}", $because);
+        return new ExpressionDescription("should have a doc block that contains {$this->docBlock}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
         if (!$theClass->containsDocBlock($this->docBlock)) {
             $violation = Violation::create(
                 $theClass->getFQCN(),
-                $this->describe($theClass, $because)->toString()
+                ViolationDescription::selfExplanatory($this->describe($theClass, $because))
             );
             $violations->add($violation);
         }

diff --git a/src/Expression/ForClasses/DocBlockNotContains.php b/src/Expression/ForClasses/DocBlockNotContains.php
@@ -5,8 +5,9 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -20,17 +21,17 @@ public function __construct(string $docBlock)
         $this->docBlock = $docBlock;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("should not have a doc block that contains {$this->docBlock}", $because);
+        return new ExpressionDescription("should not have a doc block that contains {$this->docBlock}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
         if ($theClass->containsDocBlock($this->docBlock)) {
             $violation = Violation::create(
                 $theClass->getFQCN(),
-                $this->describe($theClass, $because)->toString()
+                ViolationDescription::selfExplanatory($this->describe($theClass, $because))
             );
             $violations->add($violation);
         }

diff --git a/src/Expression/ForClasses/Extend.php b/src/Expression/ForClasses/Extend.php
@@ -5,8 +5,9 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -20,9 +21,9 @@ public function __construct(string $className)
         $this->className = $className;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("should extend {$this->className}", $because);
+        return new ExpressionDescription("should extend {$this->className}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -35,7 +36,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
 
         $violation = Violation::create(
             $theClass->getFQCN(),
-            $this->describe($theClass, $because)->toString()
+            ViolationDescription::selfExplanatory($this->describe($theClass, $because))
         );
 
         $violations->add($violation);

diff --git a/src/Expression/ForClasses/HaveAttribute.php b/src/Expression/ForClasses/HaveAttribute.php
@@ -4,8 +4,9 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -19,9 +20,9 @@ public function __construct(string $attribute)
         $this->attribute = $attribute;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("should have the attribute {$this->attribute}", $because);
+        return new ExpressionDescription("should have the attribute {$this->attribute}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -33,7 +34,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
         $violations->add(
             Violation::create(
                 $theClass->getFQCN(),
-                $this->describe($theClass, $because)->toString()
+                ViolationDescription::selfExplanatory($this->describe($theClass, $because))
             )
         );
     }

diff --git a/src/Expression/ForClasses/HaveNameMatching.php b/src/Expression/ForClasses/HaveNameMatching.php
@@ -6,8 +6,9 @@
 
 use Arkitect\Analyzer\ClassDescription;
 use Arkitect\Analyzer\FullyQualifiedClassName;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -21,9 +22,9 @@ public function __construct(string $name)
         $this->name = $name;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("should have a name that matches {$this->name}", $because);
+        return new ExpressionDescription("should have a name that matches {$this->name}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -32,7 +33,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
         if (!$fqcn->classMatches($this->name)) {
             $violation = Violation::create(
                 $theClass->getFQCN(),
-                $this->describe($theClass, $because)->toString()
+                ViolationDescription::selfExplanatory($this->describe($theClass, $because))
             );
             $violations->add($violation);
         }

diff --git a/src/Expression/ForClasses/Implement.php b/src/Expression/ForClasses/Implement.php
@@ -6,8 +6,9 @@
 
 use Arkitect\Analyzer\ClassDescription;
 use Arkitect\Analyzer\FullyQualifiedClassName;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
@@ -21,9 +22,9 @@ public function __construct(string $interface)
         $this->interface = $interface;
     }
 
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("should implement {$this->interface}", $because);
+        return new ExpressionDescription("should implement {$this->interface}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -37,7 +38,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
         if (0 === \count(array_filter($interfaces, $implements))) {
             $violation = Violation::create(
                 $theClass->getFQCN(),
-                $this->describe($theClass, $because)->toString()
+                ViolationDescription::selfExplanatory($this->describe($theClass, $because))
             );
             $violations->add($violation);
         }

diff --git a/src/Expression/ForClasses/IsAbstract.php b/src/Expression/ForClasses/IsAbstract.php
@@ -5,16 +5,17 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
 class IsAbstract implements Expression
 {
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("{$theClass->getName()} should be abstract", $because);
+        return new ExpressionDescription("{$theClass->getName()} should be abstract", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
 
         $violation = Violation::create(
             $theClass->getFQCN(),
-            $this->describe($theClass, $because)->toString()
+            ViolationDescription::selfExplanatory($this->describe($theClass, $because))
         );
 
         $violations->add($violation);

diff --git a/src/Expression/ForClasses/IsFinal.php b/src/Expression/ForClasses/IsFinal.php
@@ -5,16 +5,17 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
 class IsFinal implements Expression
 {
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("{$theClass->getName()} should be final", $because);
+        return new ExpressionDescription("{$theClass->getName()} should be final", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
 
         $violation = Violation::create(
             $theClass->getFQCN(),
-            $this->describe($theClass, $because)->toString()
+            ViolationDescription::selfExplanatory($this->describe($theClass, $because))
         );
 
         $violations->add($violation);

diff --git a/src/Expression/ForClasses/IsNotAbstract.php b/src/Expression/ForClasses/IsNotAbstract.php
@@ -5,16 +5,17 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
 class IsNotAbstract implements Expression
 {
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("{$theClass->getName()} should not be abstract", $because);
+        return new ExpressionDescription("{$theClass->getName()} should not be abstract", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
 
         $violation = Violation::create(
             $theClass->getFQCN(),
-            $this->describe($theClass, $because)->toString()
+            ViolationDescription::selfExplanatory($this->describe($theClass, $because))
         );
 
         $violations->add($violation);

diff --git a/src/Expression/ForClasses/IsNotFinal.php b/src/Expression/ForClasses/IsNotFinal.php
@@ -5,16 +5,17 @@
 namespace Arkitect\Expression\ForClasses;
 
 use Arkitect\Analyzer\ClassDescription;
-use Arkitect\Expression\Description;
 use Arkitect\Expression\Expression;
+use Arkitect\Expression\ExpressionDescription;
+use Arkitect\Expression\ViolationDescription;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
 
 class IsNotFinal implements Expression
 {
-    public function describe(ClassDescription $theClass, string $because): Description
+    public function describe(ClassDescription $theClass, string $because): ExpressionDescription
     {
-        return new Description("{$theClass->getName()} should not be final", $because);
+        return new ExpressionDescription("{$theClass->getName()} should not be final", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
@@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
 
         $violation = Violation::create(
             $theClass->getFQCN(),
-            $this->describe($theClass, $because)->toString()
+            ViolationDescription::selfExplanatory($this->describe($theClass, $because))
         );
 
         $violations->add($violation);