Repository: sharksforarms/deku
Pull request #78 (2020-07-30 02:25:07)

Add `id` attribute to top-level enum
Fixes https://github.com/sharksforarms/deku/issues/77

sharksforarms: @wcampbell0x2a take a look and let me know if it meets your use-case
codecov[bot]: # [Codecov](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=h1) Report
> Merging [#78](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=desc) into [master](https://codecov.io/gh/sharksforarms/deku/commit/6f4fdf36acf9d18ca274cf690580639616319bb8&el=desc) will **decrease** coverage by `0.97%`.
> The diff coverage is `89.28%`.

[![Impacted file tree graph](https://codecov.io/gh/sharksforarms/deku/pull/78/graphs/tree.svg?width=650&height=150&src=pr&token=MFMETW02MZ)](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #78      +/-   ##
==========================================
- Coverage   86.83%   85.86%   -0.98%     
==========================================
  Files           8        8              
  Lines         904      941      +37     
==========================================
+ Hits          785      808      +23     
- Misses        119      133      +14     
```


| [Impacted Files](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [deku-derive/src/macros/deku\_read.rs](https://codecov.io/gh/sharksforarms/deku/pull/78/diff?src=pr&el=tree#diff-ZGVrdS1kZXJpdmUvc3JjL21hY3Jvcy9kZWt1X3JlYWQucnM=) | `98.18% <85.71%> (-0.86%)` | :arrow_down: |
| [deku-derive/src/macros/deku\_write.rs](https://codecov.io/gh/sharksforarms/deku/pull/78/diff?src=pr&el=tree#diff-ZGVrdS1kZXJpdmUvc3JjL21hY3Jvcy9kZWt1X3dyaXRlLnJz) | `98.16% <90.00%> (-0.88%)` | :arrow_down: |
| [deku-derive/src/lib.rs](https://codecov.io/gh/sharksforarms/deku/pull/78/diff?src=pr&el=tree#diff-ZGVrdS1kZXJpdmUvc3JjL2xpYi5ycw==) | `75.22% <90.90%> (+1.51%)` | :arrow_up: |
| [src/error.rs](https://codecov.io/gh/sharksforarms/deku/pull/78/diff?src=pr&el=tree#diff-c3JjL2Vycm9yLnJz) | `20.68% <0.00%> (-10.89%)` | :arrow_down: |
| [src/lib.rs](https://codecov.io/gh/sharksforarms/deku/pull/78/diff?src=pr&el=tree#diff-c3JjL2xpYi5ycw==) | `93.48% <0.00%> (-0.85%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=footer). Last update [6f4fdf3...057c59b](https://codecov.io/gh/sharksforarms/deku/pull/78?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

wcampbell0x2a: > @wcampbell0x2a take a look and let me know if it meets your use-case

Works beautifully 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2431155..fcdff53 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## [Unreleased]
 
+- Added `id` attribute to top-level enums which allows to specify the enum id,
+for example a value coming via a context variable
+
 ## [0.7.0] - 2020-07-28
 
 - Added `cond` attribute which allows for conditional parsing or skipping of a field
diff --git a/deku-derive/src/lib.rs b/deku-derive/src/lib.rs
index 1d4342f..ee1ed95 100644
--- a/deku-derive/src/lib.rs
+++ b/deku-derive/src/lib.rs
@@ -20,6 +20,9 @@ struct DekuData {
     /// top-level context, argument list
     ctx: Option<syn::punctuated::Punctuated<syn::FnArg, syn::token::Comma>>,
 
+    /// enum only: `id` value
+    id: Option<TokenStream>,
+
     /// enum only: type of the enum `id`
     id_type: Option<syn::Ident>,
 
@@ -68,6 +71,7 @@ impl DekuData {
             data,
             endian: receiver.endian,
             ctx,
+            id: receiver.id,
             id_type: receiver.id_type,
             id_bits,
         })
@@ -83,6 +87,8 @@ impl DekuData {
                 // Validate id_* attributes are being used on an enum
                 if receiver.id_type.is_some() {
                     Err((receiver.id_type.span(), "`id_type` only supported on enum"))
+                } else if receiver.id.is_some() {
+                    Err((receiver.id.span(), "`id` only supported on enum"))
                 } else if receiver.id_bytes.is_some() {
                     Err((
                         receiver.id_bytes.span(),
@@ -95,16 +101,27 @@ impl DekuData {
                 }
             }
             ast::Data::Enum(_) => {
-                // Validate `id_type` is specified
-                if receiver.id_type.is_none() {
-                    return Err((receiver.ident.span(), "`id_type` must be specified on enum"));
+                // Validate `id_type` or `id` is specified
+                if receiver.id_type.is_none() && receiver.id.is_none() {
+                    return Err((
+                        receiver.ident.span(),
+                        "`id_type` or `id` must be specified on enum",
+                    ));
+                }
+
+                // Validate either `id_type` or `id` is specified
+                if receiver.id_type.is_some() && receiver.id.is_some() {
+                    return Err((
+                        receiver.ident.span(),
+                        "conflicting: both `id_type` and `id` specified on enum",
+                    ));
                 }
 
                 // Validate either `id_bits` or `id_bytes` is specified
                 if receiver.id_bits.is_some() && receiver.id_bytes.is_some() {
                     return Err((
                         receiver.id_bits.span(),
-                        "conflicting: both \"id_bits\" and \"id_bytes\" specified on field",
+                        "conflicting: both `id_bits` and `id_bytes` specified on enum",
                     ));
                 }
 
@@ -229,7 +246,7 @@ impl FieldData {
             // FIXME: Ideally we need to use `Span::join` to encompass `bits` and `bytes` together.
             return Err((
                 receiver.bits.span(),
-                "conflicting: both \"bits\" and \"bytes\" specified on field",
+                "conflicting: both `bits` and `bytes` specified on field",
             ));
         }
 
@@ -305,7 +322,7 @@ impl VariantData {
             */
             return Err((
                 receiver.id.span(),
-                "conflicting: both \"id\" and \"id_pat\" specified on variant",
+                "conflicting: both `id` and `id_pat` specified on variant",
             ));
         }
 
@@ -333,6 +350,10 @@ struct DekuReceiver {
     #[darling(default)]
     ctx: Option<syn::LitStr>,
 
+    /// enum only: `id` value
+    #[darling(default, map = "option_as_tokenstream")]
+    id: Option<TokenStream>,
+
     /// enum only: type of the enum `id`
     #[darling(default)]
     id_type: Option<syn::Ident>,
diff --git a/deku-derive/src/macros/deku_read.rs b/deku-derive/src/macros/deku_read.rs
index 5bea949..1a73112 100644
--- a/deku-derive/src/macros/deku_read.rs
+++ b/deku-derive/src/macros/deku_read.rs
@@ -104,8 +104,8 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
     let ident = quote! { #ident #ty };
     let ident_as_string = ident.to_string();
 
-    // checked in `DekuData::validate`
-    let id_type = input.id_type.as_ref().unwrap();
+    let id = input.id.as_ref();
+    let id_type = input.id_type.as_ref();
 
     let id_args = gen_id_args(input.endian.as_ref(), input.id_bits)?;
 
@@ -180,8 +180,21 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
         });
     }
 
+    let variant_id_read = if id.is_some() {
+        quote! {
+            let (new_rest, variant_id) = (rest, #id);
+        }
+    } else if id_type.is_some() {
+        quote! {
+            let (new_rest, variant_id) = #id_type::read(rest, (#id_args))?;
+        }
+    } else {
+        // either `id` or `id_type` needs to be specified
+        unreachable!();
+    };
+
     let variant_read = quote! {
-        let (new_rest, variant_id) = #id_type::read(rest, (#id_args))?;
+        #variant_id_read
 
         let value = match variant_id {
             #(#variant_matches),*
diff --git a/deku-derive/src/macros/deku_write.rs b/deku-derive/src/macros/deku_write.rs
index b4991bc..71b5d55 100644
--- a/deku-derive/src/macros/deku_write.rs
+++ b/deku-derive/src/macros/deku_write.rs
@@ -115,8 +115,8 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
     let ident = &input.ident;
     let ident = quote! { #ident #ty };
 
-    // checked in `DekuData::validate`
-    let id_type = input.id_type.as_ref().unwrap();
+    let id = input.id.as_ref();
+    let id_type = input.id_type.as_ref();
 
     let id_args = gen_id_args(input.endian.as_ref(), input.id_bits)?;
 
@@ -146,16 +146,27 @@ fn emit_enum(input: &DekuData) -> Result<TokenStream, syn::Error> {
             .map(|(i, f)| f.get_ident(i, true))
             .collect::<Vec<_>>();
 
-        let variant_id_write = if let Some(variant_id) = &variant.id {
-            let variant_id: TokenStream = variant_id.clone();
-
+        let variant_id_write = if id.is_some() {
             quote! {
+                // if we don't do this we may get a "unused variable" error if passed via `ctx`
+                // i.e. #[deku(ctx = "my_id: u8", id = "my_id")]
+                let _ = #id;
+            }
+        } else if id_type.is_some() {
+            if let Some(variant_id) = &variant.id {
+                let variant_id: TokenStream = variant_id.clone();
+
+                quote! {
                     let mut variant_id: #id_type = #variant_id;
                     let bits = variant_id.write((#id_args))?;
                     acc.extend(bits);
+                }
+            } else {
+                quote! {}
             }
         } else {
-            quote! {}
+            // either `id` or `id_type` needs to be specified
+            unreachable!();
         };
 
         let variant_match = super::gen_enum_init(variant_is_named, variant_ident, field_idents);
diff --git a/src/attributes.rs b/src/attributes.rs
index 0f30068..eddef8a 100644
--- a/src/attributes.rs
+++ b/src/attributes.rs
@@ -16,11 +16,11 @@ A documentation-only module for #[deku] attributes
 | [map](#map) | field | Apply a function over the result of reading
 | [reader](#readerwriter) | variant, field | Custom reader code
 | [writer](#readerwriter) | variant, field | Custom writer code
-| [id](#id) | variant | variant id value, paired with `id_type`
-| [id_pat](#id_pat) | variant | variant id match pattern, paired with `id_type`
-| [id_type](#id_type) | top-level (enum only) | Set the type of the variant `id`
-| [id_bits](#id_bits) | top-level (enum only) | Set the bit-size of the variant `id`
-| [id_bytes](#id_bytes) | top-level (enum only) | Set the byte-size of the variant `id`
+| enum: [id](#id) | top-level, variant | enum or variant id value
+| enum: [id_pat](#id_pat) | variant | variant id match pattern
+| enum: [id_type](#id_type) | top-level | Set the type of the variant `id`
+| enum: [id_bits](#id_bits) | top-level | Set the bit-size of the variant `id`
+| enum: [id_bytes](#id_bytes) | top-level | Set the byte-size of the variant `id`
 | [ctx](#top_level_ctx) | top-level | Context argument list for context sensitive parsing
 | [ctx](#field_level_ctx) | field | Context arguments to pass to field
 
@@ -381,7 +381,54 @@ assert_eq!(data, value);
 
 # id
 
+## id (top-level)
+
+Specify the enum id
+
+This is useful in cases when the enum `id` is already consumed or is given externally
+
+Example:
+
+```rust
+# use deku::prelude::*;
+# use std::convert::{TryInto, TryFrom};
+#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+pub struct DekuTest {
+    pub my_id: u8,
+    pub data: u8,
+    #[deku(ctx = "*my_id")]
+    pub enum_from_id: MyEnum,
+}
+
+#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+#[deku(ctx = "my_id: u8", id = "my_id")]
+pub enum MyEnum {
+    #[deku(id = "1")]
+    VariantA(u8),
+    #[deku(id = "2")]
+    VariantB,
+}
+
+let data: Vec<u8> = vec![0x01_u8, 0xff, 0xab];
+let ret_read = DekuTest::try_from(data.as_ref()).unwrap();
+
+assert_eq!(
+    DekuTest {
+        my_id: 0x01,
+        data: 0xff,
+        enum_from_id: MyEnum::VariantA(0xab),
+    },
+    ret_read
+);
+
+let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+assert_eq!(ret_write, data)
+```
+
+## id (variant)
+
 Specify the identifier of the enum variant, must be paired with [id_type](#id_type)
+or [id (top-level)](#id-top-level)
 
 **Note**: If no `id` is specified, the variant is treated as the "catch-all".
 
@@ -473,7 +520,7 @@ assert_eq!(data, variant_bytes);
 
 # id_type
 
-Specify the type of the enum variant id, see [example](#id)
+Specify the type of the enum variant id to consume, see [example](#id-variant)
 
 # id_bits
 


Test patch:
diff --git a/tests/macro_read/bits_bytes_conflict.stderr b/tests/macro_read/bits_bytes_conflict.stderr
index 3647fca..ca5651e 100644
--- a/tests/macro_read/bits_bytes_conflict.stderr
+++ b/tests/macro_read/bits_bytes_conflict.stderr
@@ -1,4 +1,4 @@
-error: conflicting: both "id_bits" and "id_bytes" specified on field
+error: conflicting: both `id_bits` and `id_bytes` specified on enum
  --> $DIR/bits_bytes_conflict.rs:3:10
   |
 3 | #[derive(DekuRead)]
@@ -6,7 +6,7 @@ error: conflicting: both "id_bits" and "id_bytes" specified on field
   |
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: conflicting: both "bits" and "bytes" specified on field
+error: conflicting: both `bits` and `bytes` specified on field
  --> $DIR/bits_bytes_conflict.rs:7:10
   |
 7 | #[derive(DekuRead)]
@@ -14,7 +14,7 @@ error: conflicting: both "bits" and "bytes" specified on field
   |
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: conflicting: both "bits" and "bytes" specified on field
+error: conflicting: both `bits` and `bytes` specified on field
   --> $DIR/bits_bytes_conflict.rs:17:10
    |
 17 | #[derive(DekuRead)]
@@ -22,7 +22,7 @@ error: conflicting: both "bits" and "bytes" specified on field
    |
    = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: conflicting: both "bits" and "bytes" specified on field
+error: conflicting: both `bits` and `bytes` specified on field
   --> $DIR/bits_bytes_conflict.rs:23:10
    |
 23 | #[derive(DekuRead)]
diff --git a/tests/macro_read/enum_validation.rs b/tests/macro_read/enum_validation.rs
new file mode 100644
index 0000000..ed1a084
--- /dev/null
+++ b/tests/macro_read/enum_validation.rs
@@ -0,0 +1,47 @@
+use deku::prelude::*;
+
+// test required attributes
+#[derive(DekuRead)]
+enum Test1 {}
+
+// test conflict `id_type` and `id`
+#[derive(DekuRead)]
+#[deku(id_type = "u8", id = "test")]
+enum Test2 {}
+
+// test conflict `id` and `id_pat`
+#[derive(DekuRead)]
+#[deku(id_type = "u8")]
+enum Test3 {
+    #[deku(id = "1", id_pat = "2..=3")] A(u8),
+}
+
+// test `id_type` only allowed on enum
+#[derive(DekuRead)]
+#[deku(id_type = "u8")]
+struct Test4 {
+    a: u8
+}
+
+// test `id_bits` only allowed on enum
+#[derive(DekuRead)]
+#[deku(id_bits = "1")]
+struct Test5 {
+    a: u8
+}
+
+// test `id_bytes` only allowed on enum
+#[derive(DekuRead)]
+#[deku(id_bits = "1")]
+struct Test6 {
+    a: u8
+}
+
+// test `id` only allowed on enum
+#[derive(DekuRead)]
+#[deku(id = "test")]
+struct Test7 {
+    a: u8
+}
+
+fn main() {}
diff --git a/tests/macro_read/enum_validation.stderr b/tests/macro_read/enum_validation.stderr
new file mode 100644
index 0000000..96c1bc8
--- /dev/null
+++ b/tests/macro_read/enum_validation.stderr
@@ -0,0 +1,45 @@
+error: `id_type` or `id` must be specified on enum
+ --> $DIR/enum_validation.rs:5:6
+  |
+5 | enum Test1 {}
+  |      ^^^^^
+
+error: conflicting: both `id_type` and `id` specified on enum
+  --> $DIR/enum_validation.rs:10:6
+   |
+10 | enum Test2 {}
+   |      ^^^^^
+
+error: conflicting: both `id` and `id_pat` specified on variant
+  --> $DIR/enum_validation.rs:16:17
+   |
+16 |     #[deku(id = "1", id_pat = "2..=3")] A(u8),
+   |                 ^^^
+
+error: `id_type` only supported on enum
+  --> $DIR/enum_validation.rs:21:18
+   |
+21 | #[deku(id_type = "u8")]
+   |                  ^^^^
+
+error: `id_bits` only supported on enum
+  --> $DIR/enum_validation.rs:27:10
+   |
+27 | #[derive(DekuRead)]
+   |          ^^^^^^^^
+   |
+   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: `id_bits` only supported on enum
+  --> $DIR/enum_validation.rs:34:10
+   |
+34 | #[derive(DekuRead)]
+   |          ^^^^^^^^
+   |
+   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: `id` only supported on enum
+  --> $DIR/enum_validation.rs:42:13
+   |
+42 | #[deku(id = "test")]
+   |             ^^^^^^
diff --git a/tests/macro_read/enum_without_id_type.rs b/tests/macro_read/enum_without_id_type.rs
deleted file mode 100644
index 5bab571..0000000
--- a/tests/macro_read/enum_without_id_type.rs
+++ /dev/null
@@ -1,6 +0,0 @@
-use deku::prelude::*;
-
-#[derive(DekuRead)]
-enum Test {}
-
-fn main() {}
diff --git a/tests/macro_read/enum_without_id_type.stderr b/tests/macro_read/enum_without_id_type.stderr
deleted file mode 100644
index f4560e8..0000000
--- a/tests/macro_read/enum_without_id_type.stderr
+++ /dev/null
@@ -1,5 +0,0 @@
-error: `id_type` must be specified on enum
- --> $DIR/enum_without_id_type.rs:4:6
-  |
-4 | enum Test {}
-  |      ^^^^
diff --git a/tests/macro_read/variant_id_id_pat_conflict.rs b/tests/macro_read/variant_id_id_pat_conflict.rs
deleted file mode 100644
index 31db7a0..0000000
--- a/tests/macro_read/variant_id_id_pat_conflict.rs
+++ /dev/null
@@ -1,9 +0,0 @@
-use deku::prelude::*;
-
-#[derive(DekuRead)]
-#[deku(id_type = "u8")]
-enum Test1 {
-    #[deku(id = "1", id_pat = "2..=3")] A(u8),
-}
-
-fn main() {}
diff --git a/tests/macro_read/variant_id_id_pat_conflict.stderr b/tests/macro_read/variant_id_id_pat_conflict.stderr
deleted file mode 100644
index 5e4ac1f..0000000
--- a/tests/macro_read/variant_id_id_pat_conflict.stderr
+++ /dev/null
@@ -1,5 +0,0 @@
-error: conflicting: both "id" and "id_pat" specified on variant
- --> $DIR/variant_id_id_pat_conflict.rs:6:17
-  |
-6 |     #[deku(id = "1", id_pat = "2..=3")] A(u8),
-  |                 ^^^
diff --git a/tests/test_macro.rs b/tests/test_macro.rs
index 32b161b..5bba1ad 100644
--- a/tests/test_macro.rs
+++ b/tests/test_macro.rs
@@ -196,6 +196,23 @@ pub mod samples {
             c: SubTypeNeedCtx,
         },
     }
+
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    pub struct StructEnumId {
+        pub my_id: u8,
+        pub data: u8,
+        #[deku(ctx = "*my_id")]
+        pub enum_from_id: EnumId,
+    }
+
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    #[deku(ctx = "my_id: u8", id = "my_id")]
+    pub enum EnumId {
+        #[deku(id = "1")]
+        VarA(u8),
+        #[deku(id = "2")]
+        VarB,
+    }
 }
 
 #[test]
@@ -500,6 +517,24 @@ fn test_top_level_ctx_enum() {
     assert_eq!(ret_write.into_vec(), &test_data[..]);
 }
 
+#[test]
+fn test_struct_enum_ctx_id() {
+    let test_data = [0x01_u8, 0xff, 0xab];
+    let ret_read = samples::StructEnumId::try_from(test_data.as_ref()).unwrap();
+
+    assert_eq!(
+        samples::StructEnumId {
+            my_id: 0x01,
+            data: 0xff,
+            enum_from_id: samples::EnumId::VarA(0xab),
+        },
+        ret_read
+    );
+
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!(ret_write, test_data)
+}
+
 #[test]
 fn test_compile() {
     let t = trybuild::TestCases::new();