From ab4a69559131ef8c7e1a14da1e09bf43f06d9976 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Thu, 2 Apr 2026 10:44:13 +0100 Subject: [PATCH] Improve pipeline indentation handling in UseConsistentIndentation rule --- Rules/UseConsistentIndentation.cs | 76 +++- .../Rules/UseConsistentIndentation.tests.ps1 | 364 ++++++++++++++++++ 2 files changed, 431 insertions(+), 9 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index 41aa4ef4d..1a01a22fe 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -130,9 +130,21 @@ public override IEnumerable AnalyzeScript(Ast ast, string file var tokens = Helper.Instance.Tokens; var diagnosticRecords = new List(); 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(); + // 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(); /* 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 @@ -188,18 +200,33 @@ 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() + .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; @@ -290,8 +317,13 @@ 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); + } } } @@ -432,6 +464,32 @@ private static PipelineAst MatchingPipelineAstEnd(List pipelineAsts, Token return matchingPipeLineAstEnd; } + /// + /// 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. + /// + private static PipelineAst FindInnermostContainingPipeline(List 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 && diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 0d26ff39d..d550be5d9 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -549,6 +549,370 @@ foo | } } + Context "When a nested multi-line pipeline is inside a pipelined script block" { + + It "Should preserve indentation with nested pipeline using " -TestCases @( + @{ + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + IdempotentScriptDefinition = @' +$Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } +'@ + } + @{ + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + IdempotentScriptDefinition = @' +$Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } +'@ + } + @{ + PipelineIndentation = 'NoIndentation' + IdempotentScriptDefinition = @' +$Test | +ForEach-Object { + Get-Process | + Select-Object -Last 1 +} +'@ + } + @{ + PipelineIndentation = 'None' + IdempotentScriptDefinition = @' +$Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 +} +'@ + } + ) { + param ($PipelineIndentation, $IdempotentScriptDefinition) + + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + + It "Should recover indentation after nested pipeline block using " -TestCases @( + @{ + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + IdempotentScriptDefinition = @' +function foo { + $Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $thisLineShouldBeAtOneIndent +} +'@ + } + @{ + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + IdempotentScriptDefinition = @' +function foo { + $Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $thisLineShouldBeAtOneIndent +} +'@ + } + @{ + PipelineIndentation = 'NoIndentation' + IdempotentScriptDefinition = @' +function foo { + $Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $thisLineShouldBeAtOneIndent +} +'@ + } + @{ + PipelineIndentation = 'None' + IdempotentScriptDefinition = @' +function foo { + $Test | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $thisLineShouldBeAtOneIndent +} +'@ + } + ) { + param ($PipelineIndentation, $IdempotentScriptDefinition) + + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + + It "Should handle multiple sequential nested pipeline blocks using " -TestCases @( + @{ + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + IdempotentScriptDefinition = @' +function foo { + $a | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $stillCorrect +} +'@ + } + @{ + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + IdempotentScriptDefinition = @' +function foo { + $a | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $stillCorrect +} +'@ + } + @{ + PipelineIndentation = 'NoIndentation' + IdempotentScriptDefinition = @' +function foo { + $a | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $stillCorrect +} +'@ + } + @{ + PipelineIndentation = 'None' + IdempotentScriptDefinition = @' +function foo { + $a | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + $stillCorrect +} +'@ + } + ) { + param ($PipelineIndentation, $IdempotentScriptDefinition) + + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + + It "Should handle inner pipeline with 3+ elements using " -TestCases @( + @{ + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + IdempotentScriptDefinition = @' +$Test | + ForEach-Object { + Get-Process | + Where-Object Path | + Select-Object -Last 1 + } +'@ + } + @{ + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + IdempotentScriptDefinition = @' +$Test | + ForEach-Object { + Get-Process | + Where-Object Path | + Select-Object -Last 1 + } +'@ + } + @{ + PipelineIndentation = 'NoIndentation' + IdempotentScriptDefinition = @' +$Test | +ForEach-Object { + Get-Process | + Where-Object Path | + Select-Object -Last 1 +} +'@ + } + @{ + PipelineIndentation = 'None' + IdempotentScriptDefinition = @' +$Test | + ForEach-Object { + Get-Process | + Where-Object Path | + Select-Object -Last 1 +} +'@ + } + ) { + param ($PipelineIndentation, $IdempotentScriptDefinition) + + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + + It "Should handle outer pipeline on same line as command using " -TestCases @( + @{ + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + IdempotentScriptDefinition = @' +$Test | ForEach-Object { + Get-Process | + Select-Object -Last 1 +} +'@ + } + @{ + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + IdempotentScriptDefinition = @' +$Test | ForEach-Object { + Get-Process | + Select-Object -Last 1 +} +'@ + } + @{ + PipelineIndentation = 'NoIndentation' + IdempotentScriptDefinition = @' +$Test | ForEach-Object { + Get-Process | + Select-Object -Last 1 +} +'@ + } + @{ + PipelineIndentation = 'None' + IdempotentScriptDefinition = @' +$Test | ForEach-Object { + Get-Process | + Select-Object -Last 1 +} +'@ + } + ) { + param ($PipelineIndentation, $IdempotentScriptDefinition) + + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + + It "Should handle deeply nested pipelines (3 levels) using " -TestCases @( + @{ + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + IdempotentScriptDefinition = @' +$a | + ForEach-Object { + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + } +'@ + } + @{ + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + IdempotentScriptDefinition = @' +$a | + ForEach-Object { + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } + } +'@ + } + @{ + PipelineIndentation = 'NoIndentation' + IdempotentScriptDefinition = @' +$a | +ForEach-Object { + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } +} +'@ + } + @{ + PipelineIndentation = 'None' + IdempotentScriptDefinition = @' +$a | + ForEach-Object { + $b | + ForEach-Object { + Get-Process | + Select-Object -Last 1 + } +} +'@ + } + ) { + param ($PipelineIndentation, $IdempotentScriptDefinition) + + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + + It "Should handle single-line inner pipeline inside multi-line outer pipeline using " -TestCases @( + @{ PipelineIndentation = 'IncreaseIndentationForFirstPipeline' } + @{ PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' } + @{ PipelineIndentation = 'NoIndentation' } + @{ PipelineIndentation = 'None' } + ) { + param ($PipelineIndentation) + + $idempotentScriptDefinition = @' +$Test | ForEach-Object { + Get-Process | Select-Object -Last 1 +} +'@ + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = $PipelineIndentation + Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition -Settings $settings | Should -Be $IdempotentScriptDefinition + } + } + Context "When tabs instead of spaces are used for indentation" { BeforeEach { $settings.Rules.PSUseConsistentIndentation.Kind = 'tab'