Make Fbp::$randomNumber private and add the missing native types - #25
Merged
Merged
Conversation
The last two cleanups planned for 2.0: - Fbp::$randomNumber was the only public, mutable property on the otherwise immutable cookie value objects. It is private now; getRandomNumber() and withRandomNumber() already existed. - User::$fbc and User::$fbp were untyped, so anything could be assigned. They are Fbc|string|null and Fbp|string|null natively. - Fb::withCreationTime() takes int|\DateTimeInterface natively instead of checking an untyped parameter at runtime, so a wrong type is a TypeError instead of an InvalidArgumentException. - withSubdomainIndex(), withCreationTime() and withAppendix() declare static as their return type instead of documenting it. The private Parameters::normalize() helper is typed as well. See UPGRADE-2.0.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The last two cleanups from the list in #16 and #24. After this, nothing in
src/is untyped, and the cookie value objects are fully immutable.Fbp::$randomNumberis private. It was the only public, mutable property on the otherwise immutableFb/Fbc/Fbpvalue objects, so$fbp->randomNumber = 1bypassedwithRandomNumber(). The getter and the wither already existed.User::$fbcandUser::$fbpare natively typed, asFbc|string|nullandFbp|string|null. They were untyped with only a docblock, so anything could be assigned.Fb::withCreationTime()takesint|\DateTimeInterfacenatively, instead of accepting anything and checking it with an assertion at runtime.withSubdomainIndex(),withCreationTime()andwithAppendix()declarestaticas their return type, replacing the@return staticdocblocks.Parameters::normalize()helper getsmixedand a native return type.Backwards compatibility
Intentional breaks, documented in a new section of
UPGRADE-2.0.md:$fbp->randomNumberno longer works; usegetRandomNumber()/withRandomNumber().Fbc/Fbp, a string or null toUser::$fbc/$fbpthrows a\TypeError.withCreationTime()throws a\TypeError, where it used to throw anInvalidArgumentException. The upgrade guide notes that a\TypeErroris PHP's own error for a programming mistake and does not implementExceptionInterface.The Roave BC job is red by design.
Tests
FbTest::it_rejects_a_creation_time_that_is_neither_an_integer_nor_a_datetime. It passed a string with a@phpstan-ignore, and only made sense for an untyped parameter; the type system enforces this now.UserTest::it_accepts_the_cookies_as_value_objects_and_as_strings: both properties default to null, and accept value objects as well as strings, with the expected payload each time.Not in this PR
Issue #20, one failing pixel stopping delivery to the rest, is still open. It was deferred on purpose and needs design decisions of its own.