Skip to content

Commit

Permalink
Merge pull request #14 from sasezaki/reportContextExceptionLogLevel
Browse files Browse the repository at this point in the history
Support reportContextExceptionLogLevel
  • Loading branch information
sasezaki authored Mar 21, 2023
2 parents f3cc238 + 18cb1f1 commit 23f3b2e
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 17 deletions.
4 changes: 3 additions & 1 deletion example/phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
# ./vendor/bin/phpstan analyse -c ./example/phpstan.neon --level 5 example/

parameters:
level: 5
level: 9
bootstrapFiles:
- vendor/autoload.php
paths:
- src
sfpPsrLog:
reportContextExceptionLogLevel: 'notice'

includes:
- ../extension.neon
Expand Down
17 changes: 13 additions & 4 deletions example/src/Example.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,25 @@ public function __construct(LoggerInterface $logger)
public function exceptionKeyOnlyAllowThrowable(\Throwable $throwable): void
{
// invalid
$this->logger->debug('foo', ['exception' => $throwable->getMessage()]);
$this->logger->notice('foo', ['exception' => $throwable->getMessage()]);
$this->logger->log('panic', 'foo', ['exception' => $throwable]);

// valid
$this->logger->log('info', 'foo', ['exception' => $throwable]);
$this->logger->log('notice', 'foo', ['exception' => $throwable]);
}

public function mustIncludesCurrentScopeThrowableIntoContext(\Throwable $throwable)
public function mustIncludesCurrentScopeThrowableIntoContext(\Throwable $throwable): void
{
// Parameter $context of logger method Psr\Log\LoggerInterface::info() requires 'exception' key. Current scope has Throwable variable - $throwable
$this->logger->info('foo');
$this->logger->notice('foo');

$this->logger->notice('foo', ['user' => 1]);
}

public function reportContextExceptionLogLevel(\Throwable $throwable): void
{
// phpstan.neon sfpPsrLog.reportContextExceptionLogLevel is 'notice'
// so bellow would not report.
$this->logger->debug('foo');
}
}
18 changes: 16 additions & 2 deletions rules.neon
Original file line number Diff line number Diff line change
@@ -1,2 +1,16 @@
rules:
- Sfp\PHPStan\Psr\Log\Rules\ContextRequireExceptionKeyRule
parameters:
sfpPsrLog:
reportContextExceptionLogLevel: 'debug'

parametersSchema:
sfpPsrLog: structure([
reportContextExceptionLogLevel: schema(string(), nullable())
])

services:
-
class: Sfp\PHPStan\Psr\Log\Rules\ContextRequireExceptionKeyRule
arguments:
reportContextExceptionLogLevel: %sfpPsrLog.reportContextExceptionLogLevel%
tags:
- phpstan.rules.rule
36 changes: 36 additions & 0 deletions src/Rules/ContextRequireExceptionKeyRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,27 @@ class ContextRequireExceptionKeyRule implements Rule
'debug',
];

private const LOGGER_LEVELS = [
'emergency' => 7,
'alert' => 6,
'critical' => 5,
'error' => 4,
'warning' => 3,
'notice' => 2,
'info' => 1,
'debug' => 0,
];

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 string */
private $reportContextExceptionLogLevel;

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

public function getNodeType(): string
{
return Node\Expr\MethodCall::class;
Expand Down Expand Up @@ -59,11 +78,15 @@ public function processNode(Node $node, Scope $scope): array

$methodName = $node->name->toLowerString();

$logLevel = $methodName;
$contextArgumentNo = 1;
if ($methodName === 'log') {
if (count($args) === 1) {
return [];
}
assert($args[0] instanceof Node\Arg);
assert($args[0]->value instanceof Node\Scalar\String_);
$logLevel = $args[0]->value->value;
$contextArgumentNo = 2;
} elseif (! in_array($methodName, self::LOGGER_LEVEL_METHODS)) {
return [];
Expand All @@ -76,6 +99,10 @@ public function processNode(Node $node, Scope $scope): array
}

if (! isset($args[$contextArgumentNo])) {
if (! $this->isReportLogLevel($logLevel)) {
return [];
}

return [sprintf(self::ERROR_MISSED_EXCEPTION_KEY, $methodName, "\${$throwable}")];
}

Expand All @@ -86,12 +113,21 @@ public function processNode(Node $node, Scope $scope): array
}

