- Fixes bug with old version where duplicate traits and methods could…

… be inserted into a class.

- Won't import identical abstract methods from a trait, but will let you declare them.
- Throw an error when importing the same method from multiple traits (use aliasing and insteadof to avoid).
- Duplicate properties must be identical type, visibility, and default literal value in order to be imported from a trait.
- Duplicate constants must be identical type, visibility, and default literal value in order to be import from a trait.
- All classes and enums checked for duplicate method names.
- Todo: index phase silently discards errors and tries to continue, but analysis phase emits errors.

src/Checks/DuplicateMemberCheck.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace BambooHR\Guardrail\Checks;
+
+use BambooHR\Guardrail\Output\OutputInterface;
+use BambooHR\Guardrail\Scope;
+use BambooHR\Guardrail\SymbolTable\SymbolTable;
+use PhpParser\Builder\Interface_;
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
+use PhpParser\Node\Stmt\Enum_;
+
+class DuplicateMemberCheck extends BaseCheck {
+
+	function getCheckNodeTypes() {
+		return [ Class_::class, Interface_::class, Enum_::class];
+	}
+
+	function run($fileName, Node $node, ClassLike $inside = null, Scope $scope = null) {
+		if ($node instanceof ClassLike) {
+			$members = ["method"=>[], "property"=>[], "constant"=>[]];
+
+			foreach ($node->stmts as $stmt) {
+				if ($stmt instanceof Node\Stmt\ClassMethod) {
+					$this->addOrEmitError($fileName, $stmt, $members, $stmt->name->name, "method");
+				} else if ($stmt instanceof Node\Stmt\Property) {
+					foreach ($stmt->props as $prop) {
+						$this->addOrEmitError($fileName, $prop, $members, $prop->name->name, "property");
+					}
+				} else if ($stmt instanceof Node\Stmt\ClassConst) {
+					foreach ($stmt->consts as $const) {
+						$this->addOrEmitError($fileName, $const, $members, $const->name->name, "constant");
+					}
+				}
+			}
+		}
+	}
+
+	function addOrEmitError(string $fileName, Node $node, array &$members, string $name,string $type) {
+		$normalizedName = ($type == 'method') ? strtolower($name) : $name;
+		if (!isset($members[$type][$normalizedName])) {
+			$members[$type][$normalizedName] = $node->getLine();
+		} else {
+			$this->emitError($fileName, $node, ErrorConstants::TYPE_DUPLICATE_NAME, "Duplicate $type " . $name . " first declared on line " . $members[$type][$normalizedName]);
+		}
+	}
+}

src/Checks/ErrorConstants.php

@@ -60,6 +60,7 @@ class ErrorConstants {
 	const TYPE_UNIMPLEMENTED_METHOD = 'Standard.Inheritance.Unimplemented';
 	const TYPE_UNKNOWN_CLASS = 'Standard.Unknown.Class';
 	const TYPE_OVERRIDE_BASE_CLASS = 'Standard.Override.Base';
+	const TYPE_DUPLICATE_NAME = 'Standard.Duplicate.Name';
 
 	const TYPE_UNDOCUMENTED_EXCEPTION = 'Standard.Undocumented.Exception';
 

src/NodeVisitors/StaticAnalyzer.php

@@ -18,6 +18,7 @@
 use BambooHR\Guardrail\Checks\CyclomaticComplexityCheck;
 use BambooHR\Guardrail\Checks\DefinedConstantCheck;
 use BambooHR\Guardrail\Checks\DocBlockTypesCheck;
+use BambooHR\Guardrail\Checks\DuplicateMemberCheck;
 use BambooHR\Guardrail\Checks\EnumCheck;
 use BambooHR\Guardrail\Checks\FunctionCallCheck;
 use BambooHR\Guardrail\Checks\GotoCheck;
@@ -158,6 +159,7 @@ function __construct(SymbolTable $index, OutputInterface $output, MetricOutputIn
 			new ThrowsCheck($this->index, $output),
 			new CountableEmptinessCheck($this->index, $output),
 			new DependenciesOnVendorCheck($this->index, $output, $metricOutput),
+			new DuplicateMemberCheck($this->index, $output),
 			//new ClassStoredAsVariableCheck($this->index, $output)
 		];
 

src/NodeVisitors/TraitImportingVisitor.php

@@ -25,11 +25,6 @@ class TraitImportingVisitor extends NodeVisitorAbstract {
 	private $importer;
 
 
-	/**
-	 * @var array
-	 */
-	private $classStack = [];
-
 	/**
 	 * TraitImportingVisitor constructor.
 	 *
@@ -39,37 +34,16 @@ public function __construct( SymbolTable $index) {
 		$this->importer  = new TraitImporter($index);
 	}
 
-
-	/**
-	 * enterNode
-	 *
-	 * @param Node $node Instance of Node
-	 *
-	 * @return null
-	 */
-	public function enterNode(Node $node) {
-		if ($node instanceof Class_ || $node instanceof Trait_ || $node instanceof Node\Stmt\Enum_) {
-			array_push($this->classStack, $node);
-		}
-		return null;
-	}
-
 	/**
 	 * leaveNode
 	 *
 	 * @param Node $node Instance of Node
 	 *
-	 * @return array|null
+	 * @return int|null|array
 	 */
 	public function leaveNode(Node $node) {
 		if ($node instanceof Class_ || $node instanceof Trait_ || $node instanceof Enum_) {
-			array_pop($this->classStack);
-		} else if ($node instanceof Node\Stmt\TraitUse) {
-
-			$class = end($this->classStack);
-			assert($class);
-			$traits = $this->importer->resolveTraits($node, $class);
-			return $traits;
+			$node->stmts = $this->importer->processClassLike($node);
 		}
 		return null;
 	}

src/Output/XUnitOutput.php

@@ -203,7 +203,7 @@ private function fileMatchesArrayOrString($fileName, $haystack) : bool {
 		if (is_array($haystack)) {
 			return $this->fileExistsInArray($fileName, $haystack);
 		} else {
-			return Glob::match("/" . $fileName, "/" . $haystack);
+			return self::globCache($fileName, $haystack);
 		}
 	}
 
@@ -213,9 +213,19 @@ private function fileMatchesArrayOrString($fileName, $haystack) : bool {
 	 *
 	 * @return bool
 	 */
+
+	private static function globCache(string $fileName, string $pattern):bool {
+		static $cacheFile = null;
+		static $matches = [];
+		if ($cacheFile == $fileName && isset($matches[$pattern])) {
+			return $matches[$pattern];
+		}
+		return $matches[$pattern] = Glob::match("/" . $fileName, "/" . $pattern);
+	}
+
 	private function fileExistsInArray($fileName, array $entryArray) : bool {
 		foreach ($entryArray as $entryItem) {
-			if (Glob::match("/" . $fileName, "/" . $entryItem)) {
+			if (self::globCache($fileName, $entryItem)) {
 				return true;
 			}
 		}

src/SymbolTable/SymbolTable.php

@@ -314,7 +314,7 @@ public function adjustBasePath(string  $fileName):string {
 	 */
 	public function removeBasePath(string $fileName):string {
 		if ($this->basePath!=="" && strpos($fileName, $this->basePath) === 0) {
-			return substr($fileName, strlen($this->basePath)+1);
+			return substr($fileName, strlen($this->basePath));
 		} else {
 			return $fileName;
 		}