Replace #[changeset_for] with #[derive(AsChangeset)]

This is required to make the API compatible with macros 1.1. While syntex could
theoretically keep the old API, I'd like to keep it in sync with nightly.
Macros 1.1 requires that we represent this as a custom derive. This does mean
that the same struct can no longer be used as a changeset for more than one
table (at least not with codegen) but that doesn't feel like something we need
to be concerned about. The stable macro is still an option for that case.

Author: sgrif

CHANGELOG.md

@@ -35,6 +35,13 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
   This was a source of trouble for people that had created views or are using
   any extension that automatically creates views (e.g. PostGIS)
 
+### Changed
+
+* `#[changeset_for(foo)]` should now be written as
+  `#[derive(AsChangeset)] #[table_name="foo"]`. If you were specifying
+  `treat_none_as_null = "true"`, you should additionally have
+  `#[changeset_options(treat_none_as_null = "true")]`.
+
 ## [0.7.2] - 2016-08-20
 
 * Updated nightly version and syntex support.

diesel/src/query_builder/functions.rs

@@ -4,8 +4,7 @@ use super::delete_statement::DeleteStatement;
 use super::insert_statement::Insert;
 
 /// Creates an update statement. Helpers for updating a single row can be
-/// generated by
-/// [`#[changeset_for]`](https://github.com/diesel-rs/diesel/tree/master/diesel_codegen#changeset_fortable_name).
+/// generated by deriving [`AsChangeset`][trait.AsChangeset.html]
 ///
 /// # Examples
 ///

diesel/src/query_builder/update_statement/changeset.rs

@@ -5,8 +5,12 @@ use result::QueryResult;
 
 /// Types which can be passed to
 /// [`update.set`](struct.IncompleteUpdateStatement.html#method.set). This can
