From c7ffc38a56b8f80c9f08b6f3ee33e1e2befb7f87 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Tue, 4 Nov 2025 21:23:57 +0000 Subject: [PATCH] Don't add inner brace whitespace to braces forming a braced member access $a.{Prop}. --- Engine/TokenOperations.cs | 160 ++++++++++++++++ Rules/UseConsistentWhitespace.cs | 11 ++ Tests/Engine/TokenOperations.tests.ps1 | 177 ++++++++++++++++++ Tests/Rules/UseConsistentWhitespace.tests.ps1 | 127 +++++++++++++ 4 files changed, 475 insertions(+) diff --git a/Engine/TokenOperations.cs b/Engine/TokenOperations.cs index fa9a1978a..4845ab8c4 100644 --- a/Engine/TokenOperations.cs +++ b/Engine/TokenOperations.cs @@ -245,5 +245,165 @@ public Ast GetAstPosition(Token token) return findAstVisitor.AstPosition; } + /// + /// Returns a list of non-overlapping ranges (startOffset,endOffset) representing the start + /// and end of braced member access expressions. These are member accesses where the name is + /// enclosed in braces. The contents of such braces are treated literally as a member name. + /// Altering the contents of these braces by formatting is likely to break code. + /// + public List> GetBracedMemberAccessRanges() + { + // A list of (startOffset, endOffset) pairs representing the start + // and end braces of braced member access expressions. + var ranges = new List>(); + + var node = tokensLL.Value.First; + while (node != null) + { + switch (node.Value.Kind) + { +#if CORECLR + // TokenKind added in PS7 + case TokenKind.QuestionDot: +#endif + case TokenKind.Dot: + break; + default: + node = node.Next; + continue; + } + + // Note: We don't check if the dot is part of an existing range. When we find + // a valid range, we skip all tokens inside it - so we won't ever evaluate a token + // which already part of a previously found range. + + // Backward scan: + // Determine if this 'dot' is part of a member access. + // Walk left over contiguous comment tokens that are 'touching'. + // After skipping comments, the preceding non-comment token must also be 'touching' + // and one of the expected TokenKinds. + var leftToken = node.Previous; + var rightToken = node; + while (leftToken != null && leftToken.Value.Kind == TokenKind.Comment) + { + if (leftToken.Value.Extent.EndOffset != rightToken.Value.Extent.StartOffset) + { + leftToken = null; + break; + } + rightToken = leftToken; + leftToken = leftToken.Previous; + } + if (leftToken == null) + { + // We ran out of tokens before finding a non-comment token to the left or there + // was intervening whitespace. + node = node.Next; + continue; + } + + if (leftToken.Value.Extent.EndOffset != rightToken.Value.Extent.StartOffset) + { + // There's whitespace between the two tokens + node = node.Next; + continue; + } + + // Limit to valid token kinds that can precede a 'dot' in a member access. + switch (leftToken.Value.Kind) + { + // Note: TokenKind.Number isn't in the list as 5.{Prop} is a syntax error + // (Unexpected token). Numbers also have no properties - only methods. + case TokenKind.Variable: + case TokenKind.Identifier: + case TokenKind.StringLiteral: + case TokenKind.StringExpandable: + case TokenKind.HereStringLiteral: + case TokenKind.HereStringExpandable: + case TokenKind.RParen: + case TokenKind.RCurly: + case TokenKind.RBracket: + // allowed + break; + default: + // not allowed + node = node.Next; + continue; + } + + // Forward Scan: + // Check that the next significant token is an LCurly + // Starting from the token after the 'dot', walk right skipping trivia tokens: + // - Comment + // - NewLine + // - LineContinuation (`) + // These may be multi-line and need not be 'touching' the dot. + // The first non-trivia token encountered must be an opening curly brace (LCurly) for + // this dot to begin a braced member access. If it is not LCurly or we run out + // of tokens, this dot is ignored. + var scan = node.Next; + while (scan != null) + { + if ( + scan.Value.Kind == TokenKind.Comment || + scan.Value.Kind == TokenKind.NewLine || + scan.Value.Kind == TokenKind.LineContinuation + ) + { + scan = scan.Next; + continue; + } + break; + } + + // If we reached the end without finding a significant token, or if the found token + // is not LCurly, continue. + if (scan == null || scan.Value.Kind != TokenKind.LCurly) + { + node = node.Next; + continue; + } + + // We have a valid token, followed by a dot, followed by an LCurly. + // Find the matching RCurly and create the range. + var lCurlyNode = scan; + + // Depth count braces to find the RCurly which closes the LCurly. + int depth = 0; + LinkedListNode rcurlyNode = null; + while (scan != null) + { + if (scan.Value.Kind == TokenKind.LCurly) depth++; + else if (scan.Value.Kind == TokenKind.RCurly) + { + depth--; + if (depth == 0) + { + rcurlyNode = scan; + break; + } + } + scan = scan.Next; + } + + // If we didn't find a matching RCurly, something has gone wrong. + // Should an unmatched pair be caught by the parser as a parse error? + if (rcurlyNode == null) + { + node = node.Next; + continue; + } + + ranges.Add(new Tuple( + lCurlyNode.Value.Extent.StartOffset, + rcurlyNode.Value.Extent.EndOffset + )); + + // Skip all tokens inside the excluded range. + node = rcurlyNode.Next; + } + + return ranges; + } } } diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index e6d4cff99..7f7550ffe 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -257,6 +257,9 @@ private IEnumerable FindOpenBraceViolations(TokenOperations to private IEnumerable FindInnerBraceViolations(TokenOperations tokenOperations) { + // Ranges which represent braced member access. Tokens within these ranges should be + // excluded from formatting. + var exclusionRanges = tokenOperations.GetBracedMemberAccessRanges(); foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly)) { if (lCurly.Next == null @@ -264,6 +267,10 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t || lCurly.Next.Value.Kind == TokenKind.NewLine || lCurly.Next.Value.Kind == TokenKind.LineContinuation || lCurly.Next.Value.Kind == TokenKind.RCurly + || exclusionRanges.Any(range => + lCurly.Value.Extent.StartOffset >= range.Item1 && + lCurly.Value.Extent.EndOffset <= range.Item2 + ) ) { continue; @@ -290,6 +297,10 @@ private IEnumerable FindInnerBraceViolations(TokenOperations t || rCurly.Previous.Value.Kind == TokenKind.NewLine || rCurly.Previous.Value.Kind == TokenKind.LineContinuation || rCurly.Previous.Value.Kind == TokenKind.AtCurly + || exclusionRanges.Any(range => + rCurly.Value.Extent.StartOffset >= range.Item1 && + rCurly.Value.Extent.EndOffset <= range.Item2 + ) ) { continue; diff --git a/Tests/Engine/TokenOperations.tests.ps1 b/Tests/Engine/TokenOperations.tests.ps1 index 97fef3958..1bb1d9298 100644 --- a/Tests/Engine/TokenOperations.tests.ps1 +++ b/Tests/Engine/TokenOperations.tests.ps1 @@ -18,4 +18,181 @@ $h = @{ $hashTableAst | Should -BeOfType [System.Management.Automation.Language.HashTableAst] $hashTableAst.Extent.Text | Should -Be '@{ z = "hi" }' } + + Context 'Braced Member Access Ranges' { + + BeforeDiscovery { + $RangeTests = @( + @{ + Name = 'No braced member access' + ScriptDef = '$object.Prop' + ExpectedRanges = @() + } + @{ + Name = 'No braced member access on braced variable name' + ScriptDef = '${object}.Prop' + ExpectedRanges = @() + } + @{ + Name = 'Braced member access' + ScriptDef = '$object.{Prop}' + ExpectedRanges = @( + ,@(8, 14) + ) + } + @{ + Name = 'Braced member access with spaces' + ScriptDef = '$object. { Prop }' + ExpectedRanges = @( + ,@(9, 17) + ) + } + @{ + Name = 'Braced member access with newline' + ScriptDef = "`$object.`n{ Prop }" + ExpectedRanges = @( + ,@(9, 17) + ) + } + @{ + Name = 'Braced member access with comment' + ScriptDef = "`$object. <#comment#>{Prop}" + ExpectedRanges = @( + ,@(20, 26) + ) + } + @{ + Name = 'Braced member access with multi-line comment' + ScriptDef = "`$object. <#`ncomment`n#>{Prop}" + ExpectedRanges = @( + ,@(22, 28) + ) + } + @{ + Name = 'Braced member access with inline comment' + ScriptDef = "`$object. #comment`n{Prop}" + ExpectedRanges = @( + ,@(18, 24) + ) + } + @{ + Name = 'Braced member access with inner curly braces' + ScriptDef = "`$object.{{Prop}}" + ExpectedRanges = @( + ,@(8, 16) + ) + } + @{ + Name = 'Indexed Braced member access' + ScriptDef = "`$object[0].{Prop}" + ExpectedRanges = @( + ,@(11, 17) + ) + } + @{ + Name = 'Parenthesized Braced member access' + ScriptDef = "(`$object).{Prop}" + ExpectedRanges = @( + ,@(10, 16) + ) + } + @{ + Name = 'Chained Braced member access' + ScriptDef = "`$object.{Prop}.{InnerProp}" + ExpectedRanges = @( + ,@(8, 14) + ,@(15, 26) + ) + } + @{ + Name = 'Multiple Braced member access in larger script' + ScriptDef = @' +$var = 1 +$a.prop.{{inner}} +$a.{ + $a.{Prop} +} +'@ + ExpectedRanges = @( + ,@(17, 26) + ,@(30, 47) + ) + } + ) + } + + It 'Should correctly identify range for ' -ForEach $RangeTests { + $tokens = $null + $parseErrors = $null + $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($ScriptDef, [ref] $tokens, [ref] $parseErrors) + $tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst) + $ranges = $tokenOperations.GetBracedMemberAccessRanges() + $ranges.Count | Should -Be $ExpectedRanges.Count + for ($i = 0; $i -lt $ranges.Count; $i++) { + $ranges[$i].Item1 | Should -Be $ExpectedRanges[$i][0] + $ranges[$i].Item2 | Should -Be $ExpectedRanges[$i][1] + } + } + + It 'Should not identify dot-sourcing as braced member access' { + $scriptText = @' +. {5+5} +$a=4;. {10+15} +'@ + $tokens = $null + $parseErrors = $null + $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors) + $tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst) + $ranges = $tokenOperations.GetBracedMemberAccessRanges() + $ranges.Count | Should -Be 0 + } + + It 'Should not return a range for an incomplete bracket pair (parse error)' { + $scriptText = @' +$object.{MemberName +'@ + $tokens = $null + $parseErrors = $null + $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors) + $tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst) + $ranges = $tokenOperations.GetBracedMemberAccessRanges() + $ranges.Count | Should -Be 0 + } + + It 'Should find the correct range for null-conditional braced member access' { + $scriptText = '$object?.{Prop}' + $tokens = $null + $parseErrors = $null + $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors) + $tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst) + $ranges = $tokenOperations.GetBracedMemberAccessRanges() + $ranges.Count | Should -Be 1 + $ExpectedRanges = @( + ,@(9, 15) + ) + for ($i = 0; $i -lt $ranges.Count; $i++) { + $ranges[$i].Item1 | Should -Be $ExpectedRanges[$i][0] + $ranges[$i].Item2 | Should -Be $ExpectedRanges[$i][1] + } + } -Skip:$($PSVersionTable.PSVersion.Major -lt 7) + + It 'Should find the correct range for nested null-conditional braced member access' { + $scriptText = '$object?.{Prop?.{InnerProp}}' + $tokens = $null + $parseErrors = $null + $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors) + $tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst) + $ranges = $tokenOperations.GetBracedMemberAccessRanges() + $ranges.Count | Should -Be 1 + $ExpectedRanges = @( + ,@(9, 28) + ) + for ($i = 0; $i -lt $ranges.Count; $i++) { + $ranges[$i].Item1 | Should -Be $ExpectedRanges[$i][0] + $ranges[$i].Item2 | Should -Be $ExpectedRanges[$i][1] + } + } -Skip:$($PSVersionTable.PSVersion.Major -lt 7) + + } + } \ No newline at end of file diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 952e49909..c2013fa31 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -684,4 +684,131 @@ bar -h i ` Should -Be $expected } } + + Context "Braced Member Accessor Handling" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $true + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.CheckParameter = $false + } + + It 'Should not find a violation for a simple braced member accessor with no whitespace' { + $def = '$variable.{Property}' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a simple braced member accessor with whitespace after dot' { + $def = '$object. {Member}' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a simple braced member accessor with newline after dot' { + $def = "`$object.`n{Member}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a simple braced member accessor with inline comment after dot' { + $def = "`$object.<#comment#>{Member}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a simple braced member accessor with inline comment before dot' { + $def = "`$object<#comment#>.{Member}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a simple braced member accessor with multiple touching inline comment before dot' { + $def = "`$object<#a#><#b#><#c#><#d#>.{Member}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for an indexed braced member accessor' { + $def = "`$object[0].{Member}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a parenthesized braced member accessor' { + $def = "(`$object).{Member}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a string literal braced member accessor' { + $def = "'StringLiteral'.{Length}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for an expandable string literal braced member accessor' { + $def = "`"StringLiteral`".{Length}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a here-string braced member accessor' { + $def = "@' +string +'@.{Length}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a doublequoted here-string braced member accessor' { + $def = "@`" +string +`"@.{Length}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for a type braced member accessor' { + $def = "[System.DayOfWeek].{Assembly}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for an braced member accessor on an identifier' { + $def = "`$Object.Property.{Prop}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for an braced member accessor with nested braces' { + $def = "`$Object.{{Prop}}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It 'Should not find a violation for an braced member accessor with nested inner dot' { + $def = "`$Object.{`$InnerObject.{Prop}}" + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + # Check that dot-sourcing still causes formatting violations + It 'Should find violations for dot-sourcing a script (no space after dot)' { + $def = '.{5+5}' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 2 + } + + It 'Should find violations for dot-sourcing a script (space after dot)' { + $def = '. {5+5}' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 2 + } + + It 'Should find violations for dot-sourcing a script (Semi-Colon before dot)' { + $def = '$a = 4;. {5+5}' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 2 + } + + # PS7 Specific behaviour. QuestionDot token. + It 'Should not find a violation for a null conditional braced member accessor' { + $def = '${Object}?.{Prop}' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } -Skip:$($PSVersionTable.PSVersion.Major -lt 7) + + It 'Should not find a violation for a nested null conditional braced member accessor' { + $def = '${Object}?.{${InnerObject}?.{Prop}}' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } -Skip:$($PSVersionTable.PSVersion.Major -lt 7) + + } }