Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to the macro test driver #38

Open
Robbepop opened this issue Nov 21, 2019 · 1 comment
Open

Improvements to the macro test driver #38

Robbepop opened this issue Nov 21, 2019 · 1 comment

Comments

@Robbepop
Copy link
Contributor

Robbepop commented Nov 21, 2019

I use synstructure to implement a simple trait for which the crate is perfectly suited and the whole thing has been implemented in approx 25 lines of code which is nice.

However testing the derive macro for struct or enum doesn't really work because the test driver is producing different whitespace in got and expected and since for the test driver whitespace is significant the tests fail.

In the current state I cannot make use of the test driver and saw that other crates aren't using it either (maybe for the same reasons?) but I think some improvements could be done to improve it significantly.

  • Ignore insignificant whitespace.
  • Normalize the generated and expected code, e.g. remove or always add trailing commas etc.

Having these improvements writing tests for macros would be less painful.


Also I think that users should not provide the synstructure generated parts into their expected test quote! - by that I mean the const _DERIVE_foo_bar: () = { ... } but they shall only provide what is inside the { ... } because that's what is actually controlled by the user, everything else shouldn't be tested since it is basically an implementation detail of how synstructure is currently dealing with it. So not a concern to users of synstructure.

@Robbepop Robbepop changed the title Insignificant whitespace should be ignored in test driver Improvements to the macro test driver Nov 21, 2019
@mystor
Copy link
Owner

mystor commented Nov 23, 2019

Whitespace should not be appearing differently, but it may be the case that proc_macro2::TokenStream's Display implementation occasionally produces inconsistent whitespace. I'd be interested in a specific example where whitespace appears inconsistent. A future version may perform more structural matching of TokenStreams to avoid some problems like this, but issues like <=> vs <= > may not be possible to fix reliably due to limitations in quote!.

Trailing commas in the generated code may (or may not) be significant, so automatically normalizing that is risky. Doing so would require understanding what type of ast node the macro is generating in order to determine whether or not the comma is necessary. I don't think that synstructure should be trying to detect that, though it may be nice to support identifying where the difference is being found more easily with better errors.

I generally agree that synstructure-generated consts can be ugly, but they may not be part of the generated code at all, as the macro is usable by people not using codegen methods such as gen_impl. The new(-ish) anonymous const feature should make the generated code much nicer once we bump the msrv above 1.37.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants