In preparation of bringing this component in compliance with the JSON-RPC 2.0 specs, I would like to update this component to a currently supported PHP version including language features (strict types, type declarations).
Planned changes/updates
@Xerkus suggested the following schedule. If possible, each point will be provided in a separate PR
to achieve green CI at each step and make reviews easier.
[ ] Smd::addService(Smd\Service $service) drop array support and argument type check [BC]
[ ] JSON-RPC SMD: Use argument type declarations as param types (deprecating type hints in PHPDocs)
[ ] Server::getSmd() Return should be changed to ?string as you get a result or nothing.
[ ] Cache::saveSmd() Return should be void. Regardless of whether caching attempt was successful just carry on.
[ ] Cache::deleteSmd() Operation result is irrelevant for the consumer. Return nothing. :void
[ ] Client::__construct() QA: rename $server to $serverAddress to better reflect its meaning
[ ] Server::handle() Not sure about the reason for false instead of null here ($request parameter). This should be changed at some point i think
[ ] Server::__call Design: return null; should be used when method is not : void. A change to be done when static analysis tool introduced.
[ ] Server::handleRequest() QA: if (! $request->isMethodError() && '' === $request->getMethod()) { Method error check block from below can be merged with this block:
if ($request->isMethodError() || empty($request->getMethod())) {
Alternatively blocks can be swapped and condition simplified to check only for empty method.
[ ] Add strict typing to laminas-server, update used version in here as well as update usages (method signatures).
PR against master
[x] Bugfix Smd::getDescription() returns $description instead of self
Ocramius
commented
2 years ago
Component maintenance
In preparation of bringing this component in compliance with the JSON-RPC 2.0 specs, I would like to update this component to a currently supported PHP version including language features (strict types, type declarations).
Planned changes/updates
@Xerkus suggested the following schedule. If possible, each point will be provided in a separate PR
This issue serves as a task list and an overview of the progress.
No changes in functionality
This "project" will not introduce features, bugfixes or BC breaks. Necessary functionality changes will be handled separately afterwards.
EDIT:
Additional changes
List of additional changes required. Initially these points resulted from the code review in #3 and will be added on if required.
PR against
dev-4.0Json::encode&Json::decodein favor of nativejson_encodeandjson_decode(add ext-json to composer.json)ExceptionInterfaceshould extendThrowablelaminas/laminas-coding-standard2.0arrayarguments more generic, probablyiterable(e.g.Smd::addServices(array $services))Smd::addService(Smd\Service $service)drop array support and argument type check [BC]Server::getSmd()Return should be changed to?stringas you get a result or nothing.Cache::saveSmd()Return should be void. Regardless of whether caching attempt was successful just carry on.Cache::deleteSmd()Operation result is irrelevant for the consumer. Return nothing.:voidClient::__construct()QA: rename$serverto$serverAddressto better reflect its meaningServer::handle()Not sure about the reason forfalseinstead ofnullhere ($requestparameter). This should be changed at some point i thinkServer::__callDesign:return null;should be used when method is not: void. A change to be done when static analysis tool introduced.Server::handleRequest()QA:if (! $request->isMethodError() && '' === $request->getMethod()) {Method error check block from below can be merged with this block: if ($request->isMethodError() || empty($request->getMethod())) { Alternatively blocks can be swapped and condition simplified to check only for empty method.laminas-server, update used version in here as well as update usages (method signatures).PR against
masterSmd::getDescription()returns$descriptioninstead ofself