literal-string becomes non-empty-string after 1000 chars

[tdtm]

tl;dr: looks like after 1000 chars and over, a literal-stringsuddenly starts being considered a non-empty-string.

Fails with a 1000-char literal
https://psalm.dev/r/05f9492c43

Same exact thing, but with 1 fewer char passes
https://psalm.dev/r/1059fb0c2f

---

Found this adding literal-string to methods taking SQL strings. Some long/complex queries were triggering warnings of ArgumentTypeCoercion despite being literals. The only apparent trigger was when the query's length.

---

Psalm 4.23.0@f1fe6ff483bf325c803df9f510d09a03fd796f88

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/05f9492c43

<?php

/**
 * @param literal-string $i
 */
function takesLiteralString($i) : string {
    return $i;
}

takesLiteralString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");

Psalm output (using commit 006f441):

ERROR: ArgumentTypeCoercion - 10:20 - Argument 1 of takesLiteralString expects literal-string, parent type non-empty-string provided

https://psalm.dev/r/1059fb0c2f

<?php

/**
 * @param literal-string $i
 */
function takesLiteralString($i) : string {
    return $i;
}

takesLiteralString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");

Psalm output (using commit 006f441):

No issues!

[weirdan]

There is an undocumented config attribute called maxStringLength:

psalm/src/Psalm/Config.php

Lines 1085 to 1088 in 4e59398

	if (isset($config_xml['maxStringLength'])) {
	$attribute_text = (int)$config_xml['maxStringLength'];
	$config->max_string_length = $attribute_text;
	}

psalm/src/Psalm/Type.php

Lines 247 to 252 in 4e59398

	if (!$type) {
	if (strlen($value) < $config->max_string_length) {
	$type = new TLiteralString($value);
	} else {
	$type = new TNonEmptyString();
	}

You may want to tweak it.

[tdtm]

That does it, wonderful. Thank you!

(ps: thanks for pointing out where that is in the codebase, that helps familiarize to it so that I can eventually contribute back!)

[AndrolGenhald]

psalm/src/Psalm/Type.php

Lines 247 to 252 in 4e59398

	if (!$type) {
	if (strlen($value) < $config->max_string_length) {
	$type = new TLiteralString($value);
	} else {
	$type = new TNonEmptyString();
	}

I assume that's from before TNonspecificLiteralString was added, shouldn't it be changed to that instead of TNonEmptyString? That should fix the issue was well without having to change the config. Or maybe people are passing long literals as non-empty-string as well and we should wait until we can do literal-string&non-empty-string?

[orklah]

shouldn't it be changed to that instead of TNonEmptyString? That should fix the issue was well without having to change the config. Or maybe people are passing long literals as non-empty-string as well and we should wait until we can do literal-string&non-empty-string?

As usual when we have to juggle between types, we're confronted to this choice. You can design a psalm.dev example that will fail for any choice you'll take.
literal-string&non-empty-string would solve that, but Psalm's type system is not designed for that.

Maybe a refactoring of TString could allow this and keep flags for literalness, non-emptyness, lowercaseness, non-falsyness etc... but it's a monstrous task

[AndrolGenhald]

Maybe a refactoring of TString could allow this and keep flags for literalness, non-emptyness, lowercaseness, non-falsyness etc... but it's a monstrous task

I'd much rather just get around to finishing my type system rewrite so that we can do literal-string&non-empty-string.

[boesing]

I came up with this as well (@orklah this is why I've reached out to you on slack recently).
I was about to create a discussion here to see how we can still pass the original string value to e.g. plugins so the parsed value is not totally lost.

I can think that the doctrine plugin for example could take advantage of it (if its plans are to support DQL analysis), etc.

But also my plugin (boesing/psalm-plugin-stringf#102) is kinda affected by the maxStringLength setting as its main purpose is to parse the $format value of e.g. sprintf, printf, fscanf and sscanf.

By raising the maxStringLength config attribute, error messages currently containing non-empty-string will be also affected. They will start displaying the whole string on CLI which might be super messy.

So yes, that might be a solution but I definitely would love to see a way how plugins can still interact with the non-shortened version of the string in some way.