easy-ext icon indicating copy to clipboard operation
easy-ext copied to clipboard

Support trait or implementation-only attributtes like `tracing::instrument`

Open Veetaha opened this issue 2 years ago • 2 comments

Context

When I put tracing::instrument on the method in the extension trait I get a compile error

use tracing::instrument;
use easy_ext::ext;

#[ext]
impl String {
    #[instrument(skip_all)]
    fn foo(&self) -> u32 {
        42
    }
}

Compile error:

error[E0308]: mismatched types
 --> src/lib.rs:5:5
  |
5 |     #[instrument(skip_all)]
  |     ^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `()`
  |
  = note: this error originates in the attribute macro `instrument` (in Nightly builds, run with -Z macro-backtrace for more info)

This is because #[instrument] must not be used on the trait method declaration. The same compile error is reproduced with this code:

use tracing::instrument;

trait StringExt {
    #[instrument(skip_all)]
    fn foo(&self) -> u32;
}

Summary

I propose to make it possible to specify with attributes to put on the trait declaration and on trait implementation. We may leave the default behavior of duplicating the attributes both on the trait declaration and implementation. This won't be a breaking change.

It could look like this. for example, in the most verbose form, with optional configs trait_only/impl_only:

#[ext(
    TraitName,
    trait_only(trait_attr_foo, trait_attr_bar),
    impl_only(impl_attr_foo, impl_attr_bar)
)]
impl String {
    #[ext(
        impl_only(instrument(skip_all), impl_attr_baz, impl_attr_bruh),
        trait_only(trait_attr_baz, trait_attr_bruh)
    )]
    fn foo(&self) -> u32 {
        42
    }
}

This will generate:

#[trait_attr_foo]
#[trait_attr_bar]
trait TraitName {
    #[trait_attr_baz]
    #[trait_attr_bruh]
    fn foo(&self) -> u32;
)

#[impl_attr_foo]
#[impl_attr_bar]
impl TraitName for String {
    #[instrument(skip_all)]
    #[impl_attr_baz]
    #[impl_attr_bruh]
    fn foo(&self) -> u32 {
        42
    }
}

Veetaha avatar Feb 03 '23 13:02 Veetaha

I would like to accept a PR to add such options (trait_only/impl_only). (As for the option names, *_only is not much clear what it means, *_attr would be better)

taiki-e avatar Feb 05 '23 04:02 taiki-e

I just ran into the same issue and have opened pull request #43.

The syntax implemented in the PR is a bit different. It uses separate attributes to designate trait-only and impl-only classification. The advantage of this approach is easier attribute configuration and debugging--e.g. with complex instrument configs--since the attribute isn't nested within an ext block. The disadvantage is it's more verbose when specifying multiple trait/impl-only attrs.

#[ext(TraitName)]
impl String {
    #[impl_attr]
    #[instrument(skip_all)]
    fn foo(&self) -> u32 {
        42
    }
}

mbazero avatar May 31 '24 08:05 mbazero