chanced / jsonptr

JSON Pointer (RFC 6901) implementation for Rust
Apache License 2.0
44 stars 4 forks source link

merge `is_valid` and `validate` #50

Closed chanced closed 4 months ago

chanced commented 4 months ago

This merges is_valid and validate so that the logic is not replicated.

@asmello we could make Pointer::parse into a const fn but it would mean accepting &str:

impl Pointer {
    const fn new(s: &str) -> &Self {
        unsafe { &*(core::ptr::from_ref::<str>(s) as *const Self) }
    }

    pub const fn parse(s: &str) -> Result<&Self, ParseError> {
        match validate(s) {
            Ok(_) => Ok(Self::new(s)),
            Err(err) => Err(err),
        }
    }
}
asmello commented 4 months ago

Ah nice, I avoided this because I wanted to minimise the risky algorithm changes, but it makes sense to do.

we could make Pointer::parse into a const fn but it would mean accepting &str

Yeah I considered that, but figured from_static would likely be more useful in const contexts (plus validate wasn't const). I'm not sure about the trade-off, I think AsRef<str> is likely more useful than having parse be const. We could add a separate parse_const instead, I'd be totally fine with that.

chanced commented 4 months ago

Ah nice, I avoided this because I wanted to minimise the risky algorithm changes, but it makes sense to do.

Yea, same, but our test coverage is solid enough now that I think this is safe enough. Plus its possible now that errors don't allocate.

We could add a separate parse_const instead, I'd be totally fine with that.

Nah, I like from_static more.