Skip to content

Commit

Permalink
Merge pull request #93 from BambooHR/jgardiner/attribute-abstraction
Browse files Browse the repository at this point in the history
Various obscure return type upgrades for 8.1, 8.2, and 8.3.  Update API for retrieving attributes().
  • Loading branch information
jongardiner authored Feb 15, 2024
2 parents 9d60dc0 + badb71a commit 7e50e0e
Show file tree
Hide file tree
Showing 23 changed files with 374 additions and 57 deletions.
15 changes: 15 additions & 0 deletions src/Abstractions/ClassMethod.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use BambooHR\Guardrail\Util;
use PhpParser\Node\Attribute;
use PhpParser\Node\ComplexType;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\NullableType;
Expand Down Expand Up @@ -53,6 +54,7 @@ public function isDeprecated() {
if (strpos($docBlock, "@deprecated") !== false) {
return true;
}
return false;
}

/**
Expand Down Expand Up @@ -170,4 +172,17 @@ public function isVariadic() {
}
return false;
}

public function getAttributes(string $name): array {
$ret=[];
foreach($this->method->attrGroups as $group) {
foreach($group->attrs as $attr) {
/** @var Attribute $attr */
if (strcasecmp($attr->name, $name)==0) {
$ret[]=$attr;
}
}
}
return $ret;
}
}
2 changes: 2 additions & 0 deletions src/Abstractions/MethodInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ public function hasNullableReturnType();


public function getComplexReturnType();

public function getAttributes(string $name):array;
}
14 changes: 11 additions & 3 deletions src/Abstractions/ReflectedClassMethod.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<?php namespace BambooHR\Guardrail\Abstractions;

use BambooHR\Guardrail\Util;
use PhpParser\Node\Attribute;
use PhpParser\Node\AttributeGroup;
use PhpParser\Node\Name;

