From 18cb1f14abcf7b0219d1b45d5de1fcc1200564f5 Mon Sep 17 00:00:00 2001 From: sasezaki Date: Tue, 21 Mar 2023 23:10:27 +0900 Subject: [PATCH] Support reportContextExceptionLogLevel --- example/phpstan.neon | 4 ++- example/src/Example.php | 17 ++++++--- rules.neon | 18 ++++++++-- src/Rules/ContextRequireExceptionKeyRule.php | 36 +++++++++++++++++++ .../ContextRequireExceptionKeyRuleTest.php | 14 ++++---- .../Rules/data/contextRequireExceptionKey.php | 12 +++++-- 6 files changed, 84 insertions(+), 17 deletions(-) diff --git a/example/phpstan.neon b/example/phpstan.neon index 734a8c7..eddc3fd 100644 --- a/example/phpstan.neon +++ b/example/phpstan.neon @@ -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 diff --git a/example/src/Example.php b/example/src/Example.php index 6e69e3a..bca4d2f 100644 --- a/example/src/Example.php +++ b/example/src/Example.php @@ -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'); } } diff --git a/rules.neon b/rules.neon index 01c6f7f..bc455dc 100644 --- a/rules.neon +++ b/rules.neon @@ -1,2 +1,16 @@ -rules: - - Sfp\PHPStan\Psr\Log\Rules\ContextRequireExceptionKeyRule \ No newline at end of file +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 \ No newline at end of file diff --git a/src/Rules/ContextRequireExceptionKeyRule.php b/src/Rules/ContextRequireExceptionKeyRule.php index f9aab96..1c92d03 100644 --- a/src/Rules/ContextRequireExceptionKeyRule.php +++ b/src/Rules/ContextRequireExceptionKeyRule.php @@ -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; @@ -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 []; @@ -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}")]; } @@ -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) { diff --git a/test/Rules/ContextRequireExceptionKeyRuleTest.php b/test/Rules/ContextRequireExceptionKeyRuleTest.php index 8147920..6629fd8 100644 --- a/test/Rules/ContextRequireExceptionKeyRuleTest.php +++ b/test/Rules/ContextRequireExceptionKeyRuleTest.php @@ -13,7 +13,7 @@ class ContextRequireExceptionKeyRuleTest extends RuleTestCase { protected function getRule(): Rule { - return new ContextRequireExceptionKeyRule(); + return new ContextRequireExceptionKeyRule('info'); } /** @test */ @@ -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, ], ]); } diff --git a/test/Rules/data/contextRequireExceptionKey.php b/test/Rules/data/contextRequireExceptionKey.php index 1af7d8a..b5734a1 100644 --- a/test/Rules/data/contextRequireExceptionKey.php +++ b/test/Rules/data/contextRequireExceptionKey.php @@ -7,15 +7,17 @@ // 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]); @@ -23,10 +25,14 @@ $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'); }