Skip to content

Fix PSUseConsistentIndentationRule to handle pipes correctly when there is a multi-line statement after a pipe and add PipelineIndentation customisation option for it #1102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Engine/Settings/CodeFormatting.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
IndentationSize = 4
}

Expand Down
1 change: 1 addition & 0 deletions Engine/Settings/CodeFormattingAllman.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
IndentationSize = 4
}

Expand Down
1 change: 1 addition & 0 deletions Engine/Settings/CodeFormattingOTBS.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
IndentationSize = 4
}

Expand Down
1 change: 1 addition & 0 deletions Engine/Settings/CodeFormattingStroustrup.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
IndentationSize = 4
}

Expand Down
24 changes: 24 additions & 0 deletions RuleDocumentation/UseConsistentIndentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Indentation should be consistent throughout the source file.
PSUseConsistentIndentation = @{
Enable = $true
IndentationSize = 4
PipelineIndentation = 'IncreaseIndentationForFirstPipeline'
Kind = 'space'
}
}
Expand All @@ -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`.
Expand Down
94 changes: 84 additions & 10 deletions Rules/UseConsistentIndentation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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];
Expand All @@ -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);
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand Down
Loading