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

feat(type): Notnull type optimizations #465

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

veewee
Copy link
Collaborator

@veewee veewee commented Apr 9, 2024

Fixes #357

There is still one situation, where you use nonnull() inside a shape().
In this situation, the type will resolve to mixed (inluding null).
This can only be solved once psalm has a TNonNull type.

Thanks @devnix !

@veewee veewee added this to the 3.0.0 milestone Apr 9, 2024
@veewee veewee added Priority: Low This issue can probably be picked up by anyone looking to contribute to the project, as an entry fix Status: Review Needed The issue has a PR attached to it which needs to be reviewed. Type: Enhancement Most issues will probably ask for additions or changes. labels Apr 9, 2024
@veewee veewee requested a review from azjezz April 9, 2024 09:22
@coveralls
Copy link

coveralls commented Apr 9, 2024

Pull Request Test Coverage Report for Build 8613784700

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.781%

Totals Coverage Status
Change from base Build 8581981192: 0.0%
Covered Lines: 4377
Relevant Lines: 4431

💛 - Coveralls

@azjezz
Copy link
Owner

azjezz commented Apr 9, 2024

I don't like that we are exposing an internal type, but i think its okay in this case.

another option is to introduce a new interface as part of the public API, like EliminiationTypeInterface and have nonnull return EliminiationTypeInterface<null> .

example:

interface EliminiationTypeInterface<TEliminated, TGroup> extends TypeInterface<TGroup>
{
    public function matches<TInput: mixed>(
        TInput|TEliminated $value
    ): ($value is TEliminated ? false : true);

    public function coerce<TInput: mixed>(
        TInput|TEliminated $value,
    ): ($value is TEliminated ? never : TGroup);

    public function assert<TInput: mixed>(
        TInput|TEliminated $value,
    ): ($value is TGroup ? ($value is TEliminated ? never : TGroup) : never);
}

@veewee
Copy link
Collaborator Author

veewee commented Apr 10, 2024

@azjezz That could work - but the interface would be broken by design:

extends TypeInterface<TGroup>

Since there is no way in psalm for doing an Exclude, it will always tell the upstream that it is of type TGroup - yet it is of type Exclude<TGroup, TEliminated>.

Not sure if that is a better solution than leaking one internal that most likely wouldn't affect much applications.
I could make a comment on the "why we did this" there ?

@azjezz azjezz merged commit 3625f2b into azjezz:next Apr 16, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low This issue can probably be picked up by anyone looking to contribute to the project, as an entry fix Status: Review Needed The issue has a PR attached to it which needs to be reviewed. Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type\not_null()
3 participants