Introduce uppercase-string by pmjones · Pull Request #3613 · phpstan/phpstan-src

pmjones · 2024-11-08T19:15:14Z

This PR introduces uppercase-string in all places where lowercase-string is recognized, except for SprintfFunctionDynamicReturnTypeExtension.

The omission is because sprintf() format strings cannot be typed reliably as uppercase-string because most of the formatting specifiers are lowercase. For example, the format string 'FOO %s BAR' is treated as a plain string rather than uppercase-string, because %s is lowercase (even though it might represent an uppercase string).

It appears that lowercase-string exhibits similar behavior. For example, consider the format string foo %F. It formats as the lowercase string 'foo' and a non-locale-aware float; the returned string will be the same after strtolower(), so it should be valid as a lowercase-string.

However, in nsrt/lowercase-string-sprintf.php, if you add assertType('lowercase-string', sprintf('foo %F', $lowercase));, it will fail with:

Expected: lowercase-string
Actual:   non-falsy-string

That's because %F is uppercase, even though it represents a float.

If you consider these to be problems, they might be resolved by adding a new type, printf-string, that can ignore the formatting specifiers when determining isLowercase() and isUppercase(), but I imagine that would have rather broad implcations.

Let me know how you'd like to proceed.

src/Type/Php/ParseUrlFunctionDynamicReturnTypeExtension.php

ondrejmirtes

This is very complete and very nice, thank you! Just one thing I noticed.

src/Type/Accessory/AccessoryNumericStringType.php

ondrejmirtes · 2024-11-09T12:28:02Z

One more thing - Windows tests are failing https://github.com/phpstan/phpstan-src/actions/runs/11749776276/job/32736693637?pr=3613

pmjones · 2024-11-09T15:21:41Z

One more thing - Windows tests are failing https://github.com/phpstan/phpstan-src/actions/runs/11749776276/job/32736693637?pr=3613

Yes; it's related to this change where two reasons are given instead of one, and the PHP_EOL between them causes the Windows failures.

After thinking about it some more, I believe it should remain as one reason (not two) and am trying to track down where the second reason is coming from. Any help there would be appreciated!

ondrejmirtes

I found a few more things.

tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php

tests/PHPStan/Analyser/nsrt/isset-coalesce-empty-type.php

tests/PHPStan/Analyser/nsrt/literal-string.php

ondrejmirtes · 2024-11-09T17:53:44Z

After you rebase on top of 1.12.x with this commit ccfb4ab, the rule error tip should appear only once.

src/Reflection/InitializerExprTypeResolver.php

pmjones · 2024-11-10T17:19:30Z

After you rebase on top of 1.12.x with this commit ccfb4ab, the rule error tip should appear only once.

That did the trick; thanks!

VincentLanglet

This PR should be copied too. #3510

IntegerRangeType::toString and IntegerType::toString are both lowercase-string and uppercase-string.

And a whole reflection on lowercase-string&uppercase-string seems needed.

Cause if this intersection is lost the current code

/** @param lowercase-string $string */
public function acceptsOnlyLowercase($string) {}

/** @var int $int */
$int = ...;

acceptsOnlyLowercase((string) $int); // Will report an error

and doesn't actually cf
https://phpstan.org/r/e722e920-af05-4c7b-8290-1205df68c4f8

src/Reflection/InitializerExprTypeResolver.php

src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php

src/Type/Accessory/AccessoryLowercaseStringType.php

src/Type/Accessory/AccessoryUppercaseStringType.php

src/Type/IntersectionType.php

stubs/core.stub

tests/PHPStan/Analyser/nsrt/isset-coalesce-empty-type.php

ondrejmirtes · 2024-11-12T09:10:20Z

In this case we need to verify that TypeCombinator::intersect() of both lowercase-string and uppercase-string leads to expected result. And both types need to tell that it's possible it can be the other too.

Should be done in TypeCombinatorTest.

Addresses phpstan#3613 (comment) and phpstan#3613 (comment) The relevate lowercase-string tests do not seem to be affected ... ?

Addresses phpstan#3613 (comment)

tests/PHPStan/Analyser/nsrt/uppercase-string-parse.php

Addresses phpstan#3613 (comment)

Addresses phpstan#3613 (comment), phpstan#3613 (comment), phpstan#3613 (comment)

