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

Feature: allow non-union assertion types #8077

Merged
merged 18 commits into from
Jun 27, 2022
Merged

Conversation

boesing
Copy link
Contributor

@boesing boesing commented Jun 8, 2022

In #5657, I was wondering why oneOf does not work as expected.
That was due to the fact that psalm is simply not able to apply assertions for non-single union types.

This PR changes this and thus is declared as feature even tho the originating issue was flagged as a bug.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
fixes vimeo#5657

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@orklah
Copy link
Collaborator

orklah commented Jun 11, 2022

Could you take a look at the CI failures?

@boesing
Copy link
Contributor Author

boesing commented Jun 11, 2022

Sure! Just need some time - gonna change this to draft for now.

@boesing boesing marked this pull request as draft June 11, 2022 11:57
This will avoid issues with invalid intersection assertions.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
…ingle

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
…ar_id`

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
…tion

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing marked this pull request as ready for review June 12, 2022 00:26
@boesing boesing marked this pull request as draft June 12, 2022 00:32
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
…eralInt` and `TLiteralString`

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
…pe, do not consider the new type as a replacement

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@orklah orklah added the release:feature The PR will be included in 'Features' section of the release notes label Jun 12, 2022
@orklah
Copy link
Collaborator

orklah commented Jun 12, 2022

Seems interesting :) remove draft mode when it's ready ;)

@boesing
Copy link
Contributor Author

boesing commented Jun 12, 2022

I am still working on it as there are some cases I'd like to work. Sadly, I have to manipulate the union in some special case and thus have to rework the current logic a bit. 😅

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing marked this pull request as ready for review June 12, 2022 17:36
@boesing
Copy link
Contributor Author

boesing commented Jun 12, 2022

Okay, I guess I have everything.
Not 100% sure about the location of the whole literal intersection.
I am open for suggestions tho 👍🏼

…ored within a union

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@orklah
Copy link
Collaborator

orklah commented Jun 27, 2022

LGTM, tell me if it's ready from your side :)

@boesing
Copy link
Contributor Author

boesing commented Jun 27, 2022

Its ready from my PoV 👌🏻

@boesing boesing requested a review from orklah June 27, 2022 18:23
@orklah orklah merged commit 7dff408 into vimeo:master Jun 27, 2022
@orklah
Copy link
Collaborator

orklah commented Jun 27, 2022

Thanks!

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.

2 participants