Improve pipeline indentation handling in UseConsistentIndentation rule (#2173)

* Improve pipeline indentation handling in UseConsistentIndentation rule

* Fix over-indentation when multiple openers appear on the same line

Author: liamjpeters

Rules/UseConsistentIndentation.cs

@@ -130,16 +130,27 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
             var tokens = Helper.Instance.Tokens;
             var diagnosticRecords = new List<DiagnosticRecord>();
             var indentationLevel = 0;
-            var currentIndenationLevelIncreaseDueToPipelines = 0;
             var onNewLine = true;
             var pipelineAsts = ast.FindAll(testAst => testAst is PipelineAst && (testAst as PipelineAst).PipelineElements.Count > 1, true).ToList();
-            /*
-                When an LParen and LBrace are on the same line, it can lead to too much de-indentation.
-                In order to prevent the RParen code from de-indenting too much, we keep a stack of when we skipped the indentation
-                caused by tokens that require a closing RParen (which are LParen, AtParen and DollarParen).
-            */
-            var lParenSkippedIndentation = new Stack<bool>();
-            
+            // Sort by end position so that inner (nested) pipelines appear before outer ones.
+            // This is required by MatchingPipelineAstEnd, whose early-break optimization
+            // would otherwise skip nested pipelines that end before their outer pipeline.
+            pipelineAsts.Sort((a, b) =>
+            {
+                int lineCmp = a.Extent.EndScriptPosition.LineNumber.CompareTo(b.Extent.EndScriptPosition.LineNumber);
+                return lineCmp != 0 ? lineCmp : a.Extent.EndScriptPosition.ColumnNumber.CompareTo(b.Extent.EndScriptPosition.ColumnNumber);
+            });
+            // Track pipeline indentation increases per PipelineAst instead of as a single
+            // flat counter. A flat counter caused all accumulated pipeline indentation to be
+            // subtracted when *any* pipeline ended, instead of only the contribution from
+            // that specific pipeline - leading to runaway indentation with nested pipelines.
+            var pipelineIndentationIncreases = new Dictionary<PipelineAst, int>();
+            // When multiple openers appear on the same line (e.g. ({ or @(@{),
+            // only the last unclosed opener should affect indentation. We
+            // track, for every opener, whether its indentation increment was
+            // skipped so that the matching closer knows not to decrement.
+            var openerSkippedIndentation = new Stack<bool>();
+
             for (int tokenIndex = 0; tokenIndex < tokens.Length; tokenIndex++)
             {
                 var token = tokens[tokenIndex];
@@ -153,27 +164,39 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
                 {
                     case TokenKind.AtCurly:
                     case TokenKind.LCurly:
-                        AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
-                        break;
-
                     case TokenKind.DollarParen:
                     case TokenKind.AtParen:
-                        lParenSkippedIndentation.Push(false);
-                        AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
+                        AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
+                        if (HasUnclosedOpenerBeforeLineEnd(tokens, tokenIndex))
+                        {
+                            openerSkippedIndentation.Push(true);
+                        }
+                        else
+                        {
+                            indentationLevel++;
+                            openerSkippedIndentation.Push(false);
+                        }
                         break;
 
                     case TokenKind.LParen:
                         AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
-                        // When a line starts with a parenthesis and it is not the last non-comment token of that line,
-                        // then indentation does not need to be increased.
+                        // When a line starts with a parenthesis and it is not the
+                        // last non-comment token of that line, indentation does
+                        // not need to be increased.
                         if ((tokenIndex == 0 || tokens[tokenIndex - 1].Kind == TokenKind.NewLine) &&
                             NextTokenIgnoringComments(tokens, tokenIndex)?.Kind != TokenKind.NewLine)
                         {
-                            onNewLine = false;
-                            lParenSkippedIndentation.Push(true);
+                            openerSkippedIndentation.Push(true);
                             break;
                         }
-                        lParenSkippedIndentation.Push(false);
+                        // General case: skip when another opener follows so that
+                        // only the last unclosed opener on a line is indent-affecting.
+                        if (HasUnclosedOpenerBeforeLineEnd(tokens, tokenIndex))
+                        {
+                            openerSkippedIndentation.Push(true);
+                            break;
+                        }
+                        openerSkippedIndentation.Push(false);
                         indentationLevel++;
                         break;
 
@@ -188,40 +211,50 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
                         if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationAfterEveryPipeline)
                         {
                             AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
-                            currentIndenationLevelIncreaseDueToPipelines++;
+                            // Attribute this increase to the innermost pipeline containing
+                            // this pipe token so it is only reversed when that specific
+                            // pipeline ends, not when an unrelated outer pipeline ends.
+                            PipelineAst containingPipeline = FindInnermostContainingPipeline(pipelineAsts, token);
+                            if (containingPipeline != null)
+                            {
+                                if (!pipelineIndentationIncreases.ContainsKey(containingPipeline))
+                                    pipelineIndentationIncreases[containingPipeline] = 0;
+                                pipelineIndentationIncreases[containingPipeline]++;
+                            }
                             break;
                         }
                         if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationForFirstPipeline)
                         {
-                            bool isFirstPipeInPipeline = pipelineAsts.Any(pipelineAst =>
-                                PositionIsEqual(LastPipeOnFirstLineWithPipeUsage((PipelineAst)pipelineAst).Extent.EndScriptPosition,
-                                   tokens[tokenIndex - 1].Extent.EndScriptPosition));
-                            if (isFirstPipeInPipeline)
+                            // Capture which specific PipelineAst this is the first pipe for,
+                            // so the indentation increase is attributed to that pipeline only.
+                            PipelineAst firstPipePipeline = pipelineAsts
+                                .Cast<PipelineAst>()
+                                .FirstOrDefault(pipelineAst =>
+                                    PositionIsEqual(LastPipeOnFirstLineWithPipeUsage(pipelineAst).Extent.EndScriptPosition,
+                                       tokens[tokenIndex - 1].Extent.EndScriptPosition));
+                            if (firstPipePipeline != null)
                             {
                                 AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
-                                currentIndenationLevelIncreaseDueToPipelines++;
+                                if (!pipelineIndentationIncreases.ContainsKey(firstPipePipeline))
+                                    pipelineIndentationIncreases[firstPipePipeline] = 0;
+                                pipelineIndentationIncreases[firstPipePipeline]++;
                             }
                         }
                         break;
 
                     case TokenKind.RParen:
-                        bool matchingLParenIncreasedIndentation = false;
-                        if (lParenSkippedIndentation.Count > 0)
+                    case TokenKind.RCurly:
+                        if (openerSkippedIndentation.Count > 0 && openerSkippedIndentation.Pop())
                         {
-                            matchingLParenIncreasedIndentation = lParenSkippedIndentation.Pop();
+                            // The matching opener skipped its increment, so we
+                            // skip the decrement but still enforce indentation.
+                            AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
                         }
-                        if (matchingLParenIncreasedIndentation)
+                        else
                         {
-                            onNewLine = false;
-                            break;
+                            indentationLevel = ClipNegative(indentationLevel - 1);
+                            AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
                         }
-                        indentationLevel = ClipNegative(indentationLevel - 1);
-                        AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
-                        break;
-
-                    case TokenKind.RCurly:
-                        indentationLevel = ClipNegative(indentationLevel - 1);
-                        AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
                         break;
 
                     case TokenKind.NewLine:
@@ -290,14 +323,62 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
                 if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationForFirstPipeline ||
                     pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationAfterEveryPipeline)
                 {
-                    indentationLevel = ClipNegative(indentationLevel - currentIndenationLevelIncreaseDueToPipelines);
-                    currentIndenationLevelIncreaseDueToPipelines = 0;
+                    // Only subtract the indentation contributed by this specific pipeline,
+                    // leaving contributions from outer/unrelated pipelines intact.
+                    if (pipelineIndentationIncreases.TryGetValue(matchingPipeLineAstEnd, out int contribution))
+                    {
+                        indentationLevel = ClipNegative(indentationLevel - contribution);
+                        pipelineIndentationIncreases.Remove(matchingPipeLineAstEnd);
+                    }
                 }
             }
 
             return diagnosticRecords;
         }
 
