attributes: support destructuring in arguments (#397)

This pull request adds support to the `#[instrument]` macro for
destructured arguments.

## Motivation

The `#[instrument]` macro automatically adds function arguments as
fields on the `Span`.  Previously, it would only do so for "plain"
`Ident` argument types, but would ignore destructured arguments.

## Solution

By recursively pattern matching each argument to find all identifiers it
creates, we can construct an authoritative list of all input names.

* attributes: extract param_names extractor

To enable the recursion needed to extract all the various
destructured parameter types.

* attributes: support destructured tuple arguments

* attributes: support destructured ref arguments

* attributes: support destructured tuple struct arguments

* attributes: support destructured struct arguments

* attributes: add crazy destructuring test

This is to test the combination of all the supported function
argument destructuring types.

* attributes: move destructuring tests to their own file

* attributes: apply cargo fmt

* attributes: add helpful comment to pattern pattern match

Since function signatures can only use irrefutable patterns,
we can be confident that no legal code will have any other
variant of `syn::Pat` than the ones we match on.  However, to
prevent anyone else from going down the rabbit hole of
wondering about it, we add a helpful comment.

* attributes: support self argument as span field

tracing-attributes/src/lib.rs

@@ -40,12 +40,14 @@
 extern crate proc_macro;
 
 use std::collections::HashSet;
+use std::iter;
 
 use proc_macro::TokenStream;
 use quote::{quote, quote_spanned, ToTokens};
 use syn::{
-    spanned::Spanned, AttributeArgs, FnArg, Ident, ItemFn, Lit, LitInt, Meta, MetaList,
-    MetaNameValue, NestedMeta, Pat, PatIdent, PatType, Signature,
+    spanned::Spanned, AttributeArgs, FieldPat, FnArg, Ident, ItemFn, Lit, LitInt, Meta, MetaList,
+    MetaNameValue, NestedMeta, Pat, PatIdent, PatReference, PatStruct, PatTuple, PatTupleStruct,
+    PatType, Signature,
 };
 
 /// Instruments a function to create and enter a `tracing` [span] every time
@@ -154,12 +156,9 @@ pub fn instrument(args: TokenStream, item: TokenStream) -> TokenStream {
     let param_names: Vec<Ident> = params
         .clone()
         .into_iter()
-        .filter_map(|param| match param {
-            FnArg::Typed(PatType { pat, .. }) => match *pat {
-                Pat::Ident(PatIdent { ident, .. }) => Some(ident),
-                _ => None,
-            },
-            _ => None,
+        .flat_map(|param| match param {
+            FnArg::Typed(PatType { pat, .. }) => param_names(*pat),
+            FnArg::Receiver(_) => Box::new(iter::once(Ident::new("self", param.span()))),
         })
         .filter(|ident| !skips.contains(ident))
         .collect();
@@ -212,6 +211,29 @@ pub fn instrument(args: TokenStream, item: TokenStream) -> TokenStream {
     .into()
 }
 
+fn param_names(pat: Pat) -> Box<dyn Iterator<Item = Ident>> {
+    match pat {
+        Pat::Ident(PatIdent { ident, .. }) => Box::new(iter::once(ident)),
+        Pat::Reference(PatReference { pat, .. }) => param_names(*pat),
+        Pat::Struct(PatStruct { fields, .. }) => Box::new(
+            fields
+                .into_iter()
+                .flat_map(|FieldPat { pat, .. }| param_names(*pat)),
+        ),
+        Pat::Tuple(PatTuple { elems, .. }) => Box::new(elems.into_iter().flat_map(param_names)),
+        Pat::TupleStruct(PatTupleStruct {
+            pat: PatTuple { elems, .. },
+            ..
+        }) => Box::new(elems.into_iter().flat_map(param_names)),
+
+        // The above *should* cover all cases of irrefutable patterns,
+        // but we purposefully don't do any funny business here
+        // (such as panicking) because that would obscure rustc's
+        // much more informative error message.
+        _ => Box::new(iter::empty()),
+    }
+}
+
 fn skips(args: &AttributeArgs) -> Result<HashSet<Ident>, impl ToTokens> {
     let mut skips = args.iter().filter_map(|arg| match arg {
         NestedMeta::Meta(Meta::List(MetaList {

tracing-attributes/tests/destructuring.rs

@@ -0,0 +1,215 @@
+mod support;
+use support::*;
+
+use tracing::subscriber::with_default;
+use tracing_attributes::instrument;
+
+#[test]
+fn destructure_tuples() {
+    #[instrument]
+    fn my_fn((arg1, arg2): (usize, usize)) {}
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone().with_field(
+                field::mock("arg1")
+                    .with_value(&format_args!("1"))
+                    .and(field::mock("arg2").with_value(&format_args!("2")))
+                    .only(),
+            ),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        my_fn((1, 2));
+    });
+
+    handle.assert_finished();
+}
+
+#[test]
+fn destructure_nested_tuples() {
+    #[instrument]
+    fn my_fn(((arg1, arg2), (arg3, arg4)): ((usize, usize), (usize, usize))) {}
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone().with_field(
+                field::mock("arg1")
+                    .with_value(&format_args!("1"))
+                    .and(field::mock("arg2").with_value(&format_args!("2")))
+                    .and(field::mock("arg3").with_value(&format_args!("3")))
+                    .and(field::mock("arg4").with_value(&format_args!("4")))
+                    .only(),
+            ),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        my_fn(((1, 2), (3, 4)));
+    });
+
+    handle.assert_finished();
+}
+
+#[test]
+fn destructure_refs() {
+    #[instrument]
+    fn my_fn(&arg1: &usize) {}
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone()
+                .with_field(field::mock("arg1").with_value(&format_args!("1")).only()),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        my_fn(&1);
+    });
+
+    handle.assert_finished();
+}
+
+#[test]
+fn destructure_tuple_structs() {
+    struct Foo(usize, usize);
+
+    #[instrument]
+    fn my_fn(Foo(arg1, arg2): Foo) {}
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone().with_field(
+                field::mock("arg1")
+                    .with_value(&format_args!("1"))
+                    .and(field::mock("arg2").with_value(&format_args!("2")))
+                    .only(),
+            ),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        my_fn(Foo(1, 2));
+    });
+
+    handle.assert_finished();
+}
+
+#[test]
+fn destructure_structs() {
+    struct Foo {
+        bar: usize,
+        baz: usize,
+    }
+
+    #[instrument]
+    fn my_fn(
+        Foo {
+            bar: arg1,
+            baz: arg2,
+        }: Foo,
+    ) {
+        let _ = (arg1, arg2);
+    }
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone().with_field(
+                field::mock("arg1")
+                    .with_value(&format_args!("1"))
+                    .and(field::mock("arg2").with_value(&format_args!("2")))
+                    .only(),
+            ),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        my_fn(Foo { bar: 1, baz: 2 });
+    });
+
+    handle.assert_finished();
+}
+
+#[test]
+fn destructure_everything() {
+    struct Foo {
+        bar: Bar,
+        baz: (usize, usize),
+        qux: NoDebug,
+    }
+    struct Bar((usize, usize));
+    struct NoDebug;
+
+    #[instrument]
+    fn my_fn(
+        &Foo {
+            bar: Bar((arg1, arg2)),
+            baz: (arg3, arg4),
+            ..
+        }: &Foo,
+    ) {
+        let _ = (arg1, arg2, arg3, arg4);
+    }
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone().with_field(
+                field::mock("arg1")
+                    .with_value(&format_args!("1"))
+                    .and(field::mock("arg2").with_value(&format_args!("2")))
+                    .and(field::mock("arg3").with_value(&format_args!("3")))
+                    .and(field::mock("arg4").with_value(&format_args!("4")))
+                    .only(),
+            ),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        let foo = Foo {
+            bar: Bar((1, 2)),
+            baz: (3, 4),
+            qux: NoDebug,
+        };
+        let _ = foo.qux; // to eliminate unused field warning
+        my_fn(&foo);
+    });
+
+    handle.assert_finished();
+}

tracing-attributes/tests/instrument.rs

@@ -166,3 +166,37 @@ fn generics() {
 
     handle.assert_finished();
 }
+
+#[test]
+fn methods() {
+    #[derive(Debug)]
+    struct Foo;
+
+    impl Foo {
+        #[instrument]
+        fn my_fn(&self, arg1: usize) {}
+    }
+
+    let span = span::mock().named("my_fn");
+
+    let (subscriber, handle) = subscriber::mock()
+        .new_span(
+            span.clone().with_field(
+                field::mock("self")
+                    .with_value(&format_args!("Foo"))
+                    .and(field::mock("arg1").with_value(&format_args!("42"))),
+            ),
+        )
+        .enter(span.clone())
+        .exit(span.clone())
+        .drop_span(span)
+        .done()
+        .run_with_handle();
+
+    with_default(subscriber, || {
+        let foo = Foo;
+        foo.my_fn(42);
+    });
+
+    handle.assert_finished();
+}