Skip to content
  • Nathan Harris's avatar
    Revisit `RESPValue` and `RESPValueConvertible` implementations. · cd9bd04f
    Nathan Harris authored
    Motivation:
    
    Johannes provided a fair code review of the project and summarized his findings in issue #48, and one of the prime offenders was all of the `unsafe*` APIs (pointers, buffers, bytes)
    that were used with `RESPValue` and `RESPValueConvertible`.
    
    He also provided great feedback and pointed out good points of confusion with the API design of `RESPValue` and `RESPValueConvertible`.
    
    Modifications:
    
    - Return to using `Array` instead of `ContiguousArray` for `RESPValue.array` storage
    - Update all documentation to be more thorough in explaining how the types should be used and conformed to.
    - Remove all uses of `unsafe*` APIs where possible
    - Change implementations to be a lot more type and memory safe, double checking assumptions
    - Remove conformance to `ExpressibleBy*Literal` as it is too easy for users to shoot themselves in the foot and saves only a few characters over `.init(bulk:)`
    - Create new `RedisNIOTestUtils` target for common test extensions, making them public
    - Move most almost all implementations of `RESPValue` computed properties into the `RESPValueConvertible` conformances
    
    Result:
    
    Users should be more safeguarded by the API against unknowingly getting incorrect `RESPValue` representations, the API design of `RESPValue` and `RESPValueConvertible` should be much clearer,
    and memory safety should be at a higher bar from these changes.
    
    This resolves issues #55 & #48, and contributes to issue #47.
    cd9bd04f