From e5f9e96729d47f394b28227e021600b611684c5d Mon Sep 17 00:00:00 2001 From: Sean Myers Date: Sun, 23 Oct 2022 14:01:37 -0700 Subject: [PATCH] Update lookup table to handle arbitrary types (#75) It turns out that cloudformation actually allows arbitrary types in Fn::Mappings. Within that, it's also not uniform types as well. This update handles the arbitrary types for non-array-based inputs and will "opt out" for non-array-based complex structures with an `any` mode, leaving it up to the user to resolve that problem in typescript. Also, in order to reuse f64 types, moved WrapperF64 to a primitives folder that spans the gap of parser and ir. --- src/ir/mappings.rs | 100 +++++++++++++++++++++- src/ir/resources.rs | 3 +- src/lib.rs | 1 + src/parser/lookup_table.rs | 22 ++++- src/parser/resource.rs | 34 +------- src/primitives/mod.rs | 34 ++++++++ src/synthesizer/typescript_synthesizer.rs | 15 +++- tests/tests.rs | 5 +- 8 files changed, 169 insertions(+), 45 deletions(-) create mode 100644 src/primitives/mod.rs diff --git a/src/ir/mappings.rs b/src/ir/mappings.rs index d9be841d..50a6d23f 100644 --- a/src/ir/mappings.rs +++ b/src/ir/mappings.rs @@ -1,3 +1,4 @@ +use crate::ir::mappings::OutputType::{Complex, Consistent}; use crate::parser::lookup_table::MappingInnerValue; use crate::CloudformationParseTree; use std::collections::HashMap; @@ -7,11 +8,34 @@ pub struct MappingInstruction { pub map: HashMap>, } +/// When printing out to a file, sometimes there are non ordinal types in mappings. +/// An example of this is something like: +/// { +/// "DisableScaleIn": true, +/// "ScaleInCooldown": 10 +/// } +/// +/// The above example has both a number and a bool. This is considered "Complex". +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum OutputType { + Consistent(MappingInnerValue), + Complex, +} + impl MappingInstruction { - pub fn find_first_type(&self) -> &MappingInnerValue { + pub fn output_type(&self) -> OutputType { let value = self.map.values().next().unwrap(); - let inner_value = value.values().next().unwrap(); - inner_value + let first_inner_value = value.values().next().unwrap(); + + for _outer_map in self.map.values() { + for inner_value in value.values() { + if std::mem::discriminant(inner_value) != std::mem::discriminant(first_inner_value) + { + return Complex; + } + } + } + Consistent(first_inner_value.clone()) } } pub fn translate(parse_tree: &CloudformationParseTree) -> Vec { @@ -25,3 +49,73 @@ pub fn translate(parse_tree: &CloudformationParseTree) -> Vec $value:expr),+ } => { + { + let mut m = ::std::collections::HashMap::new(); + $( + m.insert($key.to_string(), $value); + )+ + m + } + }; + ); + + #[test] + fn test_mapping_consistent_string() { + let mapping = MappingInstruction { + name: "TableMappings".into(), + map: map! { + "Table" => map!{ + "Key" => MappingInnerValue::String("Value".into()), + "Key2" => MappingInnerValue::String("Value2".into()) + } + }, + }; + + let actual_output = mapping.output_type(); + let expected_output = OutputType::Consistent(MappingInnerValue::String("Value".into())); + // In the end, we only care if the output is Consistent(string), not the value that is used. + assert_eq!( + std::mem::discriminant(&expected_output), + std::mem::discriminant(&actual_output) + ); + } + + #[test] + fn test_mapping_consistent_bool() { + let mapping = MappingInstruction { + name: "TableMappings".into(), + map: map! { + "Table" => map!{ + "DisableScaleIn" => MappingInnerValue::Bool(true) + } + }, + }; + + let actual_output = mapping.output_type(); + let expected_output = OutputType::Consistent(MappingInnerValue::Bool(true)); + assert_eq!(expected_output, actual_output); + } + + #[test] + fn test_mapping_complex() { + let mapping = MappingInstruction { + name: "TableMappings".into(), + map: map! { + "Table" => map!{ + "DisableScaleIn" => MappingInnerValue::Bool(true), + "Cooldown" => MappingInnerValue::Number(10) + } + }, + }; + + let actual_output = mapping.output_type(); + let expected_output = OutputType::Complex; + assert_eq!(expected_output, actual_output); + } +} diff --git a/src/ir/resources.rs b/src/ir/resources.rs index 67471b14..5c29ee2f 100644 --- a/src/ir/resources.rs +++ b/src/ir/resources.rs @@ -1,6 +1,7 @@ use crate::ir::reference::{Origin, Reference}; use crate::ir::sub::{sub_parse_tree, SubValue}; -use crate::parser::resource::{ResourceValue, WrapperF64}; +use crate::parser::resource::ResourceValue; +use crate::primitives::WrapperF64; use crate::specification::{spec, Complexity, SimpleType, Specification}; use crate::{CloudformationParseTree, TransmuteError}; use std::collections::HashMap; diff --git a/src/lib.rs b/src/lib.rs index b223a535..d41751fc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,6 +10,7 @@ use serde_json::Value; pub mod integrations; pub mod ir; pub mod parser; +pub mod primitives; pub mod specification; pub mod synthesizer; diff --git a/src/parser/lookup_table.rs b/src/parser/lookup_table.rs index 40834203..3a73e06e 100644 --- a/src/parser/lookup_table.rs +++ b/src/parser/lookup_table.rs @@ -1,3 +1,4 @@ +use crate::primitives::WrapperF64; use crate::TransmuteError; use serde_json::{Map, Value}; use std::collections::HashMap; @@ -49,12 +50,18 @@ impl MappingParseTree { /** * MappingInnerValue tracks the allowed value types in a Mapping as defined by CloudFormation in the - * link below. Right now that is either a String or List. + * link below. The values are allowed to only be a String or List: * * https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/mappings-section-structure.html#mappings-section-structure-syntax + * + * In reality, all values are allowed from the json specification. If we detect any other conflicting + * numbers, then the type becomes "Any" to allow for the strangeness. */ -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum MappingInnerValue { + Number(i64), + Float(WrapperF64), + Bool(bool), String(String), List(Vec), } @@ -68,6 +75,9 @@ impl Display for MappingInnerValue { list_val.iter().map(|val| format!("'{}'", val)).collect(); write!(f, "[{}]", quoted_list_values.join(",")) } + MappingInnerValue::Number(val) => write!(f, "{}", val), + MappingInnerValue::Float(val) => write!(f, "{}", val), + MappingInnerValue::Bool(val) => write!(f, "{}", val), }; } } @@ -143,7 +153,13 @@ fn ensure_object<'a>(name: &str, obj: &'a Value) -> Result<&'a Map Result { match obj { Value::String(x) => Ok(MappingInnerValue::String(x.to_string())), - Value::Number(x) => Ok(MappingInnerValue::String(x.to_string())), + Value::Number(x) => match x.is_f64() { + true => Ok(MappingInnerValue::Float(WrapperF64::new( + x.as_f64().unwrap(), + ))), + false => Ok(MappingInnerValue::Number(x.as_i64().unwrap())), + }, + Value::Bool(x) => Ok(MappingInnerValue::Bool(*x)), Value::Array(x) => Ok(MappingInnerValue::List(convert_to_string_vector(x, name)?)), _ => Err(TransmuteError { details: format!( diff --git a/src/parser/resource.rs b/src/parser/resource.rs index a57aaf32..cad4498f 100644 --- a/src/parser/resource.rs +++ b/src/parser/resource.rs @@ -1,8 +1,8 @@ +use crate::primitives::WrapperF64; use crate::TransmuteError; use numberkit::is_digit; use serde_json::{Map, Value}; use std::collections::HashMap; -use std::{f64, fmt}; #[derive(Debug, Eq, PartialEq)] pub enum ResourceValue { @@ -31,32 +31,6 @@ pub enum ResourceValue { impl ResourceValue {} -#[derive(Debug, Clone, Copy)] -pub struct WrapperF64 { - num: f64, -} - -impl WrapperF64 { - pub fn new(num: f64) -> WrapperF64 { - WrapperF64 { num } - } -} - -impl PartialEq for WrapperF64 { - fn eq(&self, other: &Self) -> bool { - // It's equal if the diff is very small - (self.num - other.num).abs() < 0.0000001 - } -} - -impl fmt::Display for WrapperF64 { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.num) - } -} - -impl Eq for WrapperF64 {} - #[derive(Debug, Eq, PartialEq)] pub struct ResourceParseTree { pub name: String, @@ -182,10 +156,8 @@ pub fn build_resources_recursively( if is_digit(n.to_string(), false) { return Ok(ResourceValue::Number(n.as_i64().unwrap())); } - let val = WrapperF64 { - num: n.as_f64().unwrap(), - }; - return Ok(ResourceValue::Double(val)); + let v = WrapperF64::new(n.as_f64().unwrap()); + return Ok(ResourceValue::Double(v)); } Value::Array(arr) => { let mut v = Vec::new(); diff --git a/src/primitives/mod.rs b/src/primitives/mod.rs new file mode 100644 index 00000000..ea31dd03 --- /dev/null +++ b/src/primitives/mod.rs @@ -0,0 +1,34 @@ +/** + * Primitives are for things that can be outside the scope of parsing and IR and used heavily across both + * Generally, attempt to keep this section to a minimu + * + */ +use std::fmt; + +/// WrapperF64 exists because compraisons and outputs into typescripts are annoying with the +/// default f64. Use this whenever referring to a floating point number in CFN standard. +#[derive(Debug, Clone, Copy)] +pub struct WrapperF64 { + num: f64, +} + +impl WrapperF64 { + pub fn new(num: f64) -> WrapperF64 { + WrapperF64 { num } + } +} + +impl PartialEq for WrapperF64 { + fn eq(&self, other: &Self) -> bool { + // It's equal if the diff is very small + (self.num - other.num).abs() < 0.0000001 + } +} + +impl fmt::Display for WrapperF64 { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.num) + } +} + +impl Eq for WrapperF64 {} diff --git a/src/synthesizer/typescript_synthesizer.rs b/src/synthesizer/typescript_synthesizer.rs index f86b80b4..474c39f4 100644 --- a/src/synthesizer/typescript_synthesizer.rs +++ b/src/synthesizer/typescript_synthesizer.rs @@ -1,5 +1,5 @@ use crate::ir::conditions::ConditionIr; -use crate::ir::mappings::MappingInstruction; +use crate::ir::mappings::{MappingInstruction, OutputType}; use crate::ir::resources::ResourceIr; use crate::ir::CloudformationProgramIr; use crate::parser::lookup_table::MappingInnerValue; @@ -55,9 +55,16 @@ impl TypescriptSynthesizer { append_with_newline(output, "\n\t\t// Mappings"); for mapping in ir.mappings.iter() { - let record_type = match mapping.find_first_type() { - MappingInnerValue::String(_) => "Record>", - MappingInnerValue::List(_) => "Record>>", + let record_type = match mapping.output_type() { + OutputType::Consistent(inner_type) => match inner_type { + MappingInnerValue::Number(_) | MappingInnerValue::Float(_) => { + "Record>" + } + MappingInnerValue::Bool(_) => "Record>", + MappingInnerValue::String(_) => "Record>", + MappingInnerValue::List(_) => "Record>>", + }, + OutputType::Complex => "Record>", }; append_with_newline( diff --git a/tests/tests.rs b/tests/tests.rs index 7f5c36d0..1e97ee1c 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,6 +1,5 @@ -use noctilucent::parser::resource::{ - build_resources, ResourceParseTree, ResourceValue, WrapperF64, -}; +use noctilucent::parser::resource::{build_resources, ResourceParseTree, ResourceValue}; +use noctilucent::primitives::WrapperF64; use serde_json::Value; macro_rules! map(