Declare native types and clean up code - #76
Merged
Merged
Conversation
Properties of the authentication components, the client, the command controller and the cookie middleware have native PHP types instead of @var annotations. The middleware uses constructor property promotion. Flow does not inject typed properties lazily, so these dependencies are now injected when the object is created. Named injections, such as the security logger, still default to lazy injection, which would assign a dependency proxy to the typed property. They are therefore declared with "lazy: false". The loggers are nullable, so that the classes also work without a logger in unit tests. The discovery and JWKS caches stay untyped. Flow injects properties configured in Objects.yaml lazily and has no option to change that.
Classes are imported instead of being referenced with a leading backslash, constants are typed and listed first, and docblocks which only repeated the method signature are removed. TokenSet is marked as not proxied, like other plain data objects. Unused code is removed: the security context injected into the authentication provider, the entity manager of the command controller and the default value of the middleware option "disableTrustedProxiesComponentCompatibility", which nothing reads. Configurations which still set this option keep working. A duplicated session check in the provider is merged into one.
The remaining classes and all unit tests declare strict_types, so that scalar values passed to typed parameters are no longer converted silently. IdentityToken is left unchanged for now, because it will be reworked separately.
The test matrix no longer cancels the remaining jobs after the first failure. This shows at a glance which combinations of PHP versions and dependency sets are affected.
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
1 participant
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.
This brings the code in line with current PHP and Flow conventions, without changing what the package does.
Injected and other properties have native types instead of
@varannotations. Flow injects typed properties directly instead of lazily, so dependencies are now created together with the object. Named injections such as the security logger are declared withlazy: false, because Flow would otherwise assign a dependency proxy that does not match the property type. The caches configured inObjects.yamlstay untyped for the same reason, since Flow offers no way to disable lazy injection there.Besides that, classes are imported instead of being referenced with a leading backslash, constants are typed, docblocks that only repeated the signature are removed, and some unused code is gone. The remaining classes and the tests declare
strict_types.Also, the CI matrix no longer cancels all jobs after the first failure.