Addresses phpstan#3613 (comment)

VincentLanglet

The current ToDo-list seems to be:

This comment need to be addressed
https://github.com/phpstan/phpstan-src/pull/3613/files#r1835752042
Same for https://github.com/phpstan/phpstan-src/pull/3613/files#r1835752521
appendAccessoryCasedStringTypes need to be changed.
lowercase-string|uppercase-string should be simplified as string
We need to understand why parse_url/trim doesn't produce the expected behavior.

src/Reflection/InitializerExprTypeResolver.php

tests/PHPStan/Analyser/nsrt/uppercase-string-parse.php

tests/PHPStan/Analyser/nsrt/uppercase-string-trim.php

tests/PHPStan/Type/TypeCombinatorTest.php

Addresses phpstan#3613 (comment)

@VincentLanglet

Addresses phpstan#3613 (comment), phpstan#3613 (comment), phpstan#3613 (comment) This intentionally causes tests to fail so @VincentLanglet et al. can debug.

Addresses phpstan#3613 (comment)

Addresses phpstan#3613 (comment) Instead of a 4-part if/elseif/else, this builds both lowercase and/or uppercase; then if neither was true, builds what would have been the final else case.

Co-authored-by: Markus Staab <maggus.staab@googlemail.com>

Addresses phpstan#3613 (comment)

@VincentLanglet

Addresses phpstan#3613 (comment), phpstan#3613 (comment), phpstan#3613 (comment) This intentionally causes tests to fail so @VincentLanglet et al. can debug.

Addresses phpstan#3613 (comment)

Addresses phpstan#3613 (comment) Instead of a 4-part if/elseif/else, this builds both lowercase and/or uppercase; then if neither was true, builds what would have been the final else case.

Copied from phpstan@22f0cab and phpstan@38151b7

VincentLanglet

We also need lowercase-string|uppercase-string to be simplified to string

src/Type/Php/ExplodeFunctionDynamicReturnTypeExtension.php

src/Type/Php/ParseStrParameterOutTypeExtension.php

src/Type/Php/ParseUrlFunctionDynamicReturnTypeExtension.php

@VincentLanglet

per convo w/ @VincentLanglet

Incorporates phpstan#3613 (comment), phpstan#3613 (comment)

pmjones · 2024-11-19T22:33:00Z

@VincentLanglet "This PR should be copied too. https://github.com/phpstan/phpstan-src/pull/3510"

That one is merged into 1.12.x, and I have rebased since that PR, so I think it has been included now. Let me know if I am mistaken.

Addresses phpstan#3613 (comment), phpstan#3613 (comment)

pmjones · 2024-11-19T22:46:14Z

@VincentLanglet "We also need lowercase-string|uppercase-string to be simplified to string" -- I admit I do not know where to accomplish this. Hints/tips?

VincentLanglet · 2024-11-19T23:10:06Z

@VincentLanglet "This PR should be copied too. https://github.com/phpstan/phpstan-src/pull/3510"

That one is merged into 1.12.x, and I have rebased since that PR, so I think it has been included now. Let me know if I am mistaken.

You need to update

IntegerType::toString, it returns

return new IntersectionType([
			new StringType(),
			new AccessoryLowercaseStringType(),
			new AccessoryUppercaseStringType(),
			new AccessoryNumericStringType(),
		]);

IntegerRangeType::toString, it returns

$finiteTypes = $this->getFiniteTypes();
		if ($finiteTypes !== []) {
			return TypeCombinator::union(...$finiteTypes)->toString();
		}
		$isZero = (new ConstantIntegerType(0))->isSuperTypeOf($this);
		if ($isZero->no()) {
			return new IntersectionType([
				new StringType(),
				new AccessoryLowercaseStringType(),
			    new AccessoryUppercaseStringType(),
				new AccessoryNumericStringType(),
				new AccessoryNonFalsyStringType(),
			]);
		}

		return new IntersectionType([
			new StringType(),
			new AccessoryLowercaseStringType(),
			new AccessoryUppercaseStringType(),
			new AccessoryNumericStringType(),
		]);

VincentLanglet · 2024-11-19T23:20:19Z

