Print literals from their value, not their source spelling, when building expression keys

src/Node/Printer/Printer.php

@@ -5,6 +5,9 @@
 use Override;
 use PhpParser\Node;
 use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Scalar\Int_;
+use PhpParser\Node\Scalar\InterpolatedString;
 use PhpParser\Node\Scalar\String_;
 use PhpParser\PrettyPrinter\Standard;
 use PHPStan\DependencyInjection\AutowiredService;
@@ -31,9 +34,14 @@
 use PHPStan\Node\MethodCallableNode;
 use PHPStan\Node\StaticMethodCallableNode;
 use PHPStan\Type\VerbosityLevel;
+use function addcslashes;
+use function count;
+use function in_array;
 use function preg_match;
 use function sprintf;
 use function str_contains;
+use function strtolower;
+use const PHP_INT_MAX;
 
 /**
  * @api
@@ -103,6 +111,79 @@ protected function pObjectProperty(Node $node): string
 		return parent::pObjectProperty($node);
 	}
 
+	/**
+	 * Print a string literal from its value instead of its source spelling, so
+	 * that `'a'`, `"a"`, `"\x61"` and a heredoc or nowdoc holding `a` all end up
+	 * with the same expression key. The chosen form mirrors
+	 * ConstantStringType::export().
+	 */
+	#[Override]
+	protected function pScalar_String(String_ $node): string // phpcs:ignore
+	{
+		if (addcslashes($node->value, "\0..\37") !== $node->value) {
+			return '"' . $this->escapeString($node->value, '"') . '"';
+		}
+
+		return $this->pSingleQuotedString($node->value);
+	}
+
+	/**
+	 * Always print the double-quoted form so that a heredoc and the equivalent
+	 * `"..."` interpolation share one expression key. The parts themselves are
+	 * already spelling-independent: `"$b"`, `"{$b}"` and `"${b}"` all print as
+	 * `"{$b}"`.
+	 *
+	 * Normalizing goes no further than the spelling: `"$b.value"` deliberately
+	 * keeps a different key than `$b . '.value'`, even though the two compute
+	 * the same string. Printing an InterpolatedString as a Concat would need
+	 * the node to take part in precedence handling - unlike a Scalar, which is
+	 * atomic - and without that `-"$a$b"` prints as `-$a . $b` and collides
+	 * with `(-$a) . $b`, and `"$a"` prints as `$a` and collides with the
+	 * uncast variable. A false collision hands one expression a type
+	 * established for a different one, which is worse than the narrowing this
+	 * would recover; and the concat form would then also reach every error
+	 * message that quotes the expression back to the user.
+	 */
+	#[Override]
+	protected function pScalar_InterpolatedString(InterpolatedString $node): string // phpcs:ignore
+	{
+		return '"' . $this->pEncapsList($node->parts, '"') . '"';
+	}
+
+	/**
+	 * Always print the decimal form so that `1`, `0x1`, `01` and `0b1` share one
+	 * expression key.
+	 */
+	#[Override]
+	protected function pScalar_Int(Int_ $node): string // phpcs:ignore
+	{
+		if ($node->value === -PHP_INT_MAX - 1) {
+			// PHP_INT_MIN cannot be represented as a literal, because the sign is
+			// not part of the literal.
+			return '(-' . PHP_INT_MAX . '-1)';
+		}
+
+		return (string) $node->value;
+	}
+
+	/**
+	 * Lowercase the `true`, `false` and `null` keywords, the only case-insensitive
+	 * spellings the analyser does not already report through a `*.nameCase` rule.
+	 */
+	#[Override]
+	protected function pExpr_ConstFetch(ConstFetch $node): string // phpcs:ignore
+	{
+		$name = $node->name;
+		if (count($name->getParts()) === 1 && !$name->isRelative()) {
+			$lowercasedName = strtolower($name->getFirst());
+			if (in_array($lowercasedName, ['true', 'false', 'null'], true)) {
+				return $lowercasedName;
+			}
+		}
+
+		return parent::pExpr_ConstFetch($node);
+	}
+
 	protected function pPHPStan_Node_TypeExpr(TypeExpr $expr): string // phpcs:ignore
 	{
 		return sprintf('__phpstanType(%s)', $expr->getExprType()->describe(VerbosityLevel::precise()));

tests/PHPStan/Analyser/nsrt/bug-15060.php

@@ -0,0 +1,82 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15060;
+
+use function PHPStan\Testing\assertType;
+
+function search($searchParams): void
+{
+	if (isset($searchParams['test'])) {
+		assertType('mixed~null', $searchParams['test']);
+		if ($searchParams["test"]) {
+			assertType("mixed~(0|0.0|''|'0'|array{}|false|null)", $searchParams['test']);
+			assertType("mixed~(0|0.0|''|'0'|array{}|false|null)", $searchParams["test"]);
+		}
+
+		if (is_array($searchParams["test"])) {
+			assertType('array<mixed, mixed>', $searchParams['test']);
+			assertType('array<mixed, mixed>', $searchParams["test"]);
+		}
+
+		if (is_array($searchParams["test"]) && $searchParams["test"]) {
+			assertType('non-empty-array<mixed, mixed>', $searchParams['test']);
+			assertType('non-empty-array<mixed, mixed>', $searchParams["test"]);
+		}
+	}
+}
+
+function otherStringSpellings($m): void
+{
+	if (is_array($m['test']) && $m['test']) {
+		assertType('non-empty-array<mixed, mixed>', $m['test']);
+		assertType('non-empty-array<mixed, mixed>', $m["test"]);
+		assertType('non-empty-array<mixed, mixed>', $m["\x74est"]);
+		assertType('non-empty-array<mixed, mixed>', $m[<<<'NOWDOC'
+			test
+			NOWDOC]);
+		assertType('non-empty-array<mixed, mixed>', $m[<<<HEREDOC
+			test
+			HEREDOC]);
+	}
+}
+
+function intSpellings($m): void
+{
+	if (is_array($m[1]) && $m[1]) {
+		assertType('non-empty-array<mixed, mixed>', $m[1]);
+		assertType('non-empty-array<mixed, mixed>', $m[0x1]);
+		assertType('non-empty-array<mixed, mixed>', $m[01]);
+		assertType('non-empty-array<mixed, mixed>', $m[0b1]);
+		assertType('mixed', $m[10]);
+	}
+}
+
+function interpolatedSpellings($m, string $k): void
+{
+	if (is_array($m["x$k"]) && $m["x$k"]) {
+		assertType('non-empty-array<mixed, mixed>', $m["x$k"]);
+		assertType('non-empty-array<mixed, mixed>', $m["x{$k}"]);
+		assertType('non-empty-array<mixed, mixed>', $m[<<<HEREDOC
+			x$k
+			HEREDOC]);
+	}
+}
+
+function constFetchSpellings($m): void
+{
+	if (is_array($m[true]) && $m[true]) {
+		assertType('non-empty-array<mixed, mixed>', $m[true]);
+		assertType('non-empty-array<mixed, mixed>', $m[TRUE]);
+		assertType('non-empty-array<mixed, mixed>', $m[True]);
+	}
+
+	if (is_array($m[null]) && $m[null]) {
+		assertType('non-empty-array<mixed, mixed>', $m[null]);
+		assertType('non-empty-array<mixed, mixed>', $m[NULL]);
+	}
+
+	if (is_array($m[false]) && $m[false]) {
+		assertType('non-empty-array<mixed, mixed>', $m[false]);
+		assertType('non-empty-array<mixed, mixed>', $m[FALSE]);
+	}
+}

tests/PHPStan/Node/Printer/ExprPrinterTest.php

@@ -0,0 +1,154 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node\Printer;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Stmt;
+use PHPStan\Parser\Parser;
+use PHPStan\ShouldNotHappenException;
+use PHPStan\Testing\PHPStanTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use function count;
+use function get_class;
+use function sprintf;
+
+class ExprPrinterTest extends PHPStanTestCase
+{
+
+	public static function dataEquivalentSpellings(): array
+	{
+		return [
+			'string quoting' => [
+				['$a[\'test\']', '$a["test"]', '$a["\x74est"]'],
+			],
+			'string heredoc' => [
+				['$a[\'test\']', "\$a[<<<EOT\ntest\nEOT]", "\$a[<<<'EOT'\ntest\nEOT]"],
+			],
+			'string escaping' => [
+				['$a["a\nb"]', "\$a[<<<EOT\na\nb\nEOT]"],
+			],
+			'int base' => [
+				['$a[1]', '$a[0x1]', '$a[01]', '$a[0b1]'],
+			],
+			'int separator' => [
+				['$a[10]', '$a[1_0]'],
+			],
+			'float' => [
+				['$a[1.5]', '$a[1.50]', '$a[15e-1]'],
+			],
+			'interpolated string' => [
+				['$a["x$b"]', '$a["x{$b}"]', "\$a[<<<EOT\nx\$b\nEOT]"],
+			],
+			// The three interpolation syntaxes a formatter rewrites into each
+			// other (php-cs-fixer's explicit_string_variable, Rector's
+			// SimpleToComplexStringVariableRector).
+			'interpolated variable syntax' => [
+				['$a["$b"]', '$a["{$b}"]', '$a["${b}"]'],
+			],
+			'interpolated property syntax' => [
+				['$a["$o->p"]', '$a["{$o->p}"]'],
+			],
+			'interpolated offset syntax' => [
+				['$a["$b[k]"]', '$a["{$b[\'k\']}"]'],
+			],
+			'variable variable' => [
+				['$$v', '${$v}'],
+			],
+			'true' => [
+				['$a[true]', '$a[TRUE]', '$a[True]', '$a[\true]'],
+			],
+			'false' => [
+				['$a[false]', '$a[FALSE]', '$a[\FALSE]'],
+			],
+			'null' => [
+				['$a[null]', '$a[NULL]', '$a[\null]'],
+			],
+			'object property' => [
+				['$a->b', '$a->{\'b\'}', '$a->{"b"}'],
+			],
+			'method name' => [
+				['$a->b()', '$a->{\'b\'}()', '$a->{"b"}()'],
+			],
+		];
+	}
+
+	/**
+	 * @param non-empty-list<string> $codes
+	 */
+	#[DataProvider('dataEquivalentSpellings')]
+	public function testEquivalentSpellingsPrintTheSame(array $codes): void
+	{
+		$exprPrinter = self::getContainer()->getByType(ExprPrinter::class);
+
+		$expected = null;
+		foreach ($codes as $code) {
+			$printed = $exprPrinter->printExpr($this->parseExpr($code));
+			if ($expected === null) {
+				$expected = $printed;
+				continue;
+			}
+
+			$this->assertSame($expected, $printed, sprintf('%s should print the same as %s', $code, $codes[0]));
+		}
+	}
+
+	public static function dataDifferentSpellings(): array
+	{
+		return [
+			'numeric string vs int' => [
+				'$a[1]',
+				'$a[\'1\']',
+			],
+			'single quoted backslash is literal' => [
+				'$a[\'a\nb\']',
+				'$a["a\nb"]',
+			],
+			'different int' => [
+				'$a[1]',
+				'$a[10]',
+			],
+			'other constant case is significant' => [
+				'$a[FOO]',
+				'$a[foo]',
+			],
+		];
+	}
+
+	#[DataProvider('dataDifferentSpellings')]
+	public function testDifferentSpellingsPrintDifferently(string $code, string $otherCode): void
+	{
+		$exprPrinter = self::getContainer()->getByType(ExprPrinter::class);
+
+		$this->assertNotSame(
+			$exprPrinter->printExpr($this->parseExpr($code)),
+			$exprPrinter->printExpr($this->parseExpr($otherCode)),
+		);
+	}
+
+	public function testPrintedFormNeverContainsNewline(): void
+	{
+		$exprPrinter = self::getContainer()->getByType(ExprPrinter::class);
+
+		foreach (["\$a[<<<EOT\na\nb\nEOT]", "\$a[<<<'EOT'\na\nb\nEOT]", "\$a[<<<EOT\nx\$b\ny\nEOT]"] as $code) {
+			$this->assertStringNotContainsString("\n", $exprPrinter->printExpr($this->parseExpr($code)), $code);
+		}
+	}
+
+	private function parseExpr(string $code): Expr
+	{
+		/** @var Parser $parser */
+		$parser = self::getContainer()->getService('currentPhpVersionRichParser');
+
+		/** @var Stmt[] $stmts */
+		$stmts = $parser->parseString(sprintf('<?php %s;', $code));
+		if (count($stmts) !== 1) {
+			throw new ShouldNotHappenException('Expecting code which evaluates to a single statement, got: ' . count($stmts));
+		}
+		if (!$stmts[0] instanceof Stmt\Expression) {
+			throw new ShouldNotHappenException('Expecting code contains a single statement expression, got: ' . get_class($stmts[0]));
+		}
+
+		return $stmts[0]->expr;
+	}
+
+}

tests/PHPStan/Rules/Arrays/DuplicateKeysInLiteralArraysRuleTest.php

@@ -26,7 +26,7 @@ public function testDuplicateKeys(): void
 		define('PHPSTAN_DUPLICATE_KEY', 0);
 		$this->analyse([__DIR__ . '/data/duplicate-keys.php'], [
 			[
-				'Array has 2 duplicate keys with value \'\' (null, NULL).',
+				'Array has 2 duplicate keys with value \'\' (null, null).',
 				15,
 			],
 			[

tests/PHPStan/Rules/Keywords/DeclareStrictTypesRuleTest.php

@@ -98,7 +98,7 @@ public function testNonsenseBool(): void
 	{
 		$this->analyse([__DIR__ . '/data/declare-strict-nonsense-bool.php'], [
 			[
-				'Declare strict_types must have 0 or 1 as its value, \true given.',
+				'Declare strict_types must have 0 or 1 as its value, true given.',
 				1,
 			],
 		]);