Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add attributes to ClassMethod abstraction #93

Merged
merged 10 commits into from
Feb 15, 2024
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);
}
}
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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the node here always be an instance of ClassConst?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I use the if to cast them so that static analyzers will know what we do about the type.

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, $scope)) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_CONST_TYPE,
"Type mismatch between declared type and constant value " .
TypeComparer::typeToString($node->type) . " vs " .
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
5 changes: 3 additions & 2 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 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
Loading