Make derive macros use fully-qualified paths #45
Conversation
|
@not-fl3 when do you think this is likely to be released to crates.io? I don't mean to pressure you, I'm just wondering how I should manage the releases of a project I'd like to integrate this into. |
|
If I understand correctly, the tradeoff here: With this PR we disallow using nanoserde with crate rename (like this: Honestly, I do not remember(and can't really find out on the internet fast enough) if this is the only implication here, so I just do not know enough to make a decision on merging this right now :( |
| } | ||
| Some(val) | ||
| } else { | ||
| if !field.ty.is_option { | ||
| Some(String::from("Default::default()")) | ||
| Some(String::from("::std::default::Default::default()")) |
There was a problem hiding this comment.
afaik Option/Into are pretty safe to use without ::
There was a problem hiding this comment.
also core::option is the better choice to get an Option from. But is it really a necessity to use :: for an Option?
There was a problem hiding this comment.
True, but there's no downside, and it's generally recommended to use fully qualified paths everywhere in macros, just in case the user happens to be completely mad.
There was a problem hiding this comment.
any links on a scenario when an Option is not available?
There was a problem hiding this comment.
Not links, but the user could be operating in an environment without the standard library's prelude, but with the standard library available (not sure how you'd do that, but I assume you could). What are your reservations?
There was a problem hiding this comment.
yes, but both Option and Into are parts of core, not just std
There was a problem hiding this comment.
Oh yes of course, sorry. I don't think that changes anything though, as there are no downsides to having fully-qualified paths for these. I can remove them if you'd like, but I don't see any reason; better safe than sorry, right?
|
Hmm true, I hadn't considered that. As far as I know though, this use of fully qualified paths is just about ubiquitous in other macro crates, so this behavior would be more expected by users than being forced to import things (which I've found very restrictive in using this in a macro of my own that automatically derives |
|
@not-fl3 by the way, the user renaming of the crate wouldn't work even without this PR, since there were multiple existing (and necessary) references to |
|
@not-fl3 has there been any progress on this PR? |
|
Honestly, I am not convinced And for the |
|
Alright, would you like me to change the |
|
I would just use them a is, as |
Specifically: `Option`, `Into`, `From`, and `AsRef`.
|
I've removed the absolute paths for |
|
Jumping in here, this is actually quite a blocker for my use case (writing a book where I can't have any top-level use statements). Any chance this request could be revisited? |
|
I took a swing at this in #124. |
This PR makes the macros in this crate use fully qualified paths, which prevents assumptions about what's in the user's scope. For example, this makes it possible to use
#[derive(nanoserde::SerJson)]without first writinguse nanoserde::SerJson;, which allows this crate to be used in other macros (my requirement).I think I've caught everything, but there are around a thousand lines of (very impressive) manual macros, so it's entirely possible that I've missed a few things!
By the way, thanks for this crate, it's extremely useful!