Merge pull request #92 from BambooHR/jgardiner/backed-enum-value

Fix EnumFoo::EnumCaseFoo to be in instance of EnumFoo for backed enums.
BambooHR · Feb 2, 2024 · 9d60dc0 · 9d60dc0
2 parents 626a429 + a7f1562
commit 9d60dc0
Show file tree

Hide file tree

Showing 12 changed files with 165 additions and 56 deletions.
diff --git a/README.md b/README.md
@@ -1,11 +1,11 @@
 # Guardrail - A PHP Static Analysis tool
-Copyright (c) 2017-2021 Jon Gardiner and BambooHR
+Copyright (c) 2017-2024 BambooHR
 
 [![Latest Stable Version](https://poser.pugx.org/bamboohr/guardrail/v/stable.png)](https://packagist.org/packages/bamboohr/guardrail) [![Total Downloads](https://poser.pugx.org/bamboohr/guardrail/downloads)](https://packagist.org/packages/bamboohr/guardrail) [![Latest Unstable Version](https://poser.pugx.org/bamboohr/guardrail/v/unstable)](https://packagist.org/packages/bamboohr/guardrail) [![License](https://poser.pugx.org/bamboohr/guardrail/license)](https://packagist.org/packages/bamboohr/guardrail) [![composer.lock](https://poser.pugx.org/bamboohr/guardrail/composerlock)](https://packagist.org/packages/bamboohr/guardrail)  
 
 ## Introduction
 
-Guardrail is a static analysis engine for PHP 7.  Guardrail will index your code base, learn
+Guardrail is a static analysis engine for PHP 8.3.  Guardrail will index your code base, learn
 every symbol, and then confirm that every file in the system uses those symbols in a way that
 makes sense.  For example, if you have a function call to an undefined function, it will be
 found by Guardrail.
@@ -56,50 +56,52 @@ Guardrail classifies checks by name.  Here is the standard list of errors.  Note
  start with the word "Standard."  Custom plugins, should begin with a different string.  (Ideally, an 
  organization name for the organization creating the plugin.)
 
-Name | Description
---- | ---
-Standard.Access.Violation | Accessing a protected/private variable in a context where you are not allowed to access them.
-Standard.Autoload.Unsafe | Code that executes any statements other than a class declaration.
-Standard.ConditionalAssignment | Assigning a variable in conditional expression of an if() statement.
-Standard.Constructor.MissingCall | Overriding a constructor without calling the parent constructor
-Standard.Debug | Typical debug statements such as var_dump() or print_r()
-Standard.Deprecated.Internal | Call to an internal PHP function that is deprecated
-Standard.Deprecated.User | Call to a user function that has @deprecated in the docblock.
-Standard.Exception.Base | Catching the base \Exception class instead of something more specific.
-Standard.Incorrect.Static | Static reference to a dynamic variable/method
-Standard.Incorrect.Dynamic | Dynamic reference to a static variable/method
-Standard.Inheritance.Unimplemented | Class implementing an interface fails to implement on of it's methods.
-Standard.Function.InsideFunction | Declaring a function inside of another function.  (Closures/lambdas are still allowed.)
-Standard.Global.Expression | Referencing $GLOBALS[ $expr ]
-Standard.Global.String | Referencing a global with either global $var or $GLOBALS['var']
-Standard.Goto | Any instance of a "goto" statement
-Standard.Metrics.Complexity | Any method/function with a cyclomatic complexity of 10 or greater.
-Standard.Param.Count | Failure to pass all the declared parameters to a function.
-Standard.Param.Count.Excess | Passing too many variables to a function (ignores variadic functions)
-Standard.Param.Type | Type mismatch on a parameter to a function
-Standard.Parse.Error | A parse error
-Standard.Psr4 | The namespace of the class must match in the final parts of the path with a ".php" on the end.
-Standard.Return.Type | Type mismatch on a return from a function
-Standard.Scope | Usage of parent:: or self:: when in a context where they are not available.
-Standard.Security.Eval | Code that runs eval() or create_function()
-Standard.Security.Shell | Code that runs a shell (exec, passthru, system, etc)
-Standard.Security.Backtick | The backtick operator
-Standard.Switch.Break | A switch case: statement that falls through (generally these are unintentional)
-Standard.Switch.BreakMultiple | A "continue #;" or "break #;" statement (where # is an integer)
-Standard.Unknown.Callable | A callable that can't be resolved into a class method or function.
-Standard.Unknown.Class | Reference to an undefined class
-Standard.Unknown.Class.Constant | Reference to an undefined constant inside of a class
-Standard.Unknown.Class.Method | Reference to an unknown class method
-Standard.Unknown.Class.MethodString | Occurrences of Foo::class."@bar" where Foo::bar doesn't exist.
-Standard.Unknown.Function | Reference to an unknown function
-Standard.Unknown.Global.Constant | Reference to an undefined global constant (define or const)
-Standard.Unknown.Property | Reference to a property that has not previously been declared
-Standard.Unknown.Variable | Reference to a variable that has not previously been assigned
-Standard.Unsafe.Timezone | Functions, such as date() that use a server setting for timezone instead of explicitly passing the timezone.
-Standard.Unused.Variable | A local variable is assigned but never read from.
-Standard.Unreachable | Code inside a block after a return, break, continue, etc.
-Standard.VariableFunctionCall | Call a method $foo() when $foo is a string.  (Still ok if $foo is a callable)
-Standard.VariableVariable | Referencing a variable with $$var
+| Name                                | Description                                                                                                  |
+|-------------------------------------|--------------------------------------------------------------------------------------------------------------|
+| Standard.Access.Violation           | Accessing a protected/private variable in a context where you are not allowed to access them.                |
+| Standard.Autoload.Unsafe            | Code that executes any statements other than a class declaration.                                            |
+| Standard.ConditionalAssignment      | Assigning a variable in conditional expression of an if() statement.                                         |
+| Standard.Constructor.MissingCall    | Overriding a constructor without calling the parent constructor                                              |
+| Standard.Debug                      | Typical debug statements such as var_dump() or print_r()                                                     |
+| Standard.Deprecated.Internal        | Call to an internal PHP function that is deprecated                                                          |
+| Standard.Deprecated.User            | Call to a user function that has @deprecated in the docblock.                                                |
+| Standard.Exception.Base             | Catching the base \Exception class instead of something more specific.                                       |
+| Standard.Incorrect.ReadOnly         | Attempting to build an illegal readonly property (default value or non-typed)                                |
+| Standard.Incorrect.Static           | Static reference to a dynamic variable/method                                                                |
+| Standard.Incorrect.Dynamic          | Dynamic reference to a static variable/method                                                                |
+| Standard.Inheritance.Unimplemented  | Class implementing an interface fails to implement on of it's methods.                                       |
+| Standard.Function.InsideFunction    | Declaring a function inside of another function.  (Closures/lambdas are still allowed.)                      |
+| Standard.Global.Expression          | Referencing $GLOBALS\[ $expr ]                                                                               |
+| Standard.Global.String              | Referencing a global with either global $var or $GLOBALS\['var']                                             |
+| Standard.Goto                       | Any instance of a "goto" statement                                                                           |
+| Standard.Override.Base              | Attempt to use a #\[Override] on method in a base class                                                      |
+| Standard.Metrics.Complexity         | Any method/function with a cyclomatic complexity of 10 or greater.                                           |
+| Standard.Param.Count                | Failure to pass all the declared parameters to a function.                                                   |
+| Standard.Param.Count.Excess         | Passing too many variables to a function (ignores variadic functions)                                        |
+| Standard.Param.Type                 | Type mismatch on a parameter to a function                                                                   |
+| Standard.Parse.Error                | A parse error                                                                                                |
+| Standard.Psr4                       | The namespace of the class must match in the final parts of the path with a ".php" on the end.               |
+| Standard.Return.Type                | Type mismatch on a return from a function                                                                    |
+| Standard.Scope                      | Usage of parent:: or self:: when in a context where they are not available.                                  |
+| Standard.Security.Eval              | Code that runs eval() or create_function()                                                                   |
+| Standard.Security.Shell             | Code that runs a shell (exec, passthru, system, etc)                                                         |
+| Standard.Security.Backtick          | The backtick operator                                                                                        |
+| Standard.Switch.Break               | A switch case: statement that falls through (generally these are unintentional)                              |
+| Standard.Switch.BreakMultiple       | A "continue #;" or "break #;" statement (where # is an integer)                                              |
+| Standard.Unknown.Callable           | A callable that can't be resolved into a class method or function.                                           |
+| Standard.Unknown.Class              | Reference to an undefined class                                                                              |
+| Standard.Unknown.Class.Constant     | Reference to an undefined constant inside of a class                                                         |
+| Standard.Unknown.Class.Method       | Reference to an unknown class method                                                                         |
+| Standard.Unknown.Class.MethodString | Occurrences of Foo::class."@bar" where Foo::bar doesn't exist.                                               |
+| Standard.Unknown.Function           | Reference to an unknown function                                                                             |
+| Standard.Unknown.Global.Constant    | Reference to an undefined global constant (define or const)                                                  |
+| Standard.Unknown.Property           | Reference to a property that has not previously been declared                                                |
+| Standard.Unknown.Variable           | Reference to a variable that has not previously been assigned                                                |
+| Standard.Unsafe.Timezone            | Functions, such as date() that use a server setting for timezone instead of explicitly passing the timezone. |
+| Standard.Unused.Variable            | A local variable is assigned but never read from.                                                            |
+| Standard.Unreachable                | Code inside a block after a return, break, continue, etc.                                                    |
+| Standard.VariableFunctionCall       | Call a method $foo() when $foo is a string.  (Still ok if $foo is a callable)                                |
+| Standard.VariableVariable           | Referencing a variable with $$var                                                                            |
 
 
  Guardrail has support for advanced PHP features, such as traits, interfaces, anonymous functions & classes, etc.

diff --git a/src/Abstractions/ClassAbstraction.php b/src/Abstractions/ClassAbstraction.php
@@ -154,7 +154,7 @@ public function getConstantExpr($name):null|Expr|Name {
 			foreach($constants as $enumOption) {
 				/** @var EnumCase $enumOption */
 				if (strcasecmp($enumOption->name,$name)==0) {
-					return $enumOption->expr ?? $this->class->namespacedName;
+					return $this->class->namespacedName;
 				}
 			}
 		}

diff --git a/src/Abstractions/ReflectedClass.php b/src/Abstractions/ReflectedClass.php
@@ -190,10 +190,9 @@ public function isEnum(): bool {
 	}
 
 	public function isReadOnly(): bool {
-		/*
 		if (method_exists($this->refl,"isReadOnly")) {
 			return $this->refl->isReadOnly();
-		}*/
+		}
 		return false;
 	}
 }
diff --git a/src/Checks/ErrorConstants.php b/src/Checks/ErrorConstants.php
@@ -54,6 +54,7 @@ class ErrorConstants {
 	const TYPE_SIGNATURE_TYPE_NULL = "Standard.Null.Param";
 	const TYPE_UNIMPLEMENTED_METHOD = 'Standard.Inheritance.Unimplemented';
 	const TYPE_UNKNOWN_CLASS = 'Standard.Unknown.Class';
+	const TYPE_OVERRIDE_BASE_CLASS = 'Standard.Override.Base';
 	const TYPE_UNKNOWN_CLASS_CONSTANT = 'Standard.Unknown.Class.Constant';
 	const TYPE_UNKNOWN_FUNCTION = 'Standard.Unknown.Function';
 	const TYPE_UNKNOWN_GLOBAL_CONSTANT = 'Standard.Unknown.Global.Constant';

diff --git a/src/Checks/ParamTypesCheck.php b/src/Checks/ParamTypesCheck.php
@@ -15,7 +15,6 @@
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Function_;
-use PhpParser\Node\UnionType;
 
 /**
  * Class ParamTypesCheck
@@ -80,12 +79,18 @@ public function run($fileName, Node $node, ClassLike $inside=null, Scope $scope=
 			$this->checkForNestedFunction($fileName, $node, $inside, $scope);
 		}
 
+		if ($node instanceof ClassMethod) {
+			$this->checkForBadOverride($node, $inside, $fileName);
+		}
+
 		if ($node instanceof Function_) {
 			$displayName = $node->name;
-		} else if ($node instanceof ClassMethod) {
-			$displayName = $node->name;
 		} else {
-			$displayName = "closure function";
+			if ($node instanceof ClassMethod) {
+				$displayName = $node->name;
+			} else {
+				$displayName = "closure function";
+			}
 		}
 
 		if ($node instanceof Node\FunctionLike) {
@@ -94,7 +99,7 @@ public function run($fileName, Node $node, ClassLike $inside=null, Scope $scope=
 				if ($param->type) {
 					$name = $param->type;
 					if (!$this->isAllowed($name, $inside)) {
-						$name=TypeComparer::typeToString($name);
+						$name = TypeComparer::typeToString($name);
 						$this->emitError($fileName, $node, ErrorConstants::TYPE_UNKNOWN_CLASS, "Reference to an unknown type '$name'' in parameter $index of $displayName");
 					}
 				}
@@ -103,12 +108,26 @@ public function run($fileName, Node $node, ClassLike $inside=null, Scope $scope=
 			if ($node->getReturnType()) {
 				$returnType = $node->getReturnType();
 				if (!$this->isAllowed($returnType, $inside)) {
-					$returnType=TypeComparer::typeToString($returnType);
+					$returnType = TypeComparer::typeToString($returnType);
 					$this->emitError($fileName, $node, ErrorConstants::TYPE_UNKNOWN_CLASS, "Reference to an unknown type '$returnType' in return value of $displayName");
 				}
 			}
 		}
 	}
+	function checkForBadOverride(ClassMethod $node, ClassLike $inside, string $fileName) {
+		$isOverload = Util::getPhpAttribute("Override", $node->attrGroups);
+		if ($isOverload) {
+			if ($inside instanceof Class_) {
+				if (!$inside->extends) {
+					$this->emitError($fileName, $node, ErrorConstants::TYPE_OVERRIDE_BASE_CLASS, "Attempt to override a method in a base class");
+				} else {
+					if (!Util::findAbstractedMethod($inside->extends,$node->name,$this->symbolTable)) {
+						$this->emitError($fileName, $node, ErrorConstants::TYPE_UNKNOWN_METHOD, "Impossible #[Override].  No method named ".$node->name."() found in ".$inside->extends." or any parent class.");
+					}
+				}
+			}
+		}
+	}
 
 	/**
 	 * checkForNestedFunction

diff --git a/src/Util.php b/src/Util.php
@@ -8,6 +8,8 @@
 
 use BambooHR\Guardrail\Abstractions\Property;
 use BambooHR\Guardrail\SymbolTable\SymbolTable;
+use PhpParser\Node\Attribute;
+use PhpParser\Node\AttributeGroup;
 use PhpParser\Node\Expr\Exit_;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\IntersectionType;
@@ -61,6 +63,18 @@ static public function isScalarType($name) {
 		return $name == 'bool' || $name == 'string' || $name == 'int' || $name == 'float' || $name=="false" || $name =="true";
 	}
 
+
+	static function getPhpAttribute(string $name, array $attrGroups):?Attribute {
+		foreach($attrGroups as $attrGroup) {
+			/** @var AttributeGroup $attrGroup */
+			foreach($attrGroup->attrs as $attribute) {
+				if (strcasecmp($name, $attribute->name)==0) {
+					return $attribute;
+				}
+			}
+		}
+		return null;
+	}
 	/**
 	 * isLegalNonObject
 	 *
@@ -105,14 +119,15 @@ static public function getMethodAccessLevel(ClassMethod $level) {
 			return "protected";
 		}
 		trigger_error("Impossible");
+		return "";
 	}
 
 	/**
 	 * matchesGlobs
 	 *
 	 * @param string $basePath The base path
 	 * @param string $path     The path
-	 * @param string $globArr  The rest
+	 * @param array  $globArr  The rest
 	 *
 	 * @return bool
 	 */

diff --git a/tests/units/Checks/TestData/TestEnumCheck.5.inc b/tests/units/Checks/TestData/TestEnumCheck.5.inc
@@ -0,0 +1,8 @@
+<?php
+
+enum Foo {
+	case Bar;
+	case Baz;
+}
+
+echo Foo::Bar->name;
diff --git a/tests/units/Checks/TestData/TestMethodOverride.1.inc b/tests/units/Checks/TestData/TestMethodOverride.1.inc
@@ -0,0 +1,15 @@
+<?php
+
+class Foo {
+	function foo() {
+		echo "Foo\n";
+	}
+}
+
+class Bar extends Foo
+{
+	#[Override]
+	function foo() {
+		echo "Foo\n";
+	}
+}
diff --git a/tests/units/Checks/TestData/TestMethodOverride.2.inc b/tests/units/Checks/TestData/TestMethodOverride.2.inc
@@ -0,0 +1,14 @@
+<?php
+
+class Foo {
+	function foo() {
+		echo "Foo\n";
+	}
+}
+
+class Bar {
+	#[Override]
+	function foo() {
+		echo "Foo\n";
+	}
+} 
diff --git a/tests/units/Checks/TestData/TestMethodOverride.3.inc b/tests/units/Checks/TestData/TestMethodOverride.3.inc
@@ -0,0 +1,14 @@
+<?php
+
+class Foo {
+	function foo() {
+		echo "Foo\n";
+	}
+}
+
+class Bar extends Foo {
+	#[Override]
+	function bar() {
+		echo "Foo\n";
+	}
+} 
diff --git a/tests/units/Checks/TestEnumCheck.php b/tests/units/Checks/TestEnumCheck.php
@@ -77,4 +77,7 @@ public function testLegalEnumUsage() {
 		$this->assertEquals(0, $this->runAnalyzerOnFile('.3.inc', ErrorConstants::TYPE_ILLEGAL_ENUM), "Failed to detect traits with properties" );
 	}
 
+	public function testValuesTypeFetch() {
+		$this->assertEquals(0, $this->runAnalyzerOnFile('.5.inc', ErrorConstants::TYPE_UNKNOWN_PROPERTY), "Failed retrieve the name of a specific enum case");
+	}
 }
