Repository: martinohmann/hcl-rs
Pull request #46 (2022-07-30 20:17:46)

fix: parse negate operations on numbers as negative numbers
The HCL spec does not specify negative numbers and represents them as
negate operations on positive integers and floats.

This change updates the parser to peek into unary operations and promote
negations on numbers to negative numbers instead. This behaviour will
most likely be kept even after adding the `Operation` variant to the
`Expression` type at one point.


Patch:
diff --git a/src/number.rs b/src/number.rs
index a097061..43851f3 100644
--- a/src/number.rs
+++ b/src/number.rs
@@ -1,5 +1,6 @@
 use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
 use std::fmt::{self, Display};
+use std::ops::Neg;
 
 /// Represents a HCL number.
 #[derive(Debug, PartialEq, Clone)]
@@ -176,3 +177,22 @@ impl<'de> Deserialize<'de> for Number {
         deserializer.deserialize_any(NumberVisitor)
     }
 }
+
+impl Neg for Number {
+    type Output = Number;
+
+    fn neg(self) -> Self::Output {
+        match self {
+            Number::PosInt(value) => Number::NegInt(-(value as i64)),
+            Number::NegInt(value) => {
+                let value = -value;
+                if value < 0 {
+                    Number::NegInt(value)
+                } else {
+                    Number::PosInt(value as u64)
+                }
+            }
+            Number::Float(value) => Number::Float(-value),
+        }
+    }
+}
diff --git a/src/parser/grammar/hcl.pest b/src/parser/grammar/hcl.pest
index ca301c0..24ea177 100644
--- a/src/parser/grammar/hcl.pest
+++ b/src/parser/grammar/hcl.pest
@@ -36,8 +36,8 @@ Null    =  { "null" }
 
 // Numeric literals
 NumericLit = _{ Float | Int }
-Float      = @{ "-"? ~ Decimal+ ~ (("." ~ Decimal+ ~ (ExpMark ~ Decimal+)?) | (ExpMark ~ Decimal+)) }
-Int        = @{ "-"? ~ Decimal+ }
+Float      = @{ Decimal+ ~ (("." ~ Decimal+ ~ (ExpMark ~ Decimal+)?) | (ExpMark ~ Decimal+)) }
+Int        = @{ Decimal+ }
 ExpMark    =  { ("e" | "E") ~ ("+" | "-")? }
 Decimal    =  { '0'..'9' }
 
@@ -127,7 +127,8 @@ FullSplat =  { "[*]" }
 
 // Operations
 Operation          = { UnaryOp | BinaryOp }
-UnaryOp            = { ("-" | "!") ~ Expression }
+UnaryOp            = { UnaryOperator ~ Expression }
+UnaryOperator      = { "-" | "!" }
 BinaryOp           = { ExprTerm ~ BinaryOperator ~ Expression }
 BinaryOperator     = { CompareOperator | ArithmeticOperator | LogicOperator }
 CompareOperator    = { "==" | "!=" | "<=" | ">=" | "<" | ">" }
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index bb05fb9..79af27d 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -3,8 +3,8 @@ mod tests;
 mod unescape;
 
 use crate::{
-    Attribute, Block, BlockLabel, Body, Expression, Object, ObjectKey, RawExpression, Result,
-    Structure,
+    Attribute, Block, BlockLabel, Body, Expression, Number, Object, ObjectKey, RawExpression,
+    Result, Structure,
 };
 use pest::{
     iterators::{Pair, Pairs},
@@ -115,13 +115,39 @@ fn parse_block_body(pair: Pair<Rule>) -> Result<Body> {
 }
 
 fn parse_expression(pair: Pair<Rule>) -> Result<Expression> {
-    if let Rule::ExprTerm = pair.as_rule() {
-        parse_expr_term(pair)
-    } else {
-        // @TODO(mohmann): Process Conditional and Operation at one point. This will require a
-        // PrecClimber to get precedence right though.
-        Ok(Expression::Raw(raw_expression(pair.as_str())))
+    // @TODO(mohmann): Process Conditional and Operation at one point. This will require a
+    // PrecClimber to get precedence right though.
+    match pair.as_rule() {
+        Rule::ExprTerm => parse_expr_term(pair),
+        Rule::Operation => {
+            let raw = pair.as_str();
+            // We only fully parse unary operations representing negative numbers for now.
+            // Everything else will be represented as raw expressions until parsing operations is
+            // fully implemented.
+            match parse_negative_number(inner(pair))? {
+                Some(num) => Ok(Expression::Number(num)),
+                None => Ok(Expression::Raw(raw_expression(raw))),
+            }
+        }
+        Rule::Conditional => Ok(Expression::Raw(raw_expression(pair.as_str()))),
+        rule => unexpected_rule(rule),
+    }
+}
+
+fn parse_negative_number(pair: Pair<Rule>) -> Result<Option<Number>> {
+    if let Rule::UnaryOp = pair.as_rule() {
+        let mut pairs = pair.into_inner();
+        let operator = pairs.next().unwrap();
+        let expr = pairs.next().unwrap();
+
+        if let ("-", Rule::ExprTerm) = (operator.as_str(), expr.as_rule()) {
+            if let Expression::Number(num) = parse_expr_term(expr)? {
+                return Ok(Some(-num));
+            }
+        }
     }
+
+    Ok(None)
 }
 
 fn parse_expressions(pair: Pair<Rule>) -> Result<Vec<Expression>> {


Test patch:
diff --git a/src/parser/tests.rs b/src/parser/tests.rs
index 4b57eb5..5e7bbca 100644
--- a/src/parser/tests.rs
+++ b/src/parser/tests.rs
@@ -32,10 +32,10 @@ fn parse_string() {
 fn parse_number() {
     parses_to! {
         parser: HclParser,
-        input: "-12e+10",
+        input: "12e+10",
         rule: Rule::NumericLit,
         tokens: [
-            Float(0, 7)
+            Float(0, 6)
         ]
     };
 
@@ -567,3 +567,22 @@ fn unescape_strings() {
 
     assert_eq!(body, expected);
 }
+
+#[test]
+fn negative_numbers() {
+    let input = r#"
+        float = -4.2
+        float_exp = -4.2e10
+        signed = -42
+    "#;
+
+    let body = parse(input).unwrap();
+
+    let expected = Body::builder()
+        .add_attribute(("float", -4.2f64))
+        .add_attribute(("float_exp", -4.2e10f64))
+        .add_attribute(("signed", -42))
+        .build();
+
+    assert_eq!(body, expected);
+}