From a4292c171f836e64b08bdd221883e4703444c5f7 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Sun, 18 Jun 2017 16:09:02 -0700 Subject: [PATCH 1/8] Add a property to configure indentation type --- Rules/UseConsistentIndentation.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index b7da9c629..aebc01808 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -36,6 +36,9 @@ public class UseConsistentIndentation : ConfigurableRule [ConfigurableRuleProperty(defaultValue: 4)] public int IndentationSize { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] + public bool InsertSpaces { get; protected set; } + private enum IndentationKind { Space, Tab }; // TODO make this configurable From ffa90061952f91ed7f7ad2b1621297d88b4e12cf Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 19 Jun 2017 20:33:27 -0700 Subject: [PATCH 2/8] Insert tabs if InsertSpaces is false --- Rules/UseConsistentIndentation.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index aebc01808..d1953e2ca 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -265,12 +265,14 @@ private int GetIndentationColumnNumber(int indentationLevel) private int GetIndentation(int indentationLevel) { - return indentationLevel * this.IndentationSize; + // todo if condition can be evaluated during rule configuration + return indentationLevel * (InsertSpaces ? this.IndentationSize : 1); } private char GetIndentationChar() { - return indentationKind == IndentationKind.Space ? ' ' : '\t'; + // todo can be evaluated during rule configuration + return InsertSpaces ? ' ' : '\t'; } private string GetIndentationString(int indentationLevel) From e545d5b7f0a41126f689295469b19d5badafad33 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 19 Jun 2017 20:39:37 -0700 Subject: [PATCH 3/8] Add tests for tabbed indentation --- .../Rules/UseConsistentIndentation.tests.ps1 | 53 +++++++++++++++---- 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 3dd083712..6f2664db1 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -6,17 +6,19 @@ Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") $indentationUnit = ' ' $indentationSize = 4 +$ruleConfiguration = @{ + Enable = $true + InsertSpaces = $true + IndentationSize = 4 +} + $settings = @{ IncludeRules = @("PSUseConsistentIndentation") - Rules = @{ - PSUseConsistentIndentation = @{ - Enable = $true - IndentationSize = 4 - } + Rules = @{ + PSUseConsistentIndentation = $ruleConfiguration } } - Describe "UseConsistentIndentation" { Context "When top level indentation is not consistent" { BeforeAll { @@ -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.'InsertSpaces' = $false + } + + 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 + } + } } From a255259c51f31fff5b8fac08435366378338267d Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 19 Jun 2017 20:40:09 -0700 Subject: [PATCH 4/8] Format UseConsistentIndentation.tests.ps1 --- Tests/Rules/UseConsistentIndentation.tests.ps1 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 6f2664db1..8f6f9501f 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -129,8 +129,8 @@ 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" { @@ -138,8 +138,8 @@ where-object {$_.Name -match 'powershell'} 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" { @@ -149,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" { From 92c6b5393c6ce30f99dcca29f2e99f3c931ebe8a Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 20 Jun 2017 14:43:24 -0700 Subject: [PATCH 5/8] Change configurable rule property --- Rules/UseConsistentIndentation.cs | 47 ++++++++++++++++--- .../Rules/UseConsistentIndentation.tests.ps1 | 4 +- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index d1953e2ca..8952f9fe3 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -36,13 +36,47 @@ public class UseConsistentIndentation : ConfigurableRule [ConfigurableRuleProperty(defaultValue: 4)] public int IndentationSize { get; protected set; } - [ConfigurableRuleProperty(defaultValue: true)] - public bool InsertSpaces { get; protected set; } - private enum IndentationKind { Space, Tab }; + // Cannot name to IndentationKind due to the enum type of the same name. + /// + /// 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` + /// `end` means the help is places the end of the function definition body. + /// + [ConfigurableRuleProperty(defaultValue: "space")] + public string Kind + { + get + { + return indentationKind.ToString(); + } + set + { + if (String.IsNullOrWhiteSpace(value) || + !Enum.TryParse(value, true, out indentationKind)) + { + indentationKind = IndentationKind.Space; + } + } + } + + private bool insertSpaces; + + // 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; + /// /// Analyzes the given ast to find violations. @@ -64,6 +98,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file return Enumerable.Empty(); } + insertSpaces = indentationKind == IndentationKind.Space; var tokens = Helper.Instance.Tokens; var diagnosticRecords = new List(); var indentationLevel = 0; @@ -266,13 +301,13 @@ private int GetIndentationColumnNumber(int indentationLevel) private int GetIndentation(int indentationLevel) { // todo if condition can be evaluated during rule configuration - return indentationLevel * (InsertSpaces ? this.IndentationSize : 1); + return indentationLevel * (insertSpaces ? this.IndentationSize : 1); } private char GetIndentationChar() { // todo can be evaluated during rule configuration - return InsertSpaces ? ' ' : '\t'; + return insertSpaces ? ' ' : '\t'; } private string GetIndentationString(int indentationLevel) diff --git a/Tests/Rules/UseConsistentIndentation.tests.ps1 b/Tests/Rules/UseConsistentIndentation.tests.ps1 index 8f6f9501f..89bb6d3f1 100644 --- a/Tests/Rules/UseConsistentIndentation.tests.ps1 +++ b/Tests/Rules/UseConsistentIndentation.tests.ps1 @@ -8,8 +8,8 @@ $indentationUnit = ' ' $indentationSize = 4 $ruleConfiguration = @{ Enable = $true - InsertSpaces = $true IndentationSize = 4 + Kind = 'space' } $settings = @{ @@ -173,7 +173,7 @@ $x = "this " + ` Context "When tabs instead of spaces are used for indentation" { BeforeAll { - $ruleConfiguration.'InsertSpaces' = $false + $ruleConfiguration.'Kind' = 'tab' } It "Should indent using tabs" { From b5fdbe238ddaad41816f0e6a1356a7edecf0e62d Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 20 Jun 2017 14:53:27 -0700 Subject: [PATCH 6/8] Reduce frequent variable compuation --- Rules/UseConsistentIndentation.cs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Rules/UseConsistentIndentation.cs b/Rules/UseConsistentIndentation.cs index 8952f9fe3..58880d021 100644 --- a/Rules/UseConsistentIndentation.cs +++ b/Rules/UseConsistentIndentation.cs @@ -45,8 +45,7 @@ public class UseConsistentIndentation : ConfigurableRule /// 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` - /// `end` means the help is places the end of the function definition body. + /// `tab` means a tab character, `\t`. /// [ConfigurableRuleProperty(defaultValue: "space")] public string Kind @@ -66,6 +65,8 @@ public string Kind } private bool insertSpaces; + private char indentationChar; + private int indentationLevelMultiplier; // TODO Enable auto when the rule is able to detect indentation private enum IndentationKind { @@ -98,7 +99,14 @@ public override IEnumerable AnalyzeScript(Ast ast, string file return Enumerable.Empty(); } + // 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(); var indentationLevel = 0; @@ -301,18 +309,12 @@ private int GetIndentationColumnNumber(int indentationLevel) private int GetIndentation(int indentationLevel) { // todo if condition can be evaluated during rule configuration - return indentationLevel * (insertSpaces ? this.IndentationSize : 1); - } - - private char GetIndentationChar() - { - // todo can be evaluated during rule configuration - return insertSpaces ? ' ' : '\t'; + return indentationLevel * indentationLevelMultiplier; } private string GetIndentationString(int indentationLevel) { - return new string(GetIndentationChar(), GetIndentation(indentationLevel)); + return new string(indentationChar, GetIndentation(indentationLevel)); } } } From 00352f3453eb0bb41d11c80b4a0666e91ad3092e Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 20 Jun 2017 15:01:05 -0700 Subject: [PATCH 7/8] Add doc for Indentation.Kind parameter --- RuleDocumentation/UseConsistentIndentation.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/RuleDocumentation/UseConsistentIndentation.md b/RuleDocumentation/UseConsistentIndentation.md index f89f7324a..51c32ba18 100644 --- a/RuleDocumentation/UseConsistentIndentation.md +++ b/RuleDocumentation/UseConsistentIndentation.md @@ -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`. From 7cff2648fa44eb368847cbb271ecc4e295bed6b5 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 20 Jun 2017 15:13:15 -0700 Subject: [PATCH 8/8] Update code formatting presets --- Engine/Settings/CodeFormatting.psd1 | 1 + Engine/Settings/CodeFormattingAllman.psd1 | 1 + Engine/Settings/CodeFormattingOTBS.psd1 | 1 + Engine/Settings/CodeFormattingStroustrup.psd1 | 1 + 4 files changed, 4 insertions(+) diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index c6c94129c..02a07a1da 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -24,6 +24,7 @@ PSUseConsistentIndentation = @{ Enable = $true + Kind = 'space' IndentationSize = 4 } diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index bf10dffd8..15b0ecfbd 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -24,6 +24,7 @@ PSUseConsistentIndentation = @{ Enable = $true + Kind = 'space' IndentationSize = 4 } diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 2bd6d69d4..589487235 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -24,6 +24,7 @@ PSUseConsistentIndentation = @{ Enable = $true + Kind = 'space' IndentationSize = 4 } diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 8640f6761..51cbb540a 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -25,6 +25,7 @@ PSUseConsistentIndentation = @{ Enable = $true + Kind = 'space' IndentationSize = 4 }