@VincentLanglet "We also need lowercase-string|uppercase-string to be simplified to string" -- I admit I do not know where to accomplish this. Hints/tips?

Might need to be validated with @ondrejmirtes, but I think it's in TypeCombinator::union or TypeCombinator::compareTypesInUnion

But it seems not easy to do cause
lowercase-string|uppercase-string need to be simplified to string
literal-string&lowercase-string|literal-string&uppercase-string need to be literal-string (same for other accessory)
literal-string&lowercase-string|uppercase-string should stay untouched (i think).

Depends on how hard it is to handle, and how important it is to solve ; it might be postponed in another PR.

Except my previous comment, I don't think I see anything to add on this PR ; good job

ondrejmirtes · 2024-11-20T07:23:31Z

I reviewed the whole PR again and I like it a lot! Thank you very much.

If you wanted more challenges in the type system now that you proficient in it, there's no shortage of them I can throw at you :)

staabm · 2024-11-20T07:35:50Z

great job!

VincentLanglet · 2024-11-20T08:46:38Z

I reviewed the whole PR again and I like it a lot! Thank you very much.

If you wanted more challenges in the type system now that you proficient in it, there's no shortage of them I can throw at you :)

I had a comment left about IntegerType::toString which is both lowercase and uppercase.

I made the PR #3651

pmjones · 2024-11-20T15:29:10Z

Thanks all, glad it turned out well!

thg2k · 2024-11-22T09:33:41Z

Do you think it would be doable to introduce something like html-string and sql-string? Wouldn't be too different from uppercase/lowercase-string. Idea being: mysqli_real_escape_string(string): sql-string, and htmlspecialchars(string): html-string, then concat would preserve the semantics (does it?) and stuff like that... The idea being that then mysqli_query(sql-string) and all output(html-string).. just gathering some preliminary input, i'd open a separate issue to discuss that.

ondrejmirtes · 2024-11-22T10:02:16Z

@thg2k I plan to do "nominal string type". Which means you'll be able to do whatever-string without any special meaning to PHPStan. But whatever-string will only accept another whatever-string. And it will be user's job to write their @phpstan-assert PHPDocs to narrow the types.

ondrejmirtes · 2024-11-22T10:02:37Z

The implementation would look very similar to this PR.

pmjones changed the base branch from 2.0.x to 1.12.x November 8, 2024 19:16

staabm reviewed Nov 8, 2024

View reviewed changes

src/Type/Php/ParseUrlFunctionDynamicReturnTypeExtension.php Outdated Show resolved Hide resolved

ondrejmirtes requested changes Nov 9, 2024

View reviewed changes

src/Type/Accessory/AccessoryNumericStringType.php Show resolved Hide resolved

ondrejmirtes requested changes Nov 9, 2024

View reviewed changes

tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php Show resolved Hide resolved

tests/PHPStan/Analyser/nsrt/isset-coalesce-empty-type.php Show resolved Hide resolved

tests/PHPStan/Analyser/nsrt/literal-string.php Show resolved Hide resolved

mvorisek reviewed Nov 9, 2024

View reviewed changes

src/Reflection/InitializerExprTypeResolver.php Show resolved Hide resolved

pmjones force-pushed the uppercase-string branch from 4c4f9b1 to 0767f80 Compare November 10, 2024 14:06

VincentLanglet requested changes Nov 10, 2024

View reviewed changes

VincentLanglet mentioned this pull request Nov 10, 2024

Add ArrayChangeKeyCaseFunctionReturnTypeExtension #3549

Merged

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

d472b91

Addresses phpstan#3613 (comment) and phpstan#3613 (comment) The relevate lowercase-string tests do not seem to be affected ... ?

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

c2523e7

Addresses phpstan#3613 (comment)

VincentLanglet requested changes Nov 18, 2024

View reviewed changes

tests/PHPStan/Analyser/nsrt/uppercase-string-parse.php Outdated Show resolved Hide resolved

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

85bb2bf

Addresses phpstan#3613 (comment)

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

2f78f96

Addresses phpstan#3613 (comment), phpstan#3613 (comment), phpstan#3613 (comment)

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

0c5e721

Addresses phpstan#3613 (comment)

VincentLanglet reviewed Nov 18, 2024

View reviewed changes

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

3c6a7b3

Addresses phpstan#3613 (comment)

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolves feedback

