diff --git a/README.md b/README.md index 8bbe063..d15a8ff 100644 --- 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 index 10390da..33c40f2 100644 --- 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 index 239234d..ad514d7 100644 --- 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; } } \ No newline at end of file diff --git a/src/Checks/ErrorConstants.php b/src/Checks/ErrorConstants.php index 6940076..f3da9dc 100644 --- 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 index c9efc1a..88b84cf 100644 --- 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 index d1dff4a..8d615be 100644 --- 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,6 +119,7 @@ static public function getMethodAccessLevel(ClassMethod $level) { return "protected"; } trigger_error("Impossible"); + return ""; } /** @@ -112,7 +127,7 @@ static public function getMethodAccessLevel(ClassMethod $level) { * * @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 new file mode 100644 index 0000000..bb0de3a --- /dev/null +++ b/tests/units/Checks/TestData/TestEnumCheck.5.inc @@ -0,0 +1,8 @@ +name; \ No newline at end of file diff --git a/tests/units/Checks/TestData/TestMethodOverride.1.inc b/tests/units/Checks/TestData/TestMethodOverride.1.inc new file mode 100644 index 0000000..f0fa91a --- /dev/null +++ b/tests/units/Checks/TestData/TestMethodOverride.1.inc @@ -0,0 +1,15 @@ +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"); + } } \ No newline at end of file diff --git a/tests/units/Checks/TestMethodOverride.php b/tests/units/Checks/TestMethodOverride.php new file mode 100644 index 0000000..741e150 --- /dev/null +++ b/tests/units/Checks/TestMethodOverride.php @@ -0,0 +1,19 @@ +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"); + } +} \ No newline at end of file