diff --git a/Engine/TokenOperations.cs b/Engine/TokenOperations.cs index 567fe4146..35d4b6828 100644 --- a/Engine/TokenOperations.cs +++ b/Engine/TokenOperations.cs @@ -61,7 +61,6 @@ public IEnumerable GetOpenBracesInCommandElements() return GetBraceInCommandElement(TokenKind.LCurly); } - /// /// Return tokens of kind RCurly that end a scriptblock expression in an command element. /// @@ -74,6 +73,44 @@ public IEnumerable GetCloseBracesInCommandElements() return GetBraceInCommandElement(TokenKind.RCurly); } + /// + /// Returns pairs of associatd braces. + /// + /// Tuples of tokens such that item1 is LCurly token and item2 is RCurly token. + public IEnumerable> GetBracePairs() + { + var openBraceStack = new Stack(); + foreach (var token in tokens) + { + if (token.Kind == TokenKind.LCurly) + { + openBraceStack.Push(token); + continue; + } + + if (token.Kind == TokenKind.RCurly + && openBraceStack.Count > 0) + { + yield return new Tuple(openBraceStack.Pop(), token); + } + } + } + + /// + /// Returns brace pairs that are on the same line. + /// + /// Tuples of tokens such that item1 is LCurly token and item2 is RCurly token. + public IEnumerable> GetBracePairsOnSameLine() + { + foreach (var bracePair in GetBracePairs()) + { + if (bracePair.Item1.Extent.StartLineNumber == bracePair.Item2.Extent.StartLineNumber) + { + yield return bracePair; + } + } + } + private IEnumerable GetBraceInCommandElement(TokenKind tokenKind) { var cmdElemAsts = ast.FindAll(x => x is CommandElementAst && x is ScriptBlockExpressionAst, true); @@ -110,6 +147,86 @@ private IEnumerable GetBraceInCommandElement(TokenKind tokenKind) } } + public IEnumerable GetCloseBraceInOneLineIfStatement() + { + return GetBraceInOneLineIfStatment(TokenKind.RCurly); + } + + public IEnumerable GetOpenBraceInOneLineIfStatement() + { + return GetBraceInOneLineIfStatment(TokenKind.LCurly); + } + + // TODO Fix code duplication in the following method and GetBraceInCommandElement + private IEnumerable GetBraceInOneLineIfStatment(TokenKind tokenKind) + { + var ifStatementAsts = ast.FindAll(ast => + { + var ifAst = ast as IfStatementAst; + if (ifAst == null) + { + return false; + } + + return ifAst.Extent.StartLineNumber == ifAst.Extent.EndLineNumber; + }, + true); + + if (ifStatementAsts == null) + { + yield break; + } + + var braceTokens = new List(); + foreach (var ast in ifStatementAsts) + { + var ifStatementAst = ast as IfStatementAst; + foreach (var clause in ifStatementAst.Clauses) + { + var tokenIf + = tokenKind == TokenKind.LCurly + ? GetTokens(clause.Item2).FirstOrDefault() + : GetTokens(clause.Item2).LastOrDefault(); + if (tokenIf != null) + { + yield return tokenIf; + } + } + + if (ifStatementAst.ElseClause == null) + { + continue; + } + + var tokenElse + = tokenKind == TokenKind.LCurly + ? GetTokens(ifStatementAst.ElseClause).FirstOrDefault() + : GetTokens(ifStatementAst.ElseClause).LastOrDefault(); + if (tokenElse != null) + { + yield return tokenElse; + } + } + } + + private IEnumerable GetTokens(Ast ast) + { + int k = 0; + while (k < tokens.Length && tokens[k].Extent.EndOffset <= ast.Extent.StartOffset) + { + k++; + } + + while (k < tokens.Length && tokens[k].Extent.EndOffset <= ast.Extent.EndOffset) + { + var token = tokens[k++]; + if (token.Extent.StartOffset >= ast.Extent.StartOffset) + { + yield return token; + } + } + } + public IEnumerable> GetTokenNodes(TokenKind kind) { return GetTokenNodes((token) => token.Kind == kind); diff --git a/RuleDocumentation/PlaceCloseBrace.md b/RuleDocumentation/PlaceCloseBrace.md index 417528333..12b891dc0 100644 --- a/RuleDocumentation/PlaceCloseBrace.md +++ b/RuleDocumentation/PlaceCloseBrace.md @@ -12,6 +12,7 @@ Close brace placement should follow a consistent style. It should be on a new li PSPlaceCloseBrace = @{ Enable = $true NoEmptyLineBefore = $false + IgnoreOneLineBlock = $true } } ``` @@ -23,3 +24,8 @@ Enable or disable the rule during ScriptAnalyzer invocation. #### NoEmptyLineBefore: bool (Default value is `$false`) Create violation if there is an empty line before a close brace. + +#### IgnoreOneLineBlock: bool (Default value is `$true`) +Indicates if close braces in a one line block should be ignored or not. +E.g. $x = if ($true) { "blah" } else { "blah blah" } +In the above example, if the property is set to true then the rule will not fire a violation. \ No newline at end of file diff --git a/RuleDocumentation/PlaceOpenBrace.md b/RuleDocumentation/PlaceOpenBrace.md index b4cd7f1cd..c5a1f7b45 100644 --- a/RuleDocumentation/PlaceOpenBrace.md +++ b/RuleDocumentation/PlaceOpenBrace.md @@ -13,6 +13,7 @@ Open brace placement should follow a consistent style. It can either follow KR s Enable = $true OnSameLine = $true NewLineAfter = $true + IgnoreOneLineBlock = $true } } ``` @@ -27,3 +28,8 @@ Provide an option to enforce the open brace to be on the same line as preceding #### NewLineAfter: bool (Default value is `$true`) Enforce a new line character after an open brace. The default value is true. + +#### IgnoreOneLineBlock: bool (Default value is `$true`) +Indicates if open braces in a one line block should be ignored or not. +E.g. $x = if ($true) { "blah" } else { "blah blah" } +In the above example, if the property is set to true then the rule will not fire a violation. \ No newline at end of file diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index b17d44261..671f496f3 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -36,6 +36,17 @@ public class PlaceCloseBrace : ConfigurableRule [ConfigurableRuleProperty(defaultValue:false)] public bool NoEmptyLineBefore { get; protected set; } + /// + /// Indicates if close braces in a one line block should be ignored or not. + /// E.g. $x = if ($true) { "blah" } else { "blah blah" } + /// In the above example, if the property is set to true then the rule will + /// not fire a violation. + /// + /// Default value if true. + /// + [ConfigurableRuleProperty(defaultValue: true)] + public bool IgnoreOneLineBlock { get; protected set; } + private HashSet tokensToIgnore; /// @@ -58,14 +69,24 @@ public override IEnumerable AnalyzeScript(Ast ast, string file // TODO Should have the following options // * no-empty-lines-before - // * align (if close brance and open brace on new lines align with open brace, - // if close brace is on new line but open brace is not align with the first keyword on open brace line) var tokens = Helper.Instance.Tokens; var diagnosticRecords = new List(); var curlyStack = new Stack> (); - tokensToIgnore = new HashSet ( - new TokenOperations(tokens, ast).GetCloseBracesInCommandElements()); + + // TODO move part common with PlaceOpenBrace to one place + var tokenOps = new TokenOperations(tokens, ast); + tokensToIgnore = new HashSet (tokenOps.GetCloseBracesInCommandElements()); + + // Ignore close braces that are part of a one line if-else statement + // E.g. $x = if ($true) { "blah" } else { "blah blah" } + if (IgnoreOneLineBlock) + { + foreach (var pair in tokenOps.GetBracePairsOnSameLine()) + { + tokensToIgnore.Add(pair.Item2); + } + } for (int k = 0; k < tokens.Length; k++) { diff --git a/Rules/PlaceOpenBrace.cs b/Rules/PlaceOpenBrace.cs index 8d70aaf4c..50df9f6c0 100644 --- a/Rules/PlaceOpenBrace.cs +++ b/Rules/PlaceOpenBrace.cs @@ -45,6 +45,17 @@ public class PlaceOpenBrace : ConfigurableRule [ConfigurableRuleProperty(defaultValue: true)] public bool NewLineAfter { get; protected set; } + /// + /// Indicates if open braces in a one line block should be ignored or not. + /// E.g. $x = if ($true) { "blah" } else { "blah blah" } + /// In the above example, if the property is set to true then the rule will + /// not fire a violation. + /// + /// Default value if true. + /// + [ConfigurableRuleProperty(defaultValue: true)] + public bool IgnoreOneLineBlock { get; protected set; } + private List>> violationFinders = new List>>(); @@ -100,8 +111,19 @@ public override IEnumerable AnalyzeScript(Ast ast, string file // In the above case even if OnSameLine == false, we should not // flag the open brace as it would move the brace to the next line // and will invalidate the command - tokensToIgnore = new HashSet( - new TokenOperations(tokens, ast).GetOpenBracesInCommandElements()); + var tokenOps = new TokenOperations(tokens, ast); + tokensToIgnore = new HashSet(tokenOps.GetOpenBracesInCommandElements()); + + // Ignore open braces that are part of a one line if-else statement + // E.g. $x = if ($true) { "blah" } else { "blah blah" } + if (IgnoreOneLineBlock) + { + foreach (var pair in tokenOps.GetBracePairsOnSameLine()) + { + tokensToIgnore.Add(pair.Item1); + } + } + foreach (var violationFinder in violationFinders) { diagnosticRecords.AddRange(violationFinder(tokens, ast, fileName)); diff --git a/Tests/Rules/PlaceCloseBrace.tests.ps1 b/Tests/Rules/PlaceCloseBrace.tests.ps1 index 983bb590d..8fd9a4835 100644 --- a/Tests/Rules/PlaceCloseBrace.tests.ps1 +++ b/Tests/Rules/PlaceCloseBrace.tests.ps1 @@ -1,16 +1,17 @@ Import-Module PSScriptAnalyzer -$ruleName = "PSPlaceCloseBrace" +$ruleConfiguration = @{ + Enable = $true + NoEmptyLineBefore = $true + IgnoreOneLineBlock = $true +} + $settings = @{ IncludeRules = @("PSPlaceCloseBrace") Rules = @{ - PSPlaceCloseBrace = @{ - Enable = $true - NoEmptyLineBefore = $true - } + PSPlaceCloseBrace = $ruleConfiguration } } - Describe "PlaceCloseBrace" { Context "When a close brace is not on a new line" { BeforeAll { @@ -83,11 +84,21 @@ $hashtable = @{ $def = @' Get-Process * | % { "blah" } '@ + $ruleConfiguration.'IgnoreOneLineBlock' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } It "Should not find a violation" { $violations.Count | Should Be 0 } + + It "Should ignore violations for one line if statement" { + $def = @' +$x = if ($true) { "blah" } else { "blah blah" } +'@ + $ruleConfiguration.'IgnoreOneLineBlock' = $true + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should Be 0 + } } -} \ No newline at end of file +} diff --git a/Tests/Rules/PlaceOpenBrace.tests.ps1 b/Tests/Rules/PlaceOpenBrace.tests.ps1 index 200a8777e..bbfab27de 100644 --- a/Tests/Rules/PlaceOpenBrace.tests.ps1 +++ b/Tests/Rules/PlaceOpenBrace.tests.ps1 @@ -3,6 +3,7 @@ $ruleConfiguration = @{ Enable = $true OnSameLine = $true NewLineAfter = $true + IgnoreOneLineIf = $true } $settings = @{ @@ -14,7 +15,7 @@ $settings = @{ Describe "PlaceOpenBrace" { Context "When an open brace must be on the same line" { - BeforeAll{ + BeforeAll { $def = @' function foo ($param1) { @@ -35,14 +36,15 @@ function foo ($param1) } Context "When an open brace must be on a new line" { - BeforeAll{ + BeforeAll { $def = @' function foo ($param1) { } '@ $ruleConfiguration.'OnSameLine' = $false - $ruleConfiguration.'NewLineAfter' = $true + $ruleConfiguration.'NewLineAfter' = $true + $ruleConfiguration.'IgnoreOneLineBlock' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings $defShouldIgnore = @' Get-Process | % { "blah" } @@ -61,15 +63,25 @@ Get-Process | % { "blah" } It "Should ignore violations for a command element" { $violationsShouldIgnore.Count | Should Be 0 } + + It "Should ignore violations for one line if statement" { + $def = @' +$x = if ($true) { "blah" } else { "blah blah" } +'@ + $ruleConfiguration.'IgnoreOneLineBlock' = $true + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should Be 0 + } } Context "When a new line should follow an open brace" { - BeforeAll{ + BeforeAll { $def = @' function foo { } '@ $ruleConfiguration.'OnSameLine' = $true $ruleConfiguration.'NewLineAfter' = $true + $ruleConfiguration.'IgnoreOneLineBlock' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings }