Repository: martinohmann/hcl-rs (rust)

Issue:
prefer formatting object keys as identifiers where possible
Right now this test fails because `a` gets quoted, although it is a valid identifier and the quotes could be omitted:

```rust
let value = hcl::value!({ foo = { a = "b", "with space" = "c" } });
let expected = "foo = {\n  a = \"b\"\n  \"with space\" = \"c\"\n}\n";
assert_eq!(hcl::to_string(&value).unwrap(), expected);
```

```
  left: `"foo = {\n  \"a\" = \"b\"\n  \"with space\" = \"c\"\n}\n"`
 right: `"foo = {\n  a = \"b\"\n  \"with space\" = \"c\"\n}\n"`
```

In formatted HCL, identifiers are used as object keys where possible. We could support this via a formatter options, e.g. `prefer_ident_keys` which is enabled by default, but can be disabled if desired.

Pull request:
feat(format): add `prefer_ident_keys` to `FormatterBuilder`
This allows to configure the formatting behaviour for object key strings that are valid HCL identifiers as well. This is not enabled by default.

Closes #132

Patch:
diff --git a/src/format/impls.rs b/src/format/impls.rs
index 82f174c..df8ef26 100644
--- a/src/format/impls.rs
+++ b/src/format/impls.rs
@@ -7,7 +7,7 @@ use crate::structure::{Attribute, Block, BlockLabel, Body, Structure};
 use crate::template::{
     Directive, Element, ForDirective, IfDirective, Interpolation, StripMode, Template,
 };
-use crate::util::is_templated;
+use crate::util::{is_ident, is_templated};
 use crate::{Identifier, Number, Result, Value};
 use std::io;
 
@@ -148,7 +148,7 @@ impl Format for Value {
             Value::Number(num) => num.format(fmt),
             Value::String(string) => string.format(fmt),
             Value::Array(array) => format_array(fmt, array.iter()),
-            Value::Object(object) => format_object(fmt, object.iter()),
+            Value::Object(object) => format_object(fmt, object.iter().map(|(k, v)| (StrKey(k), v))),
         }
     }
 }
@@ -173,11 +173,29 @@ impl Format for ObjectKey {
     {
         match self {
             ObjectKey::Identifier(ident) => ident.format(fmt),
+            ObjectKey::Expression(Expression::String(s)) => StrKey(s).format(fmt),
             ObjectKey::Expression(expr) => expr.format(fmt),
         }
     }
 }
 
+struct StrKey<'a>(&'a str);
+
+impl<'a> private::Sealed for StrKey<'a> {}
+
+impl<'a> Format for StrKey<'a> {
+    fn format<W>(&self, fmt: &mut Formatter<W>) -> Result<()>
+    where
+        W: io::Write,
+    {
+        if fmt.config.prefer_ident_keys && is_ident(self.0) {
+            fmt.write_string_fragment(self.0)
+        } else {
+            fmt.write_quoted_string(self.0, !is_templated(self.0))
+        }
+    }
+}
+
 impl private::Sealed for RawExpression {}
 
 impl Format for RawExpression {
diff --git a/src/format/mod.rs b/src/format/mod.rs
index 9de9088..e95293c 100644
--- a/src/format/mod.rs
+++ b/src/format/mod.rs
@@ -77,6 +77,7 @@ struct FormatConfig<'a> {
     dense: bool,
     compact_arrays: bool,
     compact_objects: bool,
+    prefer_ident_keys: bool,
 }
 
 impl<'a> Default for FormatConfig<'a> {
@@ -86,6 +87,7 @@ impl<'a> Default for FormatConfig<'a> {
             dense: false,
             compact_arrays: false,
             compact_objects: false,
+            prefer_ident_keys: false,
         }
     }
 }
@@ -249,6 +251,34 @@ impl<'a, W> FormatterBuilder<'a, W> {
         self
     }
 
+    /// Controls the object key quoting.
+    ///
+    /// By default, object keys are formatted as quoted strings (unless they are of variant
+    /// [`ObjectKey::Identifier`][ident-variant]).
+    ///
+    /// ```hcl
+    /// object = {
+    ///   "foo" = 1
+    ///   "bar baz" = 2
+    /// }
+    /// ```
+    ///
+    /// When identifier keys are preferred, object keys that are also valid HCL identifiers are
+    /// not quoted:
+    ///
+    /// ```hcl
+    /// object = {
+    ///   foo = 1
+    ///   "bar baz" = 2
+    /// }
+    /// ```
+    ///
+    /// [ident-variant]: crate::expr::ObjectKey::Identifier
+    pub fn prefer_ident_keys(mut self, yes: bool) -> Self {
+        self.config.prefer_ident_keys = yes;
+        self
+    }
+
     /// Consumes the `FormatterBuilder` and turns it into a `Formatter` which writes HCL to the
     /// provided writer.
     pub fn build(self, writer: W) -> Formatter<'a, W> {
