Proc macro for css

[WorldSEnder]

Implements a proc macro for writing css, resulting in an expression of type Sheet. The following parts are still missing or up for review already:

• As part of writing this test case https://github.com/futursolo/stylist-rs/compare/master...WorldSEnder:stylist?expand=1#diff-260b6dc657a6ef35ee73a531d3e7f5a8fd197b4ca04663d48276179cf278ac10, issue Treat pseudo selectors like emotion / styled-components? #16 came up, so this also contains a fix for that. the resolution has been benched
• Additionally fixes failing compilation on master when --target wasm32-unknown-unknown is active. I suppose at some point I can rebase against a fix on master master has been fixed
• Documentation of the proc macro itself
• Address review comments
• Round two of reviews

[futursolo]

Thank you for another PR!

However, I'd like to ask a question:

What's the deciding factor of picking this syntax over string literals?

(I feel harsh to bring this question up after you are thousands of lines in.
But I have tried to avoid this situation by asking the same question in #8 a couple days ago.)

That is:

let sheet = css! {
    background: red;
};

over:

let sheet = css!(r#"background: red;"#);

The reason why I am asking is because that we will be able to reuse the parser if a string literal is used.

For a short period of time, We've got #1 and #12 (and #16 if selector parsing is needed) that needs changes on the parser.
I can foresee that maintaining the parser will certainly take a good chunk of time in the time that I will be maintaining this project. Therefore, I want to avoid the fate of having to maintain two parsers (if possible).

If a string literal is used, the procedural macro will become as simple as something like the following:

use proc_macro::TokenStream;
use proc_macro2::TokenStream as TokenStream2;

use litrs::StringLit;
use proc_macro2::Literal;
use std::convert::TryFrom;
use stylist_core::ast::*;

use quote::quote;

fn attr_to_token_stream(attr: &StyleAttribute) -> TokenStream2 {
    let key_token = Literal::string(&attr.key);
    let value_token = Literal::string(&attr.value);

    quote! {
        ::stylist::ast::StyleAttribute {
            key: #key_token.to_string(),
            value: #value_token.to_string(),
        }
    }
}

fn block_to_token_stream(block: &Block) -> TokenStream2 {
    let cond = if let Some(ref m) = block.condition {
        let s = Literal::string(m);
        quote! { Some(#s.to_string()) }
    } else {
        quote! { None }
    };

    let mut attrs = TokenStream2::new();

    for attr in block.style_attributes.iter() {
        attrs.extend(attr_to_token_stream(attr));
        attrs.extend(quote! { ,});
    }

    quote! {
       ::stylist::ast::ScopeContent::Block( ::stylist::ast::Block {
            condition: #cond,
            style_attributes: vec![#attrs]
        })
    }
}

fn sheet_to_token_stream(sheet: &Sheet) -> TokenStream2 {
    let mut output = TokenStream2::new();

    for scope in sheet.0.iter() {
        match scope {
            ScopeContent::Block(ref m) => {
                output.extend(block_to_token_stream(m));
            }
            _ => panic!(),
        }

        output.extend(quote! { ,});
    }

    quote! {
        ::stylist::ast::Sheet(vec![#output])
    }
}

#[proc_macro]
pub fn css(input: TokenStream) -> TokenStream {
    let mut tokens = input.into_iter();

    let first_token = tokens.next().unwrap();
    let s_literal = StringLit::try_from(first_token).unwrap();
    let sheet: Sheet = s_literal.value().parse().unwrap();

    sheet_to_token_stream(&sheet).into()
}

(BTW, I did test my code this time.)

With that being said, I do not object to this syntax if it provides enough advantage that warrants maintaining a separate parser and a separate AST in the long run.

[WorldSEnder]

The advantage should be, with correct implementation, error highlighting. E.g. forgetting a ;, misspelling a css attribute name, etc. are all things that are much more easily given feedback on with quote_spanned than with the generic string parsing.

Further, as you can see you can include arbitrary expressions of impl Display with the ${...} syntax. Editor completion inside these brackets works as usual. Scope lookup works as usual.
I see it like this: the proc macro syntax is for editor support that you just can't get with string literals. For that one pays the cost of a second parser.

[WorldSEnder]

Example of what you can't do as well when you always parse (assume a fitting theme is in the context)

css! {
  backgroundColor: ${theme.background};
  fontSize: ${theme.font_size};
}

Converting the above to the style string at runtime should take almost no time, as the syntax tree is statically determined, only the values are injected at runtime. I do not see how a comparable approach could work with just using parse, as the style string is not available at compile time. You'd have to fall back to parsing at runtime:

Style::new(format!(r#"
  background-color: {bg};
  font-size: {sz};
"#, bg = theme.background, sz = theme.font_size));

[WorldSEnder]

I should also mention, I do value your input on this, you seem level headed and thoughtful :)

But don't cry for the time I sink into this, I would do it either way and at the end of the day, I will have to eat my own dogfood code in material-yewi so at least it will be somewhat battle tested.

[futursolo]

I should also mention, I do value your input on this, you seem level headed and thoughtful :)

How could I deserve such an admiration. You are a humourous person. (I mean, yewi. ;) )

The parser already reports the exact location on errors, when combined with proc_macro_error, you get:

Comparing to Native error:

Both are not precisely reporting the location, but I think both are optimisable (and acceptable for now).

It's also possible to provide error handling on interpolation values at compile time with format!-style syntax using quote_spanned:

let color = Color::from_rgb(255, 255, 255);  // implements Display

let css = css!(
    r#"
        color: ${color},
    "#,
    color = color, // quote_spanned! {} here
);

In this form, editor completion should work as well as if it's outside of a proc macro.

We just need to expand the existing parser to accept interpolation during compile time and reject interpolation during runtime and limit interpolation to only value of attributes and conditions.

Also not related but having a format!-style syntax also has other benefits (does not have to be supported this round):

let opacity = 0.1;

let css = css!(
    r#"
        /* Limit precision to 0.1 */
        opacity: ${opactity:.1};
    "#,
    opacity = opacity,
);

Although, I suspect this would also be possible for non-string literal syntax.

[futursolo]

Although, I am still slightly leaning towards a format!-like string literal syntax, purely based on it's less effort to maintain.

At the end of the day, I will accept either syntax as I am not the person implementing this PR (unresponsible).

I will leave it to you to decide. :)

[futursolo]

Since there's no development on this PR for a while and I want to experiment with #23, I have started to implemented a tentative procedural macro in master and have re-purposed css! for a different API.

The API that emits a Sheet instance has been renamed to stylist::ast::sheet!.

I will still accept this PR if you want to implement this as an alternative syntax.

[WorldSEnder]

I was on vacation for a week, I'll read through what you've done in the mean time, looks promising. I really hope I can switch to this so I can test impacts on code size 👍

[WorldSEnder]

@futursolo if you have 10 minutes time, can you give me a quick code review focused on readability, naming and points where to add internal code documentation? I'm going to work on the missing user-facing documentation now, apart from that, I consider the PR to be feature complete.

[futursolo]

The current token based parser might be "too complex" (both in amount of code and difficulty to understand) and I would like to see the complexity reduced before merging.

There're a couple ways that this could be done:

1. follow rust naming convention (e.g:stringify -> to_string)
2. abort! on first error and move all error handling into parse stage (by using proc_macro_error).
3. Follow Token -> AST(Css*) -> AST(stylist::ast::*) more closely (Parse selectors into selectors in parsing stage other than seeking for commas in output stage)
4. Remove Output* and fold_* by not doing anything that is not supported by stylist::ast.
5. Collect tokens into string literal whenever possible so output stage doesn't have to consider them.
6. Document what you are trying to do in the complex code. (Some code were quite difficult to understand for me when first looking at it.)

I recently discovered a crate jss and I think their syntax is elegant and simpler to handle (json::object! is not even a procedural macro.).

Maybe shifting our syntax a little towards jss can help reducing the complexity?

(Use some string literals, as there're cases where css tokens cannot be represented properly (like: #000000) anyways?)

[futursolo]

Other than the complexity level of code and some inefficiency in code generation (which I think it's ok to address later), I think this PR looks solid.

I do like this syntax as it is simpler than string literals when it comes to interpolations.

My rationale about reducing code complexity is based on that it may hinder future contribution from other developers if they cannot easily understand the parser internals.
I wish the codebase to stay friendly to a first-time contributor.

Thank you for another big PR.

[WorldSEnder]

I hear you. I can easily see that fold_tokens_impl has by far the highest complexity and will think of a way to reduce this before marking the PR as ready for merging and a final review. Thanks for the comments 👍

[WorldSEnder]

I think I've addressed all review comments thus far. Before I unmark this as Draft, I wanted to check if I should do an interactive rebase and try to rewrite the history a bit so it's cleaner. I think it should be possible to split this into:

• Moving the previous macro into subdirectory
• Adding the new implementation
• Adding test cases
• Adding benchmarks

And have 4 separate commits that each should be more easy to review and read in the commit history. Let me know what you think @futursolo or if there's anything else to do before that, thank you for your comments and nice discussion.

[futursolo]

In the future, it might be better to separate the changeset into multiple PRs for big changes like this.
For this time, I think it's fine to leave the git history as-is.

Thank you.

[futursolo]

I think there're still some trivial things that I didn't cover.

But I think it might be easier to fix them directly after merging this PR than pointing all of them out in the comments.
(I guess I am lazy. :) )

Again, thank you for the PR.