search

salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.14k stars 152 forks source link

Assorted macro/rust-analyzer woes #586

Open davidbarsky opened 1 month ago

davidbarsky commented 1 month ago

I noticed a few issues with Salsa's macro that impact the accuracy of rust-analyzer's semantic highlighting. While the syntax theme that I'm using has support for semantic highlighting (so there's more bandwidth for the syntax highlighting to look slightly off), I confirmed the weirdness using a VS Code command called "Developer: Inspect Editor Tokens and Scopes" to determine just exactly how slightly off the different tokens are. Using tests/is_send_sync.rs as an example:

  1. The struct keyword in struct MyInput, struct MyTracked<'db>, and struct MyInterned<'db> is reported as function, not a keyword. a. The lifetimes in the tracked and interned structs are reported as functions, not lifetimes. b. the field named field on MyInput is reported to be variable, not a property. c. The fields on MyTracked and MyInterned are reported to be methods, not fields. Arguably, this is the correct decision!
  2. the tracked function fn test(db: &dyn Database, input: MyInput)'s name ("test") is reported as a struct, not a function.

In tests/input_field_durability.rs on struct MyInput, the field types (bool and usize) are reported as unresolved references, not primitives.

I think it'd be good to resolve these either in Salsa or rust-analyzer since it's part of Salsa's fit-and-finish and these token mismatches might larger, more subtle issues in how rust-analyzer handles Salsa.

On the plus side, go-to-def/find-references on Salsa-annotated functions and structs work!

nikomatsakis commented 1 month ago

Yes, I've noticed this. Presumably we need to adjust our spans somewhat.

Veykril commented 1 month ago

Fixing 1 is easy -> https://github.com/salsa-rs/salsa/pull/606#issuecomment-2437493608 2 is more tricky, r-a currently ranks tokens by similarity and (I believe) in case of equal ranking it picks the last usage of it (which in this case is from the impl within the function). So we'd need the function definitions name to be moved to the end which isn't really doable with the current scoping tricks used. (arguably r-a should pick the first in this case -> https://github.com/rust-lang/rust-analyzer/pull/18410)

The field types rendering unresolved I don't really understand why that is happening