Repository: dtolnay/cxx (language: rust)

Issue:
Improve error on extern functions accidentally containing function body
If the user mistakenly types `{}` instead of `;` after a function signature out of muscle memory, they get a pretty poor error message.

```rust
#[cxx::bridge]
mod ffi {
    unsafe extern "C++" {
        fn f() {}
    }
}
```

```console
error[cxxbridge]: unsupported foreign item
  ┌─ src/main.rs:4:9
  │
4 │         fn f() {}
  │         ^^^^^^^^^ unsupported foreign item
```

The error message should instead point to the `{}` and say to replace with `;`.


Discussion / hints:

Fix patch:
diff --git a/syntax/parse.rs b/syntax/parse.rs
index 3a7e00f53..fbb24e287 100644
--- a/syntax/parse.rs
+++ b/syntax/parse.rs
@@ -16,9 +16,9 @@ use syn::punctuated::Punctuated;
 use syn::{
     Abi, Attribute, Error, Expr, Fields, FnArg, ForeignItem, ForeignItemFn, ForeignItemType,
     GenericArgument, GenericParam, Generics, Ident, ItemEnum, ItemImpl, ItemStruct, Lit, LitStr,
-    Pat, PathArguments, Result, ReturnType, Token, TraitBound, TraitBoundModifier,
-    Type as RustType, TypeArray, TypeBareFn, TypeParamBound, TypePath, TypeReference,
-    Variant as RustVariant, Visibility,
+    Pat, PathArguments, Result, ReturnType, Signature as RustSignature, Token, TraitBound,
+    TraitBoundModifier, Type as RustType, TypeArray, TypeBareFn, TypeParamBound, TypePath,
+    TypeReference, Variant as RustVariant, Visibility,
 };
 
 pub mod kw {
@@ -662,69 +662,90 @@ fn parse_extern_verbatim(
     |input: ParseStream| -> Result<Api> {
         let attrs = input.call(Attribute::parse_outer)?;
         let visibility: Visibility = input.parse()?;
-        let type_token: Token![type] = match input.parse()? {
-            Some(type_token) => type_token,
-            None => {
-                let span = input.cursor().token_stream();
-                return Err(Error::new_spanned(span, "unsupported foreign item"));
-            }
-        };
-        let ident: Ident = input.parse()?;
-        let generics: Generics = input.parse()?;
-        let mut lifetimes = Punctuated::new();
-        let mut has_unsupported_generic_param = false;
-        for pair in generics.params.into_pairs() {
-            let (param, punct) = pair.into_tuple();
-            match param {
-                GenericParam::Lifetime(param) => {
-                    if !param.bounds.is_empty() && !has_unsupported_generic_param {
-                        let msg = "lifetime parameter with bounds is not supported yet";
-                        cx.error(&param, msg);
-                        has_unsupported_generic_param = true;
-                    }
-                    lifetimes.push_value(param.lifetime);
-                    if let Some(punct) = punct {
-                        lifetimes.push_punct(punct);
-                    }
+        if input.peek(Token![type]) {
+            parse_extern_verbatim_type(cx, attrs, visibility, input, lang, trusted, namespace)
+        } else if input.peek(Token![fn]) {
+            parse_extern_verbatim_fn(input)
+        } else {
+            let span = input.cursor().token_stream();
+            Err(Error::new_spanned(
+                span,
+                "unsupported foreign item, expected `type` or `fn`",
+            ))
+        }
+    }
+    .parse2(tokens)
+}
+
+fn parse_extern_verbatim_type(
+    cx: &mut Errors,
+    attrs: Vec<Attribute>,
+    visibility: Visibility,
+    input: ParseStream,
+    lang: Lang,
+    trusted: bool,
+    namespace: &Namespace,
+) -> Result<Api> {
+    let type_token: Token![type] = input.parse()?;
+    let ident: Ident = input.parse()?;
+    let generics: Generics = input.parse()?;
+    let mut lifetimes = Punctuated::new();
+    let mut has_unsupported_generic_param = false;
+    for pair in generics.params.into_pairs() {
+        let (param, punct) = pair.into_tuple();
+        match param {
+            GenericParam::Lifetime(param) => {
+                if !param.bounds.is_empty() && !has_unsupported_generic_param {
+                    let msg = "lifetime parameter with bounds is not supported yet";
+                    cx.error(&param, msg);
+                    has_unsupported_generic_param = true;
                 }
-                GenericParam::Type(param) => {
-                    if !has_unsupported_generic_param {
-                        let msg = "extern type with generic type parameter is not supported yet";
-                        cx.error(&param, msg);
-                        has_unsupported_generic_param = true;
-                    }
+                lifetimes.push_value(param.lifetime);
+                if let Some(punct) = punct {
+                    lifetimes.push_punct(punct);
                 }
-                GenericParam::Const(param) => {
-                    if !has_unsupported_generic_param {
-                        let msg = "extern type with const generic parameter is not supported yet";
-                        cx.error(&param, msg);
-                        has_unsupported_generic_param = true;
-                    }
+            }
+            GenericParam::Type(param) => {
+                if !has_unsupported_generic_param {
+                    let msg = "extern type with generic type parameter is not supported yet";
+                    cx.error(&param, msg);
+                    has_unsupported_generic_param = true;
+                }
+            }
+            GenericParam::Const(param) => {
+                if !has_unsupported_generic_param {
+                    let msg = "extern type with const generic parameter is not supported yet";
+                    cx.error(&param, msg);
+                    has_unsupported_generic_param = true;
                 }
             }
-        }
-        let lifetimes = Lifetimes {
-            lt_token: generics.lt_token,
-            lifetimes,
-            gt_token: generics.gt_token,
-        };
-        let lookahead = input.lookahead1();
-        if lookahead.peek(Token![=]) {
-            // type Alias = crate::path::to::Type;
-            parse_type_alias(
-                cx, attrs, visibility, type_token, ident, lifetimes, input, lang, namespace,
-            )
-        } else if lookahead.peek(Token![:]) || lookahead.peek(Token![;]) {
-            // type Opaque: Bound2 + Bound2;
-            parse_extern_type_bounded(
-                cx, attrs, visibility, type_token, ident, lifetimes, input, lang, trusted,
-                namespace,
-            )
-        } else {
-            Err(lookahead.error())
         }
     }
-    .parse2(tokens)
+    let lifetimes = Lifetimes {
+        lt_token: generics.lt_token,
+        lifetimes,
+        gt_token: generics.gt_token,
+    };
+    let lookahead = input.lookahead1();
+    if lookahead.peek(Token![=]) {
+        // type Alias = crate::path::to::Type;
+        parse_type_alias(
+            cx, attrs, visibility, type_token, ident, lifetimes, input, lang, namespace,
+        )
+    } else if lookahead.peek(Token![:]) || lookahead.peek(Token![;]) {
+        // type Opaque: Bound2 + Bound2;
+        parse_extern_type_bounded(
+            cx, attrs, visibility, type_token, ident, lifetimes, input, lang, trusted, namespace,
+        )
+    } else {
+        Err(lookahead.error())
+    }
+}
+
+fn parse_extern_verbatim_fn(input: ParseStream) -> Result<Api> {
+    input.parse::<RustSignature>()?;
+    input.parse::<Token![;]>()?;
+    unreachable!()
 }
 
 fn parse_type_alias(


Test patch:
diff --git a/tests/ui/function_with_body.rs b/tests/ui/function_with_body.rs
new file mode 100644
index 000000000..52be79af0
--- /dev/null
+++ b/tests/ui/function_with_body.rs
@@ -0,0 +1,8 @@
+#[cxx::bridge]
+mod ffi {
+    unsafe extern "C++" {
+        fn f() {}
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/function_with_body.stderr b/tests/ui/function_with_body.stderr
new file mode 100644
index 000000000..c5641db8f
--- /dev/null
+++ b/tests/ui/function_with_body.stderr
@@ -0,0 +1,5 @@
+error: expected `;`
+ --> $DIR/function_with_body.rs:4:16
+  |
+4 |         fn f() {}
+  |                ^