b6b7b49

Addresses phpstan#3613 (comment)

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

e99bf4f

Addresses phpstan#3613 (comment), phpstan#3613 (comment), phpstan#3613 (comment) This intentionally causes tests to fail so @VincentLanglet et al. can debug.

pmjones added a commit to pmjones/phpstan-src that referenced this pull request Nov 18, 2024

resolve feedback

d068557

Addresses phpstan#3613 (comment)

pmjones and others added 5 commits November 19, 2024 13:37

Introduce uppercase-string

15551b1

Update src/Type/Php/ParseUrlFunctionDynamicReturnTypeExtension.php

09dff31

Co-authored-by: Markus Staab <maggus.staab@googlemail.com>

fix name collision

c0236db

resolve failing test on 7.4

d60c48a

reasons are now unique

f0e552c

pmjones added 7 commits November 19, 2024 13:37

resolve feedback

46f45e7

Addresses phpstan#3613 (comment)

resolve feedback

7bb56a8

Addresses phpstan#3613 (comment)

resolves feedback

b861d01

Addresses phpstan#3613 (comment)

resolve feedback

c92d7b2

Addresses phpstan#3613 (comment), phpstan#3613 (comment), phpstan#3613 (comment) This intentionally causes tests to fail so @VincentLanglet et al. can debug.

resolve feedback

78e136e

Addresses phpstan#3613 (comment)

resolve feedback

d4056d1

Addresses phpstan#3613 (comment) Instead of a 4-part if/elseif/else, this builds both lowercase and/or uppercase; then if neither was true, builds what would have been the final else case.

add extension files from @VincentLanglet

ebd50a3

Copied from phpstan@22f0cab and phpstan@38151b7

pmjones force-pushed the uppercase-string branch from 554f614 to ebd50a3 Compare November 19, 2024 19:54

pmjones added 2 commits November 19, 2024 14:05

cs fixes

96830fd

check both lower and upper, not just the one (thanks phpstan!)

b4395b2

VincentLanglet reviewed Nov 19, 2024

View reviewed changes

pmjones added 3 commits November 19, 2024 15:56

remove uppercase-string from parse_url()

356e0f8

per convo w/ @VincentLanglet

resolve linting on lower PHP versions

4c54a1a

Incorporates phpstan#3613 (comment), phpstan#3613 (comment)

resolve linting on lower PHP versions

668d970

add type combinator tests

96773a4

Addresses phpstan#3613 (comment), phpstan#3613 (comment)

ondrejmirtes merged commit ca2c937 into phpstan:1.12.x Nov 20, 2024

mvorisek mentioned this pull request Nov 20, 2024

Integer::toString() and Float::toString() are uppercase-string #3651

Merged

VincentLanglet mentioned this pull request Jan 30, 2025

Support uppercase-string vimeo/psalm#11227

Closed

VincentLanglet mentioned this pull request May 15, 2025

Add uppercase-string phpstan/phpstan#11950

Closed

Conversation

pmjones commented Nov 8, 2024

Uh oh!

Uh oh!

ondrejmirtes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ondrejmirtes commented Nov 9, 2024

Uh oh!

pmjones commented Nov 9, 2024

Uh oh!

ondrejmirtes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ondrejmirtes commented Nov 9, 2024

Uh oh!

Uh oh!

pmjones commented Nov 10, 2024

Uh oh!

VincentLanglet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ondrejmirtes commented Nov 12, 2024

Uh oh!

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pmjones commented Nov 19, 2024

Uh oh!

pmjones commented Nov 19, 2024

Uh oh!

VincentLanglet commented Nov 19, 2024

Uh oh!

VincentLanglet commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ondrejmirtes commented Nov 20, 2024

Uh oh!

staabm commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VincentLanglet commented Nov 20, 2024

Uh oh!

pmjones commented Nov 20, 2024

Uh oh!

thg2k commented Nov 22, 2024

Uh oh!

ondrejmirtes commented Nov 22, 2024

Uh oh!

ondrejmirtes commented Nov 22, 2024

Uh oh!

VincentLanglet left a comment •

edited

Loading

VincentLanglet commented Nov 19, 2024 •

edited

Loading

staabm commented Nov 20, 2024 •

edited

Loading