+        /// <summary>
+        /// Scans forward from the current opener to the end of the line.
+        /// Returns true if there is at least one unclosed opener when
+        /// the line ends, meaning the current opener should skip its
+        /// indentation increment.  If the current opener's own closer
+        /// is found on the same line (depth drops below zero), returns
+        /// false so that it indents normally.
+        /// </summary>
+        private static bool HasUnclosedOpenerBeforeLineEnd(Token[] tokens, int currentIndex)
+        {
+            int depth = 0;
+            for (int i = currentIndex + 1; i < tokens.Length; i++)
+            {
+                switch (tokens[i].Kind)
+                {
+                    case TokenKind.NewLine:
+                    case TokenKind.LineContinuation:
+                    case TokenKind.EndOfInput:
+                        return depth > 0;
+
+                    case TokenKind.LCurly:
+                    case TokenKind.AtCurly:
+                    case TokenKind.LParen:
+                    case TokenKind.AtParen:
+                    case TokenKind.DollarParen:
+                        depth++;
+                        break;
+
+                    case TokenKind.RCurly:
+                    case TokenKind.RParen:
+                        depth--;
+                        if (depth < 0)
+                        {
+                            // Our own closer was found on this line.
+                            return false;
+                        }
+                        break;
+                }
+            }
+
+            return depth > 0;
+        }
+
         private static Token NextTokenIgnoringComments(Token[] tokens, int startIndex)
         {
             if (startIndex >= tokens.Length - 1)
@@ -432,6 +513,32 @@ private static PipelineAst MatchingPipelineAstEnd(List<Ast> pipelineAsts, Token
             return matchingPipeLineAstEnd;
         }
 
+        /// <summary>
+        /// Finds the innermost (smallest) PipelineAst whose extent fully contains the given token.
+        /// Used to attribute pipeline indentation increases to the correct pipeline when
+        /// using IncreaseIndentationAfterEveryPipeline.
+        /// </summary>
+        private static PipelineAst FindInnermostContainingPipeline(List<Ast> pipelineAsts, Token token)
+        {
+            PipelineAst best = null;
+            int bestSize = int.MaxValue;
+            foreach (var ast in pipelineAsts)
+            {
+                var pipeline = (PipelineAst)ast;
+                int pipelineStart = pipeline.Extent.StartOffset;
+                int pipelineEnd = pipeline.Extent.EndOffset;
+                int pipelineSize = pipelineEnd - pipelineStart;
+                if (pipelineStart <= token.Extent.StartOffset &&
+                    token.Extent.EndOffset <= pipelineEnd &&
+                    pipelineSize < bestSize)
+                {
+                    best = pipeline;
+                    bestSize = pipelineSize;
+                }
+            }
+            return best;
+        }
+
         private static bool PositionIsEqual(IScriptPosition position1, IScriptPosition position2)
         {
             return position1.ColumnNumber == position2.ColumnNumber &&