Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
refactor(ast_codegen): use doc comments instead of insert! (#4777)
Avoid the `insert!` macro in AST codegen. Use doc comments starting with special symbol `@` instead.

* Before: `insert!("// plain comment");`
* After: `///@ plain comment`
* Or: `//!@ plain comment`

Either `///@` or `//!@` is converted to plain `//` in output.

`//!@` is legal in top-of-file position, which allows us to inline `#![allow(...)]` attributes, which in my opinion makes the generators a bit easier to read.
  • Loading branch information
overlookmotel committed Aug 9, 2024
commit f418f62e7be02843f6b9b895f29feba264d582b1
44 changes: 43 additions & 1 deletion tasks/ast_codegen/src/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ static WHITE_SPACES: &str = " \t";
///
/// We use this when inserting outer attributes (`#![allow(unused)]`) or plain comments (`//` not `///`).
/// `quote!` macro ignores plain comments, so it's not possible to produce them otherwise.
#[allow(dead_code)] // `insert!` macro is not currently used
struct InsertReplacer;

impl Replacer for InsertReplacer {
Expand Down Expand Up @@ -65,11 +66,52 @@ lazy_static! {
Regex::new(format!(r"[{WHITE_SPACES}]*{ENDL_MACRO_IDENT}!\(\);").as_str()).unwrap();
}

/// Replace doc comments which start with `@` with plain comments or line breaks.
///
/// Original comment can be either `///@` or `//!@`.
///
/// * `///@ foo` becomes `// foo`.
/// * `//!@ foo` becomes `// foo`.
/// * `///@@` is removed - i.e. line break.
/// * `//!@@` is removed - i.e. line break.
///
/// `quote!` macro ignores plain comments, but we can use these to generate plain comments
/// in generated code.
///
/// To dynamically generate a comment:
/// ```
/// let comment = format!("@ NOTE: {} doesn't exist!", name);
/// quote!(#[doc = #comment])
/// // or `quote!(#![doc = #comment])`
/// ```
///
/// `//!@@` is particularly useful for inserting a line break in a position where `endl!();`
/// is not valid syntax e.g. before an `#![allow(...)]`.
struct CommentReplacer;

impl Replacer for CommentReplacer {
fn replace_append(&mut self, caps: &Captures, dst: &mut String) {
assert_eq!(caps.len(), 2);
let body = caps.get(1).unwrap().as_str();
if body != "@" {
dst.push_str("//");
dst.push_str(body);
}
}
}

lazy_static! {
static ref COMMENT_REGEX: Regex =
Regex::new(format!(r"[{WHITE_SPACES}]*//[/!]@(.*)").as_str()).unwrap();
}

/// Pretty print
pub fn pprint(input: &TokenStream) -> String {
let result = prettyplease::unparse(&parse_file(input.to_string().as_str()).unwrap());
let result = ENDL_REGEX.replace_all(&result, EndlReplacer);
let result = INSERT_REGEX.replace_all(&result, InsertReplacer).to_string();
// `insert!` macro is not currently used
// let result = INSERT_REGEX.replace_all(&result, InsertReplacer).to_string();
let result = COMMENT_REGEX.replace_all(&result, CommentReplacer).to_string();
result
}

Expand Down
6 changes: 5 additions & 1 deletion tasks/ast_codegen/src/generators/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ impl Generator for AstBuilderGenerator {
quote! {
#header

insert!("#![allow(clippy::default_trait_access, clippy::too_many_arguments, clippy::fn_params_excessive_bools)]");
#![allow(
clippy::default_trait_access,
clippy::too_many_arguments,
clippy::fn_params_excessive_bools,
)]
endl!();

use oxc_allocator::{Allocator, Box, IntoIn, Vec};
Expand Down
2 changes: 1 addition & 1 deletion tasks/ast_codegen/src/generators/derive_get_span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn derive(
quote! {
#header

insert!("#![allow(clippy::match_same_arms)]");
#![allow(clippy::match_same_arms)]
endl!();

use oxc_span::#trait_ident;
Expand Down
11 changes: 6 additions & 5 deletions tasks/ast_codegen/src/generators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,25 +125,26 @@ pub(crate) use define_generator;
/// Similar to how `insert` macro works in the context of `quote` macro family, But this one can be
/// used outside and accepts expressions.
/// Wraps the result of the given expression in `insert!({value here});` and outputs it as `TokenStream`.
#[allow(unused)]
macro_rules! insert {
($fmt:literal $(, $args:expr)*) => {{
let txt = format!($fmt, $($args)*);
format!(r#"insert!("{}");"#, txt).parse::<proc_macro2::TokenStream>().unwrap()
}};
}
#[allow(unused_imports)]
pub(crate) use insert;

/// Creates a generated file warning + required information for a generated file.
macro_rules! generated_header {
() => {{
let file = file!().replace("\\", "/");
let edit_comment =
$crate::generators::insert!("// To edit this generated file you have to edit `{file}`");
// TODO add generation date, AST source hash, etc here.
let edit_comment = format!("@ To edit this generated file you have to edit `{file}`");
quote::quote! {
insert!("// Auto-generated code, DO NOT EDIT DIRECTLY!");
#edit_comment
endl!();
//!@ Auto-generated code, DO NOT EDIT DIRECTLY!
#![doc = #edit_comment]
//!@@
}
}};
}
Expand Down
56 changes: 24 additions & 32 deletions tasks/ast_codegen/src/generators/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use syn::{parse_quote, Ident};

use crate::{
codegen::LateCtx,
generators::{ast_kind::BLACK_LIST as KIND_BLACK_LIST, insert},
generators::ast_kind::BLACK_LIST as KIND_BLACK_LIST,
markers::{ScopeMarkers, VisitArg, VisitMarkers},
output,
schema::{EnumDef, GetIdent, StructDef, ToType, TypeDef},
Expand Down Expand Up @@ -44,28 +44,10 @@ impl Generator for VisitMutGenerator {
}
}

const CLIPPY_ALLOW: &str = "\
unused_variables,\
clippy::extra_unused_type_parameters,\
clippy::explicit_iter_loop,\
clippy::self_named_module_files,\
clippy::semicolon_if_nothing_returned,\
clippy::match_wildcard_for_single_variants";

fn generate_visit<const MUT: bool>(ctx: &LateCtx) -> TokenStream {
let header = generated_header!();
// we evaluate it outside of quote to take advantage of expression evaluation
// otherwise the `\n\` wouldn't work!
let file_docs = insert! {"\
//! Visitor Pattern\n\
//!\n\
//! See:\n\
//! * [visitor pattern](https://rust-unofficial.github.io/patterns/patterns/behavioural/visitor.html)\n\
//! * [rustc visitor](https://github.com/rust-lang/rust/blob/master/compiler/rustc_ast/src/visit.rs)\n\
"};

let (visits, walks) = VisitBuilder::new(ctx, MUT).build();
let clippy_attr = insert!("#![allow({})]", CLIPPY_ALLOW);

let walk_mod = if MUT { quote!(walk_mut) } else { quote!(walk) };
let trait_name = if MUT { quote!(VisitMut) } else { quote!(Visit) };
Expand All @@ -78,10 +60,9 @@ fn generate_visit<const MUT: bool>(ctx: &LateCtx) -> TokenStream {
quote! {
#[inline]
fn alloc<T>(&self, t: &T) -> &'a T {
insert!("// SAFETY:");
insert!("// This should be safe as long as `src` is an reference from the allocator.");
insert!("// But honestly, I'm not really sure if this is safe.");

///@ SAFETY:
///@ This should be safe as long as `src` is an reference from the allocator.
///@ But honestly, I'm not really sure if this is safe.
unsafe {
std::mem::transmute(t)
}
Expand All @@ -93,8 +74,21 @@ fn generate_visit<const MUT: bool>(ctx: &LateCtx) -> TokenStream {
quote! {
#header

#file_docs
#clippy_attr
//! Visitor Pattern
//!
//! See:
//! * [visitor pattern](https://rust-unofficial.github.io/patterns/patterns/behavioural/visitor.html)
//! * [rustc visitor](https://github.com/rust-lang/rust/blob/master/compiler/rustc_ast/src/visit.rs)
//!@@

#![allow(
unused_variables,
clippy::extra_unused_type_parameters,
clippy::explicit_iter_loop,
clippy::self_named_module_files,
clippy::semicolon_if_nothing_returned,
clippy::match_wildcard_for_single_variants
)]
endl!();

use std::cell::Cell;
Expand Down Expand Up @@ -483,13 +477,11 @@ impl<'a> VisitBuilder<'a> {
});

let node_events = if KIND_BLACK_LIST.contains(&ident.to_string().as_str()) {
(
insert!(
"// NOTE: {} doesn't exists!",
if self.is_mut { "AstType" } else { "AstKind" }
),
TokenStream::default(),
)
let comment = format!(
"@ NOTE: {} doesn't exists!",
if self.is_mut { "AstType" } else { "AstKind" }
);
(quote!(#![doc = #comment]), TokenStream::default())
} else {
let kind = self.kind_type(&ident);
(
Expand Down