Remove Deref and DerefMut for Json

[bobozaur]

Bug Description

Perhaps not a bug, but a questionable implementation decision.

The Json newtype wrapper implements Deref and DerefMut along with the more correct AsRef and AsMut traits. I'd personally say that Deref and DerefMut should be removed, perhaps in 0.8.0 as it would be a breaking change.

The rationale is that the impls are arguably redundant and even lead to confusion in some occasions for the sole benefit of a shortcut in field access (which ends up giving references instead of owned types as with regular structs field access).

Minimal Reproduction

#[derive(Clone)]
struct MyStruct {
    field: String
}

let s = MyStruct {field: "something".to_owned()};
let x = Json(s.clone());

let _ = s.field; // has type String
let _ = x.field; // has type &String - confusing
let _ = x.0.field // has type String
let _ = x.as_ref().field // has type &String - intuitive due to the as_ref() call

Info

• SQLx version: 0.7.1, most likely older
• SQLx features enabled: None
• Database server and version: N/A
• Operating system: N/A
• rustc --version: N/A

[abonander]

These impls don't exist without precedent:

• https://docs.rs/axum/latest/axum/struct.Json.html#impl-Deref-for-Json%3CT%3E
• https://docs.rs/actix-web/latest/actix_web/web/struct.Json.html#impl-Deref-for-Json%3CT%3E

It appears Actix-web experimented with removing them (on other adapters, anyway) but ultimately decided to keep them: actix/actix-web#2016 (comment)

In fact, they actually prefer Deref so much that on some adapters they went the other direction and made the tuple field private so you're forced to use it instead of destructuring: actix/actix-web#2016

When you're writing a ton of queries, e.g. for a large backend API, a little bit of convenience goes a long way. I've never had a bug result from these impls existing, nor have I heard of one happening from my coworkers who use SQLx extensively.

If you really don't like them, just don't use them.

[bobozaur]

@abonander Thanks for taking the time to answer this. I didn't mention this initially, but most of us are aware that impls of Deref and DerefMut are quite opinionated. And that's fine, really, as it's hard to have strong opinions on things as obscure as this.

Perhaps my (again, opinionated) argument against such Deref and DerefMut impls is the lack of a DerefMove trait, especially one that encapsulates the particularities of Box (like partial move, or unsized dereferencing).

I have witnessed occasions when the underlying mechanics of simple wrappers like Json create confusion due to people taking for granted how Box works and expecting the same out of such wrappers, not knowing that Box is a special snowflake.

Relying on AsRef and AsMut, perhaps paired with an into_inner() method, makes things more verbose, yes, but also more explicit and perhaps aligns more with the principle of least surprise.

Nevertheless, I was certain that the decision to implement these traits was made consciously and I wasn't necessarily expecting that to change simply from me challenging it. I appreciate you explaining the rationale behind the decision.