diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 02a07a1da..d2e5437d3 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -25,6 +25,7 @@ PSUseConsistentIndentation = @{ Enable = $true Kind = 'space' + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' IndentationSize = 4 } diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index 15b0ecfbd..e02cda261 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -25,6 +25,7 @@ PSUseConsistentIndentation = @{ Enable = $true Kind = 'space' + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' IndentationSize = 4 } diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 589487235..0fa84db8a 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -25,6 +25,7 @@ PSUseConsistentIndentation = @{ Enable = $true Kind = 'space' + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' IndentationSize = 4 } diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index f40c83988..f63cc9911 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -26,6 +26,7 @@ PSUseConsistentIndentation = @{ Enable = $true Kind = 'space' + PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' IndentationSize = 4 } diff --git a/RuleDocumentation/UseConsistentIndentation.md b/RuleDocumentation/UseConsistentIndentation.md index ade8866f4..deca041ee 100644 --- a/RuleDocumentation/UseConsistentIndentation.md +++ b/RuleDocumentation/UseConsistentIndentation.md @@ -15,6 +15,7 @@ Indentation should be consistent throughout the source file. PSUseConsistentIndentation = @{ Enable = $true IndentationSize = 4 + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' Kind = 'space' } } @@ -30,6 +31,29 @@ Enable or disable the rule during ScriptAnalyzer invocation. Indentation size in the number of space characters. +#### PipelineIndentation: string (Default value is `IncreaseIndentationForFirstPipeline`) + +Whether to increase indentation after a pipeline for multi-line statements. The settings are: + +- IncreaseIndentationForFirstPipeline (default): Indent once after the first pipeline and keep this indentation. Example: +```powershell +foo | + bar | + baz +``` +- IncreaseIndentationAfterEveryPipeline: Indent more after the first pipeline and keep this indentation. Example: +```powershell +foo | + bar | + baz +``` +- NoIndentation: Do not increase indentation. Example: +```powershell +foo | +bar | +baz +``` + #### Kind: string (Default value is `space`) Represents the kind of indentation to be used. Possible values are: `space`, `tab`. If any invalid value is given, the property defaults to `space`. diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index f3395cca5..06724c3b7 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -57,6 +57,24 @@ public string Kind } } + + [ConfigurableRuleProperty(defaultValue: "IncreaseIndentationForFirstPipeline")] + public string PipelineIndentation + { + get + { + return pipelineIndentationStyle.ToString(); + } + set + { + if (String.IsNullOrWhiteSpace(value) || + !Enum.TryParse(value, true, out pipelineIndentationStyle)) + { + pipelineIndentationStyle = PipelineIndentationStyle.IncreaseIndentationAfterEveryPipeline; + } + } + } + private bool insertSpaces; private char indentationChar; private int indentationLevelMultiplier; @@ -68,9 +86,17 @@ private enum IndentationKind { // Auto }; + private enum PipelineIndentationStyle + { + IncreaseIndentationForFirstPipeline, + IncreaseIndentationAfterEveryPipeline, + NoIndentation + } + // TODO make this configurable private IndentationKind indentationKind = IndentationKind.Space; + private PipelineIndentationStyle pipelineIndentationStyle = PipelineIndentationStyle.IncreaseIndentationAfterEveryPipeline; /// /// Analyzes the given ast to find violations. @@ -104,6 +130,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file var diagnosticRecords = new List(); 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++) { var token = tokens[k]; @@ -123,6 +150,28 @@ public override IEnumerable AnalyzeScript(Ast ast, string file AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine); break; + case TokenKind.Pipe: + bool pipelineIsFollowedByNewlineOrLineContinuation = k < tokens.Length - 1 && k > 0 && + (tokens[k + 1].Kind == TokenKind.NewLine || tokens[k + 1].Kind == TokenKind.LineContinuation); + if (!pipelineIsFollowedByNewlineOrLineContinuation) + { + break; + } + if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationAfterEveryPipeline) + { + AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine); + break; + } + if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationForFirstPipeline) + { + bool isFirstPipeInPipeline = pipelineAsts.Any(pipelineAst => PositionIsEqual(((PipelineAst)pipelineAst).PipelineElements[0].Extent.EndScriptPosition, tokens[k - 1].Extent.EndScriptPosition)); + if (isFirstPipeInPipeline) + { + AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine); + } + } + break; + case TokenKind.RParen: case TokenKind.RCurly: indentationLevel = ClipNegative(indentationLevel - 1); @@ -156,22 +205,44 @@ public override IEnumerable AnalyzeScript(Ast ast, string file { --j; } - - if (j >= 0 && tokens[j].Kind == TokenKind.Pipe) - { - ++tempIndentationLevel; - } } - AddViolation(token, tempIndentationLevel, diagnosticRecords, ref onNewLine); + var lineHasPipelineBeforeToken = tokens.Any(oneToken => + oneToken.Kind == TokenKind.Pipe && + oneToken.Extent.StartLineNumber == token.Extent.StartLineNumber && + oneToken.Extent.StartColumnNumber < token.Extent.StartColumnNumber); + + AddViolation(token, tempIndentationLevel, diagnosticRecords, ref onNewLine, lineHasPipelineBeforeToken); } break; } + + // Check if the current token matches the end of a PipelineAst + var matchingPipeLineAstEnd = pipelineAsts.FirstOrDefault(pipelineAst => + PositionIsEqual(pipelineAst.Extent.EndScriptPosition, token.Extent.EndScriptPosition)) as PipelineAst; + if (matchingPipeLineAstEnd != null) + { + if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationForFirstPipeline) + { + indentationLevel = ClipNegative(indentationLevel - 1); + } + else if (pipelineIndentationStyle == PipelineIndentationStyle.IncreaseIndentationAfterEveryPipeline) + { + indentationLevel = ClipNegative(indentationLevel - (matchingPipeLineAstEnd.PipelineElements.Count - 1)); + } + } } return diagnosticRecords; } + private static bool PositionIsEqual(IScriptPosition position1, IScriptPosition position2) + { + return position1.ColumnNumber == position2.ColumnNumber && + position1.LineNumber == position2.LineNumber && + position1.File == position2.File; + } + /// /// Retrieves the common name of this rule. /// @@ -237,7 +308,8 @@ private void AddViolation( Token token, int expectedIndentationLevel, List diagnosticRecords, - ref bool onNewLine) + ref bool onNewLine, + bool lineHasPipelineBeforeToken = false) { if (onNewLine) { @@ -265,26 +337,28 @@ private void AddViolation( GetDiagnosticSeverity(), fileName, null, - GetSuggestedCorrections(token, expectedIndentationLevel))); + GetSuggestedCorrections(token, expectedIndentationLevel, lineHasPipelineBeforeToken))); } } } private List GetSuggestedCorrections( Token token, - int indentationLevel) + int indentationLevel, + bool lineHasPipelineBeforeToken = false) { // TODO Add another constructor for correction extent that takes extent // TODO handle param block // TODO handle multiline commands var corrections = new List(); + var optionalPipeline = lineHasPipelineBeforeToken ? "| " : string.Empty; corrections.Add(new CorrectionExtent( token.Extent.StartLineNumber, token.Extent.EndLineNumber, 1, token.Extent.EndColumnNumber, - GetIndentationString(indentationLevel) + token.Extent.Text, + GetIndentationString(indentationLevel) + optionalPipeline + token.Extent.Text, token.Extent.File)); return corrections; } diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 0f91b7e3f..408423dc7 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -3,41 +3,40 @@ $testRootDirectory = Split-Path -Parent $directory Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") -$indentationUnit = ' ' -$indentationSize = 4 -$ruleConfiguration = @{ - Enable = $true - IndentationSize = 4 - Kind = 'space' -} - -$settings = @{ - IncludeRules = @("PSUseConsistentIndentation") - Rules = @{ - PSUseConsistentIndentation = $ruleConfiguration +Describe "UseConsistentIndentation" { + BeforeEach { + $indentationUnit = ' ' + $indentationSize = 4 + $ruleConfiguration = @{ + Enable = $true + IndentationSize = 4 + PipelineIndentation = 'IncreaseIndentationForFirstPipeline' + Kind = 'space' + } + + $settings = @{ + IncludeRules = @("PSUseConsistentIndentation") + Rules = @{ + PSUseConsistentIndentation = $ruleConfiguration + } + } } -} -Describe "UseConsistentIndentation" { Context "When top level indentation is not consistent" { - BeforeAll { + It "Should detect a violation" { $def = @' function foo ($param1) { } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - } - - It "Should detect a violation" { $violations.Count | Should -Be 1 } } Context "When nested indenation is not consistent" { - BeforeAll { + It "Should find a violation" { $def = @' function foo ($param1) { @@ -45,15 +44,12 @@ function foo ($param1) } '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - } - - It "Should find a violation" { $violations.Count | Should -Be 1 } } Context "When a multi-line hashtable is provided" { - BeforeAll { + It "Should find violations" { $def = @' $hashtable = @{ a = 1 @@ -62,15 +58,12 @@ b = 2 } '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - } - - It "Should find violations" { $violations.Count | Should -Be 2 } } Context "When a multi-line array is provided" { - BeforeAll { + It "Should find violations" { $def = @' $array = @( 1, @@ -78,15 +71,12 @@ $array = @( 3) '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - } - - It "Should find violations" { $violations.Count | Should -Be 2 } } Context "When a param block is provided" { - BeforeAll { + It "Should find violations" { $def = @' param( [string] $param1, @@ -99,15 +89,12 @@ $param3 ) '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - } - - It "Should find violations" { $violations.Count | Should -Be 4 } } Context "When a sub-expression is provided" { - BeforeAll { + It "Should not find a violations" { $def = @' function foo { $x = $("abc") @@ -115,9 +102,6 @@ function foo { } '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - } - - It "Should not find a violations" { $violations.Count | Should -Be 0 } } @@ -135,7 +119,9 @@ where-object {$_.Name -match 'powershell'} It "Should not find a violation if a pipleline element is indented correctly" { $def = @' get-process | - where-object {$_.Name -match 'powershell'} + where-object { + $_.Name -match 'powershell' + } '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings $violations.Count | Should -Be 0 @@ -168,11 +154,68 @@ $x = "this " + ` } Test-CorrectionExtentFromContent @params } + + It "Should indent pipelines correctly using IncreaseIndentationAfterEveryPipeline option" { + $def = @' +foo | + bar | +baz +'@ + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + $params = @{ + RawContent = $def + DiagnosticRecord = $violations[0] + CorrectionsCount = 1 + ViolationText = "baz" + CorrectionText = (New-Object -TypeName String -ArgumentList $indentationUnit, ($indentationSize * 2)) + 'baz' + } + Test-CorrectionExtentFromContent @params + } + + It "Should indent pipelines correctly using NoIndentation option" { + $def = @' +foo | + bar | + baz +'@ + $settings.Rules.PSUseConsistentIndentation.PipelineIndentation = 'NoIndentation' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 2 + $params = @{ + RawContent = $def + DiagnosticRecord = $violations[1] + CorrectionsCount = 1 + ViolationText = " baz" + CorrectionText = (New-Object -TypeName String -ArgumentList $indentationUnit, ($indentationSize * 0)) + 'baz' + } + Test-CorrectionExtentFromContent @params + } + + + It "Should indent properly after line continuation (backtick) character with pipeline" { + $def = @' +foo | + bar ` +| baz +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + $params = @{ + RawContent = $def + DiagnosticRecord = $violations[0] + CorrectionsCount = 1 + ViolationText = "| baz" + CorrectionText = (New-Object -TypeName String -ArgumentList $indentationUnit, $indentationSize) + "| baz" + } + Test-CorrectionExtentFromContent @params + } } Context "When tabs instead of spaces are used for indentation" { - BeforeAll { - $ruleConfiguration.'Kind' = 'tab' + BeforeEach { + $settings.Rules.PSUseConsistentIndentation.Kind = 'tab' } It "Should indent using tabs" { @@ -183,7 +226,9 @@ get-childitem $x=1+2 $hashtable = @{ property1 = "value" - anotherProperty = "another value" + +'@ + "`t" + @' +anotherProperty = "another value" } } '@