bumped laminas-servicemanager to v4 alongside its dependencies, remov… #103
Conversation
…ed deprecated usage functions Signed-off-by: Jurj-Bogdan <[email protected]>
Signed-off-by: Jurj-Bogdan <[email protected]>
Signed-off-by: Jurj-Bogdan <[email protected]>
Signed-off-by: Jurj-Bogdan <[email protected]>
|
@Jurj-Bogdan See #102 |
|
@laminas/technical-steering-committee So, should I create the branch 3.0.x and merge this one there ? |
Yes, create 3.0.x from the tip of the next minor release - You should probably get 2.24.x patches resolved first such as #106 so that merge-ups from 2.x have fewer conflicts |
|
@Jurj-Bogdan Please fix the conflicts |
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…ed deprecated usage functions Signed-off-by: Jurj-Bogdan <[email protected]>
Signed-off-by: Jurj-Bogdan <[email protected]>
Signed-off-by: Jurj-Bogdan <[email protected]>
| */ | ||
| protected $sessionSavePath; | ||
| protected string $sessionSavePath; |
There was a problem hiding this comment.
This property is initially null - it is not set in the constructor
| */ | ||
| protected $sessionName; | ||
| protected string $sessionName; |
There was a problem hiding this comment.
This property is initially null - it is not set in the constructor
| */ | ||
| public function setCacheStorage(CacheStorage $cacheStorage) | ||
| public function setCacheStorage(CacheStorage $cacheStorage): Cache |
There was a problem hiding this comment.
This method should be deprecated and dropped and the constructor changed to use property promotion
| @@ -41,28 +40,20 @@ class ContainerAbstractServiceFactory implements AbstractFactoryInterface | |||
| { | |||
There was a problem hiding this comment.
This class should be made final and all properties private. You should also mark this class soft final with @final in a 2.x release
| */ | ||
| protected $config; | ||
| protected array $config = []; |
There was a problem hiding this comment.
This prop is not set in a constructor, so it's type should be array|null
|
|
||
| /** @param OptionsArgument $options */ |
There was a problem hiding this comment.
Please revert as per comment on class-level doc-block
| * session?: Container, | ||
| * timeout?: ?int, | ||
| * } | ||
| */ | ||
| final class Csrf extends AbstractValidator |
There was a problem hiding this comment.
This whole class will probably need further refactoring such as deprecating and dropping all the public setters and getters
There was a problem hiding this comment.
Not suggesting that work should happen in this patch tho!
| @@ -114,7 +114,8 @@ public function testFactoryWillAddValidatorViaConfiguration(): void | |||
|
|
|||
| $manager->start(); | |||
|
|
|||
| $chain = $manager->getValidatorChain(); | |||
| $chain = $manager->getValidatorChain(); | |||
| /** @psalm-suppress ArgumentTypeCoercion **/ | |||
There was a problem hiding this comment.
Please remove this suppression and replace it with assertions.
Assert the manager is ManagerInterface
Assert the chain is a EventManagerInterface or whatever it's supposed to be
| @@ -198,7 +199,8 @@ public function testFactoryDoesNotAttachValidatorTwoTimes(): void | |||
| // Ignore exception, because we are not interested whether session validation passes in this test | |||
| } | |||
|
|
|||
| $chain = $manager->getValidatorChain(); | |||
| $chain = $manager->getValidatorChain(); | |||
| /** @psalm-suppress ArgumentTypeCoercion **/ | |||
| @@ -1019,6 +963,11 @@ | |||
| <code><![CDATA[CallbackHandler]]></code> | |||
| </UndefinedDocblockClass> | |||
| </file> | |||
| <file src="src/Validator/Csrf.php"> | |||
There was a problem hiding this comment.
We really shouldn't be expanding the baseline while moving towards 3 - This is likely due to dropping the defined array shape on the CSRF validator
Upgraded
laminas/laminas-servicemanagerto version 4, as well as the other dependencies required for it, and added various Psalm tweaks.I seems I was working on this PR alongside #102 , with the main difference seemingly being the removal of
v2functions from the codebase in preparation for a new major release.I was about to make a different PR to the current branch marking the functions removed here as
deprecated, but I'll wait for feedback regarding the desired way forward.