feat: track reference origin nullability (#129)

Allows emitting optional-chained references when the origin is conditional, so as to avoid the TypeScript compiler complaining about possibly null dereference.

.github/codecov.yml

@@ -0,0 +1,41 @@
+coverage:
+  precision: 1
+  round: nearest
+  status:
+    project:
+      default:
+        target: auto
+        threshold: 2% # Allow coverage to drop slightly
+        removed_code_behavior: adjust_base
+    patch:
+      default:
+        target: auto
+        threshold: 5% # Allow coverage to drop slightly
+        removed_code_behavior: adjust_base
+
+comment:
+  layout: header, components, diff, files, footer
+  behavior: new
+
+component_management:
+  individual_components:
+    - component_id: parser
+      name: Parser
+      paths:
+        - src/parser/**
+    - component_id: ir
+      name: Intermediate Representation
+      paths:
+        - src/ir/**
+    - component_id: synthesizer
+      name: Synthesizers
+      paths:
+        - src/synthesizer/**
+    - component_id: other
+      name: Other
+      paths:
+        - src/errors/**
+        - src/integrations/**
+        - src/primitives/**
+        - src/specification/**
+        - src/*.rs

.github/workflows/pr-actions.yml

@@ -41,12 +41,13 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-          components: llvm-tools-preview
       - name: Tests with Coverage
         run: bash tasks/coverage.sh
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
         with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-          files: target/coverage/lcov.info
+          directory: target/coverage
           fail_ci_if_error: true
+          gcov: true
+          gcov_ignore: "src/main.rs,src/**/tests.rs,tests/**"
+          token: ${{ secrets.CODECOV_TOKEN }}

Cargo.toml

@@ -38,4 +38,7 @@ opt-level = 3
 
 [profile.coverage]
 inherits = "test"
+codegen-units = 1
 incremental = false
+opt-level = 0
+panic = "abort"

src/errors/mod.rs

@@ -16,12 +16,14 @@ impl TransmuteError {
 }
 
 impl From<serde_yaml::Error> for TransmuteError {
+    #[inline]
     fn from(val: serde_yaml::Error) -> Self {
         TransmuteError::new(val)
     }
 }
 
 impl fmt::Display for TransmuteError {
+    #[inline]
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "TransmuteError: {}", self.details)
     }

src/ir/conditions.rs

@@ -52,6 +52,7 @@ pub enum ConditionIr {
 }
 
 impl ConditionIr {
+    #[inline]
     pub fn is_simple(&self) -> bool {
         matches!(self, Self::Str(_) | Self::Ref(_))
     }

src/ir/mod.rs

@@ -69,12 +69,14 @@ impl ReferenceOrigins {
                 .map(|(name, _)| (name.clone(), Origin::Parameter)),
         );
 
-        origins.extend(
-            parse_tree
-                .resources
-                .iter()
-                .map(|(name, _)| (name.clone(), Origin::LogicalId)),
-        );
+        origins.extend(parse_tree.resources.iter().map(|(name, res)| {
+            (
+                name.clone(),
+                Origin::LogicalId {
+                    conditional: res.condition.is_some(),
+                },
+            )
+        }));
 
         Self { origins }
     }
@@ -86,4 +88,13 @@ impl ReferenceOrigins {
             self.origins.get(ref_name).cloned()
         }
     }
+
+    fn is_conditional(&self, logical_id: &str) -> bool {
+        self.for_ref(logical_id)
+            .map(|orig| match orig {
+                Origin::LogicalId { conditional } => conditional,
+                _ => false,
+            })
+            .unwrap_or(false)
+    }
 }

src/ir/reference.rs

@@ -18,7 +18,11 @@ impl Reference {
             Origin::Parameter => {
                 format!("props.{}", camel_case(&self.name))
             }
-            Origin::LogicalId => format!("{}.ref", camel_case(&self.name)),
+            Origin::LogicalId { conditional } => format!(
+                "{var}{chain}ref",
+                var = camel_case(&self.name),
+                chain = if *conditional { "?." } else { "." }
+            ),
             Origin::Condition => camel_case(&self.name),
             Origin::PseudoParameter(x) => match x {
                 PseudoParameter::Partition => String::from("this.partition"),
@@ -29,7 +33,15 @@ impl Reference {
                 PseudoParameter::AccountId => String::from("this.account"),
                 PseudoParameter::NotificationArns => String::from("this.notificationArns"),
             },
-            Origin::GetAttribute(x) => format!("{}.attr{}", camel_case(&self.name), pascal_case(x)),
+            Origin::GetAttribute {
+                conditional,
+                attribute,
+            } => format!(
+                "{var_name}{chain}attr{name}",
+                var_name = camel_case(&self.name),
+                chain = if *conditional { "?." } else { "." },
+                name = pascal_case(attribute)
+            ),
         }
     }
 }
@@ -38,10 +50,14 @@ impl Reference {
 #[derive(Debug, Clone, PartialEq)]
 pub enum Origin {
     Parameter,
-    LogicalId,
+    LogicalId {
+        conditional: bool,
+    },
     Condition,
-    // String here is the attribute
-    GetAttribute(String),
+    GetAttribute {
+        attribute: String,
+        conditional: bool,
+    },
     PseudoParameter(PseudoParameter),
 }
 

src/ir/resources.rs

@@ -200,7 +200,10 @@ impl<'t> ResourceTranslator<'t> {
                         attribute_name,
                     } => Ok(ResourceIr::Ref(Reference::new(
                         &logical_name,
-                        Origin::GetAttribute(attribute_name),
+                        Origin::GetAttribute {
+                            attribute: attribute_name,
+                            conditional: self.origins.is_conditional(&logical_name),
+                        },
                     ))),
                     IntrinsicFunction::If {
                         condition_name,
@@ -293,9 +296,15 @@ impl<'t> ResourceTranslator<'t> {
         if let Some(origin) = self.origins.for_ref(x) {
             Reference::new(x, origin)
         } else if let Some((name, attribute)) = x.split_once('.') {
-            Reference::new(name, Origin::GetAttribute(attribute.into()))
+            Reference::new(
+                name,
+                Origin::GetAttribute {
+                    attribute: attribute.into(),
+                    conditional: self.origins.is_conditional(name),
+                },
+            )
         } else {
-            Reference::new(x, Origin::LogicalId)
+            Reference::new(x, Origin::LogicalId { conditional: false })
         }
     }
 
@@ -503,8 +512,8 @@ fn find_references(resource: &ResourceIr) -> Option<Vec<String>> {
         ResourceIr::Split(_, ir) => find_references(ir),
         ResourceIr::Ref(x) => match x.origin {
             Origin::Parameter | Origin::Condition | Origin::PseudoParameter(_) => None,
-            Origin::LogicalId => Some(vec![x.name.to_string()]),
-            Origin::GetAttribute(_) => Some(vec![x.name.to_string()]),
+            Origin::LogicalId { .. } => Some(vec![x.name.to_string()]),
+            Origin::GetAttribute { .. } => Some(vec![x.name.to_string()]),
         },
         ResourceIr::Sub(arr) => {
             let mut v = Vec::new();
@@ -580,10 +589,10 @@ fn find_dependencies(
         ResourceIr::Split(_, ir) => find_dependencies(resource_name, ir, topo),
         ResourceIr::Ref(x) => match x.origin {
             Origin::Parameter | Origin::Condition | Origin::PseudoParameter(_) => {}
-            Origin::LogicalId => {
+            Origin::LogicalId { .. } => {
                 topo.add_dependency(x.name.to_string(), resource_name.to_string());
             }
-            Origin::GetAttribute(_) => {
+            Origin::GetAttribute { .. } => {
                 topo.add_dependency(x.name.to_string(), resource_name.to_string());
             }
         },
@@ -647,7 +656,10 @@ mod tests {
             referrers: HashSet::default(),
             properties: create_property(
                 "something",
-                ResourceIr::Ref(Reference::new("A", Origin::LogicalId)),
+                ResourceIr::Ref(Reference::new(
+                    "A",
+                    Origin::LogicalId { conditional: false },
+                )),
             ),
         };
 
@@ -670,7 +682,10 @@ mod tests {
             referrers: HashSet::default(),
             properties: create_property(
                 "something",
-                ResourceIr::Ref(Reference::new("bar", Origin::LogicalId)),
+                ResourceIr::Ref(Reference::new(
+                    "bar",
+                    Origin::LogicalId { conditional: false },
+                )),
             ),
         };
 

tasks/coverage.sh

@@ -10,16 +10,11 @@ if ! command -v grcov >/dev/null; then
   cargo install grcov
 fi
 
-if ! rustup component list --installed | grep -e '^llvm-tools' >/dev/null; then
-  echo 'Installing the llvm-tools rustup component...'
-  rustup component add llvm-tools
-fi
-
 # We trap EXIT to collect coverage & clean-up profraw files...
 function after_tests(){
   echo 'Generating coverage reports...'
-  grcov "${COVERAGE_ROOT}/profraw"                                              \
-    --binary-path "${COVERAGE_ROOT}/deps"                                       \
+  grcov "${COVERAGE_ROOT}"                                                      \
+    --binary-path "${COVERAGE_ROOT}"                                            \
     --source-dir "${PWD}"                                                       \
     --output-types "html,lcov"                                                  \
     --branch                                                                    \
@@ -34,11 +29,11 @@ function after_tests(){
   mv "${COVERAGE_ROOT}/lcov" "${COVERAGE_ROOT}/lcov.info"
 
   echo 'Cleaning up...'
-  rm -rf "${COVERAGE_ROOT}/profraw"
+  rm -rf "${COVERAGE_ROOT}/deps/*.gcda"
 }
 trap after_tests EXIT
 
 echo 'Running tests with coverage instrumentation...'
-RUSTFLAGS='-Cinstrument-coverage'                                               \
-LLVM_PROFILE_FILE="${COVERAGE_ROOT}/profraw/%p-%m.profraw"                      \
+RUSTC_BOOTSTRAP=1                                                               \
+RUSTFLAGS='-Zprofile -Clink-dead-code -Coverflow-checks=off'                    \
 cargo test --profile=coverage

tests/end-to-end/simple/app.ts

@@ -116,7 +116,7 @@ export class NoctStack extends cdk.Stack {
 
     // Outputs
     this.bucketArn = isUsEast1
-      ? bucket.attrArn
+      ? bucket?.attrArn
       : undefined;
     if (isUsEast1) {
       new cdk.CfnOutput(this, 'BucketArn', {