Skip to content

feat(quote): implement jsquote! and jsquote_expr! #10116

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Qix-
Copy link
Contributor

@Qix- Qix- commented Mar 29, 2025

Hi! This is just a draft. Wanted to see what you might think about this implementation for a quote!-like macro for OXC, inspired heavily by the infamous quote crate. Please just consider it a proposal; if it isn't the best way forward for this, that's alright too.

ref oxc-project/backlog#151

This commit introduces the jsquote! and jsquote_expr! procedural macros, which utilize the auto-derivation system to build a proto-AST when using them in order to produce in-situ Rust invocations.

  • jsquote_expr! returns an Expression<'_> and uses .parse_expression()
  • jsquote! returns a Vec<'_, Statement<'_>> and uses .parse()

It only has half of the placeholder implementation right now but anything without placeholders should work as you'd expect. They are implemented in the latest commit but have a few caveats (see comments below).


I want to say up-front that there are some really unfortunate hacks in the proc macro to make this work on stable. Nothing "unsafe", just fragile. Until some of the long-time unstable features are no longer so, they can be improved.

I've also tried to enable the unstable features automatically depending on which compiler is used (I did it the way I did, instead of using features, since all the CI runs with --all-features, but I didn't want to muck with the rust-toolchain.toml and cause a bigger stir than this PR probably already does).

let a = Allocator::default();
let span = Span::empty(0);

let expr: Expression = jsquote_expr!(&a, span, { 1234.5 + 2 });

assert!(expr.content_eq(&Expression::BinaryExpression(Box::new_in(
    BinaryExpression {
        span,
        left: Expression::NumericLiteral(Box::new_in(
            NumericLiteral {
                span,
                value: 1234.5,
                raw: Some(Atom::new_const("1234.5"),),
                base: NumberBase::Float,
            },
            &a
        )),
        operator: BinaryOperator::Addition,
        right: Expression::NumericLiteral(Box::new_in(
            NumericLiteral {
                span,
                value: 2.0,
                raw: Some(Atom::new_const("2"),),
                base: NumberBase::Decimal,
            },
            &a
        )),
    },
    &a
))));

Copy link

graphite-app bot commented Mar 29, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added A-ast Area - AST A-ast-tools Area - AST tools C-enhancement Category - New feature or request labels Mar 29, 2025
Copy link
Contributor

autofix-ci bot commented Mar 29, 2025

Hi! I'm autofix logoautofix.ci, a bot that automatically fixes trivial issues such as code formatting in pull requests.

I would like to apply some automated changes to this pull request, but it looks like I don't have the necessary permissions to do so. To get this pull request into a mergeable state, please do one of the following two things:

  1. Allow edits by maintainers for your pull request, and then re-trigger CI (for example by pushing a new commit).
  2. Manually fix the issues identified for your pull request (see the GitHub Actions output for details on what I would like to change).

Copy link

codspeed-hq bot commented Mar 29, 2025

CodSpeed Instrumentation Performance Report

Merging #10116 will not alter performance

Comparing surplus:quote (91f16d4) with main (ac2ecbb)

Summary

✅ 3 untouched benchmarks
⁉️ 33 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
⁉️ codegen[checker.ts] 22.7 ms N/A N/A
⁉️ codegen_sourcemap[checker.ts] 65.6 ms N/A N/A
⁉️ formatter[antd.js] 7.6 ms N/A N/A
⁉️ formatter[react.development.js] 43.7 µs N/A N/A
⁉️ formatter[typescript.js] 7.6 ms N/A N/A
⁉️ isolated-declarations[vue-id.ts] 58.2 ms N/A N/A
⁉️ lexer[RadixUIAdoptionSection.jsx] 21.5 µs N/A N/A
⁉️ lexer[antd.js] 24.8 ms N/A N/A
⁉️ lexer[cal.com.tsx] 5.9 ms N/A N/A
⁉️ lexer[checker.ts] 14.8 ms N/A N/A
⁉️ lexer[pdf.mjs] 3.9 ms N/A N/A
⁉️ mangler[antd.js] 16 ms N/A N/A
⁉️ mangler[react.development.js] 295 µs N/A N/A
⁉️ mangler[typescript.js] 39.7 ms N/A N/A
⁉️ minifier[antd.js] 165.6 ms N/A N/A
⁉️ minifier[react.development.js] 1.8 ms N/A N/A
⁉️ minifier[typescript.js] 291.6 ms N/A N/A
⁉️ estree[checker.ts] 96.9 ms N/A N/A
⁉️ parser[RadixUIAdoptionSection.jsx] 92.2 µs N/A N/A
⁉️ parser[antd.js] 113.2 ms N/A N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@Qix-
Copy link
Contributor Author

