Add #[borsh(use_discriminant = <bool>)] that change enum discriminant de- and serialization behavior #148

.github/workflows/rust.yml

@@ -20,7 +20,7 @@ jobs:
   tests:
     strategy:
       matrix:
-        rust_version: [1.65.0, stable]
+        rust_version: [1.66.0, stable]
     runs-on: ubuntu-20.04
 
     steps:

Cargo.toml

@@ -11,4 +11,4 @@ members = [
 [workspace.package]
 # shared version of all public crates in the workspace
 version = "0.11.0"
-rust-version = "1.65.0"
+rust-version = "1.66.0"

README.md

@@ -1,10 +1,10 @@
-# Borsh in Rust   [![Latest Version]][crates.io] [![borsh: rustc 1.65+]][Rust 1.65] [![License Apache-2.0 badge]][License Apache-2.0] [![License MIT badge]][License MIT]
+# Borsh in Rust   [![Latest Version]][crates.io] [![borsh: rustc 1.66+]][Rust 1.66] [![License Apache-2.0 badge]][License Apache-2.0] [![License MIT badge]][License MIT]
 
 [Borsh]: https://borsh.io
 [Latest Version]: https://img.shields.io/crates/v/borsh.svg
 [crates.io]: https://crates.io/crates/borsh
-[borsh: rustc 1.65+]: https://img.shields.io/badge/rustc-1.65+-lightgray.svg
-[Rust 1.65]: https://blog.rust-lang.org/2022/11/03/Rust-1.65.0.html
+[borsh: rustc 1.66+]: https://img.shields.io/badge/rustc-1.66+-lightgray.svg
+[Rust 1.66]: https://blog.rust-lang.org/2022/12/15/Rust-1.66.0.html
 [License Apache-2.0 badge]: https://img.shields.io/badge/license-Apache2.0-blue.svg
 [License Apache-2.0]: https://opensource.org/licenses/Apache-2.0
 [License MIT badge]: https://img.shields.io/badge/license-MIT-blue.svg
@@ -76,6 +76,37 @@ struct A {
 }
 ```
 
+### Enum with explicit discriminant
+
+`#[borsh(use_discriminant=false|true])` is required if you have an enum with explicit discriminant. This settings affects `BorshSerialize` and `BorshDeserialize` behaviour at the same time.
+
+If you don't specify `use_discriminant` option for enum with explicit discriminant, you will get an error:
+
+````bash
+error: You have to specify `#[borsh(use_discriminant=true)]` or `#[borsh(use_discriminant=false)]` for all enums with explicit discriminant
+```
+```rust
+#[derive(BorshDeserialize, BorshSerialize)]
+#[borsh(use_discriminant=false)]
+enum A {
+    X,
+    Y = 10,
+}
+````
+
+Will keep old behaviour of borsh deserialization and will not use discriminant. This option is left to have backward compatability with previous versions of borsh and to have ability to deserialise data from previous versions of borsh.
+
+```rust
+#[derive(BorshDeserialize, BorshSerialize)]
+#[borsh(use_discriminant=true)]
+enum A {
+    X,
+    Y = 10,
+}
+```
+
+This one will use proper version of serialization of enum with explicit discriminant.
+
 ## Releasing
 
 The versions of all public crates in this repository are collectively managed by a single version in the [workspace manifest](https://github.com/near/borsh-rs/blob/master/Cargo.toml).

borsh-derive-internal/src/attribute_helpers.rs

@@ -1,6 +1,8 @@
 // TODO: remove this unused attribute, when the unsplit is done
 #![allow(unused)]
-use syn::{Attribute, Field, Path, WherePredicate};
+use proc_macro2::TokenStream;
+use quote::ToTokens;
+use syn::{spanned::Spanned, Attribute, DeriveInput, Expr, Field, ItemEnum, Path, WherePredicate};
 pub mod parsing_helpers;
 use parsing_helpers::get_where_predicates;
 
@@ -13,6 +15,8 @@ pub struct Symbol(pub &'static str);
 pub const BORSH: Symbol = Symbol("borsh");
 /// bound - sub-borsh nested meta, field-level only, `BorshSerialize` and `BorshDeserialize` contexts
 pub const BOUND: Symbol = Symbol("bound");
+//  use_discriminant - sub-borsh nested meta, item-level only, enums only, `BorshSerialize` and `BorshDeserialize` contexts
+pub const USE_DISCRIMINANT: &str = "use_discriminant";
 /// serialize - sub-bound nested meta attribute
 pub const SERIALIZE: Symbol = Symbol("serialize");
 /// deserialize - sub-bound nested meta attribute
@@ -42,6 +46,88 @@ pub(crate) fn contains_skip(attrs: &[Attribute]) -> bool {
     attrs.iter().any(|attr| attr.path() == SKIP)
 }
 
+pub fn check_item_attributes(derive_input: &DeriveInput) -> Result<(), TokenStream> {
+    for attr in &derive_input.attrs {
+        if attr.path().is_ident(SKIP.0) {
+            return Err(syn::Error::new(
+                derive_input.ident.span(),
+                "`borsh_skip` is not allowed as derive input attribute",
+            )
+            .to_compile_error());
+        }
+        if attr.path().is_ident(BORSH.0) {
+            attr.parse_nested_meta(|meta| {
+                if !meta.path.is_ident(USE_DISCRIMINANT) {
+                    return Err(syn::Error::new(
+                        meta.path.span(),
+                        "`use_discriminant` is the only supported attribute for `borsh`",
+                    ));
+                }
+                if meta.path.is_ident(USE_DISCRIMINANT) {
+                    let _expr: Expr = meta.value()?.parse()?;
+                    if let syn::Data::Struct(ref _data) = derive_input.data {
+                        return Err(syn::Error::new(
+                            derive_input.ident.span(),
+                            "borsh(use_discriminant=<bool>) does not support structs",
+                        ));
+                    }
+                }
+
+                Ok(())
+            })
+            .map_err(|err| err.to_compile_error())?;
+        }
+    }
+    Ok(())
+}
+
+pub(crate) fn contains_use_discriminant(input: &ItemEnum) -> Result<bool, syn::Error> {
+    if input.variants.len() > 256 {
+        return Err(syn::Error::new(
+            input.span(),
+            "up to 256 enum variants are supported",
+        ));
+    }
+
+    let attrs = &input.attrs;
+    let mut use_discriminant = None;
+    for attr in attrs {
+        if attr.path().is_ident(BORSH.0) {
+            attr.parse_nested_meta(|meta| {
+                if meta.path.is_ident(USE_DISCRIMINANT) {
+                    let value_expr: Expr = meta.value()?.parse()?;
+                    let value = value_expr.to_token_stream().to_string();
+                    match value.as_str() {
+                        "true" => {
+                            use_discriminant = Some(true);
+                        }
+                        "false" => use_discriminant = Some(false),
+                        _ => {
+                            return Err(syn::Error::new(
+                                value_expr.span(),
+                                "`use_discriminant` accepts only `true` or `false`",
+                            ));
+                        }
+                    };
+                }
+
+                Ok(())
+            })?;
+        }
+    }
+    let has_explicit_discriminants = input
+        .variants
+        .iter()
+        .any(|variant| variant.discriminant.is_some());
+    if has_explicit_discriminants && use_discriminant.is_none() {
+        return Err(syn::Error::new(
+                input.ident.span(),
+                "You have to specify `#[borsh(use_discriminant=true)]` or `#[borsh(use_discriminant=false)]` for all enums with explicit discriminant",
+            ));
+    }
+    Ok(use_discriminant.unwrap_or(false))
+}
+
 pub(crate) fn contains_initialize_with(attrs: &[Attribute]) -> Option<Path> {
     for attr in attrs.iter() {
         if attr.path() == INIT {
@@ -134,7 +220,7 @@ pub(crate) fn collect_override_bounds(
 mod tests {
     use quote::{quote, ToTokens};
     use std::fmt::Write;
-    use syn::ItemStruct;
+    use syn::{Item, ItemStruct};
 
     use crate::attribute_helpers::parse_schema_attrs;
 
@@ -387,4 +473,96 @@ mod tests {
         let schema_params = parse_schema_attrs(&first_field.attrs).unwrap();
         assert!(schema_params.is_none());
     }
+
+    use super::*;
+    #[test]
+    fn test_check_use_discriminant() {
+        let item_enum: ItemEnum = syn::parse2(quote! {
+            #[derive(BorshDeserialize, Debug)]
+            #[borsh(use_discriminant = false)]
+            enum AWithUseDiscriminantFalse {
+                X,
+                Y,
+            }
+        })
+        .unwrap();
+        let actual = contains_use_discriminant(&item_enum);
+        assert!(!actual.unwrap());
+    }
+
+    #[test]
+    fn test_check_use_discriminant_true() {
+        let item_enum: ItemEnum = syn::parse2(quote! {
+            #[derive(BorshDeserialize, Debug)]
+            #[borsh(use_discriminant = true)]
+            enum AWithUseDiscriminantTrue {
+                X,
+                Y,
+            }
+        })
+        .unwrap();
+        let actual = contains_use_discriminant(&item_enum);
+        assert!(actual.unwrap());
+    }
+
+    #[test]
+    fn test_check_use_discriminant_wrong_value() {
+        let item_enum: ItemEnum = syn::parse2(quote! {
+            #[derive(BorshDeserialize, Debug)]
+            #[borsh(use_discriminant = 111)]
+            enum AWithUseDiscriminantFalse {
+                X,
+                Y,
+            }
+        })
+        .unwrap();
+        let actual = contains_use_discriminant(&item_enum);
+        let err = match actual {
+            Ok(..) => unreachable!("expecting error here"),
+            Err(err) => err,
+        };
+        insta::assert_debug_snapshot!(err);
+    }
+    #[test]
+    fn test_check_use_discriminant_on_struct() {
+        let item_enum: DeriveInput = syn::parse2(quote! {
+            #[derive(BorshDeserialize, Debug)]
+            #[borsh(use_discriminant = false)]
+            struct AWithUseDiscriminantFalse {
+                x: X,
+                y: Y,
+            }
+        })
+        .unwrap();
+        let actual = check_item_attributes(&item_enum);
+        insta::assert_snapshot!(actual.unwrap_err().to_token_stream().to_string());
+    }
+    #[test]
+    fn test_check_use_borsh_skip_on_whole_struct() {
+        let item_enum: DeriveInput = syn::parse2(quote! {
+            #[derive(BorshDeserialize, Debug)]
+            #[borsh_skip]
+            struct AWithUseDiscriminantFalse {
+                 x: X,
+                 y: Y,
+            }
+        })
+        .unwrap();
+        let actual = check_item_attributes(&item_enum);
+        insta::assert_snapshot!(actual.unwrap_err().to_token_stream().to_string());
+    }
+    #[test]
+    fn test_check_use_borsh_invalid_on_whole_struct() {
+        let item_enum: DeriveInput = syn::parse2(quote! {
+            #[derive(BorshDeserialize, Debug)]
+            #[borsh(invalid)]
+            enum AWithUseDiscriminantFalse {
+                X,
+                Y,
+            }
+        })
+        .unwrap();
+        let actual = check_item_attributes(&item_enum);
+        insta::assert_snapshot!(actual.unwrap_err().to_token_stream().to_string());
+    }
 }

borsh-derive-internal/src/enum_de.rs

@@ -1,14 +1,15 @@
-use proc_macro2::TokenStream as TokenStream2;
-use quote::quote;
-use syn::{Fields, Ident, ItemEnum, Path, WhereClause};
-
 use crate::{
     attribute_helpers::{
-        collect_override_bounds, contains_initialize_with, contains_skip, BoundType,
+        collect_override_bounds, contains_initialize_with, contains_skip,
+        contains_use_discriminant, BoundType,
     },
     enum_discriminant_map::discriminant_map,
     generics::{compute_predicates, without_defaults, FindTyParams},
 };
+use proc_macro2::TokenStream as TokenStream2;
+use quote::quote;
+use std::convert::TryFrom;
+use syn::{Fields, Ident, ItemEnum, Path, WhereClause};
 
 pub fn enum_de(input: &ItemEnum, cratename: Ident) -> syn::Result<TokenStream2> {
     let name = &input.ident;
@@ -27,9 +28,19 @@ pub fn enum_de(input: &ItemEnum, cratename: Ident) -> syn::Result<TokenStream2>
     let mut default_params_visitor = FindTyParams::new(&generics);
 
     let init_method = contains_initialize_with(&input.attrs);
+
+    let use_discriminant = contains_use_discriminant(input)?;
+
     let mut variant_arms = TokenStream2::new();
     let discriminants = discriminant_map(&input.variants);
-    for variant in input.variants.iter() {
+
+    for (variant_idx, variant) in input.variants.iter().enumerate() {
+        let variant_idx = u8::try_from(variant_idx).map_err(|err| {
+            syn::Error::new(
+                variant.ident.span(),
+                format!("up to 256 enum variants are supported. error{}", err),
+            )
+        })?;
         let variant_ident = &variant.ident;
         let discriminant = discriminants.get(variant_ident).unwrap();
         let mut variant_header = TokenStream2::new();
@@ -85,6 +96,11 @@ pub fn enum_de(input: &ItemEnum, cratename: Ident) -> syn::Result<TokenStream2>
             }
             Fields::Unit => {}
         }
+        let discriminant = if use_discriminant {
+            quote! { #discriminant }
+        } else {
+            quote! { #variant_idx }
+        };
         variant_arms.extend(quote! {
             if variant_tag == #discriminant { #name::#variant_ident #variant_header } else
         });
@@ -291,4 +307,41 @@ mod tests {
 
         insta::assert_snapshot!(pretty_print_syn_str(&actual).unwrap());
     }
+
+    #[test]
+    fn borsh_discriminant_false() {
+        let item_enum: ItemEnum = syn::parse2(quote! {
+           #[borsh(use_discriminant = false)]
+            enum X {
+                A,
+                B = 20,
+                C,
+                D,
+                E = 10,
+                F,
+            }
+        })
+        .unwrap();
+        let actual = enum_de(&item_enum, Ident::new("borsh", Span::call_site())).unwrap();
+
+        insta::assert_snapshot!(pretty_print_syn_str(&actual).unwrap());
+    }
+    #[test]
+    fn borsh_discriminant_true() {
+        let item_enum: ItemEnum = syn::parse2(quote! {
+            #[borsh(use_discriminant = true)]
+            enum X {
+                A,
+                B = 20,
+                C,
+                D,
+                E = 10,
+                F,
+            }
+        })
+        .unwrap();
+        let actual = enum_de(&item_enum, Ident::new("borsh", Span::call_site())).unwrap();
+
+        insta::assert_snapshot!(pretty_print_syn_str(&actual).unwrap());
+    }
 }

borsh-derive-internal/src/enum_discriminant_map.rs

@@ -8,14 +8,14 @@ use syn::{punctuated::Punctuated, token::Comma, Variant};
 /// See: https://doc.rust-lang.org/reference/items/enumerations.html#assigning-discriminant-values
 pub fn discriminant_map(variants: &Punctuated<Variant, Comma>) -> HashMap<Ident, TokenStream> {
     let mut map = HashMap::new();
-
     let mut next_discriminant_if_not_specified = quote! {0};
 
     for variant in variants {
         let this_discriminant = variant.discriminant.clone().map_or_else(
             || quote! { #next_discriminant_if_not_specified },
             |(_, e)| quote! { #e },
         );
+
         next_discriminant_if_not_specified = quote! { #this_discriminant + 1 };
         map.insert(variant.ident.clone(), this_discriminant);
     }