add new lint [unsafe_block_in_proc_macro]

that detects unsafe block in `quote!`

Signed-off-by: J-ZhengLi <[email protected]>

Author: J-ZhengLi

CHANGELOG.md

@@ -4636,7 +4636,6 @@ Released 2018-09-13
 [`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
 [`large_futures`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_futures
 [`large_include_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_include_file
-[`large_memory_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_memory_allocation
 [`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays
 [`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value
 [`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
@@ -5007,6 +5006,7 @@ Released 2018-09-13
 [`unnested_or_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns
 [`unreachable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreachable
 [`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal
+[`unsafe_block_in_proc_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_block_in_proc_macro
 [`unsafe_derive_deserialize`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_derive_deserialize
 [`unsafe_removed_from_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_removed_from_name
 [`unsafe_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_vector_initialization

clippy_lints/src/declared_lints.rs

@@ -632,6 +632,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
     crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
     crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,
+    crate::unsafe_block_in_proc_macro::UNSAFE_BLOCK_IN_PROC_MACRO_INFO,
     crate::unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME_INFO,
     crate::unused_async::UNUSED_ASYNC_INFO,
     crate::unused_io_amount::UNUSED_IO_AMOUNT_INFO,

clippy_lints/src/lib.rs

@@ -310,6 +310,7 @@ mod unnecessary_self_imports;
 mod unnecessary_struct_initialization;
 mod unnecessary_wraps;
 mod unnested_or_patterns;
+mod unsafe_block_in_proc_macro;
 mod unsafe_removed_from_name;
 mod unused_async;
 mod unused_io_amount;
@@ -962,6 +963,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments));
     let mem_unsafe_functions = conf.mem_unsafe_functions.clone();
     store.register_late_pass(move |_| Box::new(guidelines::GuidelineLints::new(mem_unsafe_functions.clone())));
+    store.register_late_pass(|_| Box::new(unsafe_block_in_proc_macro::UnsafeBlockInProcMacro::new()));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/unsafe_block_in_proc_macro.rs

@@ -0,0 +1,124 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::source::snippet_opt;
+use if_chain::if_chain;
+use rustc_data_structures::fx::FxHashSet;
+use rustc_hir::Expr;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::hygiene::{ExpnKind, MacroKind};
+use rustc_span::{sym, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for unsafe block written in procedural macro
+    ///
+    /// ### Why is this bad?
+    /// It hides the unsafe code, making the safety of expended code unsound.
+    ///
+    /// ### Known problems
+    /// Possible FP when the user uses proc-macro to generate a function with unsafe block in it.
+    ///
+    /// ### Example
+    /// ```rust
+    /// #[proc_macro]
+    /// pub fn rprintf(input: TokenStream) -> TokenStream {
+    ///     let expr = parse_macro_input!(input as syn::Expr);
+    ///     quote!({
+    ///         unsafe {
+    ///             // unsafe operation
+    ///         }
+    ///     })
+    /// }
+    ///
+    /// // This allows users to use this macro without `unsafe` block
+    /// rprintf!();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// #[proc_macro]
+    /// pub fn rprintf(input: TokenStream) -> TokenStream {
+    ///     let expr = parse_macro_input!(input as syn::Expr);
+    ///     quote!({
+    ///         // unsafe operation
+    ///     })
+    /// }
+    ///
+    /// // When using this macro, an outer `unsafe` block is needed,
+    /// // making the safety of this macro much clearer.
+    /// unsafe { rprintf!(); }
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub UNSAFE_BLOCK_IN_PROC_MACRO,
+    nursery,
+    "using unsafe block in procedural macro's definition"
+}
+
+#[derive(Clone)]
+pub struct UnsafeBlockInProcMacro {
+    call_sites: FxHashSet<Span>,
+}
+
+impl UnsafeBlockInProcMacro {
+    pub fn new() -> Self {
+        Self {
+            call_sites: FxHashSet::default(),
+        }
+    }
+}
+
+impl_lint_pass!(UnsafeBlockInProcMacro => [UNSAFE_BLOCK_IN_PROC_MACRO]);
+
+impl LateLintPass<'_> for UnsafeBlockInProcMacro {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        let expn_data = expr.span.ctxt().outer_expn_data();
+        let call_site = expn_data.call_site;
+
+        if_chain! {
+            if !call_site.from_expansion();
+            if self.call_sites.insert(call_site);
+            if let ExpnKind::Macro(MacroKind::Bang, symbol) = expn_data.kind;
+            if symbol == sym::quote;
+            if let Some(code_snip) = quote_inner(cx, call_site);
+            let could_be_fn = code_snip.contains("fn ");
+            if contains_unsafe_block(&code_snip);
+            then {
+                emit_lint_message(cx, expr, call_site, could_be_fn);
+            }
+        }
+    }
+}
+
+/// Get the code snippet inside of `quote!()`
+fn quote_inner(cx: &LateContext<'_>, call_site: Span) -> Option<String> {
+    let quote_snip = snippet_opt(cx, call_site)?;
+    let (_, snip) = quote_snip.split_once('!')?;
+    let mut chars = snip.trim().chars();
+    chars.next();
+    chars.next_back();
+    Some(chars.collect())
+}
+
+fn contains_unsafe_block(code: &str) -> bool {
+    // unify input by eliminating all whitespaces
+    let without_ws: String = code.split_whitespace().collect();
+    without_ws.contains("unsafe{")
+}
+
+fn emit_lint_message(cx: &LateContext<'_>, expr: &Expr<'_>, call_site: Span, could_be_fn: bool) {
+    let parent_hid = cx.tcx.hir().parent_id(expr.hir_id);
+    let name = cx.tcx.item_name(parent_hid.owner.to_def_id());
+    let msg = &format!("procedural macro `{name}` may hiding unsafe operations");
+    let extra_sugg = if could_be_fn {
+        "if the block was in a function, \
+        put an `unsafe` keyword on its definition: `unsafe fn ...`, \n\
+        otherwise"
+    } else {
+        "and"
+    };
+    let help = &format!(
+        "consider removing unsafe block from the above `quote!` call,\n\
+        {extra_sugg} add unsafe block when calling the macro instead: `unsafe {{ {name}!{{...}} }}`"
+    );
+
+    span_lint_and_help(cx, UNSAFE_BLOCK_IN_PROC_MACRO, call_site, msg, None, help);
+}

tests/ui/unsafe_block_in_proc_macro.rs

@@ -0,0 +1,167 @@
+#![warn(clippy::unsafe_block_in_proc_macro)]
+#![allow(clippy::needless_return)]
+
+extern crate proc_macro;
+extern crate quote;
+
+use proc_macro::TokenStream;
+use quote::quote;
+
+fn do_something() {}
+fn main() {}
+
+// Ideally we can add `#[proc_macro]` attr on these following functions,
+// and check only such functions instead, but I don't know how to write such test
+// without putting `proc_macro = true` in Cargo.toml, maybe someone else can fix it.
+
+pub fn unsafe_print_foo(_: TokenStream) -> TokenStream {
+    quote!({
+        unsafe {
+            println!("foo");
+        }
+    })
+    .into()
+}
+
+#[rustfmt::skip]
+pub fn unsafe_print_bar_unformatted(_: TokenStream) -> TokenStream {
+    quote!({
+        unsafe
+        
+        
+        { println!("bar"); }
+    }).into()
+}
+
+#[rustfmt::skip]
+pub fn unsafe_print_bar_unformatted_1(_: TokenStream) -> TokenStream {
+    quote!({
+        unsafe{println!("bar");}
+    }).into()
+}
+
+pub fn unsafe_print_baz(_: TokenStream) -> TokenStream {
+    do_something();
+
+    quote!({
+        let _ = 0_u8;
+        do_something();
+        unsafe {
+            println!("baz");
+        }
+
+        do_something();
+    })
+    .into()
+}
+
+pub fn maybe_unsafe_print(_: TokenStream) -> TokenStream {
+    do_something();
+
+    if false {
+        return quote!({
+            unsafe {
+                println!("unsafe");
+            }
+        })
+        .into();
+    }
+
+    quote!({
+        println!("safe");
+    })
+    .into()
+}
+
+pub fn maybe_unsafe_print_1(_: TokenStream) -> TokenStream {
+    let condition = 1;
+    if condition > 0 {
+        return quote!({
+            println!("safe");
+        })
+        .into();
+    }
+
+    quote!({
+        unsafe {
+            println!("unsafe");
+        }
+    })
+    .into()
+}
+
+pub fn maybe_unsafe_print_2(_: TokenStream) -> TokenStream {
+    let condition = 1;
+    if condition == 0 {
+        return quote!({}).into();
+    } else if condition == 1 {
+        return quote!(unsafe {}).into();
+    } else if condition == 2 {
+        return quote!({ println!("2") }).into();
+    }
+    do_something();
+    quote!({}).into()
+}
+
+pub fn multiple_unsafe(_: TokenStream) -> TokenStream {
+    let condition = 1;
+    match condition {
+        1 => quote!(unsafe {
+            println!("1");
+        }),
+        2 => quote!(unsafe {
+            println!("2");
+        }),
+        _ => quote!(unsafe {}),
+    }
+    .into()
+}
+
+pub fn unsafe_block_in_function(_: TokenStream) -> TokenStream {
+    quote!({
+        fn print_foo() {
+            unsafe {
+                println!("foo");
+            }
+        }
+    })
+    .into()
+}
+
+// Don't lint
+pub fn print_foo(_: TokenStream) -> TokenStream {
+    quote!({
+        println!("foo");
+    })
+    .into()
+}
+
+// Don't lint
+pub fn unsafe_trait(_: TokenStream) -> TokenStream {
+    quote!({
+        unsafe trait Foo {
+            fn aaa();
+            fn bbb();
+        }
+    })
+    .into()
+}
+
+// Don't lint
+pub fn unsafe_fn(_: TokenStream) -> TokenStream {
+    quote!({
+        unsafe fn uns_foo() {}
+    })
+    .into()
+}
+
+// Don't lint
+pub fn unsafe_impl_fn(_: TokenStream) -> TokenStream {
+    quote!({
+        struct T;
+        impl T {
+            unsafe fn foo() {}
+        }
+    })
+    .into()
+}