diff --git a/src/ident.rs b/src/ident.rs
index ef11025..dc9f1b9 100644
--- a/src/ident.rs
+++ b/src/ident.rs
@@ -1,4 +1,5 @@
 use crate::expr::Variable;
+use crate::util::{is_id_continue, is_id_start, is_ident};
 use crate::{Error, Result};
 use serde::{Deserialize, Serialize};
 use std::borrow::{Borrow, Cow};
@@ -37,14 +38,7 @@ impl Identifier {
     {
         let ident = ident.into();
 
-        if ident.is_empty() {
-            return Err(Error::InvalidIdentifier(ident));
-        }
-
-        let mut chars = ident.chars();
-        let first = chars.next().unwrap();
-
-        if !is_id_start(first) || !chars.all(is_id_continue) {
+        if !is_ident(&ident) {
             return Err(Error::InvalidIdentifier(ident));
         }
 
@@ -130,16 +124,6 @@ impl Identifier {
     }
 }
 
-#[inline]
-fn is_id_start(ch: char) -> bool {
-    ch == '_' || unicode_ident::is_xid_start(ch)
-}
-
-#[inline]
-fn is_id_continue(ch: char) -> bool {
-    ch == '-' || unicode_ident::is_xid_continue(ch)
-}
-
 impl From<String> for Identifier {
     fn from(s: String) -> Self {
         Identifier::sanitized(s)
diff --git a/src/lib.rs b/src/lib.rs
index 92e394e..7264b36 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -16,6 +16,7 @@
     clippy::option_option,
     clippy::return_self_not_must_use,
     clippy::should_implement_trait,
+    clippy::struct_excessive_bools,
     clippy::unnecessary_wraps,
     clippy::wildcard_imports
 )]
diff --git a/src/util.rs b/src/util.rs
index b99114c..85162a7 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -132,6 +132,7 @@ pub fn try_unescape(s: &str) -> Cow<str> {
 ///
 /// This function only looks for start markers and does not check if the template is actually
 /// valid.
+#[inline]
 pub fn is_templated(s: &str) -> bool {
     if s.len() < 3 {
         return false;
@@ -162,6 +163,31 @@ pub fn is_templated(s: &str) -> bool {
     false
 }
 
+/// Determines if `ch` is a valid HCL identifier start character.
+#[inline]
+pub fn is_id_start(ch: char) -> bool {
+    ch == '_' || unicode_ident::is_xid_start(ch)
+}
+
+/// Determines if `ch` is a valid HCL identifier continue character.
+#[inline]
+pub fn is_id_continue(ch: char) -> bool {
+    ch == '-' || unicode_ident::is_xid_continue(ch)
+}
+
+/// Determines if `s` represents a valid HCL identifier.
+#[inline]
+pub fn is_ident(s: &str) -> bool {
+    if s.is_empty() {
+        return false;
+    }
+
+    let mut chars = s.chars();
+    let first = chars.next().unwrap();
+
+    is_id_start(first) && chars.all(is_id_continue)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;


Test patch:
diff --git a/src/format/tests.rs b/src/format/tests.rs
index caf1ed1..0aeb5dd 100644
--- a/src/format/tests.rs
+++ b/src/format/tests.rs
@@ -131,3 +131,27 @@ fn issue_131() {
 
     expect_format(value!({ a = "${\"b\"}" }), "{\n  \"a\" = \"${\"b\"}\"\n}");
 }
+
+#[test]
+fn prefer_ident_keys() {
+    let attr = Attribute::new(
+        "object",
+        expression!({
+            "foo" = 1
+            bar = 2
+            "baz qux" = 3
+        }),
+    );
+
+    expect_formatb(
+        |b| b.prefer_ident_keys(false),
+        &attr,
+        "object = {\n  \"foo\" = 1\n  bar = 2\n  \"baz qux\" = 3\n}\n",
+    );
+
+    expect_formatb(
+        |b| b.prefer_ident_keys(true),
+        &attr,
+        "object = {\n  foo = 1\n  bar = 2\n  \"baz qux\" = 3\n}\n",
+    );
+}