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

[SPN-1447] Start of Service Documentation Guardrail #126

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Checks/ErrorConstants.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class ErrorConstants {
const TYPE_VARIABLE_FUNCTION_NAME = 'Standard.VariableFunctionCall';
const TYPE_VARIABLE_VARIABLE = 'Standard.VariableVariable';
const TYPE_COUNTABLE_EMPTINESS_CHECK = 'Standard.Countable.Emptiness';
const TYPE_WEB_API_DOCUMENTATION_CHECK = 'Standard.WebApi.Documentation';
const TYPE_SERVICE_METHOD_DOCUMENTATION_CHECK = 'Standard.ServiceMethod.Documentation';


/**
Expand Down
180 changes: 180 additions & 0 deletions src/Checks/ServiceMethodDocumentationCheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
<?php

namespace BambooHR\Guardrail\Checks;

use BambooHR\Guardrail\Scope;
use PhpParser\Node;
use PhpParser\Node\Stmt\ClassLike;

class ServiceMethodDocumentationCheck extends BaseCheck {

private const array BLOCKED_SERVICE_DOCUMENTATION_TYPES = [null, 'mixed', 'object'];
/**
* getCheckNodeTypes
*
* @return string[]
*/
function getCheckNodeTypes() {
return [Node\Stmt\ClassMethod::class];
}

/**
* @param string $fileName The name of the file we are parsing
* @param Node $node Instance of the Node
* @param ClassLike|null $inside Instance of the ClassLike (the class we are parsing) [optional]
* @param Scope|null $scope Instance of the Scope (all variables in the current state) [optional]
*
* @return void
*/
public function run($fileName, Node $node, ClassLike $inside = null, Scope $scope = null) {
if ($node instanceof Node\Stmt\ClassMethod && $node->isPublic()) {
$docComment = $node->getDocComment();
if (empty($docComment)) {
$this->emitMissingDocBlockError($fileName, $node, $inside);
return;
}

$docCommentData = $this->extractDocCommentData($docComment);
$actualParams = array_map(fn($param) => $param->var, $node->getParams());
$docCommentParams = $docCommentData['params'];

$this->validateParameters($actualParams, $docCommentParams, $fileName, $node, $inside);
$this->validateReturnType($node, $docCommentData['return'], $fileName, $inside);
}
}

private function emitMissingDocBlockError(string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside): void {
$this->emitErrorOnLine($fileName, $node->getLine(),
ErrorConstants::TYPE_SERVICE_METHOD_DOCUMENTATION_CHECK,
"Method: {$node->name->name}, Class: {$inside->name->name} - All public Service methods must have a DocBlock."
);
}

private function validateParameters($actualParams, $docCommentParams, string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside): void {
if (count($docCommentParams) > count($actualParams)) {
$this->emitErrorOnLine($fileName, $node->getLine(),
ErrorConstants::TYPE_SERVICE_METHOD_DOCUMENTATION_CHECK,
"Method: {$node->name->name}, Class: {$inside->name->name} - There are extra parameters in your DocBlock that are not present in the method signature."
);
}

foreach ($actualParams as $actualParam) {
$this->validateParameter($actualParam, $docCommentParams, $fileName, $node, $inside);
}
}

private function validateParameter($actualParam, $docCommentParams, string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside): void {
$actualParamName = $actualParam->name ?? $actualParam->getString();
$docCommentParam = $docCommentParams[$actualParamName] ?? null;
if (!$docCommentParam) {
$this->emitParameterMismatchError($fileName, $node, $inside, $actualParamName);
return;
}

$actualParamAttribute = $actualParam->getAttribute('inferrer-type');
if ($this->isComplexType($actualParamAttribute)) {
$this->validateComplexType($actualParamAttribute->types, $docCommentParam['type'], $fileName, $node, $inside, $actualParamName);
} else {
$actualParamType = $actualParamAttribute?->name ?? $actualParamAttribute?->toString();
$this->validateSimpleType($actualParamType, $docCommentParam['type'], $fileName, $node, $inside, $actualParamName);
}
}

private function isComplexType($type): bool {
return $type instanceof Node\UnionType || $type instanceof Node\IntersectionType;
}

private function validateComplexType($actualParamTypes, $docCommentParamType, string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside, string $propertyName): void {
if (!is_array($docCommentParamType) || count($actualParamTypes) !== count($docCommentParamType)) {
$this->emitTypeMismatchError($fileName, $node, $inside, $propertyName);
return;
}

foreach ($actualParamTypes as $typeObject) {
$actualParamType = $typeObject->name ?? $typeObject->toString();
if (!in_array($actualParamType, $docCommentParamType) || in_array($actualParamType, self::BLOCKED_SERVICE_DOCUMENTATION_TYPES)) {
$this->emitTypeMismatchError($fileName, $node, $inside, $propertyName);
break;
}
}
}

private function validateSimpleType($actualParamType, $docCommentParamType, string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside, string $paramName): void {
if ($actualParamType !== $docCommentParamType || in_array($actualParamType, self::BLOCKED_SERVICE_DOCUMENTATION_TYPES)) {
$this->emitTypeMismatchError($fileName, $node, $inside, $paramName);
}
}

private function emitParameterMismatchError(string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside, string $paramName): void {
$this->emitErrorOnLine($fileName, $node->getLine(),
ErrorConstants::TYPE_SERVICE_METHOD_DOCUMENTATION_CHECK,
"Method: {$node->name->name}, Class: {$inside->name->name} - DocBlock does not contain matching parameter: $paramName"
);
}

private function emitTypeMismatchError(string $fileName, Node\Stmt\ClassMethod $node, Node\Stmt\ClassLike $inside, string $propertyName): void {
$this->emitErrorOnLine($fileName, $node->getLine(),
ErrorConstants::TYPE_SERVICE_METHOD_DOCUMENTATION_CHECK,
"Method: {$node->name->name}, Class: {$inside->name->name}, Property: $propertyName - DocBlock does not match method signature."
);
}

/**
* @param Node\Stmt\ClassMethod $node
* @param $docCommentReturn
* @param string $fileName
* @param ClassLike $inside
*
* @return void
*/
private function validateReturnType(Node\Stmt\ClassMethod $node, $docCommentReturn, string $fileName, Node\Stmt\ClassLike $inside): void {
$propertyName = 'return';
if (!$docCommentReturn) {
$this->emitTypeMismatchError($fileName, $node, $inside, $propertyName);
return;
}

$actualReturn = $node->getReturnType();
if ($this->isComplexType($actualReturn)) {
$this->validateComplexType($actualReturn->types, $docCommentReturn, $fileName, $node, $inside, $propertyName);
} else {
$this->validateSimpleType($actualReturn?->name ?? $actualReturn?->toString() ?? null, $docCommentReturn, $fileName, $node, $inside, $propertyName);
}
}

/**
* @param string|null $docComment
*
* @return array
*/
private function extractDocCommentData(?string $docComment) {
$result = [
'params' => [],
'return' => null,
];
if (preg_match_all('/@param\s+([^\s$]+(?:\s*[\|&]\s*[^\s$]+)*)?\s*(\$[^\s]+)?/', $docComment, $paramMatches, PREG_SET_ORDER)) {
foreach ($paramMatches as $paramMatch) {
$result['params'][$variableName] = [
'type' => $this->extractDocGetVariableType($paramMatch[1] ?? null),
'variable' => $variableName = ltrim($paramMatch[2] ?? null, '$'),
];
}
}

if (preg_match('/@return\s+([^\s$]+(?:\s*\|\s*[^\s$]+|&[^\s$]+)*)/', $docComment, $returnMatch)) {
$result['return'] = $this->extractDocGetVariableType($returnMatch[1] ?? null);
}

return $result;
}

private function extractDocGetVariableType($variableType) {
if (str_contains($variableType, '|')) {
$variableType = array_map('trim', explode('|', $variableType));
} else if (str_contains($variableType, '&')) {
$variableType = array_map('trim', explode('&', $variableType));
}

return $variableType;
}
}
4 changes: 4 additions & 0 deletions src/NodeVisitors/StaticAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use BambooHR\Guardrail\Checks\ReadOnlyPropertyCheck;
use BambooHR\Guardrail\Checks\ReturnCheck;
use BambooHR\Guardrail\Checks\DependenciesOnVendorCheck;
use BambooHR\Guardrail\Checks\ServiceMethodDocumentationCheck;
use BambooHR\Guardrail\Checks\SplatCheck;
use BambooHR\Guardrail\Checks\StaticCallCheck;
use BambooHR\Guardrail\Checks\StaticPropertyFetchCheck;
Expand All @@ -43,6 +44,7 @@
use BambooHR\Guardrail\Checks\UnsafeSuperGlobalCheck;
use BambooHR\Guardrail\Checks\UnusedPrivateMemberVariableCheck;
use BambooHR\Guardrail\Checks\UseStatementCaseCheck;
use BambooHR\Guardrail\Checks\WebApiDocumentationCheck;
use BambooHR\Guardrail\Config;
use BambooHR\Guardrail\Evaluators as Ev;
use BambooHR\Guardrail\Evaluators\ExpressionInterface;
Expand Down Expand Up @@ -157,6 +159,8 @@ function __construct(SymbolTable $index, OutputInterface $output, MetricOutputIn
new ThrowsCheck($this->index, $output),
new CountableEmptinessCheck($this->index, $output),
new DependenciesOnVendorCheck($this->index, $output, $metricOutput),
new WebApiDocumentationCheck($this->index, $output, $metricOutput),
new ServiceMethodDocumentationCheck($this->index, $output),
//new ClassStoredAsVariableCheck($this->index, $output)
];

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

