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

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

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

LeSuisse
Copy link
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
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
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
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"));
```
@AndrolGenhald AndrolGenhald merged commit 034a796 into vimeo:4.x Aug 23, 2022
@AndrolGenhald
Copy link
Collaborator

Thanks!


/**
* @psalm-pure
* @template T as string
Copy link
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
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