Merge #423

423: salsa 2022: fix input macro set_* being off by one if id field present r=XFFXFF a=jhgg

This PR fixes an issue with code generation for `#[salsa::input]` struct's `set_` methods. 

Consider the following code:

```rust
#[salsa::input(jar = Jar)]
struct BuggedInput {
    #[id]
    id: u32,
    other: String,
}
```

This will expand to:

```rust
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
struct BuggedInput(salsa::Id);

impl BuggedInput {
     // snip...
    fn set_other<'db>(
        self,
        __db: &'db mut <Jar as salsa:🫙:Jar<'_>>::DynDb,
    ) -> salsa::setter::Setter<'db, BuggedInput, u32> {
        //                                       ^^^ wrong type (should be `String`)
        let (__jar, __runtime) = <_ as salsa::storage::HasJar<Jar>>::jar_mut(__db);
        let __ingredients =
            <Jar as salsa::storage::HasIngredientsFor<BuggedInput>>::ingredient_mut(__jar);
        salsa::setter::Setter::new(__runtime, self, &mut __ingredients.0)
        //                                                             ^ wrong index (should be `1`)
   }
}
```

Here we can see that the generated `set_other` impl is improperly setting the `id` field. This bug is caused because the filtering of the id fields in `InputStruct::all_set_field_names` causes a mismatch in `InputStruct::input_inherent_impl` when zipped with the field indices and types.

This PR changes `all_set_field_names` to return an `Option<&syn::Ident>` where the None is provided when a setter should not be generated, thus not causing index mismatches because no filtering occurs. Instead, we filter inside of `input_inherent_impl` after all the zips have been applied to the iterator.

After this PR, the code generated is now correct:

```rust
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
struct BuggedInput(salsa::Id);

impl BuggedInput {
     // snip...
    fn set_other<'db>(
        self,
        __db: &'db mut <Jar as salsa:🫙:Jar<'_>>::DynDb,
    ) -> salsa::setter::Setter<'db, BuggedInput, String> {
        let (__jar, __runtime) = <_ as salsa::storage::HasJar<Jar>>::jar_mut(__db);
        let __ingredients =
            <Jar as salsa::storage::HasIngredientsFor<BuggedInput>>::ingredient_mut(__jar);
        salsa::setter::Setter::new(__runtime, self, &mut __ingredients.1)
    }
}
```


Co-authored-by: Jake Heinz <jh@discordapp.com>

components/salsa-2022-macros/src/input.rs

@@ -110,15 +110,16 @@ impl InputStruct {
             .zip(&set_field_names)
             .zip(&field_vises)
             .zip(&field_tys)
-            .map(|(((field_index, set_field_name), field_vis), field_ty)| {
-            parse_quote! {
+            .filter_map(|(((field_index, &set_field_name), field_vis), field_ty)| {
+                let set_field_name = set_field_name?;
+                Some(parse_quote! {
                     #field_vis fn #set_field_name<'db>(self, __db: &'db mut #db_dyn_ty) -> salsa::setter::Setter<'db, #ident, #field_ty>
                     {
                         let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar_mut(__db);
                         let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient_mut(__jar);
                         salsa::setter::Setter::new(__runtime, self, &mut __ingredients.#field_index)
                     }
-            }
+                })
         })
         .collect();
 
@@ -291,12 +292,13 @@ impl InputStruct {
             .collect()
     }
 
-    /// Names of setters of all fields
-    /// setters are not created for fields with #[id] tag so they'll be safe to include in debug formatting
-    pub(crate) fn all_set_field_names(&self) -> Vec<&syn::Ident> {
+    /// Names of setters of all fields that should be generated. Returns an optional Ident for the field name
+    /// that is None when the field should not generate a setter.
+    ///
+    /// Setters are not created for fields with #[id] tag so they'll be safe to include in debug formatting
+    pub(crate) fn all_set_field_names(&self) -> Vec<Option<&syn::Ident>> {
         self.all_fields()
-            .filter(|&field| !field.has_id_attr)
-            .map(|ef| ef.set_name())
+            .map(|ef| (!ef.has_id_attr).then(|| ef.set_name()))
             .collect()
     }
 

salsa-2022-tests/tests/input_with_ids.rs

@@ -8,13 +8,17 @@ struct Jar(MyInput);
 
 trait Db: salsa::DbWithJar<Jar> {}
 
+#[derive(Clone, Debug)]
+struct Field {}
+
 #[salsa::input(jar = Jar)]
 struct MyInput {
-    field: u32,
     #[id]
     id_one: u32,
     #[id]
     id_two: u16,
+
+    field: Field,
 }
 
 #[salsa::db(Jar)]
@@ -31,9 +35,16 @@ impl Db for Database {}
 fn test_debug() {
     let mut db = Database::default();
 
-    let input = MyInput::new(&mut db, 22, 50, 10);
+    let input = MyInput::new(&mut db, 50, 10, Field {});
 
     let actual = format!("{:?}", input.debug(&db));
     let expected = expect![[r#"MyInput { [salsa id]: 0, id_one: 50, id_two: 10 }"#]];
     expected.assert_eq(&actual);
 }
+
+#[test]
+fn test_set() {
+    let mut db = Database::default();
+    let input = MyInput::new(&mut db, 50, 10, Field {});
+    input.set_field(&mut db).to(Field {});
+}