Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

relm4_macros::view! triggers clippy::used_underscore_binding lint. #532

Closed
PJungkamp opened this issue Sep 15, 2023 · 2 comments · Fixed by #533
Closed

relm4_macros::view! triggers clippy::used_underscore_binding lint. #532

PJungkamp opened this issue Sep 15, 2023 · 2 comments · Fixed by #533

Comments

@PJungkamp
Copy link
Contributor

My use case

I'm using the relm4_macros::view macro standalone for convenience when working with gtk4 or libadwaita. I noticed that clippy generates lints on pretty much every view! invocation. This is probably due to Span::call_site() invocations that should really be Span::mixed_site().

Example

use relm4_macros::view;
use gtk4 as gtk;

fn main() {
  view! {
    application = gtk::Application {
      set_application_id: ("some.application.id"),
      connect_activate: build_ui,
    },
}

fn build_ui(app: &gtk::Application) {
    main_window = gtk::ApplicationWindow {
      set_application: Some(application),

      // this label here is the culprit
      #[wrap(Some)]
      set_content = &gtk::Label {
        set_text: "test",
      },
    }
  }
}

The unnamed gtk::Label here will get created under a name like _gtk_label_0.
This name has a call_site span and is thus recognized by clippy.

Workaround

Add #[allow(clippy::used_underscore_binding)] at the macro call sites or at the crate root.

Resolution

I haven't quite determined the code in relm4-macros/widgets that is responsible for giving the generated identifiers their scope, but just from looking at it at first glance, most Idents are created using Span::call_site, even if the name has been generated internally. For good macro hygiene the generated ones should probably use mixed_site hygiene. At least the _[PATH]_[NUMBER] style identifiers should really have mixed_site hygiene.

Changing the public symbols generated by the view! macro would by a subtle but breaking change.
Annotating all generated let expressions with #[allow(clippy::used_underscore_binding)] would at least fix the strange lint the user can't do anything about.

Notes

I want to add that I really love the approach taken by the view! macro. It helps readability and reduces boilerplate to a degree that makes the lack of automatic formatting pretty irrelevant. Thank you so much for imagining a better GTK development experience!

@PJungkamp PJungkamp mentioned this issue Sep 15, 2023
4 tasks
@AaronErhardt
Copy link
Member

Thanks for the report!
I wasn't really aware of the differences between call site and mixed site so far despite writing thousands of lines of proc-macro code. Should be relatively easy to fix, probably most Ident::new calls simply have to be adjusted.

@PJungkamp
Copy link
Contributor Author

I've checked most invocations of quote! and quote_spanned! and the macro hygiene isn't to great in some places. Changing the span of most idents, I've found several places where identifiers such as __current_page and similar are created or used in an invocation of quote_spanned!. This makes those identifiers assume the provided span which in some places is a call site span. These kind of identifiers should really be created outside of the quote_spanned invocation and be given a mixed site span.

Rust macros really shouldn't need the C-style underscore prefixes, proper use of Span should take care of name conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants