Rv unsafe from new unchecked

This comes from a recent Discord discussion as well as issue #270 re: constructing Json from a String, which allows a Value to be constructed and thereby to allow a query to take place (one method of many which does involve working with the large Value enum but very solid).

https://discord.com/channels/841451783728529451/1142010632053456918/1142503871474847836

https://github.com/edgedb/edgedb-rust/issues/270

I agree that the function should not be marked as unsafe as there is no possibility of undefined behaviour - there is nothing in here that the compiler can't understand and must be asked to look in the other direction. All that can happen is an error from the database that the input is invalid and a very normal Err is returned:

Err(
    Error(
        Inner {
            code: 83951616,
            messages: [
                "invalid input syntax for type json",
            ],
            error: None,
            headers: {
                257: b"edb.errors.InvalidValueError: invalid input syntax for type json\n",
            },
            fields: {
                (
                    "capabilities",
                    TypeId {
                        t: 14307968703009161034,
                    },
                ): Any { .. },
                (
                    "source_code",
                    TypeId {
                        t: 72036081375641672,
                    },
                ): Any { .. },
            },
        },
    ),
)

It also removes the only pub unsafe fn in edgedb-rust which is nice.

I gave some thought to putting an impl for Json where J: Serialize (which would add a feature to the edgedb-protocol crate) but feels like overkill/babysitting considering how ubiquitous serde and serde_json are and how lightweight the protocol crate should be. I can't imagine there being anyone who knows about EdgeDB's Rust crates but has never worked with serde or JSON before. So IMO the best thing is simply to remove unsafe, add some documentation and leave it at that.

At the same time we shouldn't add anything as convenient as From for Json as that might give the impression that we are doing something under the hood to validate the String which we are not. The _unchecked part of the function name plus the extra notes should be enough. And the function name itself is quite convenient as _unchecked is seen in a lot of unsafe functions in Rust and will raise the alert level of anyone using it.

Two other changes resulting from the removal of unsafe are:

Removing some unsafe blocks that are no longer needed,
Doing away with the _new_unchecked function (note the underscore, it's a different function) that was used internally in a single place but does the same thing: just creates a Json with a String inside.

This is okay with me.

On the contrary, I feel the _unchecked() suffix is a bit misleading, as it suggests there might be a "checked" constructor somewhere in the future, which we may not have at all, given that Json is simply a string wrapper that only tells the server it should be a JSON (or this struct can be safely parsed by the user into a JSON), just like you said in the PR:

All that can happen is an error from the database that the input is invalid

So perhaps also emphasizing such design in the comments would be nice, no matter if we want to keep the _unchecked() suffix or not?

Good idea, users will appreciate knowing that EdgeDB itself will know if the JSON is valid or not. Added a note mentioning that.

On the name: yeah, I might not name it _unchecked() if putting it together for the first time today, but as an existing function name I think it gives a good amount of caution and there might be people using it already so a change from unsafe to safe wouldn't break any code on their side and they would just get a compiler message informing them that there is no need for an unsafe block.

edgedb / edgedb-rust

Rv unsafe from new unchecked #272