Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Aug 29, 2022

No description provided.

Comment on lines 173 to 175
new MixedType(),
new ArrayType(new IntegerType(), new StringType()),
TrinaryLogic::createNo(),
Copy link
Contributor Author

@staabm staabm Aug 29, 2022

Choose a reason for hiding this comment

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

subtracting only array<int, string> could still leave e.g. a array<string, mixed> within the mixed.
therefore I am wondering, whether the only case where $mixed->isArray() can say NO is, when a general array is substracted?

Copy link
Member

Choose a reason for hiding this comment

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

Exactly. Switching the callee and the argument works even when more than just an array are subtracted, like array|null for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just see #1656 (comment), so I think we are on the same page.. will adjust the PR.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

While you're at it, please do all the other methods as well.

@staabm
Copy link
Contributor Author

staabm commented Aug 29, 2022

implemented in all methods I found useful.

here are a few notes:

  • should isIterableAtLeastOnce return NO when isIterable returns NO?
  • was not sure what todo with isOffsetAccessible and friends ..?
  • was wondering whether isCallable should also use a $this->subtractedType->isSuperTypeOf(new CallableType(...)) construction instead?
    see
    public function isCallable(): TrinaryLogic
    {
    if (
    $this->subtractedType !== null
    && $this->subtractedType->isCallable()->yes()
    ) {
    return TrinaryLogic::createNo();
    }
    return TrinaryLogic::createMaybe();
    }

@staabm staabm marked this pull request as ready for review August 29, 2022 09:27
@ondrejmirtes
Copy link
Member

should isIterableAtLeastOnce return NO when isIterable returns NO?

Exactly, it should just call return $this->isIterable(). I don't think we can get more precise than that.

  • was not sure what todo with isOffsetAccessible and friends ..?

Can we get a union of all offset-accessible types? Is it string|array|ArrayAccess? Yeah, probably.

  • was wondering whether isCallable should also use a $this->subtractedType->isSuperTypeOf(new CallableType(...)) construction instead?

Yes, definitely, the current code is wrong.

@staabm staabm force-pushed the subtract branch 2 times, most recently from 00b7b03 to e131ca2 Compare August 29, 2022 11:54
@staabm
Copy link
Contributor Author

staabm commented Aug 29, 2022

implemented and rebased

@ondrejmirtes ondrejmirtes merged commit a80a5fc into phpstan:1.8.x Aug 29, 2022
@ondrejmirtes
Copy link
Member

Thank you.

@staabm staabm deleted the subtract branch August 29, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants