add From<&str> implementation for types using `deserialize_from_str!`

[gibbz00]

Writing an authorization server which needs to be able to parse CoreResponseType from an HTTP query string 😊

[ramosbugs]

Thanks for the PR!

I'm generally wary of From<&str> since it encourages stringly-typed code that can lead to bugs in user code. For example, consider a function:

fn foo(client_id: ClientId, client_secret: ClientSecret) { ... }

If both ClientId and ClientSecret were to implement From<&str> (neither currently does), then the following code will compile fine:

foo("my_secret".into(), "my_id".into())

However, the arguments are in the wrong order, which is not at all obvious from looking at the call site. Instead, the current interface requires:

foo(ClientId::new("my_secret".to_string()), ClientSecret::new("my_id".to_string()))

(which still has the same bug, but it's a lot more obvious while looking at the code).

To avoid this issue, the newtypes throughout this crate implement an explicit new() constructor instead of From<&str>.

The same concern about .into() applies to enums, but new() doesn't feel like the right name for an explicit constructor. Instead, I think we should just make the existing from_str() methods public, which should support your use case without introducing the .into() pitfalls. I'd be happy to merge that change.

[gibbz00]

Hi!

Thanks for the well-put response. I see what you mean and it makes sense. The reason I chose to not simply make the from_str public, is because of the deny(missing_docs), as mentioned in the commit message 😊 How do you think we should go about this?

• Add #[allow(missing_docs)] to each method and put off proper documentation for later?
• Add documentation to each method? May require referencing specific specification sections.

[ramosbugs]

I would just add a generic doc message like "Construct a new <name of type>"

No need to reference the specs, which I think would be more appropriate in the type's top-level documentation rather than its constructor's documentation.

[gibbz00]

Was able to work around this by using serde_qs. Works well enough that I think this PR can be closed.