-/// be automatically generated for structs by
-/// [`#[changeset_for]`](https://github.com/diesel-rs/diesel/tree/master/diesel_codegen#changeset_fortable_name).
+/// be automatically generated for structs by `#[derive(AsChangeset)]`.
+///
+/// Structs which derive this trait must be annotated with `#[table_name = "something"]`. By
+/// default, any option fields on the struct are skipped if their value is `None`. If you would
+/// like to assign `NULL` to the field instead, you can annotate your struct with
+/// `#[changeset_options(treat_none_as_null = "true")]`
 pub trait AsChangeset {
     type Target: QuerySource;
     type Changeset;

diesel_codegen/README.md

@@ -100,20 +100,23 @@ item, targeting the given table. Can only annotate structs and tuple structs.
 Enums are not supported. See [field annotations][#field-annotations] for
 additional configurations.
 
-### `#[changeset_for(table_name)]`
+### `#[derive(AsChangeset)]`
 
 Adds an implementation of the [`AsChangeset`][as_changeset] trait to the
 annotated item, targeting the given table. At this time, it only supports
 structs with named fields. Tuple structs and enums are not supported. See [field
 annotations][#field-annotations] for additional configurations.
 
+Structs which derive `AsChangeset` must have a table name annotation like so:
+`#[table_name = "something"]`
+
 Any fields which are of the type `Option` will be skipped when their value is
 `None`. This makes it easy to support APIs where you may not want to update all
 of the fields of a record on every request.
 
 If you'd like `None` to change a field to `NULL`, instead of skipping it, you
-can pass the `treat_none_as_null` option like so: `#[changeset_for(posts,
-treat_none_as_null="true")]`
+can pass the `treat_none_as_null` option like so:
+`#[changeset_options(treat_none_as_null = "true")]`
 
 If the struct has a field for the primary key, an additional function,
 `save_changes<T: Queryable<..>>(&self, connection: &Connection) ->

diesel_codegen/src/lib.rs

@@ -15,20 +15,20 @@ pub fn register(reg: &mut rustc_plugin::Registry) {
         MultiDecorator(Box::new(associations::expand_derive_associations)),
     );
     reg.register_syntax_extension(
-        intern("derive_Queryable"),
-        MultiDecorator(Box::new(queryable::expand_derive_queryable))
+        intern("derive_AsChangeset"),
+        MultiDecorator(Box::new(update::expand_derive_as_changeset)),
     );
     reg.register_syntax_extension(
         intern("derive_Identifiable"),
         MultiDecorator(Box::new(identifiable::expand_derive_identifiable))
     );
     reg.register_syntax_extension(
-        intern("insertable_into"),
-        MultiDecorator(Box::new(insertable::expand_insert))
+        intern("derive_Queryable"),
+        MultiDecorator(Box::new(queryable::expand_derive_queryable))
     );
     reg.register_syntax_extension(
-        intern("changeset_for"),
-        MultiDecorator(Box::new(update::expand_changeset_for)),
+        intern("insertable_into"),
+        MultiDecorator(Box::new(insertable::expand_insert))
     );
     reg.register_macro("embed_migrations", migrations::expand_embed_migrations);
     reg.register_macro("infer_table_from_schema", schema_inference::expand_load_table);

diesel_codegen_syntex/src/lib.rs

@@ -30,11 +30,11 @@ pub fn register(reg: &mut syntex::Registry) {
     reg.add_attr("feature(custom_derive)");
     reg.add_attr("feature(custom_attribute)");
 
-    reg.add_decorator("derive_Queryable", queryable::expand_derive_queryable);
-    reg.add_decorator("derive_Identifiable", identifiable::expand_derive_identifiable);
+    reg.add_decorator("derive_AsChangeset", update::expand_derive_as_changeset);
     reg.add_decorator("derive_Associations", associations::expand_derive_associations);
+    reg.add_decorator("derive_Identifiable", identifiable::expand_derive_identifiable);
+    reg.add_decorator("derive_Queryable", queryable::expand_derive_queryable);
     reg.add_decorator("insertable_into", insertable::expand_insert);
-    reg.add_decorator("changeset_for", update::expand_changeset_for);
     reg.add_macro("embed_migrations", migrations::expand_embed_migrations);
     reg.add_macro("infer_table_from_schema", schema_inference::expand_load_table);
     reg.add_macro("infer_schema", schema_inference::expand_infer_schema);

diesel_codegen_syntex/src/update.rs

@@ -3,80 +3,86 @@ use syntax::attr::AttrMetaMethods;
 use syntax::codemap::Span;
 use syntax::ext::base::{Annotatable, ExtCtxt};
 use syntax::ptr::P;
-use syntax::parse::token::{InternedString, str_to_ident};
-use syntax::tokenstream::TokenTree;
 
 use model::Model;
-use util::lifetime_list_tokens;
+use util::{lifetime_list_tokens, str_value_of_attr_with_name};
 
-pub fn expand_changeset_for(
+pub fn expand_derive_as_changeset(
     cx: &mut ExtCtxt,
     span: Span,
     meta_item: &MetaItem,
     annotatable: &Annotatable,
     push: &mut FnMut(Annotatable),
 ) {
     if let Some(model) = Model::from_annotable(cx, span, annotatable) {
-        let options = changeset_options(cx, meta_item).unwrap();
+        let options = changeset_options(cx, span, annotatable.attrs()).unwrap();
         push(Annotatable::Item(changeset_impl(cx, span, &options, &model).unwrap()));
     } else {
         cx.span_err(meta_item.span,
-            "`changeset_for` may only be applied to enums and structs");
+            "`#[derive(AsChangeset)]` may only be applied to enums and structs");
     }
 }
 
 struct ChangesetOptions {
     table_name: ast::Ident,
-    treat_none_as_null: Vec<TokenTree>,
+    treat_none_as_null: bool,
 }
 
-fn changeset_options(cx: &mut ExtCtxt, meta_item: &MetaItem) -> Result<ChangesetOptions, ()> {
-    match meta_item.node {
-        MetaItemKind::List(_, ref meta_items) => {
-            let table_name = try!(table_name(cx, &meta_items[0]));
-            let treat_none_as_null = try!(boolean_option(cx, &meta_items[1..], "treat_none_as_null"));
-            Ok(ChangesetOptions {
-                table_name: str_to_ident(&table_name),
-                treat_none_as_null: treat_none_as_null,
-            })
+fn changeset_options(
+    cx: &mut ExtCtxt,
+    span: Span,
+    attributes: &[ast::Attribute]
+) -> Result<ChangesetOptions, ()> {
+    let changeset_options_attr = attributes.iter().find(|a| a.check_name("changeset_options"));
+    let treat_none_as_null = try!(changeset_options_attr
+        .map(|a| extract_treat_none_as_null(cx, a))
+        .unwrap_or(Ok(false)));
+    let table_name = match str_value_of_attr_with_name(cx, attributes, "table_name") {
+        Some(name) => name,
+        None => return missing_table_name_error(cx, span),
+    };
+
+    Ok(ChangesetOptions {
+        table_name: table_name,
+        treat_none_as_null: treat_none_as_null,
+    })
+}
+
+fn extract_treat_none_as_null(cx: &mut ExtCtxt, attr: &ast::Attribute) -> Result<bool, ()>{
+    match attr.node.value.node {
+        MetaItemKind::List(_, ref items) if items.len() == 1 => {
+            if items[0].check_name("treat_none_as_null") {
+                boolean_option(cx, &*items[0])
+            } else {
+                options_usage_error(cx, attr.span)
+            }
         }
-        _ => usage_error(cx, meta_item),
+        _ => options_usage_error(cx, attr.span),
     }
 }
 
-fn table_name(cx: &mut ExtCtxt, meta_item: &MetaItem) -> Result<InternedString, ()> {
-    match meta_item.node {
-        MetaItemKind::Word(ref word) => Ok(word.clone()),
-        _ => usage_error(cx, meta_item),
+fn boolean_option(cx: &mut ExtCtxt, item: &MetaItem) -> Result<bool, ()> {
+    match item.value_str() {
+        Some(ref s) if *s == "true" => Ok(true),
+        Some(ref s) if *s == "false" => Ok(false),
+        _ => options_usage_error(cx, item.span)
     }
 }
 
-#[allow(unused_imports)] // quote_tokens! generates warnings
-fn boolean_option(cx: &mut ExtCtxt, meta_items: &[P<MetaItem>], option_name: &str)
-    -> Result<Vec<TokenTree>, ()>
-{
-    if let Some(item) = meta_items.iter().find(|item| item.name() == option_name) {
-        match item.value_str() {
-            Some(ref s) if *s == "true" => Ok(quote_tokens!(cx, "true")),
-            Some(ref s) if *s == "false" => Ok(quote_tokens!(cx, "false")),
-            _ => {
-                cx.span_err(item.span,
-                    &format!("Expected {} to be in the form `option=\"true\"` or \
-                            option=\"false\"", option_name));
-                Err(())
-            }
-        }
-    } else {
-        Ok(quote_tokens!(cx, "false"))
-    }
+fn options_usage_error<T>(cx: &mut ExtCtxt, span: Span) -> Result<T, ()> {
+    cx.span_err(span,
+        r#"`changeset_options` must be used in the form \
+        `#[changeset_options(treat_none_as_null = "true")]`"#);
+    Err(())
 }
 
-fn usage_error<T>(cx: &mut ExtCtxt, meta_item: &MetaItem) -> Result<T, ()> {
-    cx.span_err(meta_item.span,
-        "`changeset_for` must be used in the form `#[changeset_for(table1)]`");
+fn missing_table_name_error<T>(cx: &mut ExtCtxt, span: Span) -> Result<T, ()> {
+    cx.span_err(span, r#"Structs annotated with `#[derive(AsChangeset)]` must \
+        also be annotated with `#[table_name="something"]`"#);
     Err(())
 }
 
+#[allow(unused_imports)] // quote_tokens! generates warnings
 fn changeset_impl(
     cx: &mut ExtCtxt,
     span: Span,
@@ -85,7 +91,11 @@ fn changeset_impl(
 ) -> Option<P<ast::Item>> {
     let struct_name = model.name;
     let table_name = options.table_name;
-    let treat_none_as_null = &options.treat_none_as_null;
+    let treat_none_as_null = if options.treat_none_as_null {
+        quote_tokens!(cx, "true")
+    } else {
+        quote_tokens!(cx, "false")
+    };
     let struct_ty = &model.ty;
     let lifetimes = lifetime_list_tokens(&model.generics.lifetimes, span);
 

diesel_codegen_syntex/src/util.rs

@@ -69,10 +69,11 @@ pub fn ident_value_of_attr_with_name(
 
 #[cfg(feature = "with-syntex")]
 const KNOWN_ATTRIBUTES: &'static [&'static str] = &[
-    "table_name",
+    "belongs_to",
+    "changeset_for",
     "column_name",
     "has_many",
-    "belongs_to",
+    "table_name",
 ];
 
 #[cfg(feature = "with-syntex")]

diesel_compile_tests/tests/compile-fail/codegen_does_not_add_save_changes_method_on_structs_without_primary_key.rs

@@ -14,8 +14,8 @@ table! {
     }
 }
 
-#[derive(Queryable)]
-#[changeset_for(users)]
+#[derive(Queryable, AsChangeset)]
+#[table_name = "users"]
 pub struct User {
     name: String,
     hair_color: String,

diesel_tests/tests/schema.rs

@@ -2,10 +2,10 @@ use diesel::*;
 
 infer_schema!(dotenv!("DATABASE_URL"));
 
-#[derive(PartialEq, Eq, Debug, Clone, Queryable, Identifiable, Associations)]
-#[changeset_for(users)]
+#[derive(PartialEq, Eq, Debug, Clone, Queryable, Identifiable, AsChangeset, Associations)]
 #[insertable_into(users)]
 #[has_many(posts)]
+#[table_name = "users"]
 pub struct User {
     pub id: i32,
     pub name: String,
@@ -65,9 +65,9 @@ select_column_workaround!(comments -> users (id, post_id, text));
 
 join_through!(users -> posts -> comments);
 
-#[derive(Debug, PartialEq, Eq, Queryable, Clone)]
+#[derive(Debug, PartialEq, Eq, Queryable, Clone, AsChangeset)]
 #[insertable_into(users)]
-#[changeset_for(users)]
+#[table_name = "users"]
 pub struct NewUser {
     pub name: String,
     pub hair_color: Option<String>,