Merge pull request #55 from sasezaki/message

Add `MessageStaticStringRule`
struggle-for-php · Oct 15, 2023 · 7f4aa2d · 7f4aa2d
2 parents 5c1160f + c24f70f
commit 7f4aa2d
Show file tree

Hide file tree

Showing 9 changed files with 261 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -166,6 +166,31 @@ Then, `debug`| `info` | `notice` LogLevel  is ignored for report.
 }
 ```
 
+### MessageStaticStringRule
+
+> [!IMPORTANT]
+> This Rule is currently experimental.
+
+| :pushpin: _error identifier_ |
+| --- |
+| sfp-psr-log.messageNotStaticString |
+
+* reports when $message is not static string value.
+
+```php
+// bad
+$logger->info(sprintf('Message contains %s variable', $var));
+```
+
+#### Configuration
+
+* You should add `Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule` into your `phpstan.neon` to enable.
+
+```neon
+rules:
+    - Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule
+```
+
 ## Installation
 
 To use this extension, require it in [Composer](https://getcomposer.org/):

diff --git a/example/phpstan.neon b/example/phpstan.neon
@@ -16,6 +16,9 @@ parameters:
     stubFiles:
         - ../vendor/struggle-for-php/sfp-stubs-psr-log/stubs-for-throwable/LoggerInterface.phpstub
 
+rules:
+    - Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule
+
 includes:
 # relative path can not work
 #	- %currentWorkingDirectory%/extension.neon

diff --git a/example/src/Example.php b/example/src/Example.php
@@ -7,6 +7,8 @@
 use Psr\Log\LoggerInterface;
 use Throwable;
 
+use function sprintf;
+
 class Example
 {
     /** @var LoggerInterface */
@@ -66,4 +68,14 @@ public function placeholderCorrespondToKeys(): void
         $this->logger->info('message has {notMatched1} , {matched1} , {notMatched2} .', ['matched1' => 'bar']);
         $this->logger->log('info', 'message has {notMatched} .', ['foo' => 'bar']);
     }
+
+    public function messageNotStaticString(string $var): void
+    {
+        $this->logger->info("Message contains {$var} variable");
+        $this->logger->info("Message contains $var variable");
+        $this->logger->info("Message contains " . $var . " variable");
+        $this->logger->info('Message contains ' . $var . ' variable');
+        $this->logger->info(sprintf('Message contains %s variable', $var));
+        $this->logger->log('info', sprintf('Message contains %s variable', $var));
+    }
 }
diff --git a/rules.neon b/rules.neon
@@ -10,6 +10,7 @@ parametersSchema:
 	])
 
 rules:
+#    - Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule
     - Sfp\PHPStan\Psr\Log\Rules\PlaceholderCorrespondToKeysRule
     - Sfp\PHPStan\Psr\Log\Rules\PlaceholderCharactersRule
 
@@ -25,4 +26,4 @@ services:
 		arguments:
 			contextKeyOriginalPattern: %sfpPsrLog.contextKeyOriginalPattern%
 		tags:
-			- phpstan.rules.rule
+			- phpstan.rules.rule
diff --git a/src/Rules/MessageStaticStringRule.php b/src/Rules/MessageStaticStringRule.php
@@ -0,0 +1,90 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sfp\PHPStan\Psr\Log\Rules;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\ShouldNotHappenException;
+use PHPStan\Type\ObjectType;
+
+use function assert;
+use function count;
+use function in_array;
+use function sprintf;
+
+/**
+ * @implements Rule<Node\Expr\MethodCall>
+ */
+final class MessageStaticStringRule implements Rule
+{
+    private const ERROR_MESSAGE_NOT_STATIC = 'Parameter $message of logger method Psr\Log\LoggerInterface::%s() is not a static string';
+
+    public function getNodeType(): string
+    {
+        return Node\Expr\MethodCall::class;
+    }
+
+    /**
+     * @throws ShouldNotHappenException
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! $node->name instanceof Node\Identifier) {
+            // @codeCoverageIgnoreStart
+            return []; // @codeCoverageIgnoreEnd
+        }
+
+        $calledOnType = $scope->getType($node->var);
+        if (! (new ObjectType('Psr\Log\LoggerInterface'))->isSuperTypeOf($calledOnType)->yes()) {
+            // @codeCoverageIgnoreStart
+            return []; // @codeCoverageIgnoreEnd
+        }
+
+        $args = $node->getArgs();
+        if (count($args) === 0) {
+            // @codeCoverageIgnoreStart
+            return []; // @codeCoverageIgnoreEnd
+        }
+
+        $methodName = $node->name->toLowerString();
+
+        $messageArgumentNo = 0;
+        if ($methodName === 'log') {
+            if (
+                count($args) < 2
+                || ! $args[0]->value instanceof Node\Scalar\String_
+            ) {
+                // @codeCoverageIgnoreStart
+                return []; // @codeCoverageIgnoreEnd
+            }
+
+            $messageArgumentNo = 1;
+        } elseif (! in_array($methodName, LogLevelListInterface::LOGGER_LEVEL_METHODS)) {
+            // @codeCoverageIgnoreStart
+            return []; // @codeCoverageIgnoreEnd
+        }
+
+        $message = $args[$messageArgumentNo];
+        /** @psalm-suppress RedundantConditionGivenDocblockType */
+        assert($message instanceof Node\Arg);
+        $value   = $scope->getType($message->value);
+        $strings = $value->getConstantStrings();
+
+        if (count($strings) === 0) {
+            return [
+                RuleErrorBuilder::message(
+                    sprintf(self::ERROR_MESSAGE_NOT_STATIC, $methodName)
+                )
+                    ->identifier('sfp-psr-log.messageNotStaticString')
+                    ->tip('See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages')
+                    ->build(),
+            ];
+        }
+
+        return [];
+    }
+}
diff --git a/src/Rules/PlaceholderCharactersRule.php b/src/Rules/PlaceholderCharactersRule.php
@@ -12,6 +12,7 @@
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\ObjectType;
 
+use function assert;
 use function count;
 use function implode;
 use function in_array;
@@ -73,6 +74,8 @@ public function processNode(Node $node, Scope $scope): array
         }
 
         $message = $args[$messageArgumentNo];
+        /** @psalm-suppress RedundantConditionGivenDocblockType */
+        assert($message instanceof Node\Arg);
 
         $strings = $scope->getType($message->value)->getConstantStrings();
 

diff --git a/test/Rules/MessageStaticStringRuleTest.php b/test/Rules/MessageStaticStringRuleTest.php
@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SfpTest\PHPStan\Psr\Log\Rules;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule;
+
+/**
+ * @extends RuleTestCase<MessageStaticStringRule>
+ * @covers \Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule
+ */
+final class MessageStaticStringRuleTest extends RuleTestCase
+{
+    protected function getRule(): Rule
+    {
+        return new MessageStaticStringRule();
+    }
+
+    /**
+     * @test
+     */
+    public function testProcessNode(): void
+    {
+        $this->analyse([__DIR__ . '/data/messageMustBeStatic.php'], [
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string',
+                13, // variable
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string',
+                14, // double quote escaped with braces
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string',
+                15, // double quote escaped without braces
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string',
+                16, // concat double quote
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string',
+                17, // concat single quote
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string',
+                18, // function call
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+            [
+                'Parameter $message of logger method Psr\Log\LoggerInterface::log() is not a static string',
+                20, // call log() method
+                'See https://www.php-fig.org/psr/psr-3/meta/#static-log-messages',
+            ],
+        ]);
+    }
+}
diff --git a/test/Rules/data/messageMustBeStatic.php b/test/Rules/data/messageMustBeStatic.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @phpstan-param 'literal-a'|'literal-b' $literals
+ */
+function main(Psr\Log\LoggerInterface $logger, string $m, string $literals): void
+{
+    // valid
+    $logger->info('message is valid');
+
+    $logger->info($m);
+    $logger->info("Message contains {$m} variable");
+    $logger->info("Message contains $m variable");
+    $logger->info("Message contains " . $m . " variable");
+    $logger->info('Message contains ' . $m . ' variable');
+    $logger->info(sprintf('Message contains %s variable', $m));
+
+    $logger->log('info', $m);
+
+    // Allow assign
+    $logger->info($ret = 'Invalid Request happened!');
+    echo $ret;
+
+    // Allow literal-string intersection
+    $logger->info($literals);
+}
diff --git a/test/example.output b/test/example.output
@@ -1,45 +1,63 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" failures="14" name="phpstan" tests="14" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/junit-team/junit5/r5.5.1/platform-tests/src/test/resources/jenkins-junit.xsd">
-  <testcase name="example/src/Example.php:23">
+<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" failures="20" name="phpstan" tests="20" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/junit-team/junit5/r5.5.1/platform-tests/src/test/resources/jenkins-junit.xsd">
+  <testcase name="example/src/Example.php:25">
     <failure type="ERROR" message="Parameter #2 $context of method Psr\Log\LoggerInterface::notice() expects array{exception?: Throwable}, array{exception: string} given."/>
   </testcase>
-  <testcase name="example/src/Example.php:24">
+  <testcase name="example/src/Example.php:26">
     <failure type="ERROR" message="Parameter #1 $level of method Psr\Log\LoggerInterface::log() expects 'alert'|'critical'|'debug'|'emergency'|'error'|'info'|'notice'|'warning', 'panic' given."/>
   </testcase>
-  <testcase name="example/src/Example.php:33">
+  <testcase name="example/src/Example.php:35">
     <failure type="ERROR" message="Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires 'exception' key. Current scope has Throwable variable - $throwable"/>
   </testcase>
-  <testcase name="example/src/Example.php:35">
+  <testcase name="example/src/Example.php:37">
     <failure type="ERROR" message="Parameter $context of logger method Psr\Log\LoggerInterface::notice() requires 'exception' key. Current scope has Throwable variable - $throwable"/>
   </testcase>
-  <testcase name="example/src/Example.php:47">
+  <testcase name="example/src/Example.php:49">
     <failure type="ERROR" message="Parameter $context of logger method Psr\Log\LoggerInterface::debug(), key should be non empty string."/>
   </testcase>
-  <testcase name="example/src/Example.php:52">
+  <testcase name="example/src/Example.php:54">
     <failure type="ERROR" message="Parameter $context of logger method Psr\Log\LoggerInterface::debug(), key should be match #\A[A-Za-z0-9-]+\z#."/>
   </testcase>
-  <testcase name="example/src/Example.php:57">
+  <testcase name="example/src/Example.php:59">
     <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() should not includes double braces. - {{doubleBrace}}"/>
   </testcase>
-  <testcase name="example/src/Example.php:58">
+  <testcase name="example/src/Example.php:60">
     <failure type="ERROR" message="Parameter $context of logger method Psr\Log\LoggerInterface::info(), key should be match #\A[A-Za-z0-9-]+\z#."/>
   </testcase>
-  <testcase name="example/src/Example.php:58">
+  <testcase name="example/src/Example.php:60">
     <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() has braces. But it includes invalid characters for placeholder. - { space }"/>
   </testcase>
-  <testcase name="example/src/Example.php:63">
+  <testcase name="example/src/Example.php:65">
     <failure type="ERROR" message="Parameter $context of logger method Psr\Log\LoggerInterface::info() is required, when placeholder braces exists - {nonContext}"/>
   </testcase>
-  <testcase name="example/src/Example.php:64">
+  <testcase name="example/src/Example.php:66">
     <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() has placeholder braces, but context key is not found against them. - {empty}"/>
   </testcase>
-  <testcase name="example/src/Example.php:65">
+  <testcase name="example/src/Example.php:67">
     <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() has placeholder braces, but context key is not found against them. - {notMatched}"/>
   </testcase>
-  <testcase name="example/src/Example.php:66">
+  <testcase name="example/src/Example.php:68">
     <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() has placeholder braces, but context key is not found against them. - {notMatched1},{notMatched2}"/>
   </testcase>
-  <testcase name="example/src/Example.php:67">
+  <testcase name="example/src/Example.php:69">
     <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::log() has placeholder braces, but context key is not found against them. - {notMatched}"/>
   </testcase>
+  <testcase name="example/src/Example.php:74">
+    <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string"/>
+  </testcase>
+  <testcase name="example/src/Example.php:75">
+    <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string"/>
+  </testcase>
+  <testcase name="example/src/Example.php:76">
+    <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string"/>
+  </testcase>
+  <testcase name="example/src/Example.php:77">
+    <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string"/>
+  </testcase>
+  <testcase name="example/src/Example.php:78">
+    <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::info() is not a static string"/>
+  </testcase>
+  <testcase name="example/src/Example.php:79">
+    <failure type="ERROR" message="Parameter $message of logger method Psr\Log\LoggerInterface::log() is not a static string"/>
+  </testcase>
 </testsuite>