Auto merge of #10369 - nindalf:no_mangle_lint, r=llogiq

Add new lint no_mangle_with_rust_abi

Fixes issue #10347

This PR adds a new lint `no_mangle_with_rust_abi` that suggests converting a function to the C ABI to if the function has the `#[no_mangle]` attribute and `Abi == Abi::Rust`. It will not run for any of the other variants defined in [rustc_target::spec::abi::Abi](https://doc.rust-lang.org/beta/nightly-rustc/rustc_target/spec/abi/enum.Abi.html), nor suggest any conversion other than conversion to the C ABI.

Functions that explicitly opt into the Rust ABI with `extern "Rust"` are ignored by this lint.

---

changelog: [`no_mangle_with_rust_abi`]: add lint that converts Rust ABI functions with the `#[no_mangle]` attribute to C ABI

CHANGELOG.md

@@ -4622,6 +4622,7 @@ Released 2018-09-13
 [`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect
 [`no_effect_replace`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_replace
 [`no_effect_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding
+[`no_mangle_with_rust_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
 [`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
 [`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
 [`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty

clippy_lints/src/declared_lints.rs

@@ -450,6 +450,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::no_effect::NO_EFFECT_INFO,
     crate::no_effect::NO_EFFECT_UNDERSCORE_BINDING_INFO,
     crate::no_effect::UNNECESSARY_OPERATION_INFO,
+    crate::no_mangle_with_rust_abi::NO_MANGLE_WITH_RUST_ABI_INFO,
     crate::non_copy_const::BORROW_INTERIOR_MUTABLE_CONST_INFO,
     crate::non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST_INFO,
     crate::non_expressive_names::JUST_UNDERSCORES_AND_DIGITS_INFO,

clippy_lints/src/lib.rs

@@ -219,6 +219,7 @@ mod neg_cmp_op_on_partial_ord;
 mod neg_multiply;
 mod new_without_default;
 mod no_effect;
+mod no_mangle_with_rust_abi;
 mod non_copy_const;
 mod non_expressive_names;
 mod non_octal_unix_permissions;
@@ -921,6 +922,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
             avoid_breaking_exported_api,
         ))
     });
+    store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/no_mangle_with_rust_abi.rs

