Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Feb 27, 2025

Summary

Now it also applies when a parameter is documented with a preceding ? or pending |null, but no further unionation is considered.

So previously the range check worked against parameters that were not nullable:

/**
  * @psalm-param int<1,500> $thisWasChecked
  * @psalm-param int<1,500>|null $thisWentUnderTheRadar
  * @psalm-param ?int<1,500> $thisWentUnderTheRadarAsWell
  * /

Now all cases are handled.

To keep in mind: when a parameter is passed in the query string without a value, it is considered an empty string, not a null value. To not risk breakage, we should refrain from backporting.

Checklist

@blizzz blizzz added this to the Nextcloud 32 milestone Feb 27, 2025
@blizzz blizzz requested review from a team, Altahrim, ArtificialOwl, come-nc and nickvergessen and removed request for a team February 27, 2025 18:17
@blizzz blizzz force-pushed the enh/noid/nullable-range branch from 0881c20 to 4552a7e Compare February 27, 2025 18:41
Now it also applies when a paramater is documtend with a pending |null,
but no further unionation is considered.

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz force-pushed the enh/noid/nullable-range branch from 4552a7e to 6594d7d Compare February 27, 2025 18:49
@blizzz blizzz merged commit 42d752f into master Mar 4, 2025
190 checks passed
@blizzz blizzz deleted the enh/noid/nullable-range branch March 4, 2025 13:23
@nickvergessen
Copy link
Member

Backport?

@blizzz
Copy link
Member Author

blizzz commented Mar 5, 2025

Backport?

From the description:

To keep in mind: when a parameter is passed in the query string without a value, it is considered an empty string, not a null value. To not risk breakage, we should refrain from backporting.

Do you have a different opinion on it?

@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants