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

Fix unhandled exception and stack overflow for resource parent property assignments #4384

Merged
merged 1 commit into from
Sep 14, 2021

Conversation

shenglol
Copy link
Contributor

Fixes #4212.
Fixes #4382.

@@ -249,6 +249,7 @@ public void Parent_property_self_cycles_are_blocked()
{
var (template, diags, _) = CompilationHelper.Compile(@"
resource vmExt 'Microsoft.Compute/virtualMachines/extensions@2020-06-01' = {
name: 'vmExt'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failed to catch the stack overflow when the name property is present because of this if condition in ResourceMetadataCache: https://github.com/Azure/bicep/pull/4384/files#diff-150614dce0ae8e84ce370301fb8495c5226eaceac78046576ed656b0e41d498aR47


namespace Bicep.Core.TypeSystem
{
public class ResourceParentType : TypeSymbol
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anthony-c-martin I ended up adding a new type for the parent property and doing validation in TypeValidator. I tried to modify ResourceMetadataCache to let it emit diagnostics, but I didn't find a good way to return them to the caller other than adding a IDiagnostic property to ResourceMetadata (which feels a bit hacky since it's only added for an edge case).

Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@shenglol shenglol merged commit de8b847 into main Sep 14, 2021
@shenglol shenglol deleted the shenglol/fix-issue#4212 branch September 14, 2021 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants