From 7713818dde5f209943bfa136b46bdcd1e82981c3 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 9 Aug 2024 06:25:26 +0100 Subject: [PATCH] refactor(ast_codegen): use doc comments instead of `insert!` --- tasks/ast_codegen/src/fmt.rs | 44 ++++++++++++++- .../ast_codegen/src/generators/ast_builder.rs | 6 +- .../src/generators/derive_get_span.rs | 2 +- tasks/ast_codegen/src/generators/mod.rs | 11 ++-- tasks/ast_codegen/src/generators/visit.rs | 56 ++++++++----------- 5 files changed, 79 insertions(+), 40 deletions(-) diff --git a/tasks/ast_codegen/src/fmt.rs b/tasks/ast_codegen/src/fmt.rs index 031d277d01604..cb9ef67de9916 100644 --- a/tasks/ast_codegen/src/fmt.rs +++ b/tasks/ast_codegen/src/fmt.rs @@ -15,6 +15,7 @@ static WHITE_SPACE: &str = " "; /// /// 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 { @@ -65,11 +66,52 @@ lazy_static! { Regex::new(format!(r"{WHITE_SPACE}*{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_SPACE}*//[/!]@(.*)").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 } diff --git a/tasks/ast_codegen/src/generators/ast_builder.rs b/tasks/ast_codegen/src/generators/ast_builder.rs index b18f738696ff2..f18a3f0d50d3e 100644 --- a/tasks/ast_codegen/src/generators/ast_builder.rs +++ b/tasks/ast_codegen/src/generators/ast_builder.rs @@ -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}; diff --git a/tasks/ast_codegen/src/generators/derive_get_span.rs b/tasks/ast_codegen/src/generators/derive_get_span.rs index 12e6ebd0c5276..1e20f3f82c200 100644 --- a/tasks/ast_codegen/src/generators/derive_get_span.rs +++ b/tasks/ast_codegen/src/generators/derive_get_span.rs @@ -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; diff --git a/tasks/ast_codegen/src/generators/mod.rs b/tasks/ast_codegen/src/generators/mod.rs index 55070d68894df..58e0a587317bd 100644 --- a/tasks/ast_codegen/src/generators/mod.rs +++ b/tasks/ast_codegen/src/generators/mod.rs @@ -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::().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] + //!@@ } }}; } diff --git a/tasks/ast_codegen/src/generators/visit.rs b/tasks/ast_codegen/src/generators/visit.rs index fe9856a18d69e..77b4bb977df81 100644 --- a/tasks/ast_codegen/src/generators/visit.rs +++ b/tasks/ast_codegen/src/generators/visit.rs @@ -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}, @@ -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(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) }; @@ -78,10 +60,9 @@ fn generate_visit(ctx: &LateCtx) -> TokenStream { quote! { #[inline] fn alloc(&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) } @@ -93,8 +74,21 @@ fn generate_visit(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; @@ -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); (