From 7f27040cfaa6d33e8f2974fe092f7531fea01e19 Mon Sep 17 00:00:00 2001 From: JonGardiner Date: Tue, 6 Feb 2024 13:35:39 -0700 Subject: [PATCH 1/7] Create a new getAttribute() method on MethodInterface and implement for ClassMethod and ReflectedClassMethod so that we can inspect method attributes tests. Also, change analysis rate display to only run in an interactive TTY. --- src/Abstractions/ClassMethod.php | 14 +++++++++ src/Abstractions/MethodInterface.php | 2 ++ src/Abstractions/ReflectedClassMethod.php | 10 ++++++ src/Output/ConsoleOutput.php | 2 +- src/Output/CountsOutput.php | 2 +- src/Output/OutputInterface.php | 2 ++ src/Output/XUnitOutput.php | 13 ++++++-- src/Phases/AnalyzingPhase.php | 38 ++++++++++++----------- src/Phases/IndexingPhase.php | 30 ++++++++++++++---- 9 files changed, 84 insertions(+), 29 deletions(-) diff --git a/src/Abstractions/ClassMethod.php b/src/Abstractions/ClassMethod.php index 7157b21..39ff609 100644 --- a/src/Abstractions/ClassMethod.php +++ b/src/Abstractions/ClassMethod.php @@ -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; @@ -170,4 +171,17 @@ public function isVariadic() { } return false; } + + public function getAttributes(string $name, bool $exactTypeOnly=true): 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; + } } \ No newline at end of file diff --git a/src/Abstractions/MethodInterface.php b/src/Abstractions/MethodInterface.php index 2bec9c1..0ecf54c 100644 --- a/src/Abstractions/MethodInterface.php +++ b/src/Abstractions/MethodInterface.php @@ -40,4 +40,6 @@ public function hasNullableReturnType(); public function getComplexReturnType(); + + public function getAttributes(string $name, bool $exactTypeOnly=true):array; } \ No newline at end of file diff --git a/src/Abstractions/ReflectedClassMethod.php b/src/Abstractions/ReflectedClassMethod.php index 762d1ab..7640505 100644 --- a/src/Abstractions/ReflectedClassMethod.php +++ b/src/Abstractions/ReflectedClassMethod.php @@ -1,6 +1,9 @@ refl->getAttributes($name, $exactTypeOnly ? 0 : \ReflectionAttribute::IS_INSTANCEOF ); + return array_map( function($attr) { + return new Attribute(new Name($attr->getName())); + }, $attributes); + } } \ No newline at end of file diff --git a/src/Output/ConsoleOutput.php b/src/Output/ConsoleOutput.php index c114d5a..f3a0bf1 100644 --- a/src/Output/ConsoleOutput.php +++ b/src/Output/ConsoleOutput.php @@ -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]; diff --git a/src/Output/CountsOutput.php b/src/Output/CountsOutput.php index e78ade0..04c59ae 100644 --- a/src/Output/CountsOutput.php +++ b/src/Output/CountsOutput.php @@ -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])) { diff --git a/src/Output/OutputInterface.php b/src/Output/OutputInterface.php index 779063d..fdfc22e 100644 --- a/src/Output/OutputInterface.php +++ b/src/Output/OutputInterface.php @@ -85,4 +85,6 @@ function silenceType($name); function resumeType($name); function getErrorCounts(); + + function isTTY(): bool; } \ No newline at end of file diff --git a/src/Output/XUnitOutput.php b/src/Output/XUnitOutput.php index 968d836..c699baa 100644 --- a/src/Output/XUnitOutput.php +++ b/src/Output/XUnitOutput.php @@ -53,6 +53,8 @@ class XUnitOutput implements OutputInterface { */ private $silenced = []; + private $isTTY = false; + /** * XUnitOutput constructor. * @@ -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; } /** @@ -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; @@ -285,7 +292,7 @@ public function getCounts() { */ public function outputVerbose($string) { if ($this->config->getOutputLevel() >= 1) { - echo "\n".$string."\n"; + echo $string; flush(); } } diff --git a/src/Phases/AnalyzingPhase.php b/src/Phases/AnalyzingPhase.php index eef02c1..86576c8 100644 --- a/src/Phases/AnalyzingPhase.php +++ b/src/Phases/AnalyzingPhase.php @@ -213,15 +213,15 @@ public function phase2(Config $config, OutputInterface $output, $toProcess, $tot function ($socket) use ($config) { $this->runChildAnalyzer($socket, $config); }); - $childPid = $pm->getPidForSocket($socket); - $this->output->outputExtraVerbose("Starting child $childPid with first file\n"); + if (!$output->isTTY()) { + $output->outputExtraVerbose(sprintf("%d - %s\n", $fileNumber, $toProcess[$fileNumber])); + } $this->socket_write_all($socket, "ANALYZE " . $toProcess[$fileNumber] . "\n"); } // Server process reports the errors and serves up new files to the list. $processDied = false; $bytes = 0; - $this->output->outputExtraVerbose("Parent looking for messages from the children\n"); $pm->loopWhileConnections( function ($socket, $msg) use (&$processingCount, &$fileNumber, &$bytes, $output, $toProcess, $totalBytes, $start, $pm) { $processComplete = $this->processChildMessage($socket, $msg, $processingCount, $fileNumber, $bytes, $output, $toProcess, $totalBytes, $start, $pm); @@ -235,7 +235,7 @@ function ($socket, $msg) use (&$processingCount, &$fileNumber, &$bytes, $output, protected function processChildMessage($socket, $msg, &$processingCount, &$fileNumber, &$bytes, OutputInterface $output, $toProcess, $totalBytes, $start, ProcessManager $pm) { $childPid = $pm->getPidForSocket($socket); - $output->outputExtraVerbose("parent received from $childPid: $msg\n"); + //$output->outputExtraVerbose("parent received from $childPid: $msg\n"); if ($msg === false) { echo "Error: Unexpected error reading from socket\n"; @@ -256,6 +256,7 @@ protected function processChildMessage($socket, $msg, &$processingCount, &$fileN break; case 'ERROR' : $vars = unserialize(base64_decode($details)); + $this->output->emitError( $vars['className'], $vars['file'], @@ -265,7 +266,7 @@ protected function processChildMessage($socket, $msg, &$processingCount, &$fileN ); break; case 'ANALYZED': - list($size,) = explode(' ', $details, 2); + list($size,$analyzedFileName) = explode(' ', $details, 2); if ($fileNumber < count($toProcess)) { $bytes += intval($size); $this->socket_write_all($socket, "ANALYZE " . $toProcess[$fileNumber] . "\n"); @@ -276,13 +277,17 @@ protected function processChildMessage($socket, $msg, &$processingCount, &$fileN $kbs=intdiv( intdiv($bytes, 1024), (time()-$start) ?: 1); ["total"=>$errors, "displayed"=>$displayCount] = $output->getErrorCounts(); - printf("%d/%d, %d/%d MB (%d%%), %d KB/s %d errors, %d suppressed\r", - $fileNumber, count($toProcess), - intdiv($bytes,1024*1024), intdiv($totalBytes,1024*1024), - intdiv(100*$bytes, $totalBytes), - $kbs, - $displayCount, $errors-$displayCount - ); + if ($output->isTTY()) { + printf("%d/%d, %d/%d MB (%d%%), %d KB/s %d errors, %d suppressed \r", + $fileNumber, count($toProcess), + intdiv($bytes, 1024 * 1024), intdiv($totalBytes, 1024 * 1024), + intval(round(100 * $bytes / $totalBytes)), + $kbs, + $displayCount, $errors - $displayCount + ); + } else { + $output->output(".", sprintf("%d - %s", $fileNumber-1, $analyzedFileName)); + } break; case 'TIMINGS': $this->timingResults[] = json_decode(base64_decode($details), true); @@ -312,9 +317,6 @@ protected function runChildAnalyzer($socket, Config $config) { while (1) { $buffer->read($socket); foreach ($buffer->getMessages() as $receive) { - if ($config->getOutputLevel() >= 2) { - echo "Child $pid recieved $receive\n"; - } $receive = trim($receive); if ($receive == "TIMINGS") { $this->socket_write_all($socket, "TIMINGS " . base64_encode(json_encode($this->analyzer->getTimingsAndCounts()) ). "\n"); @@ -391,7 +393,7 @@ public function run(Config $config, OutputInterface $output) { } } - $output->outputVerbose("\nAllotting work for " . $config->getPartitions() . " partitions\n"); + $output->outputVerbose("Allotting work for " . $config->getPartitions() . " partitions\n"); // Sort all the files first by size and second by name. // Once we have a list that is roughly even, then we can split @@ -428,9 +430,9 @@ public function run(Config $config, OutputInterface $output) { } } - $output->outputVerbose("Sizes: " . implode(", ", $sizes) . "\n"); + $output->outputVerbose("Sizes: " . implode(", ", $sizes)."\n"); - $output->outputVerbose("\nPartition " . ($partitionNumber + 1) . " analyzing " . count($partialList) . " files (" . $sizes[$partitionNumber] . " bytes)\n"); + $output->outputVerbose("Partition " . ($partitionNumber + 1) . " analyzing " . number_format(count($partialList) ). " files (" . number_format($sizes[$partitionNumber] ). " bytes)\n"); return $this->phase2($config, $output, $partialList, $sizes[$partitionNumber]); } diff --git a/src/Phases/IndexingPhase.php b/src/Phases/IndexingPhase.php index 6a34c05..878009b 100644 --- a/src/Phases/IndexingPhase.php +++ b/src/Phases/IndexingPhase.php @@ -122,7 +122,7 @@ function indexFile(Config $config, $pathName) { * @return void */ public function indexTraitClasses(SymbolTable $symbolTable, OutputInterface $output) { - $output->outputVerbose("Importing traits"); + $output->outputVerbose("\n\nImporting traits\n"); $symbolTable->begin(); foreach ($symbolTable->getClassesThatUseAnyTrait() as $className) { $class = $symbolTable->getClass($className); @@ -186,11 +186,18 @@ function indexList(Config $config, OutputInterface $output, $itr) { $processNumber = $fileNumber; $child = $this->createIndexingChild($processNumber, $config); socket_write($child, "INDEX " . $itr->current() . "\n"); - $output->output(".", sprintf("%d - %s", $fileNumber, $itr->current())); + + if (!$output->isTTY() && $config->getOutputLevel()==1) { + $output->outputVerbose("."); + } + if ($config->getOutputLevel()==2) { + $output->outputExtraVerbose( sprintf("%d - %s\n", $fileNumber, $itr->current()) ); + } + } $this->processManager->loopWhileConnections( - function ($socket, $msg) use (&$itr, &$fileNumber, &$bytes, $output, $start) { + function ($socket, $msg) use (&$itr, &$fileNumber, &$bytes, $output, $start, $config) { if ($msg === false) { echo "Error: Unexpected error reading from socket\n"; return ProcessManager::CLOSE_CONNECTION; @@ -200,7 +207,7 @@ function ($socket, $msg) use (&$itr, &$fileNumber, &$bytes, $output, $start) { if ($message == 'INDEXED') { [$size, $fileName, $childProcessNumber] = explode(' ', $details); $bytes += $size; - $output->output(".", sprintf("%d - %s ($childProcessNumber)", ++$fileNumber, $fileName)); + $output->outputExtraVerbose(sprintf("%d - %s ($childProcessNumber)\n", ++$fileNumber, $fileName)); if ($itr->valid()) { socket_write($socket, "INDEX " . $itr->current() ."\n"); @@ -210,7 +217,18 @@ function ($socket, $msg) use (&$itr, &$fileNumber, &$bytes, $output, $start) { return ProcessManager::CLOSE_CONNECTION; } if ($fileNumber % 50 == 0) { - $output->output("", sprintf("Processing %.1f KB/second", $bytes / 1024 / (microtime(true) - $start))); + $process= sprintf("Processing %.1f KB/second", $bytes / 1024 / (microtime(true) - $start)); + if ($config->getOutputLevel()==1) { + if (!$output->isTTY()) { + $output->outputVerbose("."); + } else { + $output->outputVerbose($process." \r"); + } + } else { + if ($config->getOutputLevel()==2) { + $output->outputExtraVerbose("\n".$process . "\n"); + } + } } } else { $output->outputVerbose($message . " D:" . $details . "\n"); @@ -238,7 +256,7 @@ public function run(Config $config, OutputInterface $output) { "Invalid or missing paths in your index config section."); exit; } - $output->outputVerbose("Index directories are valid: Indexing starting."); + $output->outputVerbose("Index directories are valid: Indexing starting.\n"); $this->indexList($config, $output, $this->getFileList($config, $indexPaths) ); From 3bcd0d05b4be9f276c005aec2bc57349d5e949a2 Mon Sep 17 00:00:00 2001 From: JonGardiner Date: Tue, 6 Feb 2024 16:23:07 -0700 Subject: [PATCH 2/7] Ansi color when outputting to TTY. --- src/Output/ConsoleOutput.php | 5 ++++- src/Output/OutputInterface.php | 2 ++ src/Output/XUnitOutput.php | 5 +++++ src/Phases/AnalyzingPhase.php | 27 ++++++++++++++++++++------- src/Phases/IndexingPhase.php | 7 ++++++- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/Output/ConsoleOutput.php b/src/Output/ConsoleOutput.php index f3a0bf1..9f30d68 100644 --- a/src/Output/ConsoleOutput.php +++ b/src/Output/ConsoleOutput.php @@ -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); @@ -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"; diff --git a/src/Output/OutputInterface.php b/src/Output/OutputInterface.php index fdfc22e..4253ec9 100644 --- a/src/Output/OutputInterface.php +++ b/src/Output/OutputInterface.php @@ -34,6 +34,8 @@ function emitError($className, $file, $line, $type, $message = ""); */ function output($verbose, $extraVerbose); + function ttyContent(string $content):string; + /** * outputVerbose * diff --git a/src/Output/XUnitOutput.php b/src/Output/XUnitOutput.php index c699baa..d023fc7 100644 --- a/src/Output/XUnitOutput.php +++ b/src/Output/XUnitOutput.php @@ -256,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 * diff --git a/src/Phases/AnalyzingPhase.php b/src/Phases/AnalyzingPhase.php index 86576c8..0afe446 100644 --- a/src/Phases/AnalyzingPhase.php +++ b/src/Phases/AnalyzingPhase.php @@ -278,12 +278,15 @@ protected function processChildMessage($socket, $msg, &$processingCount, &$fileN $kbs=intdiv( intdiv($bytes, 1024), (time()-$start) ?: 1); ["total"=>$errors, "displayed"=>$displayCount] = $output->getErrorCounts(); if ($output->isTTY()) { - printf("%d/%d, %d/%d MB (%d%%), %d KB/s %d errors, %d suppressed \r", + $white=$output->ttyContent("\33[97m"); + $red=$output->ttyContent("\33[31m"); + $reset=$output->ttyContent("\33[0m"); + printf("$white%d$reset/$white%d$reset, $white%d$reset/$white%d$reset MB ($white%d$reset%%), $white%d$reset KB/s $red%d$reset errors \r", $fileNumber, count($toProcess), intdiv($bytes, 1024 * 1024), intdiv($totalBytes, 1024 * 1024), intval(round(100 * $bytes / $totalBytes)), $kbs, - $displayCount, $errors - $displayCount + $displayCount ); } else { $output->output(".", sprintf("%d - %s", $fileNumber-1, $analyzedFileName)); @@ -320,7 +323,7 @@ protected function runChildAnalyzer($socket, Config $config) { $receive = trim($receive); if ($receive == "TIMINGS") { $this->socket_write_all($socket, "TIMINGS " . base64_encode(json_encode($this->analyzer->getTimingsAndCounts()) ). "\n"); - return 0; + return; } else { list(, $file) = explode(' ', $receive, 2); $size = $this->analyzeFile($file, $config); @@ -378,6 +381,9 @@ public function run(Config $config, OutputInterface $output) { $output->output("Invalid or missing paths in your test config section.\n", "Invalid or missing paths in your test config section.\n"); exit; } + + $white=$output->ttyContent("\33[97m"); + $reset=$output->ttyContent("\33[0m"); $output->outputVerbose("Test directories are valid: Starting Analysis\n"); $toProcess = []; if ($config->hasFileList()) { @@ -387,13 +393,20 @@ public function run(Config $config, OutputInterface $output) { } else { foreach ($indexPaths as $path) { $tmpDirectory = Util::fullDirectoryPath($baseDirectory, $path); - $output->outputVerbose("Directory: $path\n"); + $output->outputVerbose( + "Directory: " . + $white . + $path . + $reset . + $output->ttyContent("\33[0m") . + "\n" + ); $it2 = DirectoryLister::getGenerator($tmpDirectory); $this->getPhase2Files($config, $it2, $toProcess); } } - $output->outputVerbose("Allotting work for " . $config->getPartitions() . " partitions\n"); + $output->outputVerbose("Allotting work for " . $white . $config->getPartitions() . $reset . " partitions\n"); // Sort all the files first by size and second by name. // Once we have a list that is roughly even, then we can split @@ -430,9 +443,9 @@ public function run(Config $config, OutputInterface $output) { } } - $output->outputVerbose("Sizes: " . implode(", ", $sizes)."\n"); + $output->outputVerbose("Partition sizes: " . $white . implode("$reset,$white ", $sizes)."$reset\n"); - $output->outputVerbose("Partition " . ($partitionNumber + 1) . " analyzing " . number_format(count($partialList) ). " files (" . number_format($sizes[$partitionNumber] ). " bytes)\n"); + $output->outputVerbose("Partition " . $white.($partitionNumber + 1).$reset . " analyzing " . $white.number_format(count($partialList) ). $reset." files (" . $white.number_format($sizes[$partitionNumber] ).$reset. " bytes)\n"); return $this->phase2($config, $output, $partialList, $sizes[$partitionNumber]); } diff --git a/src/Phases/IndexingPhase.php b/src/Phases/IndexingPhase.php index 878009b..bf5a0d1 100644 --- a/src/Phases/IndexingPhase.php +++ b/src/Phases/IndexingPhase.php @@ -217,7 +217,12 @@ function ($socket, $msg) use (&$itr, &$fileNumber, &$bytes, $output, $start, $co return ProcessManager::CLOSE_CONNECTION; } if ($fileNumber % 50 == 0) { - $process= sprintf("Processing %.1f KB/second", $bytes / 1024 / (microtime(true) - $start)); + $process= sprintf( + "Processing %s%.1f%s KB/second", + $output->ttyContent("\33[97m"), + $bytes / 1024 / (microtime(true) - $start), + $output->ttyContent("\33[0m") + ); if ($config->getOutputLevel()==1) { if (!$output->isTTY()) { $output->outputVerbose("."); From 92e429fb10822261a299e7c095d3856592b7157f Mon Sep 17 00:00:00 2001 From: JonGardiner Date: Tue, 6 Feb 2024 16:44:42 -0700 Subject: [PATCH 3/7] Remove a couple of unused variables. --- src/Phases/AnalyzingPhase.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Phases/AnalyzingPhase.php b/src/Phases/AnalyzingPhase.php index 0afe446..9cd9f37 100644 --- a/src/Phases/AnalyzingPhase.php +++ b/src/Phases/AnalyzingPhase.php @@ -234,8 +234,6 @@ function ($socket, $msg) use (&$processingCount, &$fileNumber, &$bytes, $output, } protected function processChildMessage($socket, $msg, &$processingCount, &$fileNumber, &$bytes, OutputInterface $output, $toProcess, $totalBytes, $start, ProcessManager $pm) { - $childPid = $pm->getPidForSocket($socket); - //$output->outputExtraVerbose("parent received from $childPid: $msg\n"); if ($msg === false) { echo "Error: Unexpected error reading from socket\n"; @@ -316,7 +314,6 @@ protected function runChildAnalyzer($socket, Config $config) { } $this->initChildThread($socket, $config); $buffer = new SocketBuffer(); - $pid = getmypid(); while (1) { $buffer->read($socket); foreach ($buffer->getMessages() as $receive) { From b618725088b240a4850037b9566237b813557d51 Mon Sep 17 00:00:00 2001 From: JonGardiner Date: Wed, 7 Feb 2024 01:49:02 -0700 Subject: [PATCH 4/7] Convert getAttributes() to only take an exact name. Built a new utility function for filtering of list of potential child classes by their ancestor. --- src/Abstractions/ClassMethod.php | 3 ++- src/Abstractions/MethodInterface.php | 2 +- src/Abstractions/ReflectedClassMethod.php | 8 +++----- src/Util.php | 10 ++++++++++ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Abstractions/ClassMethod.php b/src/Abstractions/ClassMethod.php index 39ff609..9efd9e0 100644 --- a/src/Abstractions/ClassMethod.php +++ b/src/Abstractions/ClassMethod.php @@ -54,6 +54,7 @@ public function isDeprecated() { if (strpos($docBlock, "@deprecated") !== false) { return true; } + return false; } /** @@ -172,7 +173,7 @@ public function isVariadic() { return false; } - public function getAttributes(string $name, bool $exactTypeOnly=true): array { + public function getAttributes(string $name): array { $ret=[]; foreach($this->method->attrGroups as $group) { foreach($group->attrs as $attr) { diff --git a/src/Abstractions/MethodInterface.php b/src/Abstractions/MethodInterface.php index 0ecf54c..913c37e 100644 --- a/src/Abstractions/MethodInterface.php +++ b/src/Abstractions/MethodInterface.php @@ -41,5 +41,5 @@ public function hasNullableReturnType(); public function getComplexReturnType(); - public function getAttributes(string $name, bool $exactTypeOnly=true):array; + public function getAttributes(string $name):array; } \ No newline at end of file diff --git a/src/Abstractions/ReflectedClassMethod.php b/src/Abstractions/ReflectedClassMethod.php index 7640505..00f0941 100644 --- a/src/Abstractions/ReflectedClassMethod.php +++ b/src/Abstractions/ReflectedClassMethod.php @@ -92,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"; } /** @@ -182,8 +180,8 @@ public function isVariadic() { } } - public function getAttributes(string $name, bool $exactTypeOnly=true):array { - $attributes=$this->refl->getAttributes($name, $exactTypeOnly ? 0 : \ReflectionAttribute::IS_INSTANCEOF ); + public function getAttributes(string $name):array { + $attributes=$this->refl->getAttributes($name); return array_map( function($attr) { return new Attribute(new Name($attr->getName())); }, $attributes); diff --git a/src/Util.php b/src/Util.php index 8d615be..669e732 100644 --- a/src/Util.php +++ b/src/Util.php @@ -306,6 +306,16 @@ static public function callIsCompatible(ClassMethod $method,MethodCall $call) { } + static public function getFilteredChildClasses(SymbolTable $table, string $parent, string ...$potentialChildren):array { + $ret=[]; + foreach($potentialChildren as $potentialChild) { + if ($table->isParentClassOrInterface($parent, $potentialChild)) { + $ret[] = $potentialChild; + } + } + return $ret; + } + /** * configDirectoriesAreValid * From 9bf01694224afe7a3790890836ca4ce5992d5cc9 Mon Sep 17 00:00:00 2001 From: JonGardiner Date: Wed, 14 Feb 2024 17:13:59 -0700 Subject: [PATCH 5/7] Support for "never" as a return type. 8.1 Support for "true", "false", "null" as declared return types. 8.2 Support for typed constants 8.3 Checks for ?null as an illegal type (nullable null) --- src/Checks/ClassConstCheck.php | 41 ++++++++++++++++++++++++ src/Checks/ErrorConstants.php | 9 ++---- src/Checks/ParamTypesCheck.php | 6 +++- src/Checks/ReturnCheck.php | 5 +-- src/Evaluators/Expression/CallLike.php | 10 +++++- src/Evaluators/Expression/ConstFetch.php | 2 +- src/NodeVisitors/StaticAnalyzer.php | 2 ++ src/TypeComparer.php | 11 ++++++- tests/TestSuiteSetup.php | 14 ++++++++ 9 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 src/Checks/ClassConstCheck.php diff --git a/src/Checks/ClassConstCheck.php b/src/Checks/ClassConstCheck.php new file mode 100644 index 0000000..9beb2d2 --- /dev/null +++ b/src/Checks/ClassConstCheck.php @@ -0,0 +1,41 @@ +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, $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) + ); + } + } + } + } + } +} \ No newline at end of file diff --git a/src/Checks/ErrorConstants.php b/src/Checks/ErrorConstants.php index f3da9dc..4f0ebee 100644 --- a/src/Checks/ErrorConstants.php +++ b/src/Checks/ErrorConstants.php @@ -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'; @@ -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); } -} \ No newline at end of file +} diff --git a/src/Checks/ParamTypesCheck.php b/src/Checks/ParamTypesCheck.php index 88b84cf..e4ab069 100644 --- a/src/Checks/ParamTypesCheck.php +++ b/src/Checks/ParamTypesCheck.php @@ -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; @@ -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"); } diff --git a/src/Checks/ReturnCheck.php b/src/Checks/ReturnCheck.php index dcf9e16..be398cc 100644 --- a/src/Checks/ReturnCheck.php +++ b/src/Checks/ReturnCheck.php @@ -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) { diff --git a/src/Evaluators/Expression/CallLike.php b/src/Evaluators/Expression/CallLike.php index eb30e1e..e5934ee 100644 --- a/src/Evaluators/Expression/CallLike.php +++ b/src/Evaluators/Expression/CallLike.php @@ -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); + } } } } diff --git a/src/Evaluators/Expression/ConstFetch.php b/src/Evaluators/Expression/ConstFetch.php index bb58544..3009183 100644 --- a/src/Evaluators/Expression/ConstFetch.php +++ b/src/Evaluators/Expression/ConstFetch.php @@ -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. diff --git a/src/NodeVisitors/StaticAnalyzer.php b/src/NodeVisitors/StaticAnalyzer.php index fb7c245..c0133a4 100644 --- a/src/NodeVisitors/StaticAnalyzer.php +++ b/src/NodeVisitors/StaticAnalyzer.php @@ -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; @@ -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) ]; diff --git a/src/TypeComparer.php b/src/TypeComparer.php index ea2d034..4ec7c13 100644 --- a/src/TypeComparer.php +++ b/src/TypeComparer.php @@ -109,6 +109,13 @@ function areSimpleTypesCompatible(Name|Identifier|null $target, Name|Identifier| return true; } + if ( + ($targetName=="false" || $targetName=="true" || $targetName=="null") && + $valueName!=$targetName + ) { + return false; + } + if (!$strict) { if ($targetName=="mixed" || $valueName=="mixed") { return true; @@ -126,7 +133,9 @@ function areSimpleTypesCompatible(Name|Identifier|null $target, Name|Identifier| * */ static function getChainedPropertyFetchName(Node $rootNode):?string { - if ($rootNode instanceof Node\Expr\PropertyFetch && $rootNode->name instanceof Identifier) { + if (($rootNode instanceof Node\Expr\PropertyFetch || $rootNode instanceof Node\Expr\NullsafePropertyFetch) + && $rootNode->name instanceof Identifier + ) { $left = self::getChainedPropertyFetchName($rootNode->var); return $left ? ($left."->".$rootNode->name) : null; } else if ($rootNode instanceof Node\Expr\ArrayDimFetch) { diff --git a/tests/TestSuiteSetup.php b/tests/TestSuiteSetup.php index 6a0c787..eb92359 100644 --- a/tests/TestSuiteSetup.php +++ b/tests/TestSuiteSetup.php @@ -1,6 +1,7 @@ analyzeString($fileData,$additionalConfig)->getCounts(); + return array_sum($counts); + } + + public function analyzeString($fileData, $additionalConfig=[]) { + $fileName = "test.php"; + $emit = ErrorConstants::getConstants(); + unset( $emit[array_search(ErrorConstants::TYPE_AUTOLOAD_ERROR, $emit)] ); + $additionalConfig = array_merge(["basePath" => "/"], $additionalConfig); + return $this->analyzeStringToOutput($fileName, $fileData, $emit, $additionalConfig); + } + public function analyzeStringToOutput(string $fileName, string $fileData, $emit, array $additionalConfig = []) { if (!str_starts_with($fileData," Date: Wed, 14 Feb 2024 20:11:55 -0700 Subject: [PATCH 6/7] Support for "never" as a return type. 8.1 Support for "true", "false", "null" as declared return types. 8.2 Support for typed constants 8.3 Checks for ?null as an illegal type (nullable null) --- src/TypeComparer.php | 1 + tests/units/Checks/TestNullTrueFalseTypes.php | 122 ++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 tests/units/Checks/TestNullTrueFalseTypes.php diff --git a/src/TypeComparer.php b/src/TypeComparer.php index 4ec7c13..ac3f56a 100644 --- a/src/TypeComparer.php +++ b/src/TypeComparer.php @@ -69,6 +69,7 @@ function areSimpleTypesCompatible(Name|Identifier|null $target, Name|Identifier| $valueName = strtolower($value->getAttribute('namespacedName') ?: strval($value)); + //echo "Checking compatibility of $targetName and $valueName\n"; if ($targetName==$valueName || $targetName=="mixed") { return true; } diff --git a/tests/units/Checks/TestNullTrueFalseTypes.php b/tests/units/Checks/TestNullTrueFalseTypes.php new file mode 100644 index 0000000..b1d80da --- /dev/null +++ b/tests/units/Checks/TestNullTrueFalseTypes.php @@ -0,0 +1,122 @@ +assertEquals(0, $this->getStringErrorCount($code), "Failed false param"); + } + + function testTrueParam() { + $code = <<<'ENDCODE' + function foo(true $foo):void { + // WHY?!?! + var_dump($foo); + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed true param"); + } + + function testNullParam() { + $code = <<<'ENDCODE' + function foo(null $foo):void { + // WHY?!?! + var_dump($foo); + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed null param"); + } + + function testFalseReturnValue1() { + $code = <<<'ENDCODE' + function foo():false { + return false; + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed false return type"); + } + + function testFalseReturnValueReturningTrue() { + $code = <<<'ENDCODE' + function foo():false { + return true; + } + ENDCODE; + $this->assertEquals(1, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + function testNullReturnValue() { + $code = <<<'ENDCODE' + function foo():null { + return null; + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + function testNullReturnNonNull() { + $code = <<<'ENDCODE' + function foo():null { + return null; + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + function testNullableNullNotAllowed() { + $code = <<<'ENDCODE' + function foo():?null { + return null; + } + ENDCODE; + $this->assertEquals(1, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + function testTrueReturn() { + $code = <<<'ENDCODE' + function foo():true { + return true; + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + + function testTrueReturnNonBool() { + $code = <<<'ENDCODE' + function foo():true { + return 5; + } + ENDCODE; + $this->assertEquals(1, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + function testNeverReturn() { + $code = <<<'ENDCODE' + function foo():never { + exit(1); + } + ENDCODE; + $this->assertEquals(0, $this->getStringErrorCount($code), "Failed false return type returning true"); + } + + function testNeverReturnWithReturn() { + $code = <<<'ENDCODE' + function foo():never { + return 1; + } + ENDCODE; + $this->assertEquals(1, $this->getStringErrorCount($code), "Failed false return type returning true"); + } +} \ No newline at end of file From badb71a4077ab7ebcf48712243fde933633e3735 Mon Sep 17 00:00:00 2001 From: JonGardiner Date: Wed, 14 Feb 2024 20:44:23 -0700 Subject: [PATCH 7/7] Support for "never" as a return type. 8.1 Support for "true", "false", "null" as declared return types. 8.2 Support for typed constants 8.3 Checks for ?null as an illegal type (nullable null) --- src/Checks/CallCheck.php | 4 +-- src/Checks/ClassConstCheck.php | 8 ++--- src/Checks/PropertyStoreCheck.php | 2 +- src/Checks/ReturnCheck.php | 2 +- src/TypeComparer.php | 8 ++--- tests/units/Checks/TestClassConstantCheck.php | 29 +++++++++++++++++++ 6 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/Checks/CallCheck.php b/src/Checks/CallCheck.php index 9a7d82a..622355a 100644 --- a/src/Checks/CallCheck.php +++ b/src/Checks/CallCheck.php @@ -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, diff --git a/src/Checks/ClassConstCheck.php b/src/Checks/ClassConstCheck.php index 9beb2d2..8d17d28 100644 --- a/src/Checks/ClassConstCheck.php +++ b/src/Checks/ClassConstCheck.php @@ -27,11 +27,11 @@ function run($fileName, Node $node, ClassLike $inside = null, Scope $scope = nul /** @var Node\Const_ $const */ $constValue = $const->value->getAttribute(TypeComparer::INFERRED_TYPE_ATTR ); if ($node->type && $constValue) { - if (!$this->comparer->isCompatibleWithTarget($node->type, $constValue, $scope)) { + if (!$this->comparer->isCompatibleWithTarget($node->type, $constValue, forceStrict: true)) { $this->emitError($fileName, $node, ErrorConstants::TYPE_CONST_TYPE, - "Type mismatch between declared type and constant value " . - TypeComparer::typeToString($node->type) . " vs " . - TypeComparer::typeToString($constValue) + "Type mismatch between declared type (". + TypeComparer::typeToString($node->type) . ") and constant value (" . + TypeComparer::typeToString($constValue) . ")" ); } } diff --git a/src/Checks/PropertyStoreCheck.php b/src/Checks/PropertyStoreCheck.php index ce45801..4e0ffcd 100644 --- a/src/Checks/PropertyStoreCheck.php +++ b/src/Checks/PropertyStoreCheck.php @@ -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 { diff --git a/src/Checks/ReturnCheck.php b/src/Checks/ReturnCheck.php index be398cc..1fac5a5 100644 --- a/src/Checks/ReturnCheck.php +++ b/src/Checks/ReturnCheck.php @@ -85,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) . diff --git a/src/TypeComparer.php b/src/TypeComparer.php index ac3f56a..63b2f80 100644 --- a/src/TypeComparer.php +++ b/src/TypeComparer.php @@ -185,7 +185,7 @@ private function simpleTypeIsCompatibleWithIntersectionType(Name|Identifier $tar * @return bool * */ - function isCompatibleWithTarget(ComplexType|Name|Identifier|null $target, ComplexType|Name|Identifier|null $value, Scope $scope ) : bool { + function isCompatibleWithTarget(ComplexType|Name|Identifier|null $target, ComplexType|Name|Identifier|null $value, $forceStrict=false ) : bool { if ($target === NULL || $value === null) { return true; @@ -193,7 +193,7 @@ function isCompatibleWithTarget(ComplexType|Name|Identifier|null $target, Comple // Many target options, many values. Every value option must match at least one target. $ret = self::ifEveryType($value, fn($valueType) => - self::ifAnyType($target, function($targetType) use ($scope, $valueType) { + self::ifAnyType($target, function($targetType) use ($forceStrict, $valueType) { if($targetType instanceof IntersectionType) { $types = $targetType->types; @@ -204,11 +204,11 @@ function isCompatibleWithTarget(ComplexType|Name|Identifier|null $target, Comple foreach ($types as $targetComponentType) { if ($valueType instanceof IntersectionType) { - if (!$this->simpleTypeIsCompatibleWithIntersectionType($targetComponentType, $valueType, $scope->isStrict())) { + if (!$this->simpleTypeIsCompatibleWithIntersectionType($targetComponentType, $valueType,$forceStrict)) { return false; } } else { - if (!$this->areSimpleTypesCompatible($targetComponentType, $valueType, $scope->isStrict())) { + if (!$this->areSimpleTypesCompatible($targetComponentType, $valueType, $forceStrict)) { return false; } } diff --git a/tests/units/Checks/TestClassConstantCheck.php b/tests/units/Checks/TestClassConstantCheck.php index c4388b5..f6463a4 100644 --- a/tests/units/Checks/TestClassConstantCheck.php +++ b/tests/units/Checks/TestClassConstantCheck.php @@ -69,4 +69,33 @@ public function testAccessingUnknownClassConstant() { public function testAccessingUnknownClass() { $this->assertEquals(1, $this->runAnalyzerOnFile('.6.inc', ErrorConstants::TYPE_UNKNOWN_CLASS)); } + + public function testClassConstantType() { + $code = <<<'ENDCODE' + class Foo { + const string Bar = "Bar"; + const int Bad = 5; + const float Baz = 5.5; + const bool TRU = true; + const bool FALS = false; + const true TERU = true; + } + ENDCODE; + + $this->assertEquals(0, $this->getStringErrorCount($code), "Error with valid class constant."); + } + + public function testBadClassConstantType() { + $code = <<<'ENDCODE' + class Foo { + const string Bar = 5; + const int Bad = "Bad"; + const float Baz = false; + const bool TRU = 0; + const bool FALS = "Strike"; + const true TERU = 1.2; + } + ENDCODE; + $this->assertEquals(6, $this->getStringErrorCount($code), "Error with valid class constant."); + } }