@@ -0,0 +1,65 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::{Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_target::spec::abi::Abi;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for Rust ABI functions with the `#[no_mangle]` attribute.
+    ///
+    /// ### Why is this bad?
+    /// The Rust ABI is not stable, but in many simple cases matches
+    /// enough with the C ABI that it is possible to forget to add
+    /// `extern "C"` to a function called from C. Changes to the
+    /// Rust ABI can break this at any point.
+    ///
+    /// ### Example
+    /// ```rust
+    ///  #[no_mangle]
+    ///  fn example(arg_one: u32, arg_two: usize) {}
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    ///  #[no_mangle]
+    ///  extern "C" fn example(arg_one: u32, arg_two: usize) {}
+    /// ```
+    #[clippy::version = "1.69.0"]
+    pub NO_MANGLE_WITH_RUST_ABI,
+    pedantic,
+    "convert Rust ABI functions to C ABI"
+}
+declare_lint_pass!(NoMangleWithRustAbi => [NO_MANGLE_WITH_RUST_ABI]);
+
+impl<'tcx> LateLintPass<'tcx> for NoMangleWithRustAbi {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if let ItemKind::Fn(fn_sig, _, _) = &item.kind {
+            let attrs = cx.tcx.hir().attrs(item.hir_id());
+            let mut applicability = Applicability::MachineApplicable;
+            let snippet = snippet_with_applicability(cx, fn_sig.span, "..", &mut applicability);
+            for attr in attrs {
+                if let Some(ident) = attr.ident()
+                    && ident.name == rustc_span::sym::no_mangle
+                    && fn_sig.header.abi == Abi::Rust
+                    && !snippet.contains("extern") {
+
+                    let suggestion = snippet.split_once("fn")
+                        .map_or(String::new(), |(first, second)| format!(r#"{first}extern "C" fn{second}"#));
+
+                    span_lint_and_sugg(
+                        cx,
+                        NO_MANGLE_WITH_RUST_ABI,
+                        fn_sig.span,
+                        "attribute #[no_mangle] set on a Rust ABI function",
+                        "try",
+                        suggestion,
+                        applicability
+                    );
+                }
+            }
+        }
+    }
+}

tests/ui/must_use_candidates.fixed

@@ -84,7 +84,7 @@ pub unsafe fn mutates_static() -> usize {
 }
 
 #[no_mangle]
-pub fn unmangled(i: bool) -> bool {
+pub extern "C" fn unmangled(i: bool) -> bool {
     !i
 }
 

tests/ui/must_use_candidates.rs

@@ -84,7 +84,7 @@ pub unsafe fn mutates_static() -> usize {
 }
 
 #[no_mangle]
-pub fn unmangled(i: bool) -> bool {
+pub extern "C" fn unmangled(i: bool) -> bool {
     !i
 }
 

tests/ui/no_mangle_with_rust_abi.fixed

@@ -0,0 +1,48 @@
+// run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::no_mangle_with_rust_abi)]
+
+#[no_mangle]
+extern "C" fn rust_abi_fn_one(arg_one: u32, arg_two: usize) {}
+
+#[no_mangle]
+pub extern "C" fn rust_abi_fn_two(arg_one: u32, arg_two: usize) {}
+
+/// # Safety
+/// This function shouldn't be called unless the horsemen are ready
+#[no_mangle]
+pub unsafe extern "C" fn rust_abi_fn_three(arg_one: u32, arg_two: usize) {}
+
+/// # Safety
+/// This function shouldn't be called unless the horsemen are ready
+#[no_mangle]
+unsafe extern "C" fn rust_abi_fn_four(arg_one: u32, arg_two: usize) {}
+
+#[no_mangle]
+extern "C" fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
+    arg_one: u32,
+    arg_two: usize,
+) -> u32 {
+    0
+}
+
+// Must not run on functions that explicitly opt in to Rust ABI with `extern "Rust"`
+#[no_mangle]
+#[rustfmt::skip]
+extern "Rust" fn rust_abi_fn_explicit_opt_in(arg_one: u32, arg_two: usize) {}
+
+fn rust_abi_fn_again(arg_one: u32, arg_two: usize) {}
+
+#[no_mangle]
+extern "C" fn c_abi_fn(arg_one: u32, arg_two: usize) {}
+
+extern "C" fn c_abi_fn_again(arg_one: u32, arg_two: usize) {}
+
+extern "C" {
+    fn c_abi_in_block(arg_one: u32, arg_two: usize);
+}
+
+fn main() {
+    // test code goes here
+}

tests/ui/no_mangle_with_rust_abi.rs

@@ -0,0 +1,48 @@
+// run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::no_mangle_with_rust_abi)]
+
+#[no_mangle]
+fn rust_abi_fn_one(arg_one: u32, arg_two: usize) {}
+
+#[no_mangle]
+pub fn rust_abi_fn_two(arg_one: u32, arg_two: usize) {}
+
+/// # Safety
+/// This function shouldn't be called unless the horsemen are ready
+#[no_mangle]
+pub unsafe fn rust_abi_fn_three(arg_one: u32, arg_two: usize) {}
+
+/// # Safety
+/// This function shouldn't be called unless the horsemen are ready
+#[no_mangle]
+unsafe fn rust_abi_fn_four(arg_one: u32, arg_two: usize) {}
+
+#[no_mangle]
+fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
+    arg_one: u32,
+    arg_two: usize,
+) -> u32 {
+    0
+}
+
+// Must not run on functions that explicitly opt in to Rust ABI with `extern "Rust"`
+#[no_mangle]
+#[rustfmt::skip]
+extern "Rust" fn rust_abi_fn_explicit_opt_in(arg_one: u32, arg_two: usize) {}
+
+fn rust_abi_fn_again(arg_one: u32, arg_two: usize) {}
+
+#[no_mangle]
+extern "C" fn c_abi_fn(arg_one: u32, arg_two: usize) {}
+
+extern "C" fn c_abi_fn_again(arg_one: u32, arg_two: usize) {}
+
+extern "C" {
+    fn c_abi_in_block(arg_one: u32, arg_two: usize);
+}
+
+fn main() {
+    // test code goes here
+}

tests/ui/no_mangle_with_rust_abi.stderr

@@ -0,0 +1,45 @@
+error: attribute #[no_mangle] set on a Rust ABI function
+  --> $DIR/no_mangle_with_rust_abi.rs:7:1
+   |
+LL | fn rust_abi_fn_one(arg_one: u32, arg_two: usize) {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `extern "C" fn rust_abi_fn_one(arg_one: u32, arg_two: usize)`
+   |
+   = note: `-D clippy::no-mangle-with-rust-abi` implied by `-D warnings`
+
+error: attribute #[no_mangle] set on a Rust ABI function
+  --> $DIR/no_mangle_with_rust_abi.rs:10:1
+   |
+LL | pub fn rust_abi_fn_two(arg_one: u32, arg_two: usize) {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `pub extern "C" fn rust_abi_fn_two(arg_one: u32, arg_two: usize)`
+
+error: attribute #[no_mangle] set on a Rust ABI function
+  --> $DIR/no_mangle_with_rust_abi.rs:15:1
+   |
+LL | pub unsafe fn rust_abi_fn_three(arg_one: u32, arg_two: usize) {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `pub unsafe extern "C" fn rust_abi_fn_three(arg_one: u32, arg_two: usize)`
+
+error: attribute #[no_mangle] set on a Rust ABI function
+  --> $DIR/no_mangle_with_rust_abi.rs:20:1
+   |
+LL | unsafe fn rust_abi_fn_four(arg_one: u32, arg_two: usize) {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe extern "C" fn rust_abi_fn_four(arg_one: u32, arg_two: usize)`
+
+error: attribute #[no_mangle] set on a Rust ABI function
+  --> $DIR/no_mangle_with_rust_abi.rs:23:1
+   |
+LL | / fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
+LL | |     arg_one: u32,
+LL | |     arg_two: usize,
+LL | | ) -> u32 {
+   | |________^
+   |
+help: try
+   |
+LL + extern "C" fn rust_abi_multiline_function_really_long_name_to_overflow_args_to_multiple_lines(
+LL +     arg_one: u32,
+LL +     arg_two: usize,
+LL ~ ) -> u32 {
+   |
+
+error: aborting due to 5 previous errors
+