Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MessageStaticStringRule #55

Merged
merged 13 commits into from
Oct 15, 2023
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/):
Expand Down
3 changes: 3 additions & 0 deletions example/phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions example/src/Example.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use Psr\Log\LoggerInterface;
use Throwable;

use function sprintf;

class Example
{
/** @var LoggerInterface */
Expand Down Expand Up @@ -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));
}
}
3 changes: 2 additions & 1 deletion rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ parametersSchema:
])

rules:
# - Sfp\PHPStan\Psr\Log\Rules\MessageStaticStringRule
- Sfp\PHPStan\Psr\Log\Rules\PlaceholderCorrespondToKeysRule
- Sfp\PHPStan\Psr\Log\Rules\PlaceholderCharactersRule

Expand All @@ -25,4 +26,4 @@ services:
arguments:
contextKeyOriginalPattern: %sfpPsrLog.contextKeyOriginalPattern%
tags:
- phpstan.rules.rule
- phpstan.rules.rule
90 changes: 90 additions & 0 deletions src/Rules/MessageStaticStringRule.php
Original file line number Diff line number Diff line change
@@ -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 [];
}
}
3 changes: 3 additions & 0 deletions src/Rules/PlaceholderCharactersRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\ObjectType;

use function assert;
use function count;
use function implode;
use function in_array;
Expand Down Expand Up @@ -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();

Expand Down
65 changes: 65 additions & 0 deletions test/Rules/MessageStaticStringRuleTest.php
Original file line number Diff line number Diff line change
@@ -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',
],
]);
}
}
28 changes: 28 additions & 0 deletions test/Rules/data/messageMustBeStatic.php
Original file line number Diff line number Diff line change
@@ -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);
}
48 changes: 33 additions & 15 deletions test/example.output
Original file line number Diff line number Diff line change
@@ -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>