From 17c11b5b886d5a8cfa72987051575a6398612143 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 19 Aug 2019 08:25:38 +0100 Subject: [PATCH 1/5] try increase indentation after comment --- Rules/UseConsistentIndentation.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index ecf726362..de6ab3d85 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -201,9 +201,9 @@ public override IEnumerable AnalyzeScript(Ast ast, string file // Since the previous token is a newline token we start // looking for comments at the token before the newline token. int j = k - 2; - while (j > 0 && tokens[j].Kind == TokenKind.Comment) + if (j > 0 && tokens[j].Kind == TokenKind.Comment) { - --j; + ++tempIndentationLevel; } } From c30318b7d41a6d71e1a832587a26486e95d47181 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 20 Aug 2019 20:43:40 +0100 Subject: [PATCH 2/5] make line after comment in multi line command with line continuation indent correctly and add tests. TODO: make it work for more than 1 line as well --- Rules/UseConsistentIndentation.cs | 16 ++++----- .../Rules/UseConsistentIndentation.tests.ps1 | 35 +++++++++++++++++++ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index fb5d75174..70051c651 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -195,16 +195,14 @@ public override IEnumerable AnalyzeScript(Ast ast, string file { ++tempIndentationLevel; } - else + + // TODO: check if not just line before but further back + if (k > 2 && + tokens[k - 1].Kind == TokenKind.NewLine && + tokens[k - 2].Kind == TokenKind.Comment && + tokens[k - 3].Kind == TokenKind.LineContinuation) { - // Ignore comments - // Since the previous token is a newline token we start - // looking for comments at the token before the newline token. - int j = k - 2; - if (j > 0 && tokens[j].Kind == TokenKind.Comment) - { - ++tempIndentationLevel; - } + ++tempIndentationLevel; } var lineHasPipelineBeforeToken = tokens.Any(oneToken => diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 1503b8bd8..0684a20e5 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -107,6 +107,41 @@ function foo { } Context "When a multi-line command is given" { + + It "When a comment is in the middle of a multi-line statement with line continuations" { + $scriptDefinition = @' +foo ` +# comment +-bar +'@ + $expected = @' +foo ` + # comment + -bar +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 2 + Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected + } + + It "When a comment is in the middle of a multi-line statement with line continuations 2" { + $scriptDefinition = @' +foo ` +# comment +-bar +-baz +'@ + $expected = @' +foo ` + # comment + -bar ` + -baz +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 2 + Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected + } + It "Should find a violation if a pipleline element is not indented correctly" { $def = @' get-process | From 519dfd74a9975e226039f093ee2c0618b343a7c7 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 20 Aug 2019 21:43:41 +0100 Subject: [PATCH 3/5] Make it work for continued lines as well. Question: what if line continuations and pipelines are mixed? --- Rules/UseConsistentIndentation.cs | 19 +++++++++++++------ .../Rules/UseConsistentIndentation.tests.ps1 | 6 ++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index 70051c651..be39a365c 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -196,13 +196,20 @@ public override IEnumerable AnalyzeScript(Ast ast, string file ++tempIndentationLevel; } - // TODO: check if not just line before but further back - if (k > 2 && - tokens[k - 1].Kind == TokenKind.NewLine && - tokens[k - 2].Kind == TokenKind.Comment && - tokens[k - 3].Kind == TokenKind.LineContinuation) + // check for comments in between multi-line commands with line continuation + if (k > 2 && tokens[k - 1].Kind == TokenKind.NewLine + && tokens[k - 2].Kind == TokenKind.Comment) { - ++tempIndentationLevel; + int j = k - 2; + while (j > 0 && tokens[j].Kind == TokenKind.Comment) + { + --j; + } + + if (j >= 0 && tokens[j].Kind == TokenKind.LineContinuation) + { + ++tempIndentationLevel; + } } var lineHasPipelineBeforeToken = tokens.Any(oneToken => diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 0684a20e5..5a0752ce9 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -121,6 +121,7 @@ foo ` '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 2 + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected } @@ -128,7 +129,7 @@ foo ` $scriptDefinition = @' foo ` # comment --bar +-bar ` -baz '@ $expected = @' @@ -138,7 +139,8 @@ foo ` -baz '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings - $violations.Count | Should -Be 2 + $violations.Count | Should -Be 3 + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected } From cb8a227e368e6ed27283063f8606f88ffde989d8 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 21 Aug 2019 18:40:10 +0100 Subject: [PATCH 4/5] tweak c# style and add more test cases --- Rules/UseConsistentIndentation.cs | 4 +- .../Rules/UseConsistentIndentation.tests.ps1 | 48 ++++++++++++------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index be39a365c..f4a598087 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -203,12 +203,12 @@ public override IEnumerable AnalyzeScript(Ast ast, string file int j = k - 2; while (j > 0 && tokens[j].Kind == TokenKind.Comment) { - --j; + j--; } if (j >= 0 && tokens[j].Kind == TokenKind.LineContinuation) { - ++tempIndentationLevel; + tempIndentationLevel++; } } diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 5a0752ce9..2cce7dd7c 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -3,7 +3,25 @@ $testRootDirectory = Split-Path -Parent $directory Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") + Describe "UseConsistentIndentation" { + BeforeAll { + function Invoke-FormatterAssertion { + param( + [string] $ScriptDefinition, + [string] $ExcpectedScriptDefinition, + [int] $NumberOfExpectedWarnings, + [hashtable] $Settings + ) + + # Unit test just using this rule only + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be $NumberOfExpectedWarnings -Because $ScriptDefinition + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected -Because $ScriptDefinition + # Integration test with all default formatting rules + Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected -Because $ScriptDefinition + } + } BeforeEach { $indentationUnit = ' ' $indentationSize = 4 @@ -108,40 +126,36 @@ function foo { Context "When a multi-line command is given" { - It "When a comment is in the middle of a multi-line statement with line continuations" { + It "When a comment is in the middle of a multi-line statement with preceding and succeeding line continuations" { $scriptDefinition = @' foo ` # comment --bar +-bar ` +-baz '@ $expected = @' foo ` # comment - -bar + -bar ` + -baz '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings - $violations.Count | Should -Be 2 - Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected - Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected + Invoke-FormatterAssertion $scriptDefinition $expected 3 $settings } - It "When a comment is in the middle of a multi-line statement with line continuations 2" { + It "When a comment is in the middle of a multi-line statement with preceding pipeline and succeeding line continuation " { $scriptDefinition = @' -foo ` +foo | # comment --bar ` +bar ` -baz '@ $expected = @' -foo ` +foo | # comment - -bar ` - -baz + bar ` + -baz '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings - $violations.Count | Should -Be 3 - Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected - Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected + Invoke-FormatterAssertion $scriptDefinition $expected 3 $settings } It "Should find a violation if a pipleline element is not indented correctly" { From 00bf4c13ce6b19a37a1cf74f7981701a9b30012f Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 21 Aug 2019 22:00:49 +0100 Subject: [PATCH 5/5] rename indices for better readability to address PR feedback --- Rules/UseConsistentIndentation.cs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index f4a598087..1da472bf5 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -131,9 +131,9 @@ public override IEnumerable AnalyzeScript(Ast ast, string file var indentationLevel = 0; var onNewLine = true; var pipelineAsts = ast.FindAll(testAst => testAst is PipelineAst && (testAst as PipelineAst).PipelineElements.Count > 1, true); - for (int k = 0; k < tokens.Length; k++) + for (int tokenIndex = 0; tokenIndex < tokens.Length; tokenIndex++) { - var token = tokens[k]; + var token = tokens[tokenIndex]; if (token.Kind == TokenKind.EndOfInput) { @@ -151,8 +151,8 @@ public override IEnumerable AnalyzeScript(Ast ast, string file break; case TokenKind.Pipe: - bool pipelineIsFollowedByNewlineOrLineContinuation = k < tokens.Length - 1 && k > 0 && - (tokens[k + 1].Kind == TokenKind.NewLine || tokens[k + 1].Kind == TokenKind.LineContinuation); + bool pipelineIsFollowedByNewlineOrLineContinuation = tokenIndex < tokens.Length - 1 && tokenIndex > 0 && + (tokens[tokenIndex + 1].Kind == TokenKind.NewLine || tokens[tokenIndex + 1].Kind == TokenKind.LineContinuation); if (!pipelineIsFollowedByNewlineOrLineContinuation) { break; @@ -164,7 +164,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file } if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationForFirstPipeline) { - bool isFirstPipeInPipeline = pipelineAsts.Any(pipelineAst => PositionIsEqual(((PipelineAst)pipelineAst).PipelineElements[0].Extent.EndScriptPosition, tokens[k - 1].Extent.EndScriptPosition)); + bool isFirstPipeInPipeline = pipelineAsts.Any(pipelineAst => PositionIsEqual(((PipelineAst)pipelineAst).PipelineElements[0].Extent.EndScriptPosition, tokens[tokenIndex - 1].Extent.EndScriptPosition)); if (isFirstPipeInPipeline) { AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine); @@ -191,22 +191,22 @@ public override IEnumerable AnalyzeScript(Ast ast, string file var tempIndentationLevel = indentationLevel; // Check if the preceding character is an escape character - if (k > 0 && tokens[k - 1].Kind == TokenKind.LineContinuation) + if (tokenIndex > 0 && tokens[tokenIndex - 1].Kind == TokenKind.LineContinuation) { ++tempIndentationLevel; } // check for comments in between multi-line commands with line continuation - if (k > 2 && tokens[k - 1].Kind == TokenKind.NewLine - && tokens[k - 2].Kind == TokenKind.Comment) + if (tokenIndex > 2 && tokens[tokenIndex - 1].Kind == TokenKind.NewLine + && tokens[tokenIndex - 2].Kind == TokenKind.Comment) { - int j = k - 2; - while (j > 0 && tokens[j].Kind == TokenKind.Comment) + int searchForPrecedingLineContinuationIndex = tokenIndex - 2; + while (searchForPrecedingLineContinuationIndex > 0 && tokens[searchForPrecedingLineContinuationIndex].Kind == TokenKind.Comment) { - j--; + searchForPrecedingLineContinuationIndex--; } - if (j >= 0 && tokens[j].Kind == TokenKind.LineContinuation) + if (searchForPrecedingLineContinuationIndex >= 0 && tokens[searchForPrecedingLineContinuationIndex].Kind == TokenKind.LineContinuation) { tempIndentationLevel++; }