From 7e740b2393b12caec08518425eb5a0a9093d92dc Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 25 Sep 2018 20:25:53 +0100 Subject: [PATCH 01/17] Check for 1 whitespace AFTER curly brace in new InnerCurly option --- Rules/UseConsistentWhitespace.cs | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 12a47dfbd..6e69e5abf 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -41,6 +41,9 @@ private List>> violationFind [ConfigurableRuleProperty(defaultValue: true)] public bool CheckOpenBrace { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] + public bool CheckInnerBrace { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] public bool CheckOpenParen { get; protected set; } @@ -58,6 +61,11 @@ public override void ConfigureRule(IDictionary paramValueMap) violationFinders.Add(FindOpenBraceViolations); } + if (CheckInnerBrace) + { + violationFinders.Add(FindInnerBraceViolations); + } + if (CheckOpenParen) { violationFinders.Add(FindOpenParenViolations); @@ -233,6 +241,32 @@ private IEnumerable FindOpenParenViolations(TokenOperations to } } + private IEnumerable FindInnerBraceViolations(TokenOperations tokenOperations) + { + foreach (var lcurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) + { + if (lcurly.Next == null + || !IsPreviousTokenOnSameLine(lcurly) + || lcurly.Next.Value.Kind == TokenKind.LCurly + || ((lcurly.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + { + continue; + } + + if (!IsNextTokenApartByWhitespace(lcurly)) + { + yield return new DiagnosticRecord( + GetError(ErrorKind.Brace), + lcurly.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(lcurly.Previous.Value, lcurly.Value, lcurly.Next.Value, true, false).ToList()); + } + } + } + private bool IsSeparator(Token token) { return token.Kind == TokenKind.Comma || token.Kind == TokenKind.Semi; @@ -291,6 +325,12 @@ private bool IsPreviousTokenApartByWhitespace(LinkedListNode tokenNode) (tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber); } + private bool IsNextTokenApartByWhitespace(LinkedListNode tokenNode) + { + return whiteSpaceSize == + (tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber); + } + private bool IsPreviousTokenOnSameLineAndApartByWhitespace(LinkedListNode tokenNode) { return IsPreviousTokenOnSameLine(tokenNode) && IsPreviousTokenApartByWhitespace(tokenNode); From cebbaad44570071ab1d6ce978c29d85b4e6eba68 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 25 Sep 2018 20:32:40 +0100 Subject: [PATCH 02/17] check for one space in closing of inner brace --- Rules/UseConsistentWhitespace.cs | 65 +++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 6e69e5abf..ec7439f32 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -219,50 +219,73 @@ private IEnumerable FindOpenBraceViolations(TokenOperations to } } - private IEnumerable FindOpenParenViolations(TokenOperations tokenOperations) + private IEnumerable FindInnerBraceViolations(TokenOperations tokenOperations) { - foreach (var lparen in tokenOperations.GetTokenNodes(TokenKind.LParen)) + foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) { - if (lparen.Previous != null - && IsPreviousTokenOnSameLine(lparen) - && TokenTraits.HasTrait(lparen.Previous.Value.Kind, TokenFlags.Keyword) - && IsKeyword(lparen.Previous.Value) - && !IsPreviousTokenApartByWhitespace(lparen)) + if (lCurly.Next == null + || !IsPreviousTokenOnSameLine(lCurly) + || lCurly.Next.Value.Kind == TokenKind.LCurly + || ((lCurly.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + { + continue; + } + + if (!IsNextTokenApartByWhitespace(lCurly)) { yield return new DiagnosticRecord( - GetError(ErrorKind.Paren), - lparen.Value.Extent, + GetError(ErrorKind.Brace), + lCurly.Value.Extent, GetName(), GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(lparen.Previous.Value, lparen.Value, lparen.Next.Value, false, true).ToList()); + GetCorrections(lCurly.Previous.Value, lCurly.Value, lCurly.Next.Value, false, true).ToList()); } } - } - private IEnumerable FindInnerBraceViolations(TokenOperations tokenOperations) - { - foreach (var lcurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) + foreach (var rCurly in tokenOperations.GetTokenNodes(TokenKind.RCurly)) { - if (lcurly.Next == null - || !IsPreviousTokenOnSameLine(lcurly) - || lcurly.Next.Value.Kind == TokenKind.LCurly - || ((lcurly.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + if (rCurly.Previous == null + || !IsPreviousTokenOnSameLine(rCurly) + || rCurly.Previous.Value.Kind == TokenKind.LCurly + || ((rCurly.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; } - if (!IsNextTokenApartByWhitespace(lcurly)) + if (!IsPreviousTokenApartByWhitespace(rCurly)) { yield return new DiagnosticRecord( GetError(ErrorKind.Brace), - lcurly.Value.Extent, + rCurly.Value.Extent, GetName(), GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(lcurly.Previous.Value, lcurly.Value, lcurly.Next.Value, true, false).ToList()); + GetCorrections(rCurly.Previous.Value, rCurly.Value, rCurly.Next.Value, true, false).ToList()); + } + } + } + + private IEnumerable FindOpenParenViolations(TokenOperations tokenOperations) + { + foreach (var lparen in tokenOperations.GetTokenNodes(TokenKind.LParen)) + { + if (lparen.Previous != null + && IsPreviousTokenOnSameLine(lparen) + && TokenTraits.HasTrait(lparen.Previous.Value.Kind, TokenFlags.Keyword) + && IsKeyword(lparen.Previous.Value) + && !IsPreviousTokenApartByWhitespace(lparen)) + { + yield return new DiagnosticRecord( + GetError(ErrorKind.Paren), + lparen.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(lparen.Previous.Value, lparen.Value, lparen.Next.Value, false, true).ToList()); } } } From 5f444bda0ec191ae35d59084a87fe85621a32eaa Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 25 Sep 2018 22:02:10 +0100 Subject: [PATCH 03/17] add check for pipes. TODO: messages --- Rules/UseConsistentWhitespace.cs | 57 ++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index ec7439f32..79970f005 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -44,6 +44,9 @@ private List>> violationFind [ConfigurableRuleProperty(defaultValue: true)] public bool CheckInnerBrace { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] + public bool CheckPipe { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] public bool CheckOpenParen { get; protected set; } @@ -66,6 +69,11 @@ public override void ConfigureRule(IDictionary paramValueMap) violationFinders.Add(FindInnerBraceViolations); } + if (CheckPipe) + { + violationFinders.Add(FindPipeViolations); + } + if (CheckOpenParen) { violationFinders.Add(FindOpenParenViolations); @@ -268,6 +276,55 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t } } + private IEnumerable FindPipeViolations(TokenOperations tokenOperations) + { + foreach (var pipe in tokenOperations.GetTokenNodes(TokenKind.Pipe)) + { + if (pipe.Next == null + || !IsPreviousTokenOnSameLine(pipe) + || pipe.Next.Value.Kind == TokenKind.Pipe + || ((pipe.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + { + continue; + } + + if (!IsNextTokenApartByWhitespace(pipe)) + { + yield return new DiagnosticRecord( + GetError(ErrorKind.Brace), + pipe.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList()); + } + } + + foreach (var pipe in tokenOperations.GetTokenNodes(TokenKind.Pipe)) + { + if (pipe.Previous == null + || !IsPreviousTokenOnSameLine(pipe) + || pipe.Previous.Value.Kind == TokenKind.Pipe + || ((pipe.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + { + continue; + } + + if (!IsPreviousTokenApartByWhitespace(pipe)) + { + yield return new DiagnosticRecord( + GetError(ErrorKind.Brace), + pipe.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList()); + } + } + } + private IEnumerable FindOpenParenViolations(TokenOperations tokenOperations) { foreach (var lparen in tokenOperations.GetTokenNodes(TokenKind.LParen)) From 60885b0c2a445f4f3cd5ece106bd1e8786d1d68c Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 00:10:47 +0000 Subject: [PATCH 04/17] update settings files and add first set of tests for curly braces. TODO: cater for newlines --- Engine/Settings/CodeFormatting.psd1 | 12 ++--- Engine/Settings/CodeFormattingAllman.psd1 | 12 ++--- Engine/Settings/CodeFormattingOTBS.psd1 | 2 + Engine/Settings/CodeFormattingStroustrup.psd1 | 12 ++--- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 44 +++++++++++++++++++ 5 files changed, 67 insertions(+), 15 deletions(-) diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 02a07a1da..efceb62f7 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -29,11 +29,13 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckSeparator = $true + Enable = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckSeparator = $true + CheckInnerBrace = $true + CheckPipe = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index 15b0ecfbd..52b544411 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -29,11 +29,13 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckSeparator = $true + Enable = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckSeparator = $true + CheckInnerBrace = $true + CheckPipe = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 589487235..9157ec2e4 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -34,6 +34,8 @@ CheckOpenParen = $true CheckOperator = $true CheckSeparator = $true + CheckInnerBrace = $true + CheckPipe = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index f40c83988..e9a8dc539 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -30,11 +30,13 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckSeparator = $true + Enable = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckSeparator = $true + CheckInnerBrace = $true + CheckPipe = $true } PSAlignAssignmentStatement = @{ diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 75f620dee..0f5b8aa76 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -10,6 +10,8 @@ $ruleConfiguration = @{ CheckOpenParen = $false CheckOperator = $false CheckSeparator = $false + CheckInnerBrace = $false + CheckPipe = $false } $settings = @{ @@ -246,7 +248,49 @@ $x = "abc"; '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } + } + + + Context "CheckInnerBrace" { + BeforeAll { + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.CheckInnerBrace = $true + $ruleConfiguration.CheckPipe = $false + } + It "Should find a violation" { + $def = @' +if ($true) {Get-Item} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + It "Should not find a violation if there is 1 space after the opening brace and 1 before the closing brace" { + $def = @' +if($true) { Get-Item } +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should not find a violation if a new-line is after the opening brace" { + $def = @' +if ($true) { + Get-Item } +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should not find a violation if a new-line is before the closing brace" { + $def = @' +if ($true) { Get-Item +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } } + } From 692969e00e29cc58c99e710c5d11d6c3733477e6 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 10:26:57 +0000 Subject: [PATCH 05/17] fix new line handling for innerbrace --- Rules/UseConsistentWhitespace.cs | 2 ++ Tests/Rules/UseConsistentWhitespace.tests.ps1 | 12 ++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 111bae5dc..6da308cf7 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -234,6 +234,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t if (lCurly.Next == null || !IsPreviousTokenOnSameLine(lCurly) || lCurly.Next.Value.Kind == TokenKind.LCurly + || lCurly.Next.Value.Kind == TokenKind.NewLine || ((lCurly.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; @@ -257,6 +258,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t if (rCurly.Previous == null || !IsPreviousTokenOnSameLine(rCurly) || rCurly.Previous.Value.Kind == TokenKind.LCurly + || rCurly.Previous.Value.Kind == TokenKind.NewLine || ((rCurly.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 0f5b8aa76..5a02a782b 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -261,10 +261,14 @@ $x = "abc"; $ruleConfiguration.CheckPipe = $false } - It "Should find a violation" { - $def = @' -if ($true) {Get-Item} -'@ + It "Should find a violation if there is no space after opening brace" { + $def = 'if ($true) {Get-Item }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should find a violation if there is no space before closing brace" { + $def = 'if ($true) { Get-Item}' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } From 5ab67c13b831f85686a75da36a7bc8fb07c73c2e Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 12:29:58 +0000 Subject: [PATCH 06/17] fix suggested corrections and add another test case for inner brace --- Rules/UseConsistentWhitespace.cs | 9 +++++---- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 7 +++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 6da308cf7..7f9f86e52 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -249,7 +249,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(lCurly.Previous.Value, lCurly.Value, lCurly.Next.Value, false, true).ToList()); + GetCorrections(lCurly.Previous.Value, lCurly.Value, lCurly.Next.Value, true, false).ToList()); } } @@ -273,7 +273,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(rCurly.Previous.Value, rCurly.Value, rCurly.Next.Value, true, false).ToList()); + GetCorrections(rCurly.Previous.Value, rCurly.Value, rCurly.Next.Value, false, true).ToList()); } } } @@ -464,8 +464,9 @@ private List GetCorrections( Token prevToken, Token token, Token nextToken, - bool hasWhitespaceBefore, - bool hasWhitespaceAfter) + bool hasWhitespaceBefore, // if this is false, then the returned correction extent will add a whitespace before the token + bool hasWhitespaceAfter // if this is false, then the returned correction extent will add a whitespace after the token + ) { var sb = new StringBuilder(); IScriptExtent e1 = token.Extent; diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 5a02a782b..bc5ea25eb 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -280,6 +280,13 @@ if($true) { Get-Item } $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } + It "Should not find a violation if there is 1 space inside empty curly braces" { + $def = @' +if($true) { } +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + It "Should not find a violation if a new-line is after the opening brace" { $def = @' if ($true) { From a4f5c8df5f457bb01fe9c7394aa29f65e0ea3a63 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 12:45:24 +0000 Subject: [PATCH 07/17] order settings alphabetically and add tests for checkpipe --- Engine/Settings/CodeFormatting.psd1 | 4 +- Engine/Settings/CodeFormattingAllman.psd1 | 4 +- Engine/Settings/CodeFormattingOTBS.psd1 | 4 +- Engine/Settings/CodeFormattingStroustrup.psd1 | 4 +- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 69 ++++++++++++++++++- 5 files changed, 74 insertions(+), 11 deletions(-) diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index efceb62f7..297d93326 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -30,12 +30,12 @@ PSUseConsistentWhitespace = @{ Enable = $true + CheckInnerBrace = $true CheckOpenBrace = $true CheckOpenParen = $true CheckOperator = $true - CheckSeparator = $true - CheckInnerBrace = $true CheckPipe = $true + CheckSeparator = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index 52b544411..ba2664b86 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -30,12 +30,12 @@ PSUseConsistentWhitespace = @{ Enable = $true + CheckInnerBrace = $true CheckOpenBrace = $true CheckOpenParen = $true CheckOperator = $true - CheckSeparator = $true - CheckInnerBrace = $true CheckPipe = $true + CheckSeparator = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 9157ec2e4..634747a63 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -30,12 +30,12 @@ PSUseConsistentWhitespace = @{ Enable = $true + CheckInnerBrace = $true CheckOpenBrace = $true CheckOpenParen = $true CheckOperator = $true - CheckSeparator = $true - CheckInnerBrace = $true CheckPipe = $true + CheckSeparator = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index e9a8dc539..03a195b16 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -31,12 +31,12 @@ PSUseConsistentWhitespace = @{ Enable = $true + CheckInnerBrace = $true CheckOpenBrace = $true CheckOpenParen = $true CheckOperator = $true - CheckSeparator = $true - CheckInnerBrace = $true CheckPipe = $true + CheckSeparator = $true } PSAlignAssignmentStatement = @{ diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index bc5ea25eb..4d5dfeb6d 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -6,12 +6,12 @@ Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") $ruleName = "PSUseConsistentWhitespace" $ruleConfiguration = @{ Enable = $true + CheckInnerBrace = $false CheckOpenBrace = $false CheckOpenParen = $false CheckOperator = $false - CheckSeparator = $false - CheckInnerBrace = $false CheckPipe = $false + CheckSeparator = $false } $settings = @{ @@ -24,9 +24,11 @@ $settings = @{ Describe "UseWhitespace" { Context "When an open brace follows a keyword" { BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false $ruleConfiguration.CheckOpenBrace = $true $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $false } @@ -63,9 +65,11 @@ if($true) {} Context "When a parenthesis follows a keyword" { BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false $ruleConfiguration.CheckOpenBrace = $false $ruleConfiguration.CheckOpenParen = $true $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $false } @@ -114,9 +118,11 @@ $x.foo("bar") Context "When there is whitespace around assignment and binary operators" { BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOpenBrace = $false $ruleConfiguration.CheckOperator = $true + $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $false } @@ -187,9 +193,11 @@ $x = $true -and Context "When a comma is not followed by a space" { BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false $ruleConfiguration.CheckOpenBrace = $false $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $true } @@ -211,9 +219,11 @@ $x = @(1, 2) Context "When a semi-colon is not followed by a space" { BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false $ruleConfiguration.CheckOpenBrace = $false $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $true } @@ -251,14 +261,67 @@ $x = "abc"; } - Context "CheckInnerBrace" { + Context "CheckPipe" { BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false $ruleConfiguration.CheckOpenBrace = $false $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $true $ruleConfiguration.CheckSeparator = $false + } + + It "Should find a violation if there is no space after pipe" { + $def = 'Get-Item |foo' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should find a violation if there is no space before pipe" { + $def = 'Get-Item| foo' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should not find a violation if there is 1 space before and after a pipe" { + $def = 'Get-Item | foo' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should not find a violation if a new-line is before the pipe" { + $def = @' +Get-Item ` +| foo +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should not find a violation if a new-line is after the pipe" { + $def = @' +Get-Item | + foo +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should not find a violation if a backtick after the pipe" { + $def = @' +Get-Item |` +foo +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + } + + + Context "CheckInnerBrace" { + BeforeAll { $ruleConfiguration.CheckInnerBrace = $true + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOperator = $false $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckSeparator = $false } It "Should find a violation if there is no space after opening brace" { From b45cce5561949dcc1fb7441fc56bee472ab035d2 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 12:48:42 +0000 Subject: [PATCH 08/17] fix test that returned 2 warnings now due to checkinnerbrace --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index c43cac665..34ce9579b 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -181,15 +181,15 @@ Describe "Test Path" { $numFilesResult | Should -Be $numFilesExpected } } - + Context "When piping in files" { It "Can be piped in from a string" { $piped = ("$directory\TestScript.ps1" | Invoke-ScriptAnalyzer) $explicit = Invoke-ScriptAnalyzer -Path $directory\TestScript.ps1 - + $piped.Count | Should Be $explicit.Count } - + It "Can be piped from Get-ChildItem" { $piped = ( Get-ChildItem -Path $directory -Filter TestTestPath*.ps1 | Invoke-ScriptAnalyzer) $explicit = Invoke-ScriptAnalyzer -Path $directory\TestTestPath*.ps1 @@ -410,10 +410,10 @@ Describe "Test CustomizedRulePath" { Pop-Location } } - + It "resolves rule preset when passed in via pipeline" { $warnings = 'CodeFormattingStroustrup' | ForEach-Object { - Invoke-ScriptAnalyzer -ScriptDefinition 'if ($true){}' -Settings $_} + Invoke-ScriptAnalyzer -ScriptDefinition 'if ($true){ }' -Settings $_} $warnings.Count | Should -Be 1 $warnings.RuleName | Should -Be 'PSUseConsistentWhitespace' } @@ -552,8 +552,8 @@ Describe "Test -EnableExit Switch" { else { $result = powershell -command 'Invoke-Scriptanalyzer -ScriptDefinition gci -ReportSummary' } - - "$result" | Should -BeLike $reportSummaryFor1Warning + + "$result" | Should -BeLike $reportSummaryFor1Warning } It "does not print the report summary when not using -NoReportSummary switch" { if ($IsCoreCLR) { @@ -562,8 +562,8 @@ Describe "Test -EnableExit Switch" { else { $result = powershell -command 'Invoke-Scriptanalyzer -ScriptDefinition gci' } - - "$result" | Should -Not -BeLike $reportSummaryFor1Warning + + "$result" | Should -Not -BeLike $reportSummaryFor1Warning } } From 0668df9a6800e295ec62dddb8cbb8e0180f63cc0 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 13:08:36 +0000 Subject: [PATCH 09/17] fix innerPipe and write documentation --- RuleDocumentation/UseConsistentWhitespace.md | 16 ++++++++++++---- Rules/UseConsistentWhitespace.cs | 8 ++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/RuleDocumentation/UseConsistentWhitespace.md b/RuleDocumentation/UseConsistentWhitespace.md index 192f7441a..10d333971 100644 --- a/RuleDocumentation/UseConsistentWhitespace.md +++ b/RuleDocumentation/UseConsistentWhitespace.md @@ -28,18 +28,26 @@ Enable or disable the rule during ScriptAnalyzer invocation. +#### CheckInnerBrace: bool (Default value is `$true`) + +Checks if there is a space after the opening brace and a space before the closing brace. E.g. `if ($true) { foo }` instead of `if ($true) {bar}`. + #### CheckOpenBrace: bool (Default value is `$true`) -Checks if there is a space between a keyword and its corresponding open brace. E.g. `foo { }`. +Checks if there is a space between a keyword and its corresponding open brace. E.g. `foo { }` instead of `foo{ }`. #### CheckOpenParen: bool (Default value is `$true`) -Checks if there is space between a keyword and its corresponding open parenthesis. E.g. `if (true)`. +Checks if there is space between a keyword and its corresponding open parenthesis. E.g. `if (true)` instead of `if(true)`. #### CheckOperator: bool (Default value is `$true`) -Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1`. +Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1` instead of `$x=1`. #### CheckSeparator: bool (Default value is `$true`) -Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@{a = 1; b = 2}`. +Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@{a = 1; b = 2}` instead of `@(1,2,3)` or `@{a = 1;b = 2}`. + +#### CheckPipe: bool (Default value is `$true`) + +Checks if a pipe is surrounded on both sides by a space. E.g. `foo | bar` instead of `foo|bar`. \ No newline at end of file diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 7f9f86e52..0bba83a54 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -285,6 +285,8 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp if (pipe.Next == null || !IsPreviousTokenOnSameLine(pipe) || pipe.Next.Value.Kind == TokenKind.Pipe + || pipe.Next.Value.Kind == TokenKind.NewLine + || pipe.Next.Value.Kind == TokenKind.LineContinuation || ((pipe.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; @@ -299,7 +301,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList()); + GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList()); } } @@ -308,6 +310,8 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp if (pipe.Previous == null || !IsPreviousTokenOnSameLine(pipe) || pipe.Previous.Value.Kind == TokenKind.Pipe + || pipe.Previous.Value.Kind == TokenKind.NewLine + || pipe.Previous.Value.Kind == TokenKind.LineContinuation || ((pipe.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; @@ -322,7 +326,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList()); + GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList()); } } } From d74412e94c1779beb231957d5d33106cdf89b70f Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 13:36:33 +0000 Subject: [PATCH 10/17] tweak backtick scenarios --- Rules/UseConsistentWhitespace.cs | 1 + Tests/Rules/UseConsistentWhitespace.tests.ps1 | 20 +++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 0bba83a54..6b1b82157 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -235,6 +235,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t || !IsPreviousTokenOnSameLine(lCurly) || lCurly.Next.Value.Kind == TokenKind.LCurly || lCurly.Next.Value.Kind == TokenKind.NewLine + || lCurly.Next.Value.Kind == TokenKind.LineContinuation || ((lCurly.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 4d5dfeb6d..1ca0861c5 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -288,7 +288,7 @@ $x = "abc"; $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } - It "Should not find a violation if a new-line is before the pipe" { + It "Should not find a violation if a backtick is before the pipe" { $def = @' Get-Item ` | foo @@ -304,7 +304,7 @@ Get-Item | $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } - It "Should not find a violation if a backtick after the pipe" { + It "Should not find a violation if a backtick is after the pipe" { $def = @' Get-Item |` foo @@ -358,10 +358,26 @@ if ($true) { $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } + It "Should not find a violation if a backtick is after the opening brace" { + $def = @' +if ($true) {` + Get-Item } +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + It "Should not find a violation if a new-line is before the closing brace" { $def = @' if ($true) { Get-Item } +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should not find a violation if a backtick is before the closing brace" { + $def = @' +if ($true) { Get-Item` +} '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } From 43b5882fcbda457a415c0a6ea5c25ab6eb8cfa3e Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 13:43:56 +0000 Subject: [PATCH 11/17] fix 1 failing test --- Rules/UseConsistentWhitespace.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 6b1b82157..db6eaf2d1 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -260,6 +260,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t || !IsPreviousTokenOnSameLine(rCurly) || rCurly.Previous.Value.Kind == TokenKind.LCurly || rCurly.Previous.Value.Kind == TokenKind.NewLine + || rCurly.Previous.Value.Kind == TokenKind.LineContinuation || ((rCurly.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) { continue; From 73edc10f182e3265fb9e3886dc314da21f10d2aa Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 14:05:54 +0000 Subject: [PATCH 12/17] customise warning messages --- Rules/Strings.Designer.cs | 47 ++++++++++++++++--- Rules/Strings.resx | 14 +++++- Rules/UseConsistentWhitespace.cs | 24 ++++++---- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 2 +- 4 files changed, 71 insertions(+), 16 deletions(-) diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index ba1d61a4d..c01fc72fe 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -10,7 +10,6 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer { using System; - using System.Reflection; /// @@ -40,7 +39,7 @@ internal Strings() { internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Windows.PowerShell.ScriptAnalyzer.Strings", typeof(Strings).GetTypeInfo().Assembly); + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Windows.PowerShell.ScriptAnalyzer.Strings", typeof(Strings).Assembly); resourceMan = temp; } return resourceMan; @@ -537,7 +536,7 @@ internal static string AvoidTrailingWhitespaceName { return ResourceManager.GetString("AvoidTrailingWhitespaceName", resourceCulture); } } - + /// /// Looks up a localized string similar to Module Must Be Loadable. /// @@ -807,7 +806,7 @@ internal static string AvoidUsingEmptyCatchBlockName { return ResourceManager.GetString("AvoidUsingEmptyCatchBlockName", resourceCulture); } } - + /// /// Looks up a localized string similar to Avoid Using Internal URLs. /// @@ -2095,12 +2094,30 @@ internal static string UseConsistentWhitespaceDescription { } } + /// + /// Looks up a localized string similar to Use space after open brace.. + /// + internal static string UseConsistentWhitespaceErrorAfterOpeningBrace { + get { + return ResourceManager.GetString("UseConsistentWhitespaceErrorAfterOpeningBrace", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use space before closing brace.. + /// + internal static string UseConsistentWhitespaceErrorBeforeClosingInnerBrace { + get { + return ResourceManager.GetString("UseConsistentWhitespaceErrorBeforeClosingInnerBrace", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use space before open brace.. /// - internal static string UseConsistentWhitespaceErrorBeforeBrace { + internal static string UseConsistentWhitespaceErrorBeforeOpeningBrace { get { - return ResourceManager.GetString("UseConsistentWhitespaceErrorBeforeBrace", resourceCulture); + return ResourceManager.GetString("UseConsistentWhitespaceErrorBeforeOpeningBrace", resourceCulture); } } @@ -2140,6 +2157,24 @@ internal static string UseConsistentWhitespaceErrorSeparatorSemi { } } + /// + /// Looks up a localized string similar to Use space after pipe.. + /// + internal static string UseConsistentWhitespaceErrorSpaceAfterPipe { + get { + return ResourceManager.GetString("UseConsistentWhitespaceErrorSpaceAfterPipe", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use space before pipe.. + /// + internal static string UseConsistentWhitespaceErrorSpaceBeforePipe { + get { + return ResourceManager.GetString("UseConsistentWhitespaceErrorSpaceBeforePipe", resourceCulture); + } + } + /// /// Looks up a localized string similar to UseConsistentWhitespace. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index d4ebf259c..5486890ec 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -906,7 +906,7 @@ Check for whitespace between keyword and open paren/curly, around assigment operator ('='), around arithmetic operators and after separators (',' and ';') - + Use space before open brace. @@ -990,4 +990,16 @@ PossibleIncorrectUsageOfRedirectionOperator + + Use space after open brace. + + + Use space before closing brace. + + + Use space after pipe. + + + Use space before pipe. + \ No newline at end of file diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index db6eaf2d1..df2d86664 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -22,7 +22,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class UseConsistentWhitespace : ConfigurableRule { - private enum ErrorKind { Brace, Paren, Operator, SeparatorComma, SeparatorSemi }; + private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe }; private const int whiteSpaceSize = 1; private const string whiteSpace = " "; private readonly SortedSet openParenKeywordWhitelist = new SortedSet() @@ -187,10 +187,18 @@ private string GetError(ErrorKind kind) { switch (kind) { - case ErrorKind.Brace: - return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeBrace); + case ErrorKind.BeforeOpeningBrace: + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeOpeningBrace); + case ErrorKind.AfterOpeningBrace: + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorAfterOpeningBrace); + case ErrorKind.BeforeClosingBrace: + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeClosingInnerBrace); case ErrorKind.Operator: return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorOperator); + case ErrorKind.BeforePipe: + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceBeforePipe); + case ErrorKind.AfterPipe: + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceAfterPipe); case ErrorKind.SeparatorComma: return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSeparatorComma); case ErrorKind.SeparatorSemi: @@ -216,7 +224,7 @@ private IEnumerable FindOpenBraceViolations(TokenOperations to if (!IsPreviousTokenApartByWhitespace(lcurly)) { yield return new DiagnosticRecord( - GetError(ErrorKind.Brace), + GetError(ErrorKind.BeforeOpeningBrace), lcurly.Value.Extent, GetName(), GetDiagnosticSeverity(), @@ -244,7 +252,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t if (!IsNextTokenApartByWhitespace(lCurly)) { yield return new DiagnosticRecord( - GetError(ErrorKind.Brace), + GetError(ErrorKind.AfterOpeningBrace), lCurly.Value.Extent, GetName(), GetDiagnosticSeverity(), @@ -269,7 +277,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t if (!IsPreviousTokenApartByWhitespace(rCurly)) { yield return new DiagnosticRecord( - GetError(ErrorKind.Brace), + GetError(ErrorKind.BeforeClosingBrace), rCurly.Value.Extent, GetName(), GetDiagnosticSeverity(), @@ -297,7 +305,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp if (!IsNextTokenApartByWhitespace(pipe)) { yield return new DiagnosticRecord( - GetError(ErrorKind.Brace), + GetError(ErrorKind.BeforePipe), pipe.Value.Extent, GetName(), GetDiagnosticSeverity(), @@ -322,7 +330,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp if (!IsPreviousTokenApartByWhitespace(pipe)) { yield return new DiagnosticRecord( - GetError(ErrorKind.Brace), + GetError(ErrorKind.AfterPipe), pipe.Value.Extent, GetName(), GetDiagnosticSeverity(), diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 1ca0861c5..cb4a65b4d 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -376,7 +376,7 @@ if ($true) { Get-Item It "Should not find a violation if a backtick is before the closing brace" { $def = @' -if ($true) { Get-Item` +if ($true) { Get-Item ` } '@ $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null From 6b362d9a3165b534958856ae751220c78b6a42b3 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 3 Nov 2018 14:13:15 +0000 Subject: [PATCH 13/17] swap messages to be correct --- Rules/UseConsistentWhitespace.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index df2d86664..b36653a83 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -305,7 +305,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp if (!IsNextTokenApartByWhitespace(pipe)) { yield return new DiagnosticRecord( - GetError(ErrorKind.BeforePipe), + GetError(ErrorKind.AfterPipe), pipe.Value.Extent, GetName(), GetDiagnosticSeverity(), @@ -330,7 +330,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp if (!IsPreviousTokenApartByWhitespace(pipe)) { yield return new DiagnosticRecord( - GetError(ErrorKind.AfterPipe), + GetError(ErrorKind.BeforePipe), pipe.Value.Extent, GetName(), GetDiagnosticSeverity(), From ff3e3d1c26e1cf051263363ba373f5fcc9d53443 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 19 Dec 2018 14:41:38 +0000 Subject: [PATCH 14/17] add more test cases and fix small bug whereby the missing space before the closing brace would not be correctly replaced --- Rules/UseConsistentWhitespace.cs | 8 ++++---- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index b36653a83..390c189be 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -244,7 +244,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t || lCurly.Next.Value.Kind == TokenKind.LCurly || lCurly.Next.Value.Kind == TokenKind.NewLine || lCurly.Next.Value.Kind == TokenKind.LineContinuation - || ((lCurly.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + ) { continue; } @@ -269,7 +269,7 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t || rCurly.Previous.Value.Kind == TokenKind.LCurly || rCurly.Previous.Value.Kind == TokenKind.NewLine || rCurly.Previous.Value.Kind == TokenKind.LineContinuation - || ((rCurly.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + ) { continue; } @@ -297,7 +297,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp || pipe.Next.Value.Kind == TokenKind.Pipe || pipe.Next.Value.Kind == TokenKind.NewLine || pipe.Next.Value.Kind == TokenKind.LineContinuation - || ((pipe.Next.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + ) { continue; } @@ -322,7 +322,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp || pipe.Previous.Value.Kind == TokenKind.Pipe || pipe.Previous.Value.Kind == TokenKind.NewLine || pipe.Previous.Value.Kind == TokenKind.LineContinuation - || ((pipe.Previous.Value.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName)) + ) { continue; } diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index cb4a65b4d..4196f7144 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -283,6 +283,18 @@ $x = "abc"; Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } + It "Should find a violation if there is one space too much before pipe" { + $def = 'Get-Item | foo' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + + It "Should find a violation if there is one space too much after pipe" { + $def = 'Get-Item | foo' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + It "Should not find a violation if there is 1 space before and after a pipe" { $def = 'Get-Item | foo' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null From e4d6879b56277114c61078d2f1d5547e3ecb7749 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 19 Dec 2018 14:49:20 +0000 Subject: [PATCH 15/17] enforce whitespace also if there is more than 1 curly brace. TODO: add test case for that --- Rules/UseConsistentWhitespace.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 390c189be..f4f2be6ad 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -241,7 +241,6 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t { if (lCurly.Next == null || !IsPreviousTokenOnSameLine(lCurly) - || lCurly.Next.Value.Kind == TokenKind.LCurly || lCurly.Next.Value.Kind == TokenKind.NewLine || lCurly.Next.Value.Kind == TokenKind.LineContinuation ) From fb572926553ae72acb8e6b6c19e08e5698d2052a Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 19 Dec 2018 20:40:53 +0000 Subject: [PATCH 16/17] add test cases for nested parenthesis and add validation to test helper method to inform about limited functionality --- Tests/PSScriptAnalyzerTestHelper.psm1 | 3 ++- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Tests/PSScriptAnalyzerTestHelper.psm1 b/Tests/PSScriptAnalyzerTestHelper.psm1 index 499c3e29d..1601b5e17 100644 --- a/Tests/PSScriptAnalyzerTestHelper.psm1 +++ b/Tests/PSScriptAnalyzerTestHelper.psm1 @@ -39,7 +39,8 @@ Function Test-CorrectionExtentFromContent { param( [string] $rawContent, [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord] $diagnosticRecord, - [int] $correctionsCount, + [ValidateRange(0, 1)] + [int] $correctionsCount, [string] $violationText, [string] $correctionText ) diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 4196f7144..8c5500d92 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -342,12 +342,24 @@ foo Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } + It "Should find a violation if there is no space after opening brace when there are 2 braces" { + $def = 'if ($true) {{ Get-Item } }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + It "Should find a violation if there is no space before closing brace" { $def = 'if ($true) { Get-Item}' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } + It "Should find a violation if there is no space before closing brace when there are 2 braces" { + $def = 'if ($true) { { Get-Item }}' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + It "Should not find a violation if there is 1 space after the opening brace and 1 before the closing brace" { $def = @' if($true) { Get-Item } From 196b11238deef1cecbe85193d1b9e9f0f14de591 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 19 Dec 2018 20:52:33 +0000 Subject: [PATCH 17/17] add test case for more than 1 space inside curly braces and tidy up tests --- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 134 +++++++----------- 1 file changed, 52 insertions(+), 82 deletions(-) diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 8c5500d92..24ecbe78a 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -5,18 +5,18 @@ Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") $ruleName = "PSUseConsistentWhitespace" $ruleConfiguration = @{ - Enable = $true + Enable = $true CheckInnerBrace = $false - CheckOpenBrace = $false - CheckOpenParen = $false - CheckOperator = $false - CheckPipe = $false - CheckSeparator = $false + CheckOpenBrace = $false + CheckOpenParen = $false + CheckOperator = $false + CheckPipe = $false + CheckSeparator = $false } $settings = @{ IncludeRules = @($ruleName) - Rules = @{ + Rules = @{ PSUseConsistentWhitespace = $ruleConfiguration } } @@ -33,31 +33,23 @@ Describe "UseWhitespace" { } It "Should find a violation if an open brace does not follow whitespace" { - $def = @' -if ($true){} -'@ + $def = 'if ($true){}' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } It "Should not find violation if an open brace follows a whitespace" { - $def = @' -if($true) {} -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = 'if($true) {}' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find violation if an open brace follows a foreach member invocation" { - $def = @' -(1..5).foreach{$_} -'@ + $def = '(1..5).foreach{$_}' Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find violation if an open brace follows a where member invocation" { - $def = @' -(1..5).where{$_} -'@ + $def = '(1..5).where{$_}' Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } @@ -74,9 +66,7 @@ if($true) {} } It "Should find violation in an if statement" { - $def = @' -if($true) {} -'@ + $def = 'if($true) {}' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } @@ -87,7 +77,7 @@ function foo($param1) { } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation in a param block" { @@ -96,7 +86,7 @@ function foo() { param( ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation in a nested open paren" { @@ -105,14 +95,12 @@ function foo($param) { ((Get-Process)) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation on a method call" { - $def = @' -$x.foo("bar") -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = '$x.foo("bar")' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } } @@ -127,33 +115,25 @@ $x.foo("bar") } It "Should find a violation if no whitespace around an assignment operator" { - $def = @' -$x=1 -'@ + $def = '$x=1' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '=' ' = ' } It "Should find a violation if no whitespace before an assignment operator" { - $def = @' -$x= 1 -'@ + $def = '$x= 1' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } It "Should find a violation if no whitespace after an assignment operator" { - $def = @' -$x =1 -'@ + $def = '$x =1' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } It "Should find a violation if there is a whitespaces not of size 1 around an assignment operator" { - $def = @' -$x = 1 -'@ + $def = '$x = 1' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 ' = ' ' = ' } @@ -164,20 +144,16 @@ $x = @" "abc" "@ '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find violation if there are whitespaces of size 1 around an assignment operator for here string" { - $def = @' -$x = 1 -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = '$x = 1' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find violation if there are no whitespaces around DotDot operator" { - $def = @' -1..5 -'@ + $def = '1..5' Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } @@ -202,18 +178,14 @@ $x = $true -and } It "Should find a violation" { - $def = @' -$x = @(1,2) -'@ + $def = '$x = @(1,2)' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } It "Should not find a violation if a space follows a comma" { - $def = @' -$x = @(1, 2) -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = '$x = @(1, 2)' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } } @@ -228,18 +200,14 @@ $x = @(1, 2) } It "Should find a violation" { - $def = @' -$x = @{a=1;b=2} -'@ + $def = '$x = @{a=1;b=2}' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } It "Should not find a violation if a space follows a semi-colon" { - $def = @' -$x = @{a=1; b=2} -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = '$x = @{a=1; b=2}' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a new-line follows a semi-colon" { @@ -249,14 +217,14 @@ $x = @{ b=2 } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a end of input follows a semi-colon" { $def = @' $x = "abc"; '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } } @@ -297,7 +265,7 @@ $x = "abc"; It "Should not find a violation if there is 1 space before and after a pipe" { $def = 'Get-Item | foo' - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a backtick is before the pipe" { @@ -305,7 +273,7 @@ $x = "abc"; Get-Item ` | foo '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a new-line is after the pipe" { @@ -313,7 +281,7 @@ Get-Item ` Get-Item | foo '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a backtick is after the pipe" { @@ -321,7 +289,7 @@ Get-Item | Get-Item |` foo '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } } @@ -359,19 +327,21 @@ foo $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } + + It "Should find a violation if there is more than 1 space inside empty curly braces" { + $def = 'if($true) { }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } It "Should not find a violation if there is 1 space after the opening brace and 1 before the closing brace" { - $def = @' -if($true) { Get-Item } -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = 'if($true) { Get-Item }' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if there is 1 space inside empty curly braces" { - $def = @' -if($true) { } -'@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + $def = 'if($true) { }' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a new-line is after the opening brace" { @@ -379,7 +349,7 @@ if($true) { } if ($true) { Get-Item } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a backtick is after the opening brace" { @@ -387,7 +357,7 @@ if ($true) { if ($true) {` Get-Item } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a new-line is before the closing brace" { @@ -395,7 +365,7 @@ if ($true) {` if ($true) { Get-Item } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } It "Should not find a violation if a backtick is before the closing brace" { @@ -403,7 +373,7 @@ if ($true) { Get-Item if ($true) { Get-Item ` } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } }