fixed codegen for nested resource loops with complex parent index expressions

src/Bicep.Core/Emit/ExpressionConverter.cs

@@ -8,6 +8,7 @@
 using System.Linq;
 using Azure.Deployments.Core.Extensions;
 using Azure.Deployments.Expression.Expressions;
+using Bicep.Core.DataFlow;
 using Bicep.Core.Extensions;
 using Bicep.Core.Semantics;
 using Bicep.Core.Semantics.Metadata;
@@ -180,7 +181,7 @@ public ExpressionConverter CreateConverterForIndexReplacement(SyntaxBase nameSyn
                     // regardless if there is an index expression or not, we don't need to append replacements
                     return this;
 
-                case 1 when indexExpression != null:
+                case 1 when indexExpression is not null:
                     // TODO: Run data flow analysis on the array expression as well. (Will be needed for nested resource loops)
                     var @for = inaccessibleLocalLoops.Single();
                     var current = this;
@@ -510,8 +511,8 @@ public IEnumerable<LanguageExpression> GetResourceNameSegments(DeclaredResourceM
 
                 var parentNames = ancestors.SelectMany((x, i) =>
                 {
-                    var nameExpression = CreateConverterForIndexReplacement(x.Resource.NameSyntax, x.IndexExpression, resource.Symbol.NameSyntax)
-                        .ConvertExpression(x.Resource.NameSyntax);
+                    var expression = GetResourceNameAncestorSyntaxSegment(resource, i);
+                    var nameExpression = this.ConvertExpression(expression);
 
                     if (i == 0 && firstAncestorNameLength > 1)
                     {
@@ -538,6 +539,127 @@ public IEnumerable<LanguageExpression> GetResourceNameSegments(DeclaredResourceM
                     new JTokenExpression(i)));
         }
 
+        /// <summary>
+        /// Returns a collection of name segment expressions for the specified resource. Local variable replacements
+        /// are performed so the expressions are valid in the language/binding scope of the specified resource.
+        /// </summary>
+        /// <param name="resource">The resource</param>
+        public IEnumerable<SyntaxBase> GetResourceNameSyntaxSegments(DeclaredResourceMetadata resource)
+        {
+            var ancestors = this.context.SemanticModel.ResourceAncestors.GetAncestors(resource);
+            var nameExpression = resource.NameSyntax;
+
+            return ancestors
+                .Select((x, i) => GetResourceNameAncestorSyntaxSegment(resource, i))
+                .Concat(nameExpression);
+        }
+
+        /// <summary>
+        /// Calculates the expression that represents the parent name corresponding to the specified ancestor of the specified resource.
+        /// The expressions returned are modified by performing the necessary local variable replacements.
+        /// </summary>
+        /// <param name="resource">The declared resource metadata</param>
+        /// <param name="startingAncestorIndex">the index of the ancestor (0 means the ancestor closest to the root)</param>
+        private SyntaxBase GetResourceNameAncestorSyntaxSegment(DeclaredResourceMetadata resource, int startingAncestorIndex)
+        {
+            var ancestors = this.context.SemanticModel.ResourceAncestors.GetAncestors(resource);
+            if(startingAncestorIndex >= ancestors.Length)
+            {
+                // not enough ancestors
+                throw new ArgumentException($"Resource type has {ancestors.Length} ancestor types but name expression was requested for ancestor type at index {startingAncestorIndex}.");
+            }
+
+            /*
+             * Consider the following example:
+             * 
+             * resource one 'MS.Example/ones@...' = [for (_, i) in range(0, ...) : {
+             *   name: name_exp1(i)
+             * }]
+             * 
+             * resource two 'MS.Example/ones/twos@...' = [for (_, j) in range(0, ...) : {
+             *   parent: one[index_exp2(j)]
+             *   name: name_exp2(j)
+             * }]
+             * 
+             * resource three 'MS.Example/ones/twos/threes@...' = [for (_, k) in range(0, ...) : {
+             *   parent: two[index_exp3(k)]
+             *   name: name_exp3(k)
+             * }]
+             * 
+             * name_exp* and index_exp* are expressions represented here as functions
+             * 
+             * The name segment expressions for "three" are the following:
+             * 0. name_exp1(index_exp2(index_exp3(k)))
+             * 1. name_exp2(index_exp3(k))
+             * 2. name_exp3(k)
+             * 
+             * (The formula can be generalized to more levels of nesting.)
+             * 
+             * This function can be used to get 0 and 1 above by passing 0 or 1 respectively as the startingAncestorIndex.
+             * The name segment 2 above must be obtained from the resource directly.
+             * 
+             * Given that we don't have proper functions in our runtime AND that our expressions don't have side effects,
+             * the formula is implemented via local variable replacement.
+             */
+
+            // the initial ancestor gives us the base expression
+            SyntaxBase? rewritten = ancestors[startingAncestorIndex].Resource.NameSyntax;
+
+            for(int i = startingAncestorIndex; i < ancestors.Length; i++)
+            {
+                var ancestor = ancestors[i];
+
+                // local variable replacement will be done in context of the next ancestor
+                // or the resource itself if we're on the last ancestor
+                var newContext = i < ancestors.Length - 1 ? ancestors[i + 1].Resource : resource;
+
+                var inaccessibleLocals = this.context.DataFlowAnalyzer.GetInaccessibleLocalsAfterSyntaxMove(rewritten, newContext.Symbol.NameSyntax);
+                var inaccessibleLocalLoops = inaccessibleLocals.Select(local => GetEnclosingForExpression(local)).Distinct().ToList();
+
+                switch (inaccessibleLocalLoops.Count)
+                {
+                    case 0 when i == startingAncestorIndex:
+                        /*
+                         * There are no local vars to replace. It is impossible for a local var to be introduced at the next level
+                         * so we can just bail out with the result.
+                         * 
+                         * This path is followed by non-loop resources.
+                         * 
+                         * Case 0 is not possible for non-starting ancestor index because 
+                         * once we have a local variable replacement, it will propagate to the next levels
+                         */
+                        return ancestor.Resource.NameSyntax;
+
+                    case 1 when ancestor.IndexExpression is not null:
+                        if (LocalSymbolDependencyVisitor.GetLocalSymbolDependencies(this.context.SemanticModel, rewritten).SingleOrDefault(s => s.LocalKind == LocalKind.ForExpressionItemVariable) is { } loopItemSymbol)
+                        {
+                            // rewrite the straggler from previous iteration
+                            // TODO: Nested loops will require DFA on the ForSyntax.Expression
+                            rewritten = SymbolReplacer.Replace(this.context.SemanticModel, new Dictionary<Symbol, SyntaxBase> { [loopItemSymbol] = SyntaxFactory.CreateArrayAccess(GetEnclosingForExpression(loopItemSymbol).Expression, ancestor.IndexExpression) }, rewritten);
+                        }
+
+                        // TODO: Run data flow analysis on the array expression as well. (Will be needed for nested resource loops)
+                        var @for = inaccessibleLocalLoops.Single();
+
+                        var replacements = inaccessibleLocals.ToDictionary(local => (Symbol)local, local => local.LocalKind switch
+                              {
+                                  LocalKind.ForExpressionIndexVariable => ancestor.IndexExpression,
+                                  LocalKind.ForExpressionItemVariable => SyntaxFactory.CreateArrayAccess(@for.Expression, ancestor.IndexExpression),
+                                  _ => throw new NotImplementedException($"Unexpected local kind '{local.LocalKind}'.")
+                              });
+
+                        rewritten = SymbolReplacer.Replace(this.context.SemanticModel, replacements, rewritten);                        
+
+                        break;
+
+                    default:
+                        throw new NotImplementedException("Mismatch between count of index expressions and inaccessible symbols during array access index expression rewriting.");
+                }
+            }
+
+            return rewritten;
+        }
+
         public LanguageExpression GetFullyQualifiedResourceName(DeclaredResourceMetadata resource)
         {
             var nameValueSyntax = resource.NameSyntax;
@@ -603,12 +725,13 @@ public LanguageExpression GetFullyQualifiedResourceId(ResourceMetadata resource)
             }
             else if (resource is DeclaredResourceMetadata declared)
             {
+                var nameSegments = GetResourceNameSegments(declared);
                 return ScopeHelper.FormatFullyQualifiedResourceId(
                     context,
                     this,
                     context.ResourceScopeData[declared],
                     resource.TypeReference.FormatType(),
-                    GetResourceNameSegments(declared));
+                    nameSegments);
             }
             else
             {

src/Bicep.Core/Emit/ExpressionEmitter.cs

@@ -130,7 +130,9 @@ public void EmitIndexedSymbolReference(ModuleSymbol moduleSymbol, SyntaxBase ind
 
         public void EmitResourceIdReference(DeclaredResourceMetadata resource, SyntaxBase? indexExpression, SyntaxBase newContext)
         {
-            var converterForContext = this.converter.CreateConverterForIndexReplacement(resource.NameSyntax, indexExpression, newContext);
+            var nameComponents = SyntaxFactory.CreateArray(this.converter.GetResourceNameSyntaxSegments(resource));
+
+            var converterForContext = this.converter.CreateConverterForIndexReplacement(nameComponents, indexExpression, newContext);
 
             var resourceIdExpression = converterForContext.GetFullyQualifiedResourceId(resource);
             var serialized = ExpressionSerializer.SerializeExpression(resourceIdExpression);

src/Bicep.Core/Emit/ResourceDependencyVisitor.cs

@@ -237,7 +237,7 @@ public override void VisitResourceAccessSyntax(ResourceAccessSyntax syntax)
                 _ => throw new NotImplementedException($"Unexpected current declaration type '{this.currentDeclaration?.GetType().Name}'.")
             };
 
-            // using the resource/module body as the context to allow indexed depdnencies relying on the resource/module loop index to work as expected
+            // using the resource/module body as the context to allow indexed dependencies relying on the resource/module loop index to work as expected
             var inaccessibleLocals = dfa.GetInaccessibleLocalsAfterSyntaxMove(candidateIndexExpression, context);
             if (inaccessibleLocals.Any())
             {

src/Bicep.Core/Emit/SymbolReplacer.cs

@@ -0,0 +1,63 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using Bicep.Core.Semantics;
+using Bicep.Core.Syntax;
+using System.Collections.Generic;
+
+namespace Bicep.Core.Emit
+{
+    public class SymbolReplacer : SyntaxRewriteVisitor
+    {
+        private readonly SemanticModel semanticModel;
+        private IReadOnlyDictionary<Symbol, SyntaxBase> replacements;
+
+        private SymbolReplacer(SemanticModel semanticModel, IReadOnlyDictionary<Symbol, SyntaxBase> replacements)
+        {
+            this.semanticModel = semanticModel;
+            this.replacements = replacements;
+        }
+
+        public static SyntaxBase Replace(SemanticModel semanticModel, IReadOnlyDictionary<Symbol, SyntaxBase> replacements, SyntaxBase syntax) =>
+            new SymbolReplacer(semanticModel, replacements).Rewrite(syntax);
+
+        protected override SyntaxBase ReplaceVariableAccessSyntax(VariableAccessSyntax syntax)
+        {
+            if (this.semanticModel.GetSymbolInfo(syntax) is not { } symbol || !this.replacements.TryGetValue(symbol, out var replacementSyntax))
+            {
+                // unbound variable access or not a symbol that we need to replace
+                // leave syntax as-is
+                return base.ReplaceVariableAccessSyntax(syntax);
+            }
+
+            // inject the replacement syntax
+            return replacementSyntax;
+        }
+    }
+
+    public class SyntaxReplacer: SyntaxRewriteVisitor
+    {
+        private IReadOnlyDictionary<VariableAccessSyntax, SyntaxBase> replacements;
+
+        private SyntaxReplacer(IReadOnlyDictionary<VariableAccessSyntax, SyntaxBase> replacements)
+        {
+            this.replacements = replacements;
+        }
+
+        public static SyntaxBase Replace(IReadOnlyDictionary<VariableAccessSyntax, SyntaxBase> replacements, SyntaxBase syntax) =>
+            new SyntaxReplacer(replacements).Rewrite(syntax);
+
+        protected override SyntaxBase ReplaceVariableAccessSyntax(VariableAccessSyntax syntax)
+        {
+            if(!this.replacements.TryGetValue(syntax, out var replacementSyntax))
+            {
+                // no match
+                // leave syntax as-is
+                return base.ReplaceVariableAccessSyntax(syntax);
+            }
+
+            // inject the replacment syntax
+            return replacementSyntax;
+        }
+    }
+}

src/Bicep.Core/Semantics/ResourceAncestorGraph.cs

@@ -17,21 +17,7 @@ public enum ResourceAncestorType
             ParentProperty,
         }
 
-        public class ResourceAncestor
-        {
-            public ResourceAncestor(ResourceAncestorType ancestorType, DeclaredResourceMetadata resource, SyntaxBase? indexExpression)
-            {
-                AncestorType = ancestorType;
-                Resource = resource;
-                IndexExpression = indexExpression;
-            }
-
-            public ResourceAncestorType AncestorType { get; }
-
-            public DeclaredResourceMetadata Resource { get; }
-
-            public SyntaxBase? IndexExpression { get; }
-        }
+        public record ResourceAncestor(ResourceAncestorType AncestorType, DeclaredResourceMetadata Resource, SyntaxBase? IndexExpression);
 
         private readonly ImmutableDictionary<DeclaredResourceMetadata, ImmutableArray<ResourceAncestor>> data;
 

src/Bicep.Core/Syntax/SyntaxFactory.cs

@@ -130,6 +130,8 @@ public static ArraySyntax CreateArray(IEnumerable<SyntaxBase> items)
                 RightSquareToken);
         }
 
+        public static ArrayAccessSyntax CreateArrayAccess(SyntaxBase baseExpression, SyntaxBase indexExpression) => new(baseExpression, LeftSquareToken, indexExpression, RightSquareToken);
+
         public static SyntaxBase CreateObjectPropertyKey(string text)
         {
             if (Regex.IsMatch(text, "^[a-zA-Z][a-zA-Z0-9_]*$"))

src/vscode-bicep/.vscode/launch.json

@@ -18,7 +18,8 @@
       "env": {
         "BICEP_TRACING_ENABLED": "true",
         "BICEP_LANGUAGE_SERVER_PATH": "${workspaceRoot}/../Bicep.LangServer/bin/Debug/net6.0/Bicep.LangServer.dll",
-
+        //"BICEP_SYMBOLIC_NAME_CODEGEN_EXPERIMENTAL": "true",
+
         // Defaulting to "verbose" when debugging so that new telemetry IDs during development aren't accidentally sent and created before they're finalized
         "DEBUGTELEMETRY": "v", // "" or "0" or "false": send telemetry as normal; "1": debug mode (no telemetry sent); "verbose": telemetry in console, don't send (from microsoft/vscode-azext-utils)
       }