fix!: change Block identifier field type to Identifier (#107)

BREAKING CHANGE: The `Block` struct's `identifier` field type was changed from `String` to `Identifier`. Furthermore, the trait bound for the block identifier on `Block::new` changed from `Into<String>` to `Into<Identifier>`. This fixes a long standing issue. `Identifier` was introduced after `Block` and the field type wasn't updated yet.
martinohmann · Oct 26, 2022 · badce8a · badce8a
1 parent 84e1538
commit badce8a
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 27 deletions.
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
@@ -81,7 +81,7 @@ fn parse_attribute(pair: Pair<Rule>) -> Result<Attribute> {
 fn parse_block(pair: Pair<Rule>) -> Result<Block> {
     let mut pairs = pair.into_inner();
 
-    let identifier = parse_ident(pairs.next().unwrap()).into_inner();
+    let identifier = parse_ident(pairs.next().unwrap());
 
     let (labels, block_body): (Vec<Pair<Rule>>, Vec<Pair<Rule>>) =
         pairs.partition(|pair| pair.as_rule() != Rule::BlockBody);

diff --git a/src/structure/block.rs b/src/structure/block.rs
@@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize};
 #[serde(rename = "$hcl::block")]
 pub struct Block {
     /// The block identifier.
-    pub identifier: String,
+    pub identifier: Identifier,
     /// Zero or more block labels.
     pub labels: Vec<BlockLabel>,
     /// Represents the `Block`'s body.
@@ -28,7 +28,7 @@ impl Block {
     /// Creates a new `Block` from a block identifier, block labels and a block body.
     pub fn new<I, L, B>(identifier: I, labels: L, body: B) -> Block
     where
-        I: Into<String>,
+        I: Into<Identifier>,
         L: IntoIterator,
         L::Item: Into<BlockLabel>,
         B: IntoIterator,
@@ -45,7 +45,7 @@ impl Block {
     /// identifier.
     pub fn builder<I>(identifier: I) -> BlockBuilder
     where
-        I: Into<String>,
+        I: Into<Identifier>,
     {
         BlockBuilder::new(identifier)
     }
@@ -74,14 +74,14 @@ impl From<Block> for Value {
 
 impl<I, B> From<(I, B)> for Block
 where
-    I: Into<String>,
+    I: Into<Identifier>,
     B: Into<Body>,
 {
-    fn from(pair: (I, B)) -> Block {
+    fn from((ident, body): (I, B)) -> Block {
         Block {
-            identifier: pair.0.into(),
+            identifier: ident.into(),
             labels: Vec::new(),
-            body: pair.1.into(),
+            body: body.into(),
         }
     }
 }
@@ -174,7 +174,7 @@ where
 /// ```
 #[derive(Debug)]
 pub struct BlockBuilder {
-    identifier: String,
+    identifier: Identifier,
     labels: Vec<BlockLabel>,
     body: BodyBuilder,
 }
@@ -184,7 +184,7 @@ impl BlockBuilder {
     /// identifier.
     pub fn new<I>(identifier: I) -> BlockBuilder
     where
-        I: Into<String>,
+        I: Into<Identifier>,
     {
         BlockBuilder {
             identifier: identifier.into(),

diff --git a/src/structure/de.rs b/src/structure/de.rs
@@ -139,7 +139,7 @@ impl<'de> IntoDeserializer<'de, Error> for Block {
 }
 
 pub struct BlockAccess {
-    identifier: Option<String>,
+    identifier: Option<Identifier>,
     labels: Option<Vec<BlockLabel>>,
     body: Option<Body>,
 }

diff --git a/src/structure/mod.rs b/src/structure/mod.rs
@@ -388,7 +388,7 @@ impl IntoNodeMap for Block {
         let node = match labels.next() {
             Some(label) => {
                 let block = Block {
-                    identifier: label.into_inner(),
+                    identifier: Identifier::unchecked(label.into_inner()),
                     labels: labels.collect(),
                     body: self.body,
                 };
@@ -398,7 +398,7 @@ impl IntoNodeMap for Block {
             None => Node::BlockInner(vec![self.body]),
         };
 
-        Map::from_iter(std::iter::once((self.identifier, node)))
+        Map::from_iter(std::iter::once((self.identifier.into_inner(), node)))
     }
 }
 

diff --git a/src/structure/ser/block.rs b/src/structure/ser/block.rs
@@ -1,6 +1,6 @@
 use super::{
     body::BodySerializer, expression::ExpressionSerializer, structure::StructureSerializer,
-    IdentifierSerializer, SeqSerializer, StringSerializer,
+    IdentifierSerializer, SeqSerializer,
 };
 use crate::{Attribute, Block, BlockLabel, Body, Error, Identifier, Result, Structure};
 use serde::ser::{self, Impossible, Serialize};
@@ -38,7 +38,7 @@ impl ser::Serializer for BlockSerializer {
         T: ?Sized + Serialize,
     {
         Ok(Block {
-            identifier: variant.to_owned(),
+            identifier: Identifier::new(variant)?,
             labels: Vec::new(),
             body: value.serialize(BodySerializer)?,
         })
@@ -55,7 +55,7 @@ impl ser::Serializer for BlockSerializer {
         variant: &'static str,
         len: usize,
     ) -> Result<Self::SerializeTupleVariant> {
-        Ok(SerializeBlockVariant::new(variant, len))
+        Ok(SerializeBlockVariant::new(Identifier::new(variant)?, len))
     }
 
     fn serialize_map(self, _len: Option<usize>) -> Result<Self::SerializeMap> {
@@ -73,12 +73,12 @@ impl ser::Serializer for BlockSerializer {
         variant: &'static str,
         len: usize,
     ) -> Result<Self::SerializeStructVariant> {
-        Ok(SerializeBlockVariant::new(variant, len))
+        Ok(SerializeBlockVariant::new(Identifier::new(variant)?, len))
     }
 }
 
 pub struct SerializeBlockSeq {
-    identifier: Option<String>,
+    identifier: Option<Identifier>,
     labels: Option<Vec<BlockLabel>>,
     body: Option<Body>,
 }
@@ -102,7 +102,7 @@ impl ser::SerializeSeq for SerializeBlockSeq {
         T: ?Sized + ser::Serialize,
     {
         if self.identifier.is_none() {
-            self.identifier = Some(value.serialize(StringSerializer)?);
+            self.identifier = Some(value.serialize(IdentifierSerializer)?);
         } else if self.labels.is_none() {
             self.labels = Some(value.serialize(SeqSerializer::new(BlockLabelSerializer))?);
         } else if self.body.is_none() {
@@ -131,14 +131,14 @@ impl ser::SerializeTupleStruct for SerializeBlockSeq {
 }
 
 pub struct SerializeBlockVariant {
-    identifier: String,
+    identifier: Identifier,
     structures: Vec<Structure>,
 }
 
 impl SerializeBlockVariant {
-    pub fn new(variant: &'static str, len: usize) -> Self {
+    pub fn new(identifier: Identifier, len: usize) -> Self {
         SerializeBlockVariant {
-            identifier: variant.to_owned(),
+            identifier,
             structures: Vec::with_capacity(len),
         }
     }
@@ -169,6 +169,7 @@ impl ser::SerializeStructVariant for SerializeBlockVariant {
     where
         T: ?Sized + ser::Serialize,
     {
+        let key = Identifier::new(key)?;
         let expr = value.serialize(ExpressionSerializer)?;
         self.structures.push(Attribute::new(key, expr).into());
         Ok(())
@@ -184,7 +185,7 @@ impl ser::SerializeStructVariant for SerializeBlockVariant {
 }
 
 pub struct SerializeBlockMap {
-    identifier: Option<String>,
+    identifier: Option<Identifier>,
     body: Option<Body>,
 }
 
@@ -206,7 +207,7 @@ impl ser::SerializeMap for SerializeBlockMap {
         T: ?Sized + ser::Serialize,
     {
         if self.identifier.is_none() {
-            self.identifier = Some(key.serialize(StringSerializer)?);
+            self.identifier = Some(key.serialize(IdentifierSerializer)?);
             Ok(())
         } else {
             Err(ser::Error::custom("expected map with 1 entry"))
@@ -227,14 +228,18 @@ impl ser::SerializeMap for SerializeBlockMap {
 
     fn end(self) -> Result<Self::Ok> {
         match (self.identifier, self.body) {
-            (Some(ident), Some(body)) => Ok(Block::new(ident, Vec::<BlockLabel>::new(), body)),
+            (Some(identifier), Some(body)) => Ok(Block {
+                identifier,
+                labels: Vec::new(),
+                body,
+            }),
             (_, _) => Err(ser::Error::custom("expected map with 1 entry")),
         }
     }
 }
 
 pub struct SerializeBlockStruct {
-    identifier: Option<String>,
+    identifier: Option<Identifier>,
     labels: Option<Vec<BlockLabel>>,
     body: Option<Body>,
 }
@@ -258,7 +263,7 @@ impl ser::SerializeStruct for SerializeBlockStruct {
         T: ?Sized + ser::Serialize,
     {
         match key {
-            "identifier" => self.identifier = Some(value.serialize(StringSerializer)?),
+            "identifier" => self.identifier = Some(value.serialize(IdentifierSerializer)?),
             "labels" => {
                 self.labels = Some(value.serialize(SeqSerializer::new(BlockLabelSerializer))?)
             }