class MyService {
// This method should emit 2 errors
/**
* @param int $a
* @param string $b
*
* @return int
*/
public function noParamTypes($a, $b): int {
return 123;
}

// This method should emit 2 errors

/**
* @param mixed $a
* @param object $b
*
* @return int
*/
public function specialParamTypes(mixed $a, object $b): int {
return 123;
}

// This method should emit 1 error

/**
* @param int|string|object $a
* @param int|float $b
*
* @return int
*/
public function unionParamTypes(int | string | object $a, int | float $b): int {
return 123;
}

// This method should emit 0 errors

/**
* @param MyObj2&MyObj $a
*
* @return int
*/
public function intersectionParamTypes(MyObj&MyObj2 $a): int {
return 123;
}

// This method should emit 0 errors

/**
* @param bool $a
* @param int $b
* @param string $c
*
* @return int
*/
public function withNormalTypedParams(bool $a, int $b, string $c): int {
return 123;
}
}

class MyObj {
}

class MyObj2 {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

class MyService {
// This method should emit 1 error
/**
* @return int
*/
public function noReturnTypes() {
return 123;
}

// This method should emit 1 error

/**
* @return mixed
*/
public function specialReturnType(): mixed {
return 123;
}

// This method should emit 1 error

/**
* @return object
*/
public function specialReturnType2(): object {
return 123;
}

// This method should emit 1 error

/**
* @return int|string|object
*/
public function unionParamTypes(): int | string | object {
return 123;
}

// This method should emit 0 errors

/**
* @return MyObj2&MyObj
*/
public function intersectionParamTypes(): MyObj&MyObj2 {
}

// This method should emit 0 errors

/**
* @return bool
*/
public function withNormalTypedParams(): bool {
return true;
}
}

class MyObj {
}

class MyObj2 {
}
Loading