Skip to content
Draft
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
3 changes: 2 additions & 1 deletion composer-require-checker.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"PHPStan\\Type\\ArrayType",
"PHPStan\\Type\\Constant\\ConstantStringType",
"PHPStan\\Type\\ObjectType",
"PHPStan\\Type\\Type"
"PHPStan\\Type\\Type",
"PHPStan\\Type\\ErrorType"
]
}
12 changes: 8 additions & 4 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,18 @@ rules:

services:
-
class: Sfp\PHPStan\Psr\Log\Rules\ContextRequireExceptionKeyRule
class: Sfp\PHPStan\Psr\Log\Rules\ContextKeyRule
arguments:
reportContextExceptionLogLevel: %sfpPsrLog.reportContextExceptionLogLevel%
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
contextKeyOriginalPattern: %sfpPsrLog.contextKeyOriginalPattern%
tags:
- phpstan.rules.rule
-
class: Sfp\PHPStan\Psr\Log\Rules\ContextKeyRule
class: Sfp\PHPStan\Psr\Log\Rules\ContextRequireExceptionKeyRule
arguments:
contextKeyOriginalPattern: %sfpPsrLog.contextKeyOriginalPattern%
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
reportMaybes: %reportMaybes%
reportContextExceptionLogLevel: %sfpPsrLog.reportContextExceptionLogLevel%
tags:
- phpstan.rules.rule

16 changes: 13 additions & 3 deletions src/Rules/ContextKeyRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,17 @@ final class ContextKeyRule implements Rule

private const ERROR_NOT_MATCH_ORIGINAL_PATTERN = 'Parameter $context of logger method Psr\Log\LoggerInterface::%s(), key should be match %s.';

/** @var bool */
private $treatPhpDocTypesAsCertain;

/** @var string|null */
private $contextKeyOriginalPattern;

