Fix PSUseConsistentIndentationRule to handle pipes correctly when the…

…re is a multi-line statement after a pipe and add PipelineIndentation customisation option for it (PowerShell#1102) * Fix PSUseConsistentIndentationRule to handle pipes correctly when there is a multi-line statement after it: Do not just add temporary indentation for the next line but keep the indentation level * correctly decrement indentation count for multiple pipes * add customisation * take pipeline into account and add docs. TODO: Change shipped setting files * update formatting setting files * add tests and refactor/improve test suite * Apply suggestions from code review Co-Authored-By: bergmeister <[email protected]> * Remove else block by using break
bergmeister · Mar 22, 2019 · 941e546 · 941e546
1 parent 50b4648
commit 941e546
Show file tree

Hide file tree

Showing 7 changed files with 200 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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
@@ -26,6 +26,7 @@
         PSUseConsistentIndentation = @{
             Enable          = $true
             Kind            = 'space'
+            PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
             IndentationSize = 4
         }
 

diff --git 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
@@ -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;
 
         /// <summary>
         /// Analyzes the given ast to find violations.
@@ -104,6 +130,7 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
             var diagnosticRecords = new List<DiagnosticRecord>();
             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<DiagnosticRecord> 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<DiagnosticRecord> 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;
+        }
+
         /// <summary>
         /// Retrieves the common name of this rule.
         /// </summary>
@@ -237,7 +308,8 @@ private void AddViolation(
             Token token,
             int expectedIndentationLevel,
             List<DiagnosticRecord> 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<CorrectionExtent> 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<CorrectionExtent>();
+            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;
         }