/**
* Guardrail. Copyright (c) 2016-2023, BambooHR.
Expand Down Expand Up @@ -89,12 +92,10 @@ public function getAccessLevel() {
if ($this->refl->isPrivate()) {
return "private";
}
if ($this->refl->isPublic()) {
return "public";
}
if ($this->refl->isProtected()) {
return "protected";
}
return "public";
}

/**
Expand Down Expand Up @@ -178,4 +179,11 @@ public function isVariadic() {
return true; // We assume internal functions are variadic so that we don't get bombarded with warnings.
}
}

public function getAttributes(string $name):array {
$attributes=$this->refl->getAttributes($name);
return array_map( function($attr) {
return new Attribute(new Name($attr->getName()));
}, $attributes);
}
}
4 changes: 2 additions & 2 deletions src/Checks/CallCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ protected function checkParam($fileName, $node, $name, Scope $scope, ClassLike $
// Type mismatch
$checker = new TypeComparer($this->symbolTable);

if ($type && !$checker->isCompatibleWithTarget($expectedType, $type, $scope)) {
if ($type && !$checker->isCompatibleWithTarget($expectedType, $type, $scope->isStrict())) {
$nullOnlyError = false;
$typeStr=TypeComparer::typeToString($type);
if ($type instanceof Node\UnionType || $type instanceof Node\NullableType || TypeComparer::isNamedIdentifier($type,"null")) {
$typeWithOutNull = TypeComparer::removeNullOption($type);
$nullOnlyError = $checker->isCompatibleWithTarget($expectedType, $typeWithOutNull, $scope);
$nullOnlyError = $checker->isCompatibleWithTarget($expectedType, $typeWithOutNull, $scope->isStrict());
}
$this->emitError($fileName, $node,
$nullOnlyError ? ErrorConstants::TYPE_SIGNATURE_TYPE_NULL : ErrorConstants::TYPE_SIGNATURE_TYPE,
Expand Down
41 changes: 41 additions & 0 deletions src/Checks/ClassConstCheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace BambooHR\Guardrail\Checks;

use BambooHR\Guardrail\Output\OutputInterface;
use BambooHR\Guardrail\Scope;
use BambooHR\Guardrail\SymbolTable\SymbolTable;
use BambooHR\Guardrail\TypeComparer;
use PhpParser\Node;
use PhpParser\Node\Stmt\ClassConst;
use PhpParser\Node\Stmt\ClassLike;

class ClassConstCheck extends BaseCheck {
private $comparer;
function __construct(SymbolTable $symbolTable, OutputInterface $doc) {
parent::__construct($symbolTable, $doc);
$this->comparer=new TypeComparer($symbolTable);
}

function getCheckNodeTypes() {
return [ClassConst::class];
}

function run($fileName, Node $node, ClassLike $inside = null, Scope $scope = null) {
if ($node instanceof ClassConst) {
foreach($node->consts as $const) {
/** @var Node\Const_ $const */
$constValue = $const->value->getAttribute(TypeComparer::INFERRED_TYPE_ATTR );
if ($node->type && $constValue) {
if (!$this->comparer->isCompatibleWithTarget($node->type, $constValue, forceStrict: true)) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_CONST_TYPE,
"Type mismatch between declared type (".
TypeComparer::typeToString($node->type) . ") and constant value (" .
TypeComparer::typeToString($constValue) . ")"
);
}
}
}
}
}
}
9 changes: 3 additions & 6 deletions src/Checks/ErrorConstants.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class ErrorConstants {
const TYPE_SIGNATURE_COUNT_EXCESS = 'Standard.Param.Count.Excess';
const TYPE_SIGNATURE_RETURN = 'Standard.Return.Type';
const TYPE_SIGNATURE_TYPE = 'Standard.Param.Type';
const TYPE_CONST_TYPE = 'Standard.Const.Type';
const TYPE_SIGNATURE_TYPE_NULL = "Standard.Null.Param";
const TYPE_UNIMPLEMENTED_METHOD = 'Standard.Inheritance.Unimplemented';
const TYPE_UNKNOWN_CLASS = 'Standard.Unknown.Class';
Expand Down Expand Up @@ -79,13 +80,9 @@ class ErrorConstants {
* @return string[]
*/
static function getConstants() {
$ret = [];
$selfReflection = new \ReflectionClass(self::class);
$constants = $selfReflection->getConstants();
sort($constants);
foreach ($constants as $name => $value) {
$ret[] = $value;
}
return $ret;
return array_values($constants);
}
}
}
6 changes: 5 additions & 1 deletion src/Checks/ParamTypesCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ public function getCheckNodeTypes() {
*/
protected function isAllowed(Node\ComplexType|Node\NullableType|Node\Name|Node\Identifier|null $name , ClassLike $inside=null) {
$return = true;

if ($name instanceof Node\NullableType && TypeComparer::isNamedIdentifier($name->type,"null")) {
return false;
}
TypeComparer::forEachAnyEveryType($name, function($name2) use ($inside, &$return) {
if($name2===null) {
return;
Expand Down Expand Up @@ -107,7 +111,7 @@ public function run($fileName, Node $node, ClassLike $inside=null, Scope $scope=

if ($node->getReturnType()) {
$returnType = $node->getReturnType();
if (!$this->isAllowed($returnType, $inside)) {
if (!TypeComparer::isNamedIdentifier($returnType,"never") && !$this->isAllowed($returnType, $inside)) {
$returnType = TypeComparer::typeToString($returnType);
$this->emitError($fileName, $node, ErrorConstants::TYPE_UNKNOWN_CLASS, "Reference to an unknown type '$returnType' in return value of $displayName");
}
Expand Down
2 changes: 1 addition & 1 deletion src/Checks/PropertyStoreCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function run($fileName, Node $node, ClassLike $inside=null, Scope $scope=
});


if (!$this->typeComparer->isCompatibleWithTarget($targetType, $valueType, $scope)) {
if (!$this->typeComparer->isCompatibleWithTarget($targetType, $valueType, $scope->isStrict())) {
if($targetType instanceof Node\Identifier && util::isScalarType(strval($targetType))) {
$errorType = ErrorConstants::TYPE_ASSIGN_MISMATCH_SCALAR;
} else {
Expand Down
7 changes: 4 additions & 3 deletions src/Checks/ReturnCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ public function run($fileName, Node $node, ClassLike $inside = null, Scope $scop
$returnType = $insideFunc->getReturnType();

$returnIsVoid = TypeComparer::isNamedIdentifier($returnType,"void");
if ($returnIsVoid) {
$returnIsNever = TypeComparer::isNamedIdentifier($returnType,"never");
if ($returnIsVoid || $returnIsNever) {
if ($node->expr != null) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_SIGNATURE_RETURN, "Attempt to return a value from a void function $functionName");
$this->emitError($fileName, $node, ErrorConstants::TYPE_SIGNATURE_RETURN, "Attempt to return a value from a ".TypeComparer::typeToString($returnType)." function $functionName");
return;
}
} else if ($returnType && $node->expr == null) {
Expand All @@ -84,7 +85,7 @@ public function run($fileName, Node $node, ClassLike $inside = null, Scope $scop
$returnType = TypeComparer::nameFromName( $inside->namespacedName );
}

if (!$this->typeComparer->isCompatibleWithTarget($returnType, $exprType, $scope)) {
if (!$this->typeComparer->isCompatibleWithTarget($returnType, $exprType, $scope->isStrict())) {
$functionName = $this->getFunctionName($inside, $insideFunc);
$msg = "Value returned from $functionName()" .
" must be a " . TypeComparer::typeToString($returnType) .
Expand Down
10 changes: 9 additions & 1 deletion src/Evaluators/Expression/CallLike.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,16 @@ function checkForPropertyCastedCall(Node\Expr\FuncCall $func, ScopeStack $scope)
) {
$varName = NodePatterns::getVariableOrPropertyName($func->args[0]->value);
$type = $this->getCastedCallType(strtolower($func->name));

if (!is_null($type) && !is_null($varName)) {
$this->tagScopeAsType($func, $scope, $varName,$type);

if ($func->args[0]->value instanceof Node\Expr\NullsafePropertyFetch) {
// TODO: confirm all elements are non-null or null-safe, then set all
// different lengths of chains as non-null
//$list = explode("->", $varName);
} else {
$this->tagScopeAsType($func, $scope, $varName, $type);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Evaluators/Expression/ConstFetch.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function getType(SymbolTable $table, Node\Expr\ConstFetch $expr):?Node\Identifie
return TypeComparer::identifierFromName("null");
}
if (strcasecmp($expr->name, "false") == 0 || strcasecmp($expr->name, "true") == 0) {
return TypeComparer::identifierFromName("bool");
return TypeComparer::identifierFromName($expr->name);
}
if (defined($expr->name)) {
// Guardrail doesn't declare any global constants. Any that exist are from the runtime.
Expand Down
2 changes: 2 additions & 0 deletions src/NodeVisitors/StaticAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use BambooHR\Guardrail\Checks\BreakCheck;
use BambooHR\Guardrail\Checks\CatchCheck;
use BambooHR\Guardrail\Checks\ClassConstantCheck;
use BambooHR\Guardrail\Checks\ClassConstCheck;
use BambooHR\Guardrail\Checks\ClassMethodStringCheck;
use BambooHR\Guardrail\Checks\ConditionalAssignmentCheck;
use BambooHR\Guardrail\Checks\ConstructorCheck;
Expand Down Expand Up @@ -149,6 +150,7 @@ function __construct($basePath, $index, OutputInterface $output, $config)
new UnsafeSuperGlobalCheck($this->index, $output),
new UseStatementCaseCheck($this->index, $output),
new ReadOnlyPropertyCheck($this->index, $output),
new ClassConstCheck($this->index, $output),
//new ClassStoredAsVariableCheck($this->index, $output)
];

Expand Down
7 changes: 5 additions & 2 deletions src/Output/ConsoleOutput.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function emitError($className, $fileName, $lineNumber, $name, $message =
return;
}
$this->displayedErrors++;
if ($this->emitErrors) {
if ($this->emitErrors && !$this->isTTY()) {
echo "E";
}
$this->errors[$fileName][] = ["line" => $lineNumber, "message" => $message];
Expand All @@ -34,8 +34,10 @@ public function emitError($className, $fileName, $lineNumber, $name, $message =
*/
public function renderResults() {
echo "\n";
$white=$this->ttyContent("\33[97m");
$reset=$this->ttyContent("\33[0m");
foreach ($this->errors as $fileName => $errors) {
echo " Line | $fileName\n";
echo " ${white}Line${reset} | ${white}$fileName${reset}\n";
echo "-------+----------------------------------------------------------------\n";
usort($errors, function ($cmpa, $cmpb) {
return $cmpa['line'] > $cmpb['line'] ? 1 : ($cmpa['line'] == $cmpb['line'] ? 0 : -1);
Expand All @@ -44,6 +46,7 @@ public function renderResults() {
if (!is_int($error['line'])) {
var_dump($error);
}

printf("%6d | %s\n", $error['line'], $error['message']);
}
echo "\n";
Expand Down
2 changes: 1 addition & 1 deletion src/Output/CountsOutput.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function emitError($className, $fileName, $lineNumber, $name, $message =
return;
}
$this->displayedErrors++;
if ($this->emitErrors) {
if ($this->emitErrors && !$this->isTTY()) {
echo "E";
}
if (!isset($this->errors[$name])) {
Expand Down
4 changes: 4 additions & 0 deletions src/Output/OutputInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ function emitError($className, $file, $line, $type, $message = "");
*/
function output($verbose, $extraVerbose);

function ttyContent(string $content):string;

/**
* outputVerbose
*
Expand Down Expand Up @@ -85,4 +87,6 @@ function silenceType($name);
function resumeType($name);

function getErrorCounts();

function isTTY(): bool;
}
18 changes: 15 additions & 3 deletions src/Output/XUnitOutput.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class XUnitOutput implements OutputInterface {
*/
private $silenced = [];

private $isTTY = false;

/**
* XUnitOutput constructor.
*
Expand All @@ -66,6 +68,11 @@ public function __construct(Config $config) {
$this->emitErrors = $config->getOutputLevel() == 1;
$this->emitList = $config->getEmitList();

$this->isTTY = posix_isatty(STDOUT );
}

public function isTTY():bool {
return $this->isTTY;
}

/**
Expand Down Expand Up @@ -238,8 +245,8 @@ public function emitError($className, $fileName, $lineNumber, $name, $message=""

$message .= " on line " . $lineNumber;
$case->addFailure( $this->escapeText($name . ":" . $message), "error");
if ($this->emitErrors) {
// echo "E";
if ($this->emitErrors && !$this->isTTY()) {
echo "E";
}
if (!isset($this->counts[$name])) {
$this->counts[$name] = 1;
Expand All @@ -249,6 +256,11 @@ public function emitError($className, $fileName, $lineNumber, $name, $message=""
$this->outputExtraVerbose("ERROR: $fileName $lineNumber: $name: $message\n");
}

public function ttyContent($content):string {
return $this->isTTY ? $content : "";
}


/**
* output
*
Expand Down Expand Up @@ -285,7 +297,7 @@ public function getCounts() {
*/
public function outputVerbose($string) {
if ($this->config->getOutputLevel() >= 1) {
echo "\n".$string."\n";
echo $string;
flush();
}
}
Expand Down
Loading

0 comments on commit 7e50e0e

Please sign in to comment.