derive(Insertable) into multiple tables #3535
Conversation
6293
force-pushed
the
multiple-table-names
branch
from
64afc77 to
cafc7f8
Compare
diesel_derives/src/model.rs
Outdated
| match self.table_names.len() { | ||
| 0 => &self.name, | ||
| 1 => &self.table_names[0], | ||
| _ => abort_call_site!("Multiple table name attributes are not supported"), |
There was a problem hiding this comment.
| _ => abort_call_site!("Multiple table name attributes are not supported"), | |
| _ => abort_call_site!("expected a single table name attribute"), |
Let's avoid erroring out on the call site. Let's try doing it on the spans for table names.
There was a problem hiding this comment.
ty for the insight. applied
diesel_derives/src/model.rs
Outdated
| self.table_name.as_ref().unwrap_or(&self.name) | ||
| match self.table_names.len() { | ||
| 0 => &self.name, | ||
| 1 => &self.table_names[0], |
There was a problem hiding this comment.
Is there a reason for just returning the first table name and not any other table name?
There was a problem hiding this comment.
While Insertable calls table_names method, other derives would call this to check if there is only one table name
|
Haha I was just thinking about this today! How unlikely! |
6293
force-pushed
the
multiple-table-names
branch
from
cafc7f8 to
61928d0
Compare
6293
requested review from
pksunkara and
JosueMolinaMorales
and removed request for
pksunkara and
JosueMolinaMorales
| error: expected a single table name attribute | ||
| --> tests/fail/derive/multiple_table_names.rs:29:43 | ||
| | | ||
| 29 | #[diesel(table_name = users, table_name = users_)] | ||
| | ^^^^^^ |
There was a problem hiding this comment.
It would be even better, if the error would be reported in this way:
| error: expected a single table name attribute | |
| --> tests/fail/derive/multiple_table_names.rs:29:43 | |
| | | |
| 29 | #[diesel(table_name = users, table_name = users_)] | |
| | ^^^^^^ | |
| error: expected a single table name attribute | |
| --> tests/fail/derive/multiple_table_names.rs:29:43 | |
| | | |
| 29 | #[diesel(table_name = users, table_name = users_)] | |
| | ^^^^^^^^^^^^^^^^^^^ note: remove this attribute |
There was a problem hiding this comment.
agree. I updated the code to check the number of table_name attributes when building Model, instead of when retrieving them from Model. Now, the error will point to the table_name attribute, with a friendly note.
Btw I'm not sure how to put the note right after the highlighting carets as you provided, using proc_macro_error. Would be nice if you had any advice on this.
There was a problem hiding this comment.
That's currently not possible, as the rust-lang proc macro crate does not provide API's for that. See rust-lang/rust#54140 for details.
Closes #1314