Skip to content

Add tabbed indentation capability to UseConsistentIndentation rule #790

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
merged 8 commits into from
Jun 20, 2017
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 @@ -24,6 +24,7 @@

PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
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 @@ -24,6 +24,7 @@

PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
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 @@ -24,6 +24,7 @@

PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
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 @@ -25,6 +25,7 @@

PSUseConsistentIndentation = @{
Enable = $true
Kind = 'space'
IndentationSize = 4
}

Expand Down
7 changes: 7 additions & 0 deletions RuleDocumentation/UseConsistentIndentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,10 @@ Enable or disable the rule during ScriptAnalyzer invocation.
#### IndentationSize: bool (Default value is `4`)

Indentation size in the number of space characters.

#### 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`.

`space` means `IndentationSize` number of `space` characters are used to provide one level of indentation.
`tab` means a tab character, `\t`.
60 changes: 51 additions & 9 deletions Rules/UseConsistentIndentation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,48 @@ public class UseConsistentIndentation : ConfigurableRule
[ConfigurableRuleProperty(defaultValue: 4)]
public int IndentationSize { get; protected set; }

private enum IndentationKind { Space, Tab };

// Cannot name to IndentationKind due to the enum type of the same name.
/// <summary>
/// Represents the kind of indentation to be used.
///
/// Possible values are: `space`, `tab`. If any invalid value is given, the
/// property defaults to `space`.
///
/// `space` means `IndentationSize` number of `space` characters are used to provide one level of indentation.
/// `tab` means a tab character, `\t`.
///</summary>
[ConfigurableRuleProperty(defaultValue: "space")]
public string Kind
{
get
{
return indentationKind.ToString();
}
set
{
if (String.IsNullOrWhiteSpace(value) ||
!Enum.TryParse<IndentationKind>(value, true, out indentationKind))
{
indentationKind = IndentationKind.Space;
}
}
}

private bool insertSpaces;
private char indentationChar;
private int indentationLevelMultiplier;

// TODO Enable auto when the rule is able to detect indentation
private enum IndentationKind {
Space,
Tab,
// Auto
};

// TODO make this configurable
private readonly IndentationKind indentationKind = IndentationKind.Space;
private IndentationKind indentationKind = IndentationKind.Space;


/// <summary>
/// Analyzes the given ast to find violations.
Expand All @@ -61,6 +99,14 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
return Enumerable.Empty<DiagnosticRecord>();
}

// It is more efficient to initialize these fields in ConfigurRule method
// but when the rule will enable `Auto` IndentationKind, we will anyways need to move
// the setting of these variables back here after the rule detects the indentation kind for
// each invocation.
insertSpaces = indentationKind == IndentationKind.Space;
indentationChar = insertSpaces ? ' ' : '\t';
indentationLevelMultiplier = insertSpaces ? IndentationSize : 1;

var tokens = Helper.Instance.Tokens;
var diagnosticRecords = new List<DiagnosticRecord>();
var indentationLevel = 0;
Expand Down Expand Up @@ -262,17 +308,13 @@ private int GetIndentationColumnNumber(int indentationLevel)

private int GetIndentation(int indentationLevel)
{
return indentationLevel * this.IndentationSize;
}

private char GetIndentationChar()
{
return indentationKind == IndentationKind.Space ? ' ' : '\t';
// todo if condition can be evaluated during rule configuration
return indentationLevel * indentationLevelMultiplier;
}

private string GetIndentationString(int indentationLevel)
{
return new string(GetIndentationChar(), GetIndentation(indentationLevel));
return new string(indentationChar, GetIndentation(indentationLevel));
}
}
}
65 changes: 50 additions & 15 deletions Tests/Rules/UseConsistentIndentation.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@ Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1")

$indentationUnit = ' '
$indentationSize = 4
$ruleConfiguration = @{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, looks like you used the formatter to format your formatter test script, nice!

Enable = $true
IndentationSize = 4
Kind = 'space'
}

$settings = @{
IncludeRules = @("PSUseConsistentIndentation")
Rules = @{
PSUseConsistentIndentation = @{
Enable = $true
IndentationSize = 4
}
Rules = @{
PSUseConsistentIndentation = $ruleConfiguration
}
}


Describe "UseConsistentIndentation" {
Context "When top level indentation is not consistent" {
BeforeAll {
Expand Down Expand Up @@ -127,17 +129,17 @@ function foo {
get-process |
where-object {$_.Name -match 'powershell'}
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 1
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 1
}

It "Should not find a violation if a pipleline element is indented correctly" {
$def = @'
get-process |
where-object {$_.Name -match 'powershell'}
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 0
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 0
}

It "Should ignore comment in the pipleline" {
Expand All @@ -147,8 +149,8 @@ get-process |
select Name,Id |
format-list
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 3
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 3
}

It "Should indent properly after line continuation (backtick) character" {
Expand All @@ -159,13 +161,46 @@ $x = "this " + `
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 1
$params = @{
RawContent = $def
RawContent = $def
DiagnosticRecord = $violations[0]
CorrectionsCount = 1
ViolationText = "`"Should be indented properly`""
CorrectionText = (New-Object -TypeName String -ArgumentList $indentationUnit,$indentationSize) + "`"Should be indented properly`""
ViolationText = "`"Should be indented properly`""
CorrectionText = (New-Object -TypeName String -ArgumentList $indentationUnit, $indentationSize) + "`"Should be indented properly`""
}
Test-CorrectionExtentFromContent @params
}
}

Context "When tabs instead of spaces are used for indentation" {
BeforeAll {
$ruleConfiguration.'Kind' = 'tab'
}

It "Should indent using tabs" {
$def = @'
function foo
{
get-childitem
$x=1+2
$hashtable = @{
property1 = "value"
anotherProperty = "another value"
}
}
'@
${t} = "`t"
$expected = @"
function foo
{
${t}get-childitem
${t}`$x=1+2
${t}`$hashtable = @{
${t}${t}property1 = "value"
${t}${t}anotherProperty = "another value"
${t}}
}
"@
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should Be $expected
}
}
}