Skip to content

Commit

Permalink
Add error identifiers (#140)
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Oct 5, 2023
1 parent dd0c725 commit a67f18f
Show file tree
Hide file tree
Showing 34 changed files with 247 additions and 98 deletions.
14 changes: 11 additions & 3 deletions src/Rule/AllowComparingOnlyComparableTypesRule.php
Expand Up @@ -12,6 +12,8 @@
use PhpParser\Node\Expr\BinaryOp\Spaceship;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\FloatType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\ObjectType;
Expand All @@ -33,7 +35,7 @@ public function getNodeType(): string

/**
* @param BinaryOp $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand All @@ -54,11 +56,17 @@ public function processNode(Node $node, Scope $scope): array
$rightTypeDescribed = $rightType->describe(VerbosityLevel::typeOnly());

if (!$this->isComparable($leftType) || !$this->isComparable($rightType)) {
return ["Comparison {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed} contains non-comparable type, only int|float|string|DateTimeInterface is allowed."];
$error = RuleErrorBuilder::message("Comparison {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed} contains non-comparable type, only int|float|string|DateTimeInterface is allowed.")
->identifier('shipmonk.comparingNonComparableTypes')
->build();
return [$error];
}

if (!$this->isComparableTogether($leftType, $rightType)) {
return ["Cannot compare different types in {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed}."];
$error = RuleErrorBuilder::message("Cannot compare different types in {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed}.")
->identifier('shipmonk.comparingNonComparableTypes')
->build();
return [$error];
}

return [];
Expand Down
9 changes: 7 additions & 2 deletions src/Rule/AllowNamedArgumentOnlyInAttributesRule.php
Expand Up @@ -6,6 +6,8 @@
use PhpParser\Node\Arg;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use ShipMonk\PHPStan\Visitor\NamedArgumentSourceVisitor;

/**
Expand All @@ -21,7 +23,7 @@ public function getNodeType(): string

/**
* @param Arg $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand All @@ -33,7 +35,10 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

return ['Named arguments are allowed only within native attributes'];
$error = RuleErrorBuilder::message('Named arguments are allowed only within native attributes')
->identifier('shipmonk.namedArgumentOutsideAttribute')
->build();
return [$error];
}

}
9 changes: 7 additions & 2 deletions src/Rule/BackedEnumGenericsRule.php
Expand Up @@ -8,6 +8,8 @@
use PHPStan\Node\InClassNode;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\VerbosityLevel;

/**
Expand All @@ -23,7 +25,7 @@ public function getNodeType(): string

/**
* @param InClassNode $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand All @@ -47,7 +49,10 @@ public function processNode(Node $node, Scope $scope): array
}
}

return ["Class {$classReflection->getName()} extends generic BackedEnum, but does not specify its type. Use @implements $expectedTag"];
$error = RuleErrorBuilder::message("Class {$classReflection->getName()} extends generic BackedEnum, but does not specify its type. Use @implements $expectedTag")
->identifier('shipmonk.missingImplementsOnBackedEnum')
->build();
return [$error];
}

private function hasGenericsTag(ClassReflection $classReflection, string $expectedTag): bool
Expand Down
9 changes: 7 additions & 2 deletions src/Rule/ClassSuffixNamingRule.php
Expand Up @@ -6,6 +6,8 @@
use PHPStan\Analyser\Scope;
use PHPStan\Node\InClassNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use function strlen;
use function substr_compare;

Expand Down Expand Up @@ -35,7 +37,7 @@ public function getNodeType(): string

/**
* @param InClassNode $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(
Node $node,
Expand All @@ -60,7 +62,10 @@ public function processNode(
$className = $classReflection->getName();

if (substr_compare($className, $suffix, -strlen($suffix)) !== 0) {
return ["Class name $className should end with $suffix suffix"];
$error = RuleErrorBuilder::message("Class name $className should end with $suffix suffix")
->identifier('shipmonk.invalidClassSuffix')
->build();
return [$error];
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/Rule/EnforceEnumMatchRule.php
Expand Up @@ -8,6 +8,8 @@
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Enum\EnumCaseObjectType;
use function array_map;
use function array_merge;
Expand All @@ -27,7 +29,7 @@ public function getNodeType(): string

/**
* @param BinaryOp $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand Down Expand Up @@ -57,7 +59,10 @@ public function processNode(Node $node, Scope $scope): array
}

$trueFalse = $conditionType->isTrue()->yes() ? 'true' : 'false';
return ["This condition contains always-$trueFalse enum comparison of $enumCases[0]. Use match expression instead, PHPStan will report unhandled enum cases"];
$error = RuleErrorBuilder::message("This condition contains always-$trueFalse enum comparison of $enumCases[0]. Use match expression instead, PHPStan will report unhandled enum cases")
->identifier('shipmonk.enumMatchNotUsed')
->build();
return [$error];
}

return [];
Expand Down
2 changes: 1 addition & 1 deletion src/Rule/EnforceIteratorToArrayPreserveKeysRule.php
Expand Up @@ -50,7 +50,7 @@ public function processNode(Node $node, Scope $scope): array

return [RuleErrorBuilder::message('Calling iterator_to_array without 2nd parameter $preserve_keys. Default value true might cause failures or data loss.')
->line($node->getLine())
->identifier('iteratorToArrayWithoutPreserveKeys')
->identifier('shipmonk.iteratorToArrayWithoutPreserveKeys')
->build()];
}

Expand Down
9 changes: 7 additions & 2 deletions src/Rule/EnforceListReturnRule.php
Expand Up @@ -10,6 +10,8 @@
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Accessory\AccessoryArrayListType;
use PHPStan\Type\VerbosityLevel;
use function count;
Expand All @@ -27,7 +29,7 @@ public function getNodeType(): string

/**
* @param ReturnStatementsNode $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand All @@ -46,7 +48,10 @@ public function processNode(Node $node, Scope $scope): array
? 'Method'
: 'Function';

return ["{$callLikeType} {$methodReflection->getName()} always return list, but is marked as {$this->getReturnPhpDoc($methodReflection)}"];
$error = RuleErrorBuilder::message("{$callLikeType} {$methodReflection->getName()} always return list, but is marked as {$this->getReturnPhpDoc($methodReflection)}")
->identifier('shipmonk.returnListNotUsed')
->build();
return [$error];
}

return [];
Expand Down
11 changes: 7 additions & 4 deletions src/Rule/EnforceNativeReturnTypehintRule.php
Expand Up @@ -9,6 +9,8 @@
use PHPStan\Node\ReturnStatementsNode;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ArrayType;
use PHPStan\Type\BooleanType;
use PHPStan\Type\CallableType;
Expand Down Expand Up @@ -62,7 +64,7 @@ public function getNodeType(): string

/**
* @param ReturnStatementsNode $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand Down Expand Up @@ -92,9 +94,10 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

return [
sprintf('Missing native return typehint %s', $typeHint),
];
$error = RuleErrorBuilder::message(sprintf('Missing native return typehint %s', $typeHint))
->identifier('shipmonk.missingNativeReturnTypehint')
->build();
return [$error];
}

private function getTypehintByType(
Expand Down
9 changes: 7 additions & 2 deletions src/Rule/EnforceReadonlyPublicPropertyRule.php
Expand Up @@ -7,6 +7,8 @@
use PHPStan\Node\ClassPropertyNode;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;

/**
* @implements Rule<ClassPropertyNode>
Expand All @@ -28,7 +30,7 @@ public function getNodeType(): string

/**
* @param ClassPropertyNode $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand All @@ -50,7 +52,10 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

return ["Public property `{$node->getName()}` not marked as readonly."];
$error = RuleErrorBuilder::message("Public property `{$node->getName()}` not marked as readonly.")
->identifier('shipmonk.publicPropertyNotReadonly')
->build();
return [$error];
}

}
18 changes: 11 additions & 7 deletions src/Rule/ForbidAssignmentNotMatchingVarDocRule.php
Expand Up @@ -8,6 +8,8 @@
use PHPStan\Analyser\Scope;
use PHPStan\PhpDoc\Tag\VarTag;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ArrayType;
use PHPStan\Type\FileTypeMapper;
use PHPStan\Type\IterableType;
Expand Down Expand Up @@ -44,7 +46,7 @@ public function getNodeType(): string

/**
* @param Assign $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand Down Expand Up @@ -109,14 +111,16 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

return [
"Invalid var phpdoc of \${$variableName}. Cannot narrow {$valueTypeString} to {$varPhpDocTypeString}",
];
$error = RuleErrorBuilder::message("Invalid var phpdoc of \${$variableName}. Cannot narrow {$valueTypeString} to {$varPhpDocTypeString}")
->identifier('shipmonk.invalidVarDocAssignment')
->build();
return [$error];
}

return [
"Invalid var phpdoc of \${$variableName}. Cannot assign {$valueTypeString} to {$varPhpDocTypeString}",
];
$error = RuleErrorBuilder::message("Invalid var phpdoc of \${$variableName}. Cannot assign {$valueTypeString} to {$varPhpDocTypeString}")
->identifier('shipmonk.invalidVarDocAssignment')
->build();
return [$error];
}

private function weakenTypeToKeepShapeOnly(Type $type): Type
Expand Down
9 changes: 7 additions & 2 deletions src/Rule/ForbidCastRule.php
Expand Up @@ -14,6 +14,8 @@
use PhpParser\Node\Expr\Cast\Unset_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use function get_class;
use function in_array;

Expand Down Expand Up @@ -45,14 +47,17 @@ public function getNodeType(): string

/**
* @param Cast $node
* @return list<string>
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
$castString = $this->getCastString($node);

if (in_array($castString, $this->blacklist, true)) {
return ["Using $castString is discouraged, please avoid using that."];
$error = RuleErrorBuilder::message("Using $castString is discouraged, please avoid using that.")
->identifier('shipmonk.forbiddenCast')
->build();
return [$error];
}

return [];
Expand Down
6 changes: 5 additions & 1 deletion src/Rule/ForbidCheckedExceptionInCallableRule.php
Expand Up @@ -184,6 +184,7 @@ public function processClosure(
if ($this->exceptionTypeResolver->isCheckedException($exceptionClass, $throwPoint->getScope())) {
$errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in closure!")
->line($throwPoint->getNode()->getLine())
->identifier('shipmonk.checkedExceptionInCallable')
->build();
}
}
Expand Down Expand Up @@ -227,6 +228,7 @@ static function (): void {
if ($this->exceptionTypeResolver->isCheckedException($exceptionClass, $throwPoint->getScope())) {
$errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in arrow function!")
->line($throwPoint->getNode()->getLine())
->identifier('shipmonk.checkedExceptionInArrowFunction')
->build();
}
}
Expand Down Expand Up @@ -269,7 +271,9 @@ private function processThrowType(

foreach ($throwType->getObjectClassNames() as $exceptionClass) {
if ($this->exceptionTypeResolver->isCheckedException($exceptionClass, $scope)) {
$errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in first-class-callable!")->build();
$errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in first-class-callable!")
->identifier('shipmonk.checkedExceptionInCallable')
->build();
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Rule/ForbidCheckedExceptionInYieldingMethodRule.php
Expand Up @@ -52,6 +52,7 @@ public function processNode(
if ($this->exceptionTypeResolver->isCheckedException($exceptionClass, $throwPoint->getScope())) {
$errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in yielding method is denied as it gets thrown upon Generator iteration")
->line($throwPoint->getNode()->getLine())
->identifier('shipmonk.checkedExceptionInYieldingMethod')
->build();
}
}
Expand Down

0 comments on commit a67f18f

Please sign in to comment.