public function __construct(?string $contextKeyOriginalPattern = null)
{
public function __construct(
bool $treatPhpDocTypesAsCertain,
?string $contextKeyOriginalPattern = null
) {
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->contextKeyOriginalPattern = $contextKeyOriginalPattern;
}

Expand Down Expand Up @@ -85,7 +91,11 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

$arrayType = $scope->getType($context->value);
if ($this->treatPhpDocTypesAsCertain) {
$arrayType = $scope->getType($context->value);
} else {
$arrayType = $scope->getNativeType($context->value);
}

if ($arrayType->isIterableAtLeastOnce()->no()) {
// @codeCoverageIgnoreStart
Expand Down
56 changes: 45 additions & 11 deletions src/Rules/ContextRequireExceptionKeyRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\ArrayType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\ObjectType;
use Throwable;

Expand All @@ -37,11 +37,22 @@ final class ContextRequireExceptionKeyRule implements Rule

private const ERROR_MISSED_EXCEPTION_KEY = 'Parameter $context of logger method Psr\Log\LoggerInterface::%s() requires \'exception\' key. Current scope has Throwable variable - %s';

/** @var bool */
private $treatPhpDocTypesAsCertain;

/** @var bool */
private $reportMaybes;

/** @var string */
private $reportContextExceptionLogLevel;

public function __construct(string $reportContextExceptionLogLevel = 'debug')
{
public function __construct(
bool $treatPhpDocTypesAsCertain,
bool $reportMaybes,
string $reportContextExceptionLogLevel = 'debug'
) {
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->reportMaybes = $reportMaybes;
$this->reportContextExceptionLogLevel = $reportContextExceptionLogLevel;
}

Expand Down Expand Up @@ -116,7 +127,7 @@ public function processNode(Node $node, Scope $scope): array
/** @psalm-suppress RedundantConditionGivenDocblockType */
assert($context instanceof Node\Arg);

if (self::contextDoesNotHaveExceptionKey($context, $scope)) {
if ($this->contextDoesNotHaveExceptionKey($context, $scope)) {
if (! $this->hasReportLogLevel($logLevels)) {
return [];
}
Expand Down Expand Up @@ -161,16 +172,39 @@ private function findCurrentScopeThrowableVariable(Scope $scope): ?string
return null;
}

private static function contextDoesNotHaveExceptionKey(Node\Arg $context, Scope $scope): bool
private function contextDoesNotHaveExceptionKey(Node\Arg $context, Scope $scope): ?bool
{
$type = $scope->getType($context->value);
if ($type instanceof ArrayType) {
if ($type->hasOffsetValueType(new ConstantStringType('exception'))->yes()) {
return false;
if ($this->treatPhpDocTypesAsCertain) {
$type = $scope->getType($context->value);
} else {
$type = $scope->getNativeType($context->value);
}

if ($type instanceof ErrorType) {
return null;
}

$exceptionOffset = $type->hasOffsetValueType(new ConstantStringType('exception'));

if (! $this->reportMaybes) {
return $exceptionOffset->no();
}

if ($exceptionOffset->yes()) {
return false;
}

if ($exceptionOffset->maybe()) {
// check unions
foreach ($type->getConstantArrays() as $constantArray) {
if ($constantArray->hasOffsetValueType(new ConstantStringType('exception'))->no()) {
return true;
}
}
return true;

return false;
}

return false;
return true;
}
}
56 changes: 40 additions & 16 deletions test/Rules/ContextKeyRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,42 @@
use PHPStan\Testing\RuleTestCase;
use Sfp\PHPStan\Psr\Log\Rules\ContextKeyRule;

use function array_pop;

/**
* @extends RuleTestCase<ContextKeyRule>
* @covers \Sfp\PHPStan\Psr\Log\Rules\ContextKeyRule
*/
final class ContextKeyRuleTest extends RuleTestCase
{
/** @var bool */
private $treatPhpDocTypesAsCertain = false;

/** @var null|string */
private $contextKeyOriginalPattern;

protected function getRule(): Rule
{
return new ContextKeyRule($this->contextKeyOriginalPattern);
return new ContextKeyRule($this->treatPhpDocTypesAsCertain, $this->contextKeyOriginalPattern);
}

public function testAlwaysShouldBeCheckedNonEmptyString(): void
/**
* @dataProvider provideNonEmptyStringKeyPattern
* @phpstan-param list<array{0:string, 1:int}> $expectedErrors
*/
public function testAlwaysShouldBeCheckedNonEmptyString(bool $treatPhpDocTypesAsCertain, array $expectedErrors): void
{
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->contextKeyOriginalPattern = null;
$this->analyse([__DIR__ . '/data/contextKey_nonEmptyString.php'], [
$this->analyse([__DIR__ . '/data/contextKey_nonEmptyString.php'], $expectedErrors);
}

/**
* @phpstan-return list<array{treatPhpDocTypesAsCertain: bool, list<array{0:string, 1:int}>}>
*/
public static function provideNonEmptyStringKeyPattern(): array
{
$expectedErrors = [
[
'Parameter $context of logger method Psr\Log\LoggerInterface::info(), key should be non empty string.',
20, // empty string
Expand All @@ -48,29 +66,35 @@ public function testAlwaysShouldBeCheckedNonEmptyString(): void
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::info(), key should be non empty string.',
27, // union parameter
29, // inline array
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::info(), key should be non empty string.',
32, // inline array
32, // union parameter
],
]);
}
];

public function testWithPattern(): void
{
$this->contextKeyOriginalPattern = '#\A[A-Za-z0-9-]+';
$this->analyse([__DIR__ . '/data/contextKey_originalPattern.php'], [
$expectedErrors1 = $expectedErrors;
array_pop($expectedErrors);

return [
[
'Your contextKeyOriginalPattern #\A[A-Za-z0-9-]+ seems not valid regex. Failed.',
8,
'treatPhpDocTypesAsCertain' => true,
$expectedErrors1,
],
[
'Your contextKeyOriginalPattern #\A[A-Za-z0-9-]+ seems not valid regex. Failed.',
9,
'treatPhpDocTypesAsCertain' => false,
$expectedErrors,
],
];
}

public function testWithPattern(): void
{
$this->contextKeyOriginalPattern = '#\A[A-Za-z0-9-]+\z#';
$this->analyse([__DIR__ . '/data/contextKey_originalPattern.php'], [
[
'Your contextKeyOriginalPattern #\A[A-Za-z0-9-]+ seems not valid regex. Failed.',
'Parameter $context of logger method Psr\Log\LoggerInterface::info(), key should be match #\A[A-Za-z0-9-]+\z#.',
14,
],
]);
Expand Down
61 changes: 51 additions & 10 deletions test/Rules/ContextRequireExceptionKeyRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,26 @@
*/
final class ContextRequireExceptionKeyRuleTest extends RuleTestCase
{
/** @var bool */
private $treatPhpDocTypesAsCertain = false;

/** @var bool */
private $reportMaybes = false;

/** @phpstan-var 'debug'|'info' */
private $reportContextExceptionLogLevel = 'debug';

protected function getRule(): Rule
{
return new ContextRequireExceptionKeyRule($this->reportContextExceptionLogLevel);
return new ContextRequireExceptionKeyRule(
$this->treatPhpDocTypesAsCertain,
$this->reportMaybes,
$this->reportContextExceptionLogLevel
);
}

/** @test */
public function shouldNotBeReportsIfLogLevelIsUnder(): void
public function shouldNotBeReportedIfLogLevelIsNotReached(): void
{
$this->reportContextExceptionLogLevel = 'info';

Expand All @@ -35,44 +45,75 @@ public function shouldNotBeReportsIfLogLevelIsUnder(): void
]);
}

/**
* @testWith [ false, false, 0, [] ]
* [ false, true, 0, [] ]
* [ true, false, 1, [26] ]
* [ true, true, 2, [23, 26] ]
* @phpstan-param list<int> $expectedErrorLines
*/
public function testParameterSettings(
bool $treatPhpDocTypesAsCertain,
bool $reportMaybes,
int $expectedErrorCount,
array $expectedErrorLines
): void {
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->reportMaybes = $reportMaybes;
$this->reportContextExceptionLogLevel = 'debug';

$errors = $this->gatherAnalyserErrors([__DIR__ . '/data/ContextRequireExceptionKeyRule/parameters.php']);

$this->assertCount($expectedErrorCount, $errors);

$errorLines = [];
foreach ($errors as $error) {
$errorLines[] = $error->getLine();
}

$this->assertSame($expectedErrorLines, $errorLines);
}

/**
* @test
*/
public function testProcessNode(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->reportMaybes = false;
$this->reportContextExceptionLogLevel = 'debug';
$this->analyse([__DIR__ . '/data/contextRequireExceptionKey.php'], [
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception',
24,
22,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception',
25,
23,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::log() requires \'exception\' key. Current scope has Throwable variable - $exception',
28,
26,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::log() requires \'exception\' key. Current scope has Throwable variable - $exception',
29,
27,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception',
33,
31,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception',
35,
33,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception',
37,
35,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception',
38,
36,
],
[
'Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires \'exception\' key. Current scope has Throwable variable - $exception2',
Expand Down
28 changes: 28 additions & 0 deletions test/Rules/data/ContextRequireExceptionKeyRule/parameters.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

/**
* @phpstan-param array{exception: \Exception} $context
* @phpstan-param array{exception: \Exception}|array{exception2: \Exception} $unionContext
*
* NOTES: union param would be reported level7(reportMaybes)
* https://phpstan.org/r/8f1158f4-0cac-4ef9-9abe-29a892d502af
*/
function main(
Psr\Log\LoggerInterface $logger,
array $nonTypedArray,
array $context,
array $unionContext
): void {
try {
throw new InvalidArgumentException();
} catch (InvalidArgumentException $exception) {
$logger->notice('foo', $nonTypedArray);
$logger->notice('foo', $context);
$logger->notice('foo', $unionContext);

$logger->notice('foo', array_merge(['foo' => 1], ['exception' => $exception]));
$logger->notice('foo', array_merge(['foo' => 1], ['exception2' => $exception]));
}
}
Loading