if ($context instanceof Node\Arg && self::contextDoesNotHavExceptionKey($context)) {
if (! $this->isReportLogLevel($logLevel)) {
return [];
}

return [sprintf(self::ERROR_MISSED_EXCEPTION_KEY, $methodName, "\${$throwable}")];
}

return [];
}

public function isReportLogLevel(string $logLevel): bool
{
return self::LOGGER_LEVELS[$logLevel] >= self::LOGGER_LEVELS[$this->reportContextExceptionLogLevel];
}

private function findCurrentScopeThrowableVariable(Scope $scope): ?string
{
foreach ($scope->getDefinedVariables() as $var) {
Expand Down
14 changes: 7 additions & 7 deletions test/Rules/ContextRequireExceptionKeyRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class ContextRequireExceptionKeyRuleTest extends RuleTestCase
{
protected function getRule(): Rule
{
return new ContextRequireExceptionKeyRule();
return new ContextRequireExceptionKeyRule('info');
}

/** @test */
Expand All @@ -22,27 +22,27 @@ public function testProcessNode()
$this->analyse([__DIR__ . '/data/contextRequireExceptionKey.php'], [
'missing context' => [
'Parameter $context of logger method Psr\Log\LoggerInterface::info() requires \'exception\' key. Current scope has Throwable variable - $exception',
15,
17,
],
'invalid key' => [
'Parameter $context of logger method Psr\Log\LoggerInterface::info() requires \'exception\' key. Current scope has Throwable variable - $exception',
16,
18,
],
'missing context - log method' => [
'Parameter $context of logger method Psr\Log\LoggerInterface::log() requires \'exception\' key. Current scope has Throwable variable - $exception',
19,
21,
],
'invalid key - log method' => [
'Parameter $context of logger method Psr\Log\LoggerInterface::log() requires \'exception\' key. Current scope has Throwable variable - $exception',
20,
22,
],
'missing context - other catch' => [
'Parameter $context of logger method Psr\Log\LoggerInterface::critical() requires \'exception\' key. Current scope has Throwable variable - $exception2',
26,
29,
],
'invalid key - other catch' => [
'Parameter $context of logger method Psr\Log\LoggerInterface::log() requires \'exception\' key. Current scope has Throwable variable - $exception2',
27,
30,
],
]);
}
Expand Down
12 changes: 9 additions & 3 deletions test/Rules/data/contextRequireExceptionKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,32 @@
// phpcs:enable

try {
$logger->debug("foo"); // allow
$logger->log('debug', "foo"); // allow
$logger->debug("foo");
$logger->log('debug', "foo");

throw new InvalidArgumentException();
} catch (LogicException $exception) {
// ng
$logger->debug("foo"); // but would not be report
$logger->info("foo");
$logger->info('foo', ['throwable' => $exception]);

// log method
// ng. (by `log` call)
$logger->log('notice', 'foo');
$logger->log('notice', 'foo', ['throwable' => $exception]);

// ok
$logger->alert("foo", ['exception' => $exception]);
$logger->log('alert', 'foo', ['exception' => $exception]);
} catch (RuntimeException | Throwable $exception2) {
// ng
$logger->critical('foo');
$logger->log('critical', 'foo');
$logger->debug("foo", ['exception' => new DateTimeImmutable()]); // but would not be report

// ok
$logger->critical("foo", ['exception' => $exception2]);
} finally {
// ok
$logger->emergency('foo');
}

0 comments on commit 23f3b2e

Please sign in to comment.