Skip to content

Commit c7ffc38

Browse files
liamjpetersliamjpeters
committed
Don't add inner brace whitespace to braces forming a braced member access $a.{Prop}.
1 parent a9898a6 commit c7ffc38

File tree

4 files changed

+475
-0
lines changed

4 files changed

+475
-0
lines changed

‎Engine/TokenOperations.cs‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,5 +245,165 @@ public Ast GetAstPosition(Token token)
245245
return findAstVisitor.AstPosition;
246246
}
247247

248+
/// <summary>
249+
/// Returns a list of non-overlapping ranges (startOffset,endOffset) representing the start
250+
/// and end of braced member access expressions. These are member accesses where the name is
251+
/// enclosed in braces. The contents of such braces are treated literally as a member name.
252+
/// Altering the contents of these braces by formatting is likely to break code.
253+
/// </summary>
254+
public List<Tuple<int, int>> GetBracedMemberAccessRanges()
255+
{
256+
// A list of (startOffset, endOffset) pairs representing the start
257+
// and end braces of braced member access expressions.
258+
var ranges = new List<Tuple<int, int>>();
259+
260+
var node = tokensLL.Value.First;
261+
while (node != null)
262+
{
263+
switch (node.Value.Kind)
264+
{
265+
#if CORECLR
266+
// TokenKind added in PS7
267+
case TokenKind.QuestionDot:
268+
#endif
269+
case TokenKind.Dot:
270+
break;
271+
default:
272+
node = node.Next;
273+
continue;
274+
}
275+
276+
// Note: We don't check if the dot is part of an existing range. When we find
277+
// a valid range, we skip all tokens inside it - so we won't ever evaluate a token
278+
// which already part of a previously found range.
279+
280+
// Backward scan:
281+
// Determine if this 'dot' is part of a member access.
282+
// Walk left over contiguous comment tokens that are 'touching'.
283+
// After skipping comments, the preceding non-comment token must also be 'touching'
284+
// and one of the expected TokenKinds.
285+
var leftToken = node.Previous;
286+
var rightToken = node;
287+
while (leftToken != null && leftToken.Value.Kind == TokenKind.Comment)
288+
{
289+
if (leftToken.Value.Extent.EndOffset != rightToken.Value.Extent.StartOffset)
290+
{
291+
leftToken = null;
292+
break;
293+
}
294+
rightToken = leftToken;
295+
leftToken = leftToken.Previous;
296+
}
297+
if (leftToken == null)
298+
{
299+
// We ran out of tokens before finding a non-comment token to the left or there
300+
// was intervening whitespace.
301+
node = node.Next;
302+
continue;
303+
}
304+
305+
if (leftToken.Value.Extent.EndOffset != rightToken.Value.Extent.StartOffset)
306+
{
307+
// There's whitespace between the two tokens
308+
node = node.Next;
309+
continue;
310+
}
311+
312+
// Limit to valid token kinds that can precede a 'dot' in a member access.
313+
switch (leftToken.Value.Kind)
314+
{
315+
// Note: TokenKind.Number isn't in the list as 5.{Prop} is a syntax error
316+
// (Unexpected token). Numbers also have no properties - only methods.
317+
case TokenKind.Variable:
318+
case TokenKind.Identifier:
319+
case TokenKind.StringLiteral:
320+
case TokenKind.StringExpandable:
321+
case TokenKind.HereStringLiteral:
322+
case TokenKind.HereStringExpandable:
323+
case TokenKind.RParen:
324+
case TokenKind.RCurly:
325+
case TokenKind.RBracket:
326+
// allowed
327+
break;
328+
default:
329+
// not allowed
330+
node = node.Next;
331+
continue;
332+
}
333+
334+
// Forward Scan:
335+
// Check that the next significant token is an LCurly
336+
// Starting from the token after the 'dot', walk right skipping trivia tokens:
337+
// - Comment
338+
// - NewLine
339+
// - LineContinuation (`)
340+
// These may be multi-line and need not be 'touching' the dot.
341+
// The first non-trivia token encountered must be an opening curly brace (LCurly) for
342+
// this dot to begin a braced member access. If it is not LCurly or we run out
343+
// of tokens, this dot is ignored.
344+
var scan = node.Next;
345+
while (scan != null)
346+
{
347+
if (
348+
scan.Value.Kind == TokenKind.Comment ||
349+
scan.Value.Kind == TokenKind.NewLine ||
350+
scan.Value.Kind == TokenKind.LineContinuation
351+
)
352+
{
353+
scan = scan.Next;
354+
continue;
355+
}
356+
break;
357+
}
358+
359+
// If we reached the end without finding a significant token, or if the found token
360+
// is not LCurly, continue.
361+
if (scan == null || scan.Value.Kind != TokenKind.LCurly)
362+
{
363+
node = node.Next;
364+
continue;
365+
}
366+
367+
// We have a valid token, followed by a dot, followed by an LCurly.
368+
// Find the matching RCurly and create the range.
369+
var lCurlyNode = scan;
370+
371+
// Depth count braces to find the RCurly which closes the LCurly.
372+
int depth = 0;
373+
LinkedListNode<Token> rcurlyNode = null;
374+
while (scan != null)
375+
{
376+
if (scan.Value.Kind == TokenKind.LCurly) depth++;
377+
else if (scan.Value.Kind == TokenKind.RCurly)
378+
{
379+
depth--;
380+
if (depth == 0)
381+
{
382+
rcurlyNode = scan;
383+
break;
384+
}
385+
}
386+
scan = scan.Next;
387+
}
388+
389+
// If we didn't find a matching RCurly, something has gone wrong.
390+
// Should an unmatched pair be caught by the parser as a parse error?
391+
if (rcurlyNode == null)
392+
{
393+
node = node.Next;
394+
continue;
395+
}
396+
397+
ranges.Add(new Tuple<int, int>(
398+
lCurlyNode.Value.Extent.StartOffset,
399+
rcurlyNode.Value.Extent.EndOffset
400+
));
401+
402+
// Skip all tokens inside the excluded range.
403+
node = rcurlyNode.Next;
404+
}
405+
406+
return ranges;
407+
}
248408
}
249409
}

