Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: track reference origin nullability #129

Merged
merged 10 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .github/codecov.yml
Original file line number Diff line number Diff line change
@@ -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
7 changes: 4 additions & 3 deletions .github/workflows/pr-actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,7 @@ opt-level = 3

[profile.coverage]
inherits = "test"
codegen-units = 1
incremental = false
opt-level = 0
panic = "abort"
2 changes: 2 additions & 0 deletions src/errors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions src/ir/conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub enum ConditionIr {
}

impl ConditionIr {
#[inline]
pub fn is_simple(&self) -> bool {
matches!(self, Self::Str(_) | Self::Ref(_))
}
Expand Down
23 changes: 17 additions & 6 deletions src/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand All @@ -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)
}
}
26 changes: 21 additions & 5 deletions src/ir/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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)
),
}
}
}
Expand All @@ -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),
}

Expand Down
33 changes: 24 additions & 9 deletions src/ir/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 })
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
}
},
Expand Down Expand Up @@ -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 },
)),
),
};

Expand All @@ -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 },
)),
),
};

Expand Down
15 changes: 5 additions & 10 deletions tasks/coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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
2 changes: 1 addition & 1 deletion tests/end-to-end/simple/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down