diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index 2c77787c6..0e6680e63 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -275,17 +275,16 @@ public override IEnumerable AnalyzeScript(Ast ast, string file ++tempIndentationLevel; } - // check for comments in between multi-line commands with line continuation + // Check for comments in between multi-line commands with line continuation. if (tokenIndex > 2 && tokens[tokenIndex - 1].Kind == TokenKind.NewLine && tokens[tokenIndex - 2].Kind == TokenKind.Comment) { - int searchForPrecedingLineContinuationIndex = tokenIndex - 2; - while (searchForPrecedingLineContinuationIndex > 0 && tokens[searchForPrecedingLineContinuationIndex].Kind == TokenKind.Comment) - { - searchForPrecedingLineContinuationIndex--; - } - - if (searchForPrecedingLineContinuationIndex >= 0 && tokens[searchForPrecedingLineContinuationIndex].Kind == TokenKind.LineContinuation) + GetCommentRunIndentationOffset( + tokens, + tokenIndex - 2, + out bool commentRunStartsAfterLineContinuation, + out _); + if (commentRunStartsAfterLineContinuation) { tempIndentationLevel++; } @@ -298,7 +297,25 @@ public override IEnumerable AnalyzeScript(Ast ast, string file } bool lineHasPipelineBeforeToken = LineHasPipelineBeforeToken(tokens, tokenIndex, token); - AddViolation(token, tempIndentationLevel, diagnosticRecords, ref onNewLine, lineHasPipelineBeforeToken); + int commentIndentationOffset = 0; + if (token.Kind == TokenKind.Comment) + { + commentIndentationOffset = GetCommentRunIndentationOffset( + tokens, + tokenIndex, + out bool commentRunStartsAfterLineContinuation, + out bool commentRunStartsAfterPipeline); + if (pipelineIndentationStyle == PipelineIndentationStyle.None && + commentRunStartsAfterPipeline) + { + onNewLine = false; + continue; + } + tempIndentationLevel = indentationLevel + + (commentRunStartsAfterLineContinuation ? 1 : 0); + } + + AddViolation(token, tempIndentationLevel, diagnosticRecords, ref onNewLine, lineHasPipelineBeforeToken, commentIndentationOffset); } break; } @@ -336,6 +353,58 @@ public override IEnumerable AnalyzeScript(Ast ast, string file return diagnosticRecords; } + private static int GetCommentRunIndentationOffset( + Token[] tokens, + int tokenIndex, + out bool commentRunStartsAfterLineContinuation, + out bool commentRunStartsAfterPipeline) + { + int firstCommentIndex = tokenIndex; + int currentLine = tokens[tokenIndex].Extent.StartLineNumber; + int precedingIndex = tokenIndex - 1; + while (precedingIndex >= 0 && + (tokens[precedingIndex].Kind == TokenKind.NewLine || + tokens[precedingIndex].Kind == TokenKind.LineContinuation)) + { + precedingIndex--; + if (precedingIndex < 0 || + tokens[precedingIndex].Kind != TokenKind.Comment || + tokens[precedingIndex].Extent.EndLineNumber != currentLine - 1 || + (precedingIndex > 0 && + tokens[precedingIndex - 1].Kind != TokenKind.NewLine && + tokens[precedingIndex - 1].Kind != TokenKind.LineContinuation)) + { + break; + } + + firstCommentIndex = precedingIndex; + currentLine = tokens[precedingIndex].Extent.StartLineNumber; + precedingIndex--; + } + + commentRunStartsAfterLineContinuation = + firstCommentIndex > 0 && + tokens[firstCommentIndex - 1].Kind == TokenKind.LineContinuation; + commentRunStartsAfterPipeline = + PreviousLineEndedWithPipe(tokens, firstCommentIndex, tokens[firstCommentIndex]); + int leastIndentedColumn = tokens[firstCommentIndex].Extent.StartColumnNumber; + for (int nextCommentIndex = firstCommentIndex + 2; + nextCommentIndex < tokens.Length && + (tokens[nextCommentIndex - 1].Kind == TokenKind.NewLine || + tokens[nextCommentIndex - 1].Kind == TokenKind.LineContinuation) && + tokens[nextCommentIndex].Kind == TokenKind.Comment && + tokens[nextCommentIndex].Extent.StartLineNumber == tokens[nextCommentIndex - 2].Extent.EndLineNumber + 1; + nextCommentIndex += 2) + { + leastIndentedColumn = Math.Min( + leastIndentedColumn, + tokens[nextCommentIndex].Extent.StartColumnNumber); + } + + return tokens[tokenIndex].Extent.StartColumnNumber - + leastIndentedColumn; + } + /// /// Scans forward from the current opener to the end of the line. /// Returns true if there is at least one unclosed opener when @@ -612,12 +681,14 @@ private void AddViolation( int expectedIndentationLevel, List diagnosticRecords, ref bool onNewLine, - bool lineHasPipelineBeforeToken = false) + bool lineHasPipelineBeforeToken = false, + int indentationOffset = 0) { if (onNewLine) { onNewLine = false; - if (token.Extent.StartColumnNumber - 1 != GetIndentation(expectedIndentationLevel)) + int expectedIndentation = Math.Max(0, GetIndentation(expectedIndentationLevel) + indentationOffset); + if (token.Extent.StartColumnNumber - 1 != expectedIndentation) { var fileName = token.Extent.File; var extent = token.Extent; @@ -640,14 +711,14 @@ private void AddViolation( GetDiagnosticSeverity(), fileName, null, - GetSuggestedCorrections(token, expectedIndentationLevel, lineHasPipelineBeforeToken))); + GetSuggestedCorrections(token, expectedIndentation, lineHasPipelineBeforeToken))); } } } private List GetSuggestedCorrections( Token token, - int indentationLevel, + int indentation, bool lineHasPipelineBeforeToken = false) { // TODO Add another constructor for correction extent that takes extent @@ -661,7 +732,7 @@ private List GetSuggestedCorrections( token.Extent.EndLineNumber, 1, token.Extent.EndColumnNumber, - GetIndentationString(indentationLevel) + optionalPipeline + token.Extent.Text, + new string(indentationChar, indentation) + optionalPipeline + token.Extent.Text, token.Extent.File)); return corrections; } @@ -677,9 +748,5 @@ private int GetIndentation(int indentationLevel) return indentationLevel * indentationLevelMultiplier; } - private string GetIndentationString(int indentationLevel) - { - return new string(indentationChar, GetIndentation(indentationLevel)); - } } } diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 6bf241116..d07a7b0a5 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -204,8 +204,139 @@ function foo { } } - Context "When a multi-line command is given" { + Context "When a commented-out code block is provided" { + + It 'Should preserve relative indentation in commented-out code with indentation' -TestCases @( + @{ Kind = 'space'; Indentation = ' ' } + @{ Kind = 'tab'; Indentation = "`t" } + ) { + param([string] $Kind, [string] $Indentation) + $scriptDefinition = @( + 'if ($a) {' + "$Indentation# if (`$b) {" + "$Indentation$Indentation# doThing" + "$Indentation# }" + "$($Indentation)Write-Output 'after'" + '}' + ) -join "`n" + $settings.Rules.PSUseConsistentIndentation.Kind = $Kind + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'NoIndentation' + + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | + Should -Be $scriptDefinition + } + + It 'Should shift a commented-out code block without flattening it with indentation' -TestCases @( + @{ Kind = 'space'; Indentation = ' ' } + @{ Kind = 'tab'; Indentation = "`t" } + ) { + param([string] $Kind, [string] $Indentation) + $scriptDefinition = @( + 'if ($a) {' + '# if ($b) {' + "$Indentation# doThing" + '# }' + "$($Indentation)Write-Output 'after'" + '}' + ) -join "`n" + $expectedScriptDefinition = @( + 'if ($a) {' + "$Indentation# if (`$b) {" + "$Indentation$Indentation# doThing" + "$Indentation# }" + "$($Indentation)Write-Output 'after'" + '}' + ) -join "`n" + $settings.Rules.PSUseConsistentIndentation.Kind = $Kind + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'NoIndentation' + + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | + Should -Be $expectedScriptDefinition + } + + It 'Should preserve relative indentation in comments after a line continuation' { + $scriptDefinition = @' +Write-Output ` + # first + # nested + # last + -InputObject 'value' +'@ + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'NoIndentation' + + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | + Should -Be $scriptDefinition + } + + It 'Should preserve indentation when the first comment is not the least-indented with indentation' -TestCases @( + @{ Kind = 'space'; Indentation = ' ' } + @{ Kind = 'tab'; Indentation = "`t" } + ) { + param([string] $Kind, [string] $Indentation) + $scriptDefinition = @( + "$Indentation# inner" + '# outer' + ) -join "`n" + $settings.Rules.PSUseConsistentIndentation.Kind = $Kind + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'NoIndentation' + + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | + Should -Be $scriptDefinition + } + It 'Should preserve a comment run containing an internal line continuation with indentation' -TestCases @( + @{ Kind = 'space'; Indentation = ' ' } + @{ Kind = 'tab'; Indentation = "`t" } + ) { + param([string] $Kind, [string] $Indentation) + $scriptDefinition = @( + '<# first #> `' + "$Indentation<# second #>" + "Write-Output 'after'" + ) -join "`n" + $settings.Rules.PSUseConsistentIndentation.Kind = $Kind + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 0 + $formatted = Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings + $formatted | Should -Be $scriptDefinition + Invoke-Formatter -ScriptDefinition $formatted -Settings $settings | + Should -Be $formatted + } + + It 'Should shift a comment run containing an internal line continuation uniformly with indentation' -TestCases @( + @{ Kind = 'space'; Indentation = ' ' } + @{ Kind = 'tab'; Indentation = "`t" } + ) { + param([string] $Kind, [string] $Indentation) + $scriptDefinition = @( + 'if ($a) {' + '<# first #> `' + "$Indentation<# second #>" + "$($Indentation)Write-Output 'after'" + '}' + ) -join "`n" + $expectedScriptDefinition = @( + 'if ($a) {' + "$Indentation<# first #> ``" + "$Indentation$Indentation<# second #>" + "$($Indentation)Write-Output 'after'" + '}' + ) -join "`n" + $settings.Rules.PSUseConsistentIndentation.Kind = $Kind + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 2 + $formatted = Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings + $formatted | Should -Be $expectedScriptDefinition + Invoke-Formatter -ScriptDefinition $formatted -Settings $settings | + Should -Be $formatted + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $formatted -Settings $settings + $violations.Count | Should -Be 0 + } + } + + Context "When a multi-line command is given" { It "When a comment is in the middle of a multi-line statement with preceding and succeeding line continuations" { $scriptDefinition = @' foo ` @@ -530,6 +661,19 @@ foo | Test-CorrectionExtentFromContent @params } + It "Should preserve indentation in a comment run after a pipeline with PipelineIndentation None" { + $def = @' +foo | + # first comment + # second comment +bar +'@ + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'None' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations | Should -BeNullOrEmpty + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $def + } + It "Should indent properly after line continuation (backtick) character with pipeline" { $def = @' foo | diff --git a/docs/Rules/UseConsistentIndentation.md b/docs/Rules/UseConsistentIndentation.md index 5e6392772..4d020bbd4 100644 --- a/docs/Rules/UseConsistentIndentation.md +++ b/docs/Rules/UseConsistentIndentation.md @@ -15,6 +15,9 @@ title: UseConsistentIndentation This rule detects inconsistent indentation patterns within a source file. Indentation should be consistent throughout your script. +Consecutive comment lines are shifted together to preserve their relative indentation, including +when a line continuation occurs within the comment run. + ## Example ### Noncompliant