‎Rules/UseConsistentWhitespace.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,20 @@ private IEnumerable<DiagnosticRecord> FindOpenBraceViolations(TokenOperations to
257257

258258
private IEnumerable<DiagnosticRecord> FindInnerBraceViolations(TokenOperations tokenOperations)
259259
{
260+
// Ranges which represent braced member access. Tokens within these ranges should be
261+
// excluded from formatting.
262+
var exclusionRanges = tokenOperations.GetBracedMemberAccessRanges();
260263
foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly))
261264
{
262265
if (lCurly.Next == null
263266
|| !(lCurly.Previous == null || IsPreviousTokenOnSameLine(lCurly))
264267
|| lCurly.Next.Value.Kind == TokenKind.NewLine
265268
|| lCurly.Next.Value.Kind == TokenKind.LineContinuation
266269
|| lCurly.Next.Value.Kind == TokenKind.RCurly
270+
|| exclusionRanges.Any(range =>
271+
lCurly.Value.Extent.StartOffset >= range.Item1 &&
272+
lCurly.Value.Extent.EndOffset <= range.Item2
273+
)
267274
)
268275
{
269276
continue;
@@ -290,6 +297,10 @@ private IEnumerable<DiagnosticRecord> FindInnerBraceViolations(TokenOperations t
290297
|| rCurly.Previous.Value.Kind == TokenKind.NewLine
291298
|| rCurly.Previous.Value.Kind == TokenKind.LineContinuation
292299
|| rCurly.Previous.Value.Kind == TokenKind.AtCurly
300+
|| exclusionRanges.Any(range =>
301+
rCurly.Value.Extent.StartOffset >= range.Item1 &&
302+
rCurly.Value.Extent.EndOffset <= range.Item2
303+
)
293304
)
294305
{
295306
continue;

‎Tests/Engine/TokenOperations.tests.ps1‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,181 @@ $h = @{
1818
$hashTableAst | Should -BeOfType [System.Management.Automation.Language.HashTableAst]
1919
$hashTableAst.Extent.Text | Should -Be '@{ z = "hi" }'
2020
}
21+
22+
Context 'Braced Member Access Ranges' {
23+
24+
BeforeDiscovery {
25+
$RangeTests = @(
26+
@{
27+
Name = 'No braced member access'
28+
ScriptDef = '$object.Prop'
29+
ExpectedRanges = @()
30+
}
31+
@{
32+
Name = 'No braced member access on braced variable name'
33+
ScriptDef = '${object}.Prop'
34+
ExpectedRanges = @()
35+
}
36+
@{
37+
Name = 'Braced member access'
38+
ScriptDef = '$object.{Prop}'
39+
ExpectedRanges = @(
40+
,@(8, 14)
41+
)
42+
}
43+
@{
44+
Name = 'Braced member access with spaces'
45+
ScriptDef = '$object. { Prop }'
46+
ExpectedRanges = @(
47+
,@(9, 17)
48+
)
49+
}
50+
@{
51+
Name = 'Braced member access with newline'
52+
ScriptDef = "`$object.`n{ Prop }"
53+
ExpectedRanges = @(
54+
,@(9, 17)
55+
)
56+
}
57+
@{
58+
Name = 'Braced member access with comment'
59+
ScriptDef = "`$object. <#comment#>{Prop}"
60+
ExpectedRanges = @(
61+
,@(20, 26)
62+
)
63+
}
64+
@{
65+
Name = 'Braced member access with multi-line comment'
66+
ScriptDef = "`$object. <#`ncomment`n#>{Prop}"
67+
ExpectedRanges = @(
68+
,@(22, 28)
69+
)
70+
}
71+
@{
72+
Name = 'Braced member access with inline comment'
73+
ScriptDef = "`$object. #comment`n{Prop}"
74+
ExpectedRanges = @(
75+
,@(18, 24)
76+
)
77+
}
78+
@{
79+
Name = 'Braced member access with inner curly braces'
80+
ScriptDef = "`$object.{{Prop}}"
81+
ExpectedRanges = @(
82+
,@(8, 16)
83+
)
84+
}
85+
@{
86+
Name = 'Indexed Braced member access'
87+
ScriptDef = "`$object[0].{Prop}"
88+
ExpectedRanges = @(
89+
,@(11, 17)
90+
)
91+
}
92+
@{
93+
Name = 'Parenthesized Braced member access'
94+
ScriptDef = "(`$object).{Prop}"
95+
ExpectedRanges = @(
96+
,@(10, 16)
97+
)
98+
}
99+
@{
100+
Name = 'Chained Braced member access'
101+
ScriptDef = "`$object.{Prop}.{InnerProp}"
102+
ExpectedRanges = @(
103+
,@(8, 14)
104+
,@(15, 26)
105+
)
106+
}
107+
@{
108+
Name = 'Multiple Braced member access in larger script'
109+
ScriptDef = @'
110+
$var = 1
111+
$a.prop.{{inner}}
112+
$a.{
113+
$a.{Prop}
114+
}
115+
'@
116+
ExpectedRanges = @(
117+
,@(17, 26)
118+
,@(30, 47)
119+
)
120+
}
121+
)
122+
}
123+
124+
It 'Should correctly identify range for <Name>' -ForEach $RangeTests {
125+
$tokens = $null
126+
$parseErrors = $null
127+
$scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($ScriptDef, [ref] $tokens, [ref] $parseErrors)
128+
$tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst)
129+
$ranges = $tokenOperations.GetBracedMemberAccessRanges()
130+
$ranges.Count | Should -Be $ExpectedRanges.Count
131+
for ($i = 0; $i -lt $ranges.Count; $i++) {
132+
$ranges[$i].Item1 | Should -Be $ExpectedRanges[$i][0]
133+
$ranges[$i].Item2 | Should -Be $ExpectedRanges[$i][1]
134+
}
135+
}
136+
137+
It 'Should not identify dot-sourcing as braced member access' {
138+
$scriptText = @'
139+
. {5+5}
140+
$a=4;. {10+15}
141+
'@
142+
$tokens = $null
143+
$parseErrors = $null
144+
$scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors)
145+
$tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst)
146+
$ranges = $tokenOperations.GetBracedMemberAccessRanges()
147+
$ranges.Count | Should -Be 0
148+
}
149+
150+
It 'Should not return a range for an incomplete bracket pair (parse error)' {
151+
$scriptText = @'
152+
$object.{MemberName
153+
'@
154+
$tokens = $null
155+
$parseErrors = $null
156+
$scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors)
157+
$tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst)
158+
$ranges = $tokenOperations.GetBracedMemberAccessRanges()
159+
$ranges.Count | Should -Be 0
160+
}
161+
162+
It 'Should find the correct range for null-conditional braced member access' {
163+
$scriptText = '$object?.{Prop}'
164+
$tokens = $null
165+
$parseErrors = $null
166+
$scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors)
167+
$tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst)
168+
$ranges = $tokenOperations.GetBracedMemberAccessRanges()
169+
$ranges.Count | Should -Be 1
170+
$ExpectedRanges = @(
171+
,@(9, 15)
172+
)
173+
for ($i = 0; $i -lt $ranges.Count; $i++) {
174+
$ranges[$i].Item1 | Should -Be $ExpectedRanges[$i][0]
175+
$ranges[$i].Item2 | Should -Be $ExpectedRanges[$i][1]
176+
}
177+
} -Skip:$($PSVersionTable.PSVersion.Major -lt 7)
178+
179+
It 'Should find the correct range for nested null-conditional braced member access' {
180+
$scriptText = '$object?.{Prop?.{InnerProp}}'
181+
$tokens = $null
182+
$parseErrors = $null
183+
$scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors)
184+
$tokenOperations = [Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations]::new($tokens, $scriptAst)
185+
$ranges = $tokenOperations.GetBracedMemberAccessRanges()
186+
$ranges.Count | Should -Be 1
187+
$ExpectedRanges = @(
188+
,@(9, 28)
189+
)
190+
for ($i = 0; $i -lt $ranges.Count; $i++) {
191+
$ranges[$i].Item1 | Should -Be $ExpectedRanges[$i][0]
192+
$ranges[$i].Item2 | Should -Be $ExpectedRanges[$i][1]
193+
}
194+
} -Skip:$($PSVersionTable.PSVersion.Major -lt 7)
195+
196+
}
197+
21198
}

0 commit comments

Comments
 (0)