Qix- commented Mar 30, 2025

I'm grappling a bit with substitutions. All of the differentiating types (Expression vs Argument, as well as BindingIdentifier) makes it tricky to substitute in items that aren't exactly the AST type.

Throwing an .into() onto everything causes issues with how vectors are built up, too, given that Rust isn't (yet) able to solve for these types until Chalk comes around, I'd think. So that negates things like identifier_expr.into::<BindingIdentifier>() (pseudocode) being the 'magic' sauce here.

Further, doing what quote! actually does, which is generating tokens, and then re-parsing is out of the question here since the parser infrastructure is rigid and doesn't allow for a generic .parse()? to work either - especially given the disparity in parse() and parse_expression(). Otherwise, for the dubious cases, a #(#ident?) sort of syntax could be crafted such that ident.codegen().parse()? (pseudocode) could be done at a small performance overhead.

So for now, I'll just add as many exceptions as I can with the hopes that maybe someday this can be formalized in a bit more of a generic way.

@Qix-
Copy link
Contributor Author

Qix- commented Mar 30, 2025

Another way to go about this is to have a dedicated set of utility types that can be converted into various like-types. For example:

let a = oxc_allocator::Allocator::default();
let s = oxc_span::Span::empty(0);
let ident = oxc_quote::Ident(&a, s, "foobar");

let _ = jsquote!(&a, s, {
    var #ident = 10;
    console.log(#ident);
});

Which would expand to something like...

// ...
VariableDeclarator {
    // ...
    id: BindingPattern {
        kind: ident.into(), // Ident -> BindingPatternKind::BindingIdentifier(...)
        // ...
    },
    // ...
}
// ...
{
    arguments: [
        ident.into() // Ident -> Argument::IdentifierReference(...)
    ]
}

But it might actually be easier to implement a flag in the parser to explicitly parse template variables as AST nodes.

I might try that approach too.

@Qix- Qix- force-pushed the quote branch 2 times, most recently from 6d031bc to d90cf82 Compare March 30, 2025 19:54
@Qix-
Copy link
Contributor Author

Qix- commented Mar 30, 2025

For some reason I'm getting this on CI, but I can't reproduce locally by running the same command.

error[E0609]: no field `lossy` on type `&ast::literal::StringLiteral<'_>`
    --> crates/oxc_ast/src/generated/derive_to_rust.rs:5726:32
     |
57[26](https://github.com/oxc-project/oxc/actions/runs/14159257451/job/39662477714?pr=10116#step:4:27) |                 ("lossy", self.lossy.to_rust())
     |                                ^^^^^ unknown field
     |
     = note: available fields are: `span`, `value`, `raw`, `lone_surrogates`

Also, grepping for lone_surrogates across the entire project gives me nothing, so I'm not sure what I'm missing here.

ref oxc-project/backlog#151

This commit introduces the `jsquote!` and `jsquote_expr!`
procedural macros, which utilize the auto-derivation
system to build a proto-AST when using them in order to
produce in-situ Rust invocations.

Inspired heavily by the infamous `quote` crate.
@Dunqing
Copy link
Member

Dunqing commented Mar 31, 2025

Thank you for working on this great staff! We will take a look at this later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-ast-tools Area - AST tools C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants