From 7e60fb33a6ca0d217a2cd6ebcf63b002632c665d Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Tue, 17 Jun 2025 07:06:21 -0500 Subject: [PATCH 1/3] fix: Formatting misses certain keywords (unary operators, 'until') --- Rules/UseConsistentWhitespace.cs | 126 +++++++++++-- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 172 ++++++++++++++++++ 2 files changed, 278 insertions(+), 20 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index e6d4cff99..35ed69714 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -22,8 +22,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class UseConsistentWhitespace : ConfigurableRule { - private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, - AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe, BetweenParameter }; + private enum ErrorKind + { + BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, + AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe, BetweenParameter + }; + private const int whiteSpaceSize = 1; private const string whiteSpace = " "; private readonly SortedSet openParenKeywordAllowList = new SortedSet() @@ -33,7 +37,9 @@ private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, Se TokenKind.Switch, TokenKind.For, TokenKind.Foreach, - TokenKind.While + TokenKind.While, + TokenKind.Until, + TokenKind.Do }; private List>> violationFinders @@ -72,6 +78,7 @@ public override void ConfigureRule(IDictionary paramValueMap) if (CheckOpenBrace) { violationFinders.Add(FindOpenBraceViolations); + violationFinders.Add(FindKeywordAfterBraceViolations); } if (CheckInnerBrace) @@ -194,6 +201,7 @@ private bool IsOperator(Token token) return TokenTraits.HasTrait(token.Kind, TokenFlags.AssignmentOperator) || TokenTraits.HasTrait(token.Kind, TokenFlags.BinaryPrecedenceAdd) || TokenTraits.HasTrait(token.Kind, TokenFlags.BinaryPrecedenceMultiply) + || TokenTraits.HasTrait(token.Kind, TokenFlags.UnaryOperator) || token.Kind == TokenKind.AndAnd || token.Kind == TokenKind.OrOr; } @@ -229,7 +237,6 @@ private IEnumerable FindOpenBraceViolations(TokenOperations to { foreach (var lcurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) { - if (lcurly.Previous == null || !IsPreviousTokenOnSameLine(lcurly) || lcurly.Previous.Value.Kind == TokenKind.LCurly @@ -239,11 +246,28 @@ private IEnumerable FindOpenBraceViolations(TokenOperations to continue; } + if (lcurly.Previous.Value.Kind == TokenKind.RCurly && lcurly.Previous.Previous != null) + { + var keywordBeforeBrace = lcurly.Previous.Previous.Value; + if (IsKeyword(keywordBeforeBrace) && !IsPreviousTokenApartByWhitespace(lcurly.Previous)) + { + yield return new DiagnosticRecord( + GetError(ErrorKind.BeforeOpeningBrace), + lcurly.Previous.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(keywordBeforeBrace, lcurly.Previous.Value, lcurly.Value, false, true).ToList()); + } + continue; + } + if (IsPreviousTokenApartByWhitespace(lcurly) || IsPreviousTokenLParen(lcurly)) { continue; } - + yield return new DiagnosticRecord( GetError(ErrorKind.BeforeOpeningBrace), lcurly.Value.Extent, @@ -255,6 +279,46 @@ private IEnumerable FindOpenBraceViolations(TokenOperations to } } + private IEnumerable FindKeywordAfterBraceViolations(TokenOperations tokenOperations) + { + foreach (var keywordNode in tokenOperations.GetTokenNodes(IsKeyword)) + { + var keyword = keywordNode.Value; + + if (keywordNode.Previous != null) + { + if (keywordNode.Previous.Value.Kind == TokenKind.RCurly && + IsPreviousTokenOnSameLine(keywordNode)) + { + var hasWhitespace = IsPreviousTokenApartByWhitespace(keywordNode); + + if (!hasWhitespace) + { + var corrections = new List + { + new CorrectionExtent( + keywordNode.Previous.Value.Extent.EndLineNumber, + keyword.Extent.StartLineNumber, + keywordNode.Previous.Value.Extent.EndColumnNumber, + keyword.Extent.StartColumnNumber, + " ", + keyword.Extent.File) + }; + + yield return new DiagnosticRecord( + GetError(ErrorKind.BeforeOpeningBrace), + keyword.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + corrections); + } + } + } + } + } + private IEnumerable FindInnerBraceViolations(TokenOperations tokenOperations) { foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) @@ -509,7 +573,7 @@ private static bool IsPreviousTokenApartByWhitespace(LinkedListNode token hasRedundantWhitespace = actualWhitespaceSize - whiteSpaceSize > 0; return whiteSpaceSize == actualWhitespaceSize; } - + private static bool IsPreviousTokenLParen(LinkedListNode tokenNode) { return tokenNode.Previous.Value.Kind == TokenKind.LParen; @@ -536,17 +600,30 @@ private IEnumerable FindOperatorViolations(TokenOperations tok { foreach (var tokenNode in tokenOperations.GetTokenNodes(IsOperator)) { - if (tokenNode.Previous == null - || tokenNode.Next == null - || tokenNode.Value.Kind == TokenKind.DotDot) + var token = tokenNode.Value; + + if (tokenNode.Previous == null || tokenNode.Next == null || token.Kind == TokenKind.DotDot) { continue; } - // exclude unary operator for cases like $foo.bar(-$Var) - if (TokenTraits.HasTrait(tokenNode.Value.Kind, TokenFlags.UnaryOperator) && - tokenNode.Previous.Value.Kind == TokenKind.LParen && - tokenNode.Next.Value.Kind == TokenKind.Variable) + // Check unary operator handling + bool isUnaryInMethodCall = false; + if (TokenTraits.HasTrait(token.Kind, TokenFlags.UnaryOperator)) + { + // Only skip if it's a unary operator in a method call like $foo.bar(-$var) + if (tokenNode.Previous.Value.Kind == TokenKind.LParen && + tokenNode.Next.Value.Kind == TokenKind.Variable && + tokenNode.Previous.Previous != null) + { + var beforeLParen = tokenNode.Previous.Previous.Value; + + isUnaryInMethodCall = beforeLParen.Kind == TokenKind.Dot || + (beforeLParen.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName; + } + } + + if (isUnaryInMethodCall) { continue; } @@ -561,22 +638,30 @@ private IEnumerable FindOperatorViolations(TokenOperations tok } } + // Check whitespace var hasWhitespaceBefore = IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode); - var hasWhitespaceAfter = tokenNode.Next.Value.Kind == TokenKind.NewLine - || IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode.Next); + var hasWhitespaceAfter = tokenNode.Next.Value.Kind == TokenKind.NewLine || + IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode.Next); + + // Special case: Don't require space before unary operator if preceded by LParen + if (TokenTraits.HasTrait(token.Kind, TokenFlags.UnaryOperator) && + tokenNode.Previous.Value.Kind == TokenKind.LParen) + { + hasWhitespaceBefore = true; + } if (!hasWhitespaceAfter || !hasWhitespaceBefore) { yield return new DiagnosticRecord( GetError(ErrorKind.Operator), - tokenNode.Value.Extent, + token.Extent, GetName(), GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, GetCorrections( tokenNode.Previous.Value, - tokenNode.Value, + token, tokenNode.Next.Value, hasWhitespaceBefore, hasWhitespaceAfter)); @@ -614,7 +699,9 @@ private List GetCorrections( var extent = new ScriptExtent( new ScriptPosition(e1.File, e1.EndLineNumber, e1.EndColumnNumber, null), - new ScriptPosition(e2.File, e2.StartLineNumber, e2.StartColumnNumber, null)); + new ScriptPosition(e2.File, e2.StartLineNumber, e2.StartColumnNumber, null) + ); + return new List() { new CorrectionExtent( @@ -630,6 +717,5 @@ private static bool IsPreviousTokenOnSameLine(LinkedListNode lparen) { return lparen.Previous.Value.Extent.EndLineNumber == lparen.Value.Extent.StartLineNumber; } - } -} +} \ No newline at end of file diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 952e49909..07b4c740b 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -684,4 +684,176 @@ bar -h i ` Should -Be $expected } } + + Context "When keywords follow closing braces" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false + $ruleConfiguration.CheckOpenBrace = $true + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckSeparator = $false + } + + It "Should find a violation if no space between } and while" { + $def = 'do { "test" }while($true)' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should find a violation if no space between } and until" { + $def = 'do { "test" }until($false)' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should not find a violation if there is space between } and while" { + $def = 'do { "test" } while($true)' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It "Should not find a violation if there is space between } and until" { + $def = 'do { "test" } until($false)' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + } + + Context "When checking unary operators" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOperator = $true + $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckSeparator = $false + } + + It "Should find a violation if no space after -not operator" { + $def = 'if (-not$true) { }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should find a violation if no space after -bnot operator" { + $def = '$x = -bnot$value' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + } + + It "Should not find a violation if space after -not operator" { + $def = 'if (-not $true) { }' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It "Should not find a violation for unary operator in method call" { + $def = '$foo.bar(-$value)' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It "Should not find a violation for unary operator in property access" { + $def = '$object.Property(-$x)' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It "Should find a violation for unary operator not in method call context" { + $def = 'if(-not$x) { }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + } + + It "Should handle multiple unary operators in same expression" { + $def = 'while(-not$a -and -not$b) { }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 2 + } + } + + Context "Invoke-Formatter validates do-while/do-until and unary operator fixes" { + BeforeAll { + $settings = @{ + Rules = @{ + PSUseConsistentWhitespace = @{ + Enable = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + } + } + } + } + + It "Should format the original bug repro correctly" { + $def = @' +if(-not$false) { + do{ + "Hello!" + }until( + $True + ) + do{ + "Oh, hi!" + }while( + -not$True + ) + while(-not$True) { + "This won't show up." + } +} +'@ + $expected = @' +if (-not $false) { + do { + "Hello!" + } until ( + $True + ) + do { + "Oh, hi!" + } while ( + -not $True + ) + while (-not $True) { + "This won't show up." + } } +'@ + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should add space between } and while" { + $def = 'do { Get-Process }while($true)' + $expected = 'do { Get-Process } while ($true)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should add space between } and until" { + $def = 'do { Get-Process }until($false)' + $expected = 'do { Get-Process } until ($false)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should add space after -not operator" { + $def = 'if (-not$variable) { "test" }' + $expected = 'if (-not $variable) { "test" }' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should add space after -bnot operator" { + $def = '$result = -bnot$value' + $expected = '$result = -bnot $value' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should not add space after unary minus in method call" { + $def = '$object.Method(-$value)' + $expected = '$object.Method(-$value)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle all unary operators correctly" { + $def = 'if (-not$a -and -bnot$b -and $c.Method(-$d)) { }' + $expected = 'if (-not $a -and -bnot $b -and $c.Method(-$d)) { }' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + } +} \ No newline at end of file From deb3512999e99ac927a5a9d578fd188fcd178b94 Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Tue, 17 Jun 2025 15:40:42 -0500 Subject: [PATCH 2/3] wip: Before-Optimized UseConsistentWhitespace --- Rules/UseConsistentWhitespace.cs | 220 ++++++++++++++---- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 210 +++++++++++++++-- 2 files changed, 374 insertions(+), 56 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 35ed69714..eee80edd2 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -39,7 +39,10 @@ private enum ErrorKind TokenKind.Foreach, TokenKind.While, TokenKind.Until, - TokenKind.Do + TokenKind.Do, + TokenKind.Else, + TokenKind.Catch, + TokenKind.Finally }; private List>> violationFinders @@ -78,6 +81,7 @@ public override void ConfigureRule(IDictionary paramValueMap) if (CheckOpenBrace) { violationFinders.Add(FindOpenBraceViolations); + violationFinders.Add(FindSpaceAfterClosingBraceViolations); violationFinders.Add(FindKeywordAfterBraceViolations); } @@ -301,7 +305,7 @@ private IEnumerable FindKeywordAfterBraceViolations(TokenOpera keyword.Extent.StartLineNumber, keywordNode.Previous.Value.Extent.EndColumnNumber, keyword.Extent.StartColumnNumber, - " ", + whiteSpace, keyword.Extent.File) }; @@ -321,20 +325,40 @@ private IEnumerable FindKeywordAfterBraceViolations(TokenOpera private IEnumerable FindInnerBraceViolations(TokenOperations tokenOperations) { + // Handle opening braces foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) { if (lCurly.Next == null - || !(lCurly.Previous == null || IsPreviousTokenOnSameLine(lCurly)) + || (lCurly.Previous != null && !IsPreviousTokenOnSameLine(lCurly)) || lCurly.Next.Value.Kind == TokenKind.NewLine - || lCurly.Next.Value.Kind == TokenKind.LineContinuation - || lCurly.Next.Value.Kind == TokenKind.RCurly - ) + || lCurly.Next.Value.Kind == TokenKind.LineContinuation) + { + continue; + } + + // Special handling for empty braces - they should have a space + if (lCurly.Next.Value.Kind == TokenKind.RCurly) { + if (!IsNextTokenApartByWhitespace(lCurly)) + { + var prevToken = lCurly.Previous?.Value ?? lCurly.Value; + var nextToken = lCurly.Next?.Value ?? lCurly.Value; + + yield return new DiagnosticRecord( + GetError(ErrorKind.AfterOpeningBrace), + lCurly.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(prevToken, lCurly.Value, nextToken, true, false).ToList()); + } continue; } if (!IsNextTokenApartByWhitespace(lCurly)) { + var prevToken = lCurly.Previous?.Value ?? lCurly.Value; yield return new DiagnosticRecord( GetError(ErrorKind.AfterOpeningBrace), lCurly.Value.Extent, @@ -342,25 +366,48 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(lCurly.Previous.Value, lCurly.Value, lCurly.Next.Value, true, false).ToList()); + GetCorrections(prevToken, lCurly.Value, lCurly.Next.Value, true, false).ToList()); } } + // Handle closing braces foreach (var rCurly in tokenOperations.GetTokenNodes(TokenKind.RCurly)) { - if (rCurly.Previous == null - || !IsPreviousTokenOnSameLine(rCurly) - || rCurly.Previous.Value.Kind == TokenKind.LCurly + if (rCurly.Previous == null) + { + continue; + } + + if (!IsPreviousTokenOnSameLine(rCurly) || rCurly.Previous.Value.Kind == TokenKind.NewLine || rCurly.Previous.Value.Kind == TokenKind.LineContinuation - || rCurly.Previous.Value.Kind == TokenKind.AtCurly - ) + || rCurly.Previous.Value.Kind == TokenKind.AtCurly) + { + continue; + } + + // Skip empty braces that already have space + if (rCurly.Previous.Value.Kind == TokenKind.LCurly && IsPreviousTokenApartByWhitespace(rCurly)) { continue; } - if (!IsPreviousTokenApartByWhitespace(rCurly)) + // Use AST to check if this is a hashtable + var ast = tokenOperations.GetAstPosition(rCurly.Value); + + if (ast is HashtableAst hashtableAst) + { + if (rCurly.Value.Extent.EndOffset == hashtableAst.Extent.EndOffset) + { + continue; + } + } + + bool hasSpace = IsPreviousTokenApartByWhitespace(rCurly); + + if (!hasSpace) { + var nextToken = rCurly.Next?.Value ?? rCurly.Value; yield return new DiagnosticRecord( GetError(ErrorKind.BeforeClosingBrace), rCurly.Value.Extent, @@ -368,7 +415,41 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t GetDiagnosticSeverity(), tokenOperations.Ast.Extent.File, null, - GetCorrections(rCurly.Previous.Value, rCurly.Value, rCurly.Next.Value, false, true).ToList()); + GetCorrections(rCurly.Previous.Value, rCurly.Value, nextToken, false, true).ToList()); + } + } + } + + private IEnumerable FindSpaceAfterClosingBraceViolations(TokenOperations tokenOperations) + { + foreach (var rCurly in tokenOperations.GetTokenNodes(TokenKind.RCurly)) + { + if (rCurly.Next == null + || !IsPreviousTokenOnSameLine(rCurly.Next) + || rCurly.Next.Value.Kind == TokenKind.NewLine + || rCurly.Next.Value.Kind == TokenKind.EndOfInput + || rCurly.Next.Value.Kind == TokenKind.Semi + || rCurly.Next.Value.Kind == TokenKind.Comma + || rCurly.Next.Value.Kind == TokenKind.RParen) + { + continue; + } + + // Need space after } before keywords, numbers, or another } + if ((IsKeyword(rCurly.Next.Value) + || rCurly.Next.Value.Kind == TokenKind.Number + || rCurly.Next.Value.Kind == TokenKind.RCurly) + && !IsNextTokenApartByWhitespace(rCurly)) + { + var prevToken = rCurly.Previous?.Value ?? rCurly.Value; + yield return new DiagnosticRecord( + GetError(ErrorKind.BeforeOpeningBrace), + rCurly.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(prevToken, rCurly.Value, rCurly.Next.Value, true, false).ToList()); } } } @@ -456,8 +537,7 @@ private IEnumerable FindOpenParenViolations(TokenOperations to private IEnumerable FindParameterViolations(Ast ast) { - IEnumerable commandAsts = ast.FindAll( - testAst => testAst is CommandAst, true); + IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); foreach (CommandAst commandAst in commandAsts) { /// When finding all the command parameter elements, there is no guarantee that @@ -471,6 +551,7 @@ private IEnumerable FindParameterViolations(Ast ast) ).ThenBy( e => e.Extent.StartColumnNumber ).ToList(); + for (int i = 0; i < commandParameterAstElements.Count - 1; i++) { IScriptExtent leftExtent = commandParameterAstElements[i].Extent; @@ -511,33 +592,90 @@ private bool IsSeparator(Token token) private IEnumerable FindSeparatorViolations(TokenOperations tokenOperations) { - Func, bool> predicate = node => + foreach (var tokenNode in tokenOperations.GetTokenNodes(IsSeparator)) { - return node.Next != null - && node.Next.Value.Kind != TokenKind.NewLine - && node.Next.Value.Kind != TokenKind.Comment - && node.Next.Value.Kind != TokenKind.EndOfInput // semicolon can be followed by end of input - && !IsPreviousTokenApartByWhitespace(node.Next); - }; + if (tokenNode.Next == null + || tokenNode.Next.Value.Kind == TokenKind.NewLine + || tokenNode.Next.Value.Kind == TokenKind.Comment + || tokenNode.Next.Value.Kind == TokenKind.EndOfInput) + { + continue; + } - foreach (var tokenNode in tokenOperations.GetTokenNodes(IsSeparator).Where(predicate)) - { - var errorKind = tokenNode.Value.Kind == TokenKind.Comma - ? ErrorKind.SeparatorComma - : ErrorKind.SeparatorSemi; - yield return getDiagnosticRecord( - tokenNode.Value, - errorKind, - GetCorrections( - tokenNode.Previous.Value, - tokenNode.Value, - tokenNode.Next.Value, - true, - false)); + var separator = tokenNode.Value; + + // Check if comma is part of a parameter value by looking at surrounding tokens + if (separator.Kind == TokenKind.Comma) + { + // Look for pattern: word,word (no spaces) which indicates parameter value + if (tokenNode.Previous != null && tokenNode.Next != null) + { + var prevTok = tokenNode.Previous.Value; + var nextTok = tokenNode.Next.Value; + + // Skip if comma appears to be within a parameter value (no spaces around it) + if ((prevTok.Kind == TokenKind.Identifier || prevTok.Kind == TokenKind.Generic) && + (nextTok.Kind == TokenKind.Identifier || nextTok.Kind == TokenKind.Generic) && + prevTok.Extent.EndColumnNumber == separator.Extent.StartColumnNumber && + separator.Extent.EndColumnNumber == nextTok.Extent.StartColumnNumber) + { + // This looks like key=value,key=value pattern + continue; + } + } + } + + var prevToken = tokenNode.Previous.Value; + var nextToken = tokenNode.Next.Value; + + // Check for space before separator (should not exist) + if (tokenNode.Previous != null && IsPreviousTokenOnSameLine(tokenNode)) + { + var spaceBefore = separator.Extent.StartColumnNumber - prevToken.Extent.EndColumnNumber; + if (spaceBefore > 0) + { + // Remove space before separator + yield return new DiagnosticRecord( + GetError(separator.Kind == TokenKind.Comma ? ErrorKind.SeparatorComma : ErrorKind.SeparatorSemi), + separator.Extent, + GetName(), + GetDiagnosticSeverity(), + separator.Extent.File, + null, + new List { + new CorrectionExtent( + prevToken.Extent.EndLineNumber, + separator.Extent.StartLineNumber, + prevToken.Extent.EndColumnNumber, + separator.Extent.StartColumnNumber, + string.Empty, + separator.Extent.File) + }); + } + } + + // Check for space after separator (should exist) + if (!IsPreviousTokenApartByWhitespace(tokenNode.Next)) + { + var errorKind = separator.Kind == TokenKind.Comma ? ErrorKind.SeparatorComma : ErrorKind.SeparatorSemi; + + yield return GetDiagnosticRecord( + separator, + errorKind, + new List { + new CorrectionExtent( + separator.Extent.EndLineNumber, + nextToken.Extent.StartLineNumber, + separator.Extent.EndColumnNumber, + nextToken.Extent.StartColumnNumber, + whiteSpace, + separator.Extent.File) + }); + } } } - private DiagnosticRecord getDiagnosticRecord( + private DiagnosticRecord GetDiagnosticRecord( Token token, ErrorKind errKind, List corrections) @@ -602,6 +740,11 @@ private IEnumerable FindOperatorViolations(TokenOperations tok { var token = tokenNode.Value; + if (IsSeparator(token)) + { + continue; + } + if (tokenNode.Previous == null || tokenNode.Next == null || token.Kind == TokenKind.DotDot) { continue; @@ -617,7 +760,6 @@ private IEnumerable FindOperatorViolations(TokenOperations tok tokenNode.Previous.Previous != null) { var beforeLParen = tokenNode.Previous.Previous.Value; - isUnaryInMethodCall = beforeLParen.Kind == TokenKind.Dot || (beforeLParen.TokenFlags & TokenFlags.MemberName) == TokenFlags.MemberName; } diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 07b4c740b..2cedd2b11 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -25,7 +25,6 @@ BeforeAll { } } - Describe "UseWhitespace" { Context "When an open brace follows a keyword" { BeforeAll { @@ -569,7 +568,6 @@ $Array = @( } - Context "CheckParameter" { BeforeAll { $ruleConfiguration.CheckInnerBrace = $true @@ -693,18 +691,25 @@ bar -h i ` $ruleConfiguration.CheckOperator = $false $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.CheckParameter = $false } It "Should find a violation if no space between } and while" { $def = 'do { "test" }while($true)' - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + + # 2114 changed $def to multiple violations rather than 1. + [Object[]] $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -BeGreaterThan 0 + Test-CorrectionExtentFromContent $def $violations[0] 1 '' ' ' } It "Should find a violation if no space between } and until" { $def = 'do { "test" }until($false)' - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - Test-CorrectionExtentFromContent $def $violations 1 '' ' ' + + # 2114 changed $def to multiple violations rather than 1. + [Object[]] $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -BeGreaterThan 0 + Test-CorrectionExtentFromContent $def $violations[0] 1 '' ' ' } It "Should not find a violation if there is space between } and while" { @@ -721,11 +726,12 @@ bar -h i ` Context "When checking unary operators" { BeforeAll { $ruleConfiguration.CheckInnerBrace = $false - $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOperator = $true $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.CheckParameter = $false } It "Should find a violation if no space after -not operator" { @@ -770,16 +776,13 @@ bar -h i ` Context "Invoke-Formatter validates do-while/do-until and unary operator fixes" { BeforeAll { - $settings = @{ - Rules = @{ - PSUseConsistentWhitespace = @{ - Enable = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - } - } - } + $ruleConfiguration.CheckInnerBrace = $true + $ruleConfiguration.CheckOpenBrace = $true + $ruleConfiguration.CheckOpenParen = $true + $ruleConfiguration.CheckOperator = $true + $ruleConfiguration.CheckPipe = $true + $ruleConfiguration.CheckSeparator = $true + $ruleConfiguration.CheckParameter = $false } It "Should format the original bug repro correctly" { @@ -856,4 +859,177 @@ if (-not $false) { Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected } } + + # 2114 Tests + Context "Invoke-Formatter comprehensive regression tests" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $true + $ruleConfiguration.CheckOpenBrace = $true + $ruleConfiguration.CheckOpenParen = $true + $ruleConfiguration.CheckOperator = $true + $ruleConfiguration.CheckPipe = $true + $ruleConfiguration.CheckSeparator = $true + $ruleConfiguration.CheckParameter = $false + } + + # Operator tests + It "Should format assignment operators correctly" { + $def = '$x=1;$y = 2;$z = 3' + $expected = '$x = 1; $y = 2; $z = 3' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should format arithmetic operators correctly" { + $def = '$a+$b-$c*$d/$e%$f' + $expected = '$a + $b - $c * $d / $e % $f' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should format comparison operators correctly" { + $def = 'if($a-eq$b -and $c-ne$d){}' + $expected = 'if ($a -eq $b -and $c -ne $d) { }' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should not add spaces around .. operator" { + $def = '1..10 | ForEach-Object { $_ }' + $expected = '1..10 | ForEach-Object { $_ }' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + # Separator tests + It "Should format array separators correctly" { + $def = '@(1,2,3,4,5)' + $expected = '@(1, 2, 3, 4, 5)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should format hashtable separators correctly" { + $def = '@{a=1;b=2;c=3}' + $expected = '@{a = 1; b = 2; c = 3}' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should format parameter separators correctly" { + $def = 'Get-Process -Name notepad,explorer,cmd' + $expected = 'Get-Process -Name notepad,explorer,cmd' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle separators with existing spacing" { + $def = '$a = @(1 , 2, 3,4)' + $expected = '$a = @(1, 2, 3, 4)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + # Brace tests + It "Should format if/else statements correctly" { + $def = 'if($true){Write-Host "yes"}else{Write-Host "no"}' + $expected = 'if ($true) { Write-Host "yes" } else { Write-Host "no" }' + Invoke-Formatter $def -Settings $settings | Should -BeExactly $expected + } + + It "Should format switch statements correctly" { + $def = 'switch($x){1{"one"}2{"two"}}' + $expected = 'switch ($x) { 1 { "one" } 2 { "two" } }' + Invoke-Formatter $def -Settings $settings | Should -BeExactly $expected + } + + It "Should format try/catch/finally correctly" { + $def = 'try{Get-Item}catch{Write-Error $_}finally{Clean-Up}' + $expected = 'try { Get-Item } catch { Write-Error $_ } finally { Clean-Up }' + Invoke-Formatter $def -Settings $settings | Should -BeExactly $expected + } + + # Mixed scenarios + It "Should handle nested structures correctly" { + $def = '@{a=@(1,2,3);b=@{x=1;y=2}}' + $expected = '@{a = @(1, 2, 3); b = @{x = 1; y = 2} }' + Invoke-Formatter $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle complex expressions correctly" { + $def = 'if($a-eq$b-and($c-ne$d-or$e-like$f)){$result=$true}' + $expected = 'if ($a -eq $b -and ($c -ne $d -or $e -like $f)) { $result = $true }' + Invoke-Formatter $def -Settings $settings | Should -BeExactly $expected + } + + It "Should preserve newlines and not add spaces" { + $def = @' +$hash = @{ + Key1 = "Value1" + Key2 = "Value2" +} +'@ + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $def + } + + It "Should handle pipeline correctly" { + $def = 'Get-Process|Where-Object{$_.CPU -gt 10}|Sort-Object CPU' + $expected = 'Get-Process | Where-Object { $_.CPU -gt 10 } | Sort-Object CPU' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle member access correctly" { + $def = '$object.Method($param1,$param2)' + $expected = '$object.Method($param1, $param2)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should not modify method calls with unary operators" { + $def = '$result = $object.Calculate(-$value)' + $expected = '$result = $object.Calculate(-$value)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle splatting correctly" { + $def = 'Get-Process @PSBoundParameters -Name notepad, explorer' + $expected = 'Get-Process @PSBoundParameters -Name notepad, explorer' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle subexpressions correctly" { + $def = 'Result: $(1+2*3)' + $expected = 'Result: $(1 + 2 * 3)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle array indexing correctly" { + $def = '$array[0]+$array[1]-$array[2]' + $expected = '$array[0] + $array[1] - $array[2]' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle multiple statements on one line" { + $def = '$a=1;$b=2;if($a-eq$b){$c=3}' + $expected = '$a = 1; $b = 2; if ($a -eq $b) { $c = 3 }' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should only add space after comma, not before" { + $def = 'Get-ChildItem -Path ".",".\"' + $expected = 'Get-ChildItem -Path ".", ".\"' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should handle array with no spaces correctly" { + $def = '$arr = @(1,2,3,4)' + $expected = '$arr = @(1, 2, 3, 4)' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + # 2114 - Fixes #2094 + It "Should not add space after comma" { + $def = 'docker build --secret id=NUGET_USER,env=NUGET_USER' + $expected = 'docker build --secret id=NUGET_USER,env=NUGET_USER' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + # 2114 - Fixes #2094 + It "Should not remove space after comma if provided" { + $def = 'docker build --secret id=NUGET_USER, env=NUGET_USER' + $expected = 'docker build --secret id=NUGET_USER, env=NUGET_USER' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + } } \ No newline at end of file From ac6859800ecf22afa2d87d91ae9bf4ea832d4b68 Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Tue, 17 Jun 2025 17:42:42 -0500 Subject: [PATCH 3/3] fix: #1561 --- Rules/UseConsistentWhitespace.cs | 21 +++++++++++++++++++ Tests/Rules/UseConsistentWhitespace.tests.ps1 | 21 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index eee80edd2..2c66b7e0c 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -556,11 +556,32 @@ private IEnumerable FindParameterViolations(Ast ast) { IScriptExtent leftExtent = commandParameterAstElements[i].Extent; IScriptExtent rightExtent = commandParameterAstElements[i + 1].Extent; + + // Skip if elements are on different lines if (leftExtent.EndLineNumber != rightExtent.StartLineNumber) { continue; } + // # 1561 - Skip if the whitespace is inside a string literal + // Check if any string in the command contains this whitespace region + var stringAsts = commandAst.FindAll(a => a is StringConstantExpressionAst || a is ExpandableStringExpressionAst, true); + bool isInsideString = false; + foreach (var stringAst in stringAsts) + { + if (stringAst.Extent.StartOffset < leftExtent.EndOffset && + stringAst.Extent.EndOffset > rightExtent.StartOffset) + { + isInsideString = true; + break; + } + } + + if (isInsideString) + { + continue; + } + var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1; if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent) { diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 2cedd2b11..5ca6050da 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -681,6 +681,27 @@ bar -h i ` Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected } + + # Tests for #1561 + It "Should not remove whitespace inside string literals" { + $def = @' + $InputList | ForEach-Object { + $_.Name + } | Select-Object -First 2 | Join-String -sep ", " -OutputPrefix 'Results: ' +'@ + $expected = @' + $InputList | ForEach-Object { + $_.Name + } | Select-Object -First 2 | Join-String -sep ", " -OutputPrefix 'Results: ' +'@ + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } + + It "Should not remove whitespace from string parameters with multiple arguments" { + $def = 'Get-Process | Out-String -Stream | Select-String -Pattern "chrome", "firefox" -SimpleMatch' + $expected = 'Get-Process | Out-String -Stream | Select-String -Pattern "chrome", "firefox" -SimpleMatch' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -BeExactly $expected + } } Context "When keywords follow closing braces" {