Skip to content

Commit d89d877

Browse files
liamjpetersliamjpeters
committed
Fix over-indentation when multiple openers appear on the same line
1 parent ab4a695 commit d89d877

2 files changed

Lines changed: 196 additions & 31 deletions

File tree

‎Rules/UseConsistentIndentation.cs‎

Lines changed: 80 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,14 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
143143
// Track pipeline indentation increases per PipelineAst instead of as a single
144144
// flat counter. A flat counter caused all accumulated pipeline indentation to be
145145
// subtracted when *any* pipeline ended, instead of only the contribution from
146-
// that specific pipeline — leading to runaway indentation with nested pipelines.
146+
// that specific pipeline - leading to runaway indentation with nested pipelines.
147147
var pipelineIndentationIncreases = new Dictionary<PipelineAst, int>();
148-
/*
149-
When an LParen and LBrace are on the same line, it can lead to too much de-indentation.
150-
In order to prevent the RParen code from de-indenting too much, we keep a stack of when we skipped the indentation
151-
caused by tokens that require a closing RParen (which are LParen, AtParen and DollarParen).
152-
*/
153-
var lParenSkippedIndentation = new Stack<bool>();
154-
148+
// When multiple openers appear on the same line (e.g. ({ or @(@{),
149+
// only the last unclosed opener should affect indentation. We
150+
// track, for every opener, whether its indentation increment was
151+
// skipped so that the matching closer knows not to decrement.
152+
var openerSkippedIndentation = new Stack<bool>();
153+
155154
for (int tokenIndex = 0; tokenIndex < tokens.Length; tokenIndex++)
156155
{
157156
var token = tokens[tokenIndex];
@@ -165,27 +164,39 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
165164
{
166165
case TokenKind.AtCurly:
167166
case TokenKind.LCurly:
168-
AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
169-
break;
170-
171167
case TokenKind.DollarParen:
172168
case TokenKind.AtParen:
173-
lParenSkippedIndentation.Push(false);
174-
AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
169+
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
170+
if (HasUnclosedOpenerBeforeLineEnd(tokens, tokenIndex))
171+
{
172+
openerSkippedIndentation.Push(true);
173+
}
174+
else
175+
{
176+
indentationLevel++;
177+
openerSkippedIndentation.Push(false);
178+
}
175179
break;
176180

177181
case TokenKind.LParen:
178182
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
179-
// When a line starts with a parenthesis and it is not the last non-comment token of that line,
180-
// then indentation does not need to be increased.
183+
// When a line starts with a parenthesis and it is not the
184+
// last non-comment token of that line, indentation does
185+
// not need to be increased.
181186
if ((tokenIndex == 0 || tokens[tokenIndex - 1].Kind == TokenKind.NewLine) &&
182187
NextTokenIgnoringComments(tokens, tokenIndex)?.Kind != TokenKind.NewLine)
183188
{
184-
onNewLine = false;
185-
lParenSkippedIndentation.Push(true);
189+
openerSkippedIndentation.Push(true);
190+
break;
191+
}
192+
// General case: skip when another opener follows so that
193+
// only the last unclosed opener on a line is indent-affecting.
194+
if (HasUnclosedOpenerBeforeLineEnd(tokens, tokenIndex))
195+
{
196+
openerSkippedIndentation.Push(true);
186197
break;
187198
}
188-
lParenSkippedIndentation.Push(false);
199+
openerSkippedIndentation.Push(false);
189200
indentationLevel++;
190201
break;
191202

@@ -232,23 +243,18 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
232243
break;
233244

234245
case TokenKind.RParen:
235-
bool matchingLParenIncreasedIndentation = false;
236-
if (lParenSkippedIndentation.Count > 0)
246+
case TokenKind.RCurly:
247+
if (openerSkippedIndentation.Count > 0 && openerSkippedIndentation.Pop())
237248
{
238-
matchingLParenIncreasedIndentation = lParenSkippedIndentation.Pop();
249+
// The matching opener skipped its increment, so we
250+
// skip the decrement but still enforce indentation.
251+
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
239252
}
240-
if (matchingLParenIncreasedIndentation)
253+
else
241254
{
242-
onNewLine = false;
243-
break;
255+
indentationLevel = ClipNegative(indentationLevel - 1);
256+
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
244257
}
245-
indentationLevel = ClipNegative(indentationLevel - 1);
246-
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
247-
break;
248-
249-
case TokenKind.RCurly:
250-
indentationLevel = ClipNegative(indentationLevel - 1);
251-
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
252258
break;
253259

254260
case TokenKind.NewLine:
@@ -330,6 +336,49 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
330336
return diagnosticRecords;
331337
}
332338

339+
/// <summary>
340+
/// Scans forward from the current opener to the end of the line.
341+
/// Returns true if there is at least one unclosed opener when
342+
/// the line ends, meaning the current opener should skip its
343+
/// indentation increment. If the current opener's own closer
344+
/// is found on the same line (depth drops below zero), returns
345+
/// false so that it indents normally.
346+
/// </summary>
347+
private static bool HasUnclosedOpenerBeforeLineEnd(Token[] tokens, int currentIndex)
348+
{
349+
int depth = 0;
350+
for (int i = currentIndex + 1; i < tokens.Length; i++)
351+
{
352+
switch (tokens[i].Kind)
353+
{
354+
case TokenKind.NewLine:
355+
case TokenKind.LineContinuation:
356+
case TokenKind.EndOfInput:
357+
return depth > 0;
358+
359+
case TokenKind.LCurly:
360+
case TokenKind.AtCurly:
361+
case TokenKind.LParen:
362+
case TokenKind.AtParen:
363+
case TokenKind.DollarParen:
364+
depth++;
365+
break;
366+
367+
case TokenKind.RCurly:
368+
case TokenKind.RParen:
369+
depth--;
370+
if (depth < 0)
371+
{
372+
// Our own closer was found on this line.
373+
return false;
374+
}
375+
break;
376+
}
377+
}
378+
379+
return depth > 0;
380+
}
381+
333382
private static Token NextTokenIgnoringComments(Token[] tokens, int startIndex)
334383
{
335384
if (startIndex >= tokens.Length - 1)

‎Tests/Rules/UseConsistentIndentation.tests.ps1‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,122 @@ $Test | ForEach-Object {
913913
}
914914
}
915915

916+
Context "When multiple openers appear on the same line" {
917+
It "Should not double-indent for paren-then-brace: .foreach({" {
918+
$def = @'
919+
@('a', 'b').foreach({
920+
$_.ToUpper()
921+
})
922+
'@
923+
$expected = @'
924+
@('a', 'b').foreach({
925+
$_.ToUpper()
926+
})
927+
'@
928+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
929+
}
930+
931+
It "Should not double-indent for brace-then-paren: {(" {
932+
$def = @'
933+
@('a', 'b').foreach({(
934+
$_.ToUpper()
935+
)})
936+
'@
937+
$expected = @'
938+
@('a', 'b').foreach({(
939+
$_.ToUpper()
940+
)})
941+
'@
942+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
943+
}
944+
945+
It "Should not double-indent for array-then-hashtable on same line: @(@{" {
946+
$idempotentScriptDefinition = @'
947+
$x = @(@{
948+
key = 'value'
949+
})
950+
'@
951+
Invoke-Formatter -ScriptDefinition $idempotentScriptDefinition -Settings $settings | Should -Be $idempotentScriptDefinition
952+
}
953+
954+
It "Should not double-indent when non-opener tokens separate openers: ([PSCustomObject]@{" {
955+
$def = @'
956+
$list.Add([PSCustomObject]@{
957+
Name = "Test"
958+
Value = 123
959+
})
960+
'@
961+
$expected = @'
962+
$list.Add([PSCustomObject]@{
963+
Name = "Test"
964+
Value = 123
965+
})
966+
'@
967+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
968+
}
969+
970+
It "Should indent normally when all openers are closed on the same line" {
971+
$idempotentScriptDefinition = @'
972+
$list.Add([PSCustomObject]@{Name = "Test"; Value = 123})
973+
'@
974+
Invoke-Formatter -ScriptDefinition $idempotentScriptDefinition -Settings $settings | Should -Be $idempotentScriptDefinition
975+
}
976+
977+
It "Should handle closing brace and paren on separate lines" {
978+
$def = @'
979+
@('a', 'b').foreach({
980+
$_.ToUpper()
981+
}
982+
)
983+
'@
984+
$expected = @'
985+
@('a', 'b').foreach({
986+
$_.ToUpper()
987+
}
988+
)
989+
'@
990+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
991+
}
992+
993+
It "Should handle nested .foreach({ }) calls" {
994+
$def = @'
995+
@(1, 2).foreach({
996+
@('a', 'b').foreach({
997+
"$_ and $_"
998+
})
999+
})
1000+
'@
1001+
$expected = @'
1002+
@(1, 2).foreach({
1003+
@('a', 'b').foreach({
1004+
"$_ and $_"
1005+
})
1006+
})
1007+
'@
1008+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
1009+
}
1010+
1011+
It "Should still indent each opener separately when on different lines" {
1012+
$idempotentScriptDefinition = @'
1013+
$x = @(
1014+
@{
1015+
key = 'value'
1016+
}
1017+
)
1018+
'@
1019+
Invoke-Formatter -ScriptDefinition $idempotentScriptDefinition -Settings $settings | Should -Be $idempotentScriptDefinition
1020+
}
1021+
1022+
It "Should still indent normally for sub-expressions" {
1023+
$idempotentScriptDefinition = @'
1024+
$(
1025+
Get-Process
1026+
)
1027+
'@
1028+
Invoke-Formatter -ScriptDefinition $idempotentScriptDefinition -Settings $settings | Should -Be $idempotentScriptDefinition
1029+
}
1030+
}
1031+
9161032
Context "When tabs instead of spaces are used for indentation" {
9171033
BeforeEach {
9181034
$settings.Rules.PSUseConsistentIndentation.Kind = 'tab'

0 commit comments

Comments
 (0)