cranetm / yii2-json-rpc-2.0

Other
26 stars 18 forks source link

Support multiple types in phpdoc #13

Open evpav opened 7 years ago

evpav commented 7 years ago

https://www.phpdoc.org/docs/latest/guides/types.html#multiple-types-combined

cranetm commented 7 years ago

Actually I cant see any benefits here. Strict type validation becomes to something else. If you have similar DTO, but one or more properties have different type - you can create new DTO, extend from the old one, override needed properties and create new web-service for it.

cranetm commented 7 years ago

www.phpdoc.org shows 2 examples: {code}/* @return string|null /{code} {code}/**

First one already works without multiple declaration. Second one very interesting, but only for Objects. And in my opinion, this Objects should have same interface, be logically combined.

you can propose another solution for Objects with the same interface.

evpav commented 7 years ago

Actually I cant see any benefits here. Strict type validation becomes to something else.

That is a valid way to describe union types without creating weird class hierarchies or multiple API methods for the same purpose but with different types of arguments.

First one already works without multiple declaration.

it passes VarValidator, but without any actual type validation(coercion), see #12

Second one very interesting, but only for Objects. And in my opinion, this Objects should have same interface, be logically combined.

Validating only shared fields is not enough. Here is a contrived example where this can be used: DTOs:

abstract A:
  int id
B extends A:
  int attrB
C extends A:
  string attrC

Currently a method response type must be declared as @returns A, and thus leaving attrB or attrC without validation. With this we can declare @returns B|C and be sure that the shape of the response will strictly match that of B or C.

cranetm commented 7 years ago

That is a valid way to describe union types without creating weird class hierarchies or multiple API methods for the same purpose but with different types of arguments.

Its not weird, its polymorphism. In strict-type languages its general situation. This library about strict validation, so.

Currently a method response type must be declared as @returns A, and thus leaving attrB or attrC without validation. With this we can declare @returns B|C and be sure that the shape of the response will strictly match that of B or C.

Objects should have same interface, be logically combined.

Class A should be an Interface A. Than validator extract interfaces from B and C and if its the same strictly match value to the first best one. But also, what about error handling? If B and C implement same interface, but some property inside both invalid. Example class B: string @minSize 20 attrA class C: string @minSize 10 attrA

attrA in passed value equal to "123". What error should be shown, about min size 20 or 10?

evpav commented 7 years ago

Supporting multiple types doesn't mean omitting strict type validation.

I know that in the current state this PR doesn't provide very useful error messages, but it can be improved if you will generally agree on the proposal, otherwise we will just part our ways. To improve the error messages we will need to track which attributes didn't pass the validation and generate error messages according to the request/response structure. This is where using \yii\base\Model would be helpful.

cranetm commented 7 years ago

Generally I agree for complex multiple types with same interface. But for the time being there is no solution for this. So, I will add long forms of simple types for parsing (integer, boolean), I'll investigate \yii\base\Model for the best experience.