Skip to content

Allow *bin2hex and *bin2base64 functions to keep non-empty-string type#8431

Merged
AndrolGenhald merged 1 commit intovimeo:4.xfrom
LeSuisse:bin2hex-base64-stub
Aug 23, 2022
Merged

Allow *bin2hex and *bin2base64 functions to keep non-empty-string type#8431
AndrolGenhald merged 1 commit intovimeo:4.xfrom
LeSuisse:bin2hex-base64-stub

Conversation

@LeSuisse
Copy link
Copy Markdown
Contributor

@LeSuisse LeSuisse commented Aug 23, 2022

Those functions should not return a string when they receive a non-empty-string in input.

The following example is expected to work:

<?php

/**
 * @param non-empty-string $i
 */
function takesNonEmptyString(string $i): void {
    echo $i;
}

takesNonEmptyString(bin2hex("a"));
takesNonEmptyString(base64_encode("a"));

https://psalm.dev/r/0d5b2b3965

@AndrolGenhald
Copy link
Copy Markdown
Collaborator

I believe the sodium_* functions require an extension to be installed? Could we add a separate stub for that and conditionally include it like the other extension stubs?

@AndrolGenhald AndrolGenhald added the release:feature The PR will be included in 'Features' section of the release notes label Aug 23, 2022
@LeSuisse
Copy link
Copy Markdown
Contributor Author

I believe the sodium_* functions require an extension to be installed? Could we add a separate stub for that and conditionally include it like the other extension stubs?

Hum maybe? I took the same approach than for the sodium_* functions that were already in the file.

@AndrolGenhald
Copy link
Copy Markdown
Collaborator

I took the same approach than for the sodium_* functions that were already in the file.

Well, if they're already there I guess this is fine then. Maybe I'll get around to moving them to a separate extension stub for Psalm 5.

Those functions should not return a string when they receive a
non-empty-string in input.

The following example is expected to work:
```php
<?php

/**
 * @param non-empty-string $i
 */
function takesNonEmptyString(string $i): void {
    echo $i;
}

takesNonEmptyString(bin2hex("a"));
takesNonEmptyString(base64_encode("a"));
```
@LeSuisse LeSuisse force-pushed the bin2hex-base64-stub branch from b466a96 to 4b1adaa Compare August 23, 2022 14:40
@AndrolGenhald AndrolGenhald merged commit 034a796 into vimeo:4.x Aug 23, 2022
@AndrolGenhald
Copy link
Copy Markdown
Collaborator

Thanks!


/**
* @psalm-pure
* @template T as string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a headsup from the review in phpstan. we don't need a template in phpstan for this case, see phpstan/phpstan-src#1664

maybe thats also the case for psalm and this can be simplified

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, same on Psalm, but stubs are transparent for users so it doesn't really matters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:feature The PR will be included in 'Features' section of the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants