From e0febd904ac254588f3eec456e49dd3e8e8af2bc Mon Sep 17 00:00:00 2001 From: Jake Date: Thu, 24 Oct 2024 16:37:28 -0700 Subject: [PATCH 1/2] salsa-macros: handle invalid inputs in a way friendlier to rust-analyzer --- components/salsa-macros/src/accumulator.rs | 5 +- components/salsa-macros/src/db.rs | 8 +- components/salsa-macros/src/input.rs | 5 +- components/salsa-macros/src/interned.rs | 6 +- components/salsa-macros/src/lib.rs | 23 +++- components/salsa-macros/src/tracked.rs | 6 +- .../input_struct_incompatibles.stderr | 6 + .../interned_struct_incompatibles.stderr | 6 + .../tracked_fn_incompatibles.stderr | 31 +++++ .../tracked_impl_incompatibles.stderr | 127 ++++++++++++++++++ .../tracked_method_incompatibles.stderr | 50 +++++++ .../tracked_method_on_untracked_impl.stderr | 12 ++ 12 files changed, 270 insertions(+), 15 deletions(-) diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs index 0c9f39bf..1e09cb08 100644 --- a/components/salsa-macros/src/accumulator.rs +++ b/components/salsa-macros/src/accumulator.rs @@ -3,6 +3,7 @@ use proc_macro2::TokenStream; use crate::{ hygiene::Hygiene, options::{AllowedOptions, Options}, + token_stream_with_error, }; // #[salsa::accumulator(jar = Jar0)] @@ -14,7 +15,7 @@ pub(crate) fn accumulator( ) -> proc_macro::TokenStream { let hygiene = Hygiene::from1(&input); let args = syn::parse_macro_input!(args as Options); - let struct_item = syn::parse_macro_input!(input as syn::ItemStruct); + let struct_item = parse_macro_input!(input as syn::ItemStruct); let ident = struct_item.ident.clone(); let m = StructMacro { hygiene, @@ -23,7 +24,7 @@ pub(crate) fn accumulator( }; match m.try_expand() { Ok(v) => crate::debug::dump_tokens(ident, v).into(), - Err(e) => e.to_compile_error().into(), + Err(e) => token_stream_with_error(input, e), } } diff --git a/components/salsa-macros/src/db.rs b/components/salsa-macros/src/db.rs index 02fcef4b..e9f553bb 100644 --- a/components/salsa-macros/src/db.rs +++ b/components/salsa-macros/src/db.rs @@ -1,7 +1,7 @@ use proc_macro2::TokenStream; use syn::parse::Nothing; -use crate::hygiene::Hygiene; +use crate::{hygiene::Hygiene, token_stream_with_error}; // Source: // @@ -16,11 +16,11 @@ pub(crate) fn db( ) -> proc_macro::TokenStream { let _nothing = syn::parse_macro_input!(args as Nothing); let hygiene = Hygiene::from1(&input); - let input = syn::parse_macro_input!(input as syn::Item); + let item = parse_macro_input!(input as syn::Item); let db_macro = DbMacro { hygiene }; - match db_macro.try_db(input) { + match db_macro.try_db(item) { Ok(v) => crate::debug::dump_tokens("db", v).into(), - Err(e) => e.to_compile_error().into(), + Err(e) => token_stream_with_error(input, e), } } diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index e5d67d53..9ad44491 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -2,6 +2,7 @@ use crate::{ hygiene::Hygiene, options::Options, salsa_struct::{SalsaStruct, SalsaStructAllowedOptions}, + token_stream_with_error, }; use proc_macro2::TokenStream; @@ -16,7 +17,7 @@ pub(crate) fn input( ) -> proc_macro::TokenStream { let args = syn::parse_macro_input!(args as InputArgs); let hygiene = Hygiene::from1(&input); - let struct_item = syn::parse_macro_input!(input as syn::ItemStruct); + let struct_item = parse_macro_input!(input as syn::ItemStruct); let m = Macro { hygiene, args, @@ -24,7 +25,7 @@ pub(crate) fn input( }; match m.try_macro() { Ok(v) => v.into(), - Err(e) => e.to_compile_error().into(), + Err(e) => token_stream_with_error(input, e), } } diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index aea9a4e1..2fcae64e 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -2,7 +2,7 @@ use crate::{ db_lifetime, hygiene::Hygiene, options::Options, - salsa_struct::{SalsaStruct, SalsaStructAllowedOptions}, + salsa_struct::{SalsaStruct, SalsaStructAllowedOptions}, token_stream_with_error, }; use proc_macro2::TokenStream; @@ -17,7 +17,7 @@ pub(crate) fn interned( ) -> proc_macro::TokenStream { let args = syn::parse_macro_input!(args as InternedArgs); let hygiene = Hygiene::from1(&input); - let struct_item = syn::parse_macro_input!(input as syn::ItemStruct); + let struct_item = parse_macro_input!(input as syn::ItemStruct); let m = Macro { hygiene, args, @@ -25,7 +25,7 @@ pub(crate) fn interned( }; match m.try_macro() { Ok(v) => v.into(), - Err(e) => e.to_compile_error().into(), + Err(e) => token_stream_with_error(input, e), } } diff --git a/components/salsa-macros/src/lib.rs b/components/salsa-macros/src/lib.rs index 61edb1d5..2b2de522 100644 --- a/components/salsa-macros/src/lib.rs +++ b/components/salsa-macros/src/lib.rs @@ -20,6 +20,20 @@ macro_rules! parse_quote { } } +/// Similar to `syn::parse_macro_input`, however, when a parse error is encountered, it will return +/// the input token stream in addition to the error. This will make it so that rust-analyzer can work +/// with incomplete code. +macro_rules! parse_macro_input { + ($tokenstream:ident as $ty:ty) => { + match syn::parse::<$ty>($tokenstream.clone()) { + Ok(data) => data, + Err(err) => { + return $crate::token_stream_with_error($tokenstream, err); + } + } + }; +} + mod accumulator; mod db; mod db_lifetime; @@ -64,9 +78,14 @@ pub fn tracked(args: TokenStream, input: TokenStream) -> TokenStream { #[proc_macro_derive(Update)] pub fn update(input: TokenStream) -> TokenStream { - let item = syn::parse_macro_input!(input as syn::DeriveInput); + let item = parse_macro_input!(input as syn::DeriveInput); match update::update_derive(item) { Ok(tokens) => tokens.into(), - Err(err) => err.to_compile_error().into(), + Err(error) => token_stream_with_error(input, error), } } + +pub(crate) fn token_stream_with_error(mut tokens: TokenStream, error: syn::Error) -> TokenStream { + tokens.extend(TokenStream::from(error.into_compile_error())); + tokens +} diff --git a/components/salsa-macros/src/tracked.rs b/components/salsa-macros/src/tracked.rs index a8709603..34529abc 100644 --- a/components/salsa-macros/src/tracked.rs +++ b/components/salsa-macros/src/tracked.rs @@ -1,10 +1,12 @@ use syn::{spanned::Spanned, Item}; +use crate::token_stream_with_error; + pub(crate) fn tracked( args: proc_macro::TokenStream, input: proc_macro::TokenStream, ) -> proc_macro::TokenStream { - let item = syn::parse_macro_input!(input as Item); + let item = parse_macro_input!(input as Item); let res = match item { syn::Item::Struct(item) => crate::tracked_struct::tracked_struct(args, item), syn::Item::Fn(item) => crate::tracked_fn::tracked_fn(args, item), @@ -16,6 +18,6 @@ pub(crate) fn tracked( }; match res { Ok(s) => s.into(), - Err(err) => err.into_compile_error().into(), + Err(err) => token_stream_with_error(input, err), } } diff --git a/tests/compile-fail/input_struct_incompatibles.stderr b/tests/compile-fail/input_struct_incompatibles.stderr index 9c705390..1a68fb55 100644 --- a/tests/compile-fail/input_struct_incompatibles.stderr +++ b/tests/compile-fail/input_struct_incompatibles.stderr @@ -40,3 +40,9 @@ error: `#[id]` cannot be used with `#[salsa::input]` 21 | / #[id] 22 | | field: u32, | |______________^ + +error: cannot find attribute `id` in this scope + --> tests/compile-fail/input_struct_incompatibles.rs:21:7 + | +21 | #[id] + | ^^ diff --git a/tests/compile-fail/interned_struct_incompatibles.stderr b/tests/compile-fail/interned_struct_incompatibles.stderr index 72daa2fd..335a1536 100644 --- a/tests/compile-fail/interned_struct_incompatibles.stderr +++ b/tests/compile-fail/interned_struct_incompatibles.stderr @@ -40,3 +40,9 @@ error: `#[id]` cannot be used with `#[salsa::interned]` 33 | / #[id] 34 | | field: u32, | |______________^ + +error: cannot find attribute `id` in this scope + --> tests/compile-fail/interned_struct_incompatibles.rs:33:7 + | +33 | #[id] + | ^^ diff --git a/tests/compile-fail/tracked_fn_incompatibles.stderr b/tests/compile-fail/tracked_fn_incompatibles.stderr index 882f9209..dbdb3827 100644 --- a/tests/compile-fail/tracked_fn_incompatibles.stderr +++ b/tests/compile-fail/tracked_fn_incompatibles.stderr @@ -46,6 +46,20 @@ error: only a single lifetime parameter is accepted 67 | fn tracked_fn_with_multiple_lts<'db1, 'db2>(db: &'db1 dyn Db, interned: MyInterned<'db2>) -> u32 { | ^^^^ +error: `self` parameter is only allowed in associated functions + --> tests/compile-fail/tracked_fn_incompatibles.rs:27:55 + | +27 | fn tracked_fn_with_receiver_not_applied_to_impl_block(&self, db: &dyn Db) -> u32 {} + | ^^^^^ not semantically valid as function parameter + | + = note: associated functions are those in `impl` or `trait` definitions + +error[E0415]: identifier `input` is bound more than once in this parameter list + --> tests/compile-fail/tracked_fn_incompatibles.rs:33:5 + | +33 | input: MyInput, + | ^^^^^ used as parameter more than once + error[E0106]: missing lifetime specifier --> tests/compile-fail/tracked_fn_incompatibles.rs:61:15 | @@ -64,3 +78,20 @@ error[E0308]: mismatched types | ----------------- implicitly returns `()` as its body has no tail or `return` expression 24 | fn tracked_fn_with_one_input(db: &dyn Db) -> u32 {} | ^^^ expected `u32`, found `()` + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_fn_incompatibles.rs:27:78 + | +27 | fn tracked_fn_with_receiver_not_applied_to_impl_block(&self, db: &dyn Db) -> u32 {} + | -------------------------------------------------- ^^^ expected `u32`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_fn_incompatibles.rs:34:6 + | +30 | fn tracked_fn_with_too_many_arguments_for_specify( + | ---------------------------------------------- implicitly returns `()` as its body has no tail or `return` expression +... +34 | ) -> u32 { + | ^^^ expected `u32`, found `()` diff --git a/tests/compile-fail/tracked_impl_incompatibles.stderr b/tests/compile-fail/tracked_impl_incompatibles.stderr index 4e3e1ed2..01eb5ee2 100644 --- a/tests/compile-fail/tracked_impl_incompatibles.stderr +++ b/tests/compile-fail/tracked_impl_incompatibles.stderr @@ -46,6 +46,69 @@ error: unexpected token 41 | #[salsa::tracked(constructor = Constructor)] | ^^^^^^^^^^^ +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:12:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +12 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:17:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +17 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:22:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +22 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:27:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +27 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:32:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +32 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:37:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +37 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + +error[E0119]: conflicting implementations of trait `Default` for type `MyTracked<'_>` + --> tests/compile-fail/tracked_impl_incompatibles.rs:42:1 + | +7 | impl<'db> std::default::Default for MyTracked<'db> { + | -------------------------------------------------- first implementation here +... +42 | impl<'db> std::default::Default for MyTracked<'db> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyTracked<'_>` + error[E0117]: only traits defined in the current crate can be implemented for arbitrary types --> tests/compile-fail/tracked_impl_incompatibles.rs:47:1 | @@ -57,6 +120,70 @@ error[E0117]: only traits defined in the current crate can be implemented for ar | = note: define and implement a trait or new type instead +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:8:21 + | +8 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:13:21 + | +13 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:18:21 + | +18 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:23:21 + | +23 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:28:21 + | +28 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:33:21 + | +33 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:38:21 + | +38 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error[E0308]: mismatched types + --> tests/compile-fail/tracked_impl_incompatibles.rs:43:21 + | +43 | fn default() -> Self {} + | ------- ^^^^ expected `MyTracked<'_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + error[E0308]: mismatched types --> tests/compile-fail/tracked_impl_incompatibles.rs:48:21 | diff --git a/tests/compile-fail/tracked_method_incompatibles.stderr b/tests/compile-fail/tracked_method_incompatibles.stderr index c38afa94..72a27a33 100644 --- a/tests/compile-fail/tracked_method_incompatibles.stderr +++ b/tests/compile-fail/tracked_method_incompatibles.stderr @@ -1,23 +1,73 @@ +error: #[salsa::tracked] must also be applied to the impl block for tracked methods + --> tests/compile-fail/tracked_method_incompatibles.rs:9:17 + | +9 | fn ref_self(&self, db: &dyn salsa::Database) {} + | ^^^^^ + error: tracked methods's first argument must be declared as `self`, not `&self` or `&mut self` --> tests/compile-fail/tracked_method_incompatibles.rs:9:17 | 9 | fn ref_self(&self, db: &dyn salsa::Database) {} | ^ +error: #[salsa::tracked] must also be applied to the impl block for tracked methods + --> tests/compile-fail/tracked_method_incompatibles.rs:15:21 + | +15 | fn ref_mut_self(&mut self, db: &dyn salsa::Database) {} + | ^^^^^^^^^ + error: tracked methods's first argument must be declared as `self`, not `&self` or `&mut self` --> tests/compile-fail/tracked_method_incompatibles.rs:15:21 | 15 | fn ref_mut_self(&mut self, db: &dyn salsa::Database) {} | ^ +error: #[salsa::tracked] must also be applied to the impl block for tracked methods + --> tests/compile-fail/tracked_method_incompatibles.rs:21:33 + | +21 | fn multiple_lifetimes<'db1>(&mut self, db: &'db1 dyn salsa::Database) {} + | ^^^^^^^^^ + error: tracked method already has a lifetime parameter in scope --> tests/compile-fail/tracked_method_incompatibles.rs:21:27 | 21 | fn multiple_lifetimes<'db1>(&mut self, db: &'db1 dyn salsa::Database) {} | ^^^^ +error: only a single lifetime parameter is accepted + --> tests/compile-fail/tracked_method_incompatibles.rs:27:22 + | +27 | fn type_generics(&mut self, db: &dyn salsa::Database) -> T { + | ^ + error: tracked methods cannot have non-lifetime generic parameters --> tests/compile-fail/tracked_method_incompatibles.rs:27:22 | 27 | fn type_generics(&mut self, db: &dyn salsa::Database) -> T { | ^ + +warning: unused variable: `db` + --> tests/compile-fail/tracked_method_incompatibles.rs:9:24 + | +9 | fn ref_self(&self, db: &dyn salsa::Database) {} + | ^^ help: if this is intentional, prefix it with an underscore: `_db` + | + = note: `#[warn(unused_variables)]` on by default + +warning: unused variable: `db` + --> tests/compile-fail/tracked_method_incompatibles.rs:15:32 + | +15 | fn ref_mut_self(&mut self, db: &dyn salsa::Database) {} + | ^^ help: if this is intentional, prefix it with an underscore: `_db` + +warning: unused variable: `db` + --> tests/compile-fail/tracked_method_incompatibles.rs:21:44 + | +21 | fn multiple_lifetimes<'db1>(&mut self, db: &'db1 dyn salsa::Database) {} + | ^^ help: if this is intentional, prefix it with an underscore: `_db` + +warning: unused variable: `db` + --> tests/compile-fail/tracked_method_incompatibles.rs:27:36 + | +27 | fn type_generics(&mut self, db: &dyn salsa::Database) -> T { + | ^^ help: if this is intentional, prefix it with an underscore: `_db` diff --git a/tests/compile-fail/tracked_method_on_untracked_impl.stderr b/tests/compile-fail/tracked_method_on_untracked_impl.stderr index 2807c74d..f1e00622 100644 --- a/tests/compile-fail/tracked_method_on_untracked_impl.stderr +++ b/tests/compile-fail/tracked_method_on_untracked_impl.stderr @@ -3,3 +3,15 @@ error: #[salsa::tracked] must also be applied to the impl block for tracked meth | 8 | fn tracked_method_on_untracked_impl(self, db: &dyn Db) -> u32 { | ^^^^ + +error[E0405]: cannot find trait `Db` in this scope + --> tests/compile-fail/tracked_method_on_untracked_impl.rs:8:56 + | +8 | fn tracked_method_on_untracked_impl(self, db: &dyn Db) -> u32 { + | ^^ not found in this scope + +error[E0425]: cannot find value `input` in this scope + --> tests/compile-fail/tracked_method_on_untracked_impl.rs:9:9 + | +9 | input.field(db) + | ^^^^^ not found in this scope From 10d14f0131a3480c2bc6a5bb1a252a0b84f2c86b Mon Sep 17 00:00:00 2001 From: Jake Date: Thu, 24 Oct 2024 16:47:44 -0700 Subject: [PATCH 2/2] fix cargo fmt --- components/salsa-macros/src/interned.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 2fcae64e..8caba77e 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -2,7 +2,8 @@ use crate::{ db_lifetime, hygiene::Hygiene, options::Options, - salsa_struct::{SalsaStruct, SalsaStructAllowedOptions}, token_stream_with_error, + salsa_struct::{SalsaStruct, SalsaStructAllowedOptions}, + token_stream_with_error, }; use proc_macro2::TokenStream;