diff --git a/tests/units/Checks/TestMethodOverride.php b/tests/units/Checks/TestMethodOverride.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace BambooHR\Guardrail\Tests\units\Checks;
+
+use BambooHR\Guardrail\Checks\ErrorConstants;
+use BambooHR\Guardrail\Tests\TestSuiteSetup;
+
+class TestMethodOverride extends TestSuiteSetup {
+	function testSuccessfulOverride() {
+		$this->assertEquals(0, $this->runAnalyzerOnFile('.1.inc', ErrorConstants::TYPE_UNKNOWN_METHOD), "Detected an error when the #[Override] was valid");
+		$this->assertEquals(0, $this->runAnalyzerOnFile('.1.inc', ErrorConstants::TYPE_OVERRIDE_BASE_CLASS), "Detected an error when the #[Override] was valid");
+	}
+	function testDoesntExtend() {
+		$this->assertEquals(1, $this->runAnalyzerOnFile('.2.inc', ErrorConstants::TYPE_OVERRIDE_BASE_CLASS), "Failed to detect #[Override] of in a base class");
+	}
+	function testParentDoesntHaveMethod() {
+		$this->assertEquals(1, $this->runAnalyzerOnFile('.3.inc', ErrorConstants::TYPE_UNKNOWN_METHOD), "Failed to detect #[Override] when no parent implements the same method");
+	}
+}