Skip to content

Commit 1c344d6

Browse files
committed
C#: Adjust conditional access locations
1 parent 7c43ca7 commit 1c344d6

20 files changed

+337
-307
lines changed

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expression.cs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -314,24 +314,46 @@ public static bool IsDynamic(Context cx, ExpressionSyntax node)
314314
}
315315

316316
/// <summary>
317-
/// Given b in a?.b.c, return a.
317+
/// Given `b` in `a?.b.c`, return `(a?.b, a?.b)`.
318+
///
319+
/// Given `c` in `a?.b?.c.d`, return `(b?.c, a?.b?.c)`.
318320
/// </summary>
319321
/// <param name="node">A MemberBindingExpression.</param>
320-
/// <returns>The qualifier of the conditional access.</returns>
321-
protected static ExpressionSyntax FindConditionalQualifier(ExpressionSyntax node)
322+
/// <returns>The conditional access.</returns>
323+
public static (ConditionalAccessExpressionSyntax Parent, ConditionalAccessExpressionSyntax Root) FindConditionalAccessParent(ExpressionSyntax node)
322324
{
323-
for (SyntaxNode? n = node; n is not null; n = n.Parent)
325+
(ConditionalAccessExpressionSyntax, ConditionalAccessExpressionSyntax)? res = null;
326+
SyntaxNode? prev = null;
327+
328+
for (SyntaxNode? n = node; n is not null; prev = n, n = n.Parent)
324329
{
325-
if (n.Parent is ConditionalAccessExpressionSyntax conditionalAccess &&
326-
conditionalAccess.WhenNotNull == n)
330+
if (n is ConditionalAccessExpressionSyntax conditionalAccess &&
331+
(prev is null || conditionalAccess.WhenNotNull == prev))
332+
{
333+
res = res is null ? (conditionalAccess, conditionalAccess) : (res.Value.Item1, conditionalAccess);
334+
}
335+
else if (res.HasValue)
327336
{
328-
return conditionalAccess.Expression;
337+
break;
329338
}
330339
}
331340

341+
if (res.HasValue)
342+
{
343+
return res.Value;
344+
}
345+
332346
throw new InternalError(node, "Unable to locate a ConditionalAccessExpression");
333347
}
334348

349+
/// <summary>
350+
/// Given b in a?.b.c, return a.
351+
/// </summary>
352+
/// <param name="node">A MemberBindingExpression.</param>
353+
/// <returns>The qualifier of the conditional access.</returns>
354+
protected static ExpressionSyntax FindConditionalQualifier(ExpressionSyntax node) =>
355+
FindConditionalAccessParent(node).Parent.Expression;
356+
335357
public void MakeConditional(TextWriter trapFile)
336358
{
337359
trapFile.conditional_access(this);

csharp/extractor/Semmle.Extraction.CSharp/Entities/ExpressionNodeInfo.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,6 @@ public Extraction.Entities.Location Location
125125
cachedLocation = Context.CreateLocation(CodeAnalysisLocation);
126126
return cachedLocation;
127127
}
128-
129-
set
130-
{
131-
cachedLocation = value;
132-
}
133128
}
134129

135130
public ExprKind Kind { get; set; } = ExprKind.UNKNOWN;

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ElementAccess.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ private NormalElementAccess(ExpressionNodeInfo info)
7979
internal class BindingElementAccess : ElementAccess
8080
{
8181
private BindingElementAccess(ExpressionNodeInfo info)
82-
: base(info, FindConditionalQualifier(info.Node), ((ElementBindingExpressionSyntax)info.Node).ArgumentList) { }
82+
: base(info, FindConditionalQualifier(info.Node), ((ElementBindingExpressionSyntax)info.Node).ArgumentList)
83+
{
84+
}
8385

8486
public static Expression Create(ExpressionNodeInfo info) => new BindingElementAccess(info).TryPopulate();
8587

csharp/extractor/Semmle.Extraction.CSharp/Populators/Locations.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public static class LocationExtensions
1313
/// <param name="l1">The location to extend.</param>
1414
/// <param name="n2">The node to extend the location to.</param>
1515
/// <returns>Extended location.</returns>
16-
public static Location ExtendLocation(this Location l1, SyntaxNode n2)
16+
public static Location ExtendLocation(this Location l1, SyntaxNode n2, bool onlyStart = false)
1717
{
1818
if (n2 is null)
1919
{
@@ -22,7 +22,7 @@ public static Location ExtendLocation(this Location l1, SyntaxNode n2)
2222

2323
var l2 = n2.FixedLocation();
2424
var start = System.Math.Min(l1.SourceSpan.Start, l2.SourceSpan.Start);
25-
var end = System.Math.Max(l1.SourceSpan.End, l2.SourceSpan.End);
25+
var end = onlyStart ? l1.SourceSpan.End : System.Math.Max(l1.SourceSpan.End, l2.SourceSpan.End);
2626
return Location.Create(n2.SyntaxTree, new Microsoft.CodeAnalysis.Text.TextSpan(start, end - start));
2727
}
2828

@@ -85,6 +85,17 @@ public static Location FixedLocation(this SyntaxNode node)
8585
return ((CatchDeclarationSyntax)node).Identifier.GetLocation();
8686
case SyntaxKind.LabeledStatement:
8787
return ((LabeledStatementSyntax)node).Identifier.GetLocation();
88+
case SyntaxKind.ElementBindingExpression:
89+
return node.GetLocation().ExtendLocation(Entities.Expression.FindConditionalAccessParent((ElementBindingExpressionSyntax)node).Root, onlyStart: true);
90+
case SyntaxKind.MemberBindingExpression:
91+
return node.GetLocation().ExtendLocation(Entities.Expression.FindConditionalAccessParent((MemberBindingExpressionSyntax)node).Root, onlyStart: true);
92+
case SyntaxKind.ElementAccessExpression:
93+
return node.GetLocation().ExtendLocation(((ElementAccessExpressionSyntax)node).Expression);
94+
case SyntaxKind.SimpleMemberAccessExpression:
95+
return node.GetLocation().ExtendLocation(((MemberAccessExpressionSyntax)node).Expression);
96+
case SyntaxKind.InvocationExpression:
97+
return node.GetLocation().ExtendLocation(((InvocationExpressionSyntax)node).Expression);
98+
8899
default:
89100
result = node.GetLocation();
90101
break;

csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -242,39 +242,39 @@
242242
| ConditionalAccess.cs:1:7:1:23 | enter ConditionalAccess | ConditionalAccess.cs:1:7:1:23 | exit ConditionalAccess | 5 |
243243
| ConditionalAccess.cs:3:12:3:13 | enter M1 | ConditionalAccess.cs:3:26:3:26 | access to parameter i | 2 |
244244
| ConditionalAccess.cs:3:12:3:13 | exit M1 (normal) | ConditionalAccess.cs:3:12:3:13 | exit M1 | 2 |
245-
| ConditionalAccess.cs:3:28:3:38 | call to method ToString | ConditionalAccess.cs:3:28:3:38 | call to method ToString | 1 |
246-
| ConditionalAccess.cs:3:40:3:49 | call to method ToLower | ConditionalAccess.cs:3:40:3:49 | call to method ToLower | 1 |
245+
| ConditionalAccess.cs:3:26:3:38 | call to method ToString | ConditionalAccess.cs:3:26:3:38 | call to method ToString | 1 |
246+
| ConditionalAccess.cs:3:26:3:49 | call to method ToLower | ConditionalAccess.cs:3:26:3:49 | call to method ToLower | 1 |
247247
| ConditionalAccess.cs:5:10:5:11 | enter M2 | ConditionalAccess.cs:5:26:5:26 | access to parameter s | 2 |
248248
| ConditionalAccess.cs:5:10:5:11 | exit M2 (normal) | ConditionalAccess.cs:5:10:5:11 | exit M2 | 2 |
249-
| ConditionalAccess.cs:5:28:5:34 | access to property Length | ConditionalAccess.cs:5:28:5:34 | access to property Length | 1 |
249+
| ConditionalAccess.cs:5:26:5:34 | access to property Length | ConditionalAccess.cs:5:26:5:34 | access to property Length | 1 |
250250
| ConditionalAccess.cs:7:10:7:11 | enter M3 | ConditionalAccess.cs:7:39:7:40 | access to parameter s1 | 2 |
251251
| ConditionalAccess.cs:7:10:7:11 | exit M3 (normal) | ConditionalAccess.cs:7:10:7:11 | exit M3 | 2 |
252+
| ConditionalAccess.cs:7:38:7:55 | access to property Length | ConditionalAccess.cs:7:38:7:55 | access to property Length | 1 |
252253
| ConditionalAccess.cs:7:39:7:46 | ... ?? ... | ConditionalAccess.cs:7:39:7:46 | ... ?? ... | 1 |
253254
| ConditionalAccess.cs:7:39:7:46 | [non-null] ... ?? ... | ConditionalAccess.cs:7:39:7:46 | [non-null] ... ?? ... | 1 |
254255
| ConditionalAccess.cs:7:39:7:46 | [null] ... ?? ... | ConditionalAccess.cs:7:39:7:46 | [null] ... ?? ... | 1 |
255256
| ConditionalAccess.cs:7:45:7:46 | access to parameter s2 | ConditionalAccess.cs:7:45:7:46 | access to parameter s2 | 1 |
256-
| ConditionalAccess.cs:7:49:7:55 | access to property Length | ConditionalAccess.cs:7:49:7:55 | access to property Length | 1 |
257257
| ConditionalAccess.cs:9:9:9:10 | enter M4 | ConditionalAccess.cs:9:25:9:25 | access to parameter s | 2 |
258+
| ConditionalAccess.cs:9:25:9:33 | access to property Length | ConditionalAccess.cs:9:25:9:33 | access to property Length | 1 |
258259
| ConditionalAccess.cs:9:25:9:38 | ... ?? ... | ConditionalAccess.cs:9:9:9:10 | exit M4 | 3 |
259-
| ConditionalAccess.cs:9:27:9:33 | access to property Length | ConditionalAccess.cs:9:27:9:33 | access to property Length | 1 |
260260
| ConditionalAccess.cs:9:38:9:38 | 0 | ConditionalAccess.cs:9:38:9:38 | 0 | 1 |
261261
| ConditionalAccess.cs:11:9:11:10 | enter M5 | ConditionalAccess.cs:13:13:13:13 | access to parameter s | 4 |
262262
| ConditionalAccess.cs:11:9:11:10 | exit M5 (normal) | ConditionalAccess.cs:11:9:11:10 | exit M5 | 2 |
263-
| ConditionalAccess.cs:13:15:13:21 | access to property Length | ConditionalAccess.cs:13:15:13:21 | access to property Length | 1 |
263+
| ConditionalAccess.cs:13:13:13:21 | access to property Length | ConditionalAccess.cs:13:13:13:21 | access to property Length | 1 |
264264
| ConditionalAccess.cs:13:25:13:25 | 0 | ConditionalAccess.cs:13:13:13:25 | ... > ... | 3 |
265265
| ConditionalAccess.cs:14:20:14:20 | 0 | ConditionalAccess.cs:14:13:14:21 | return ...; | 2 |
266266
| ConditionalAccess.cs:16:20:16:20 | 1 | ConditionalAccess.cs:16:13:16:21 | return ...; | 2 |
267267
| ConditionalAccess.cs:19:12:19:13 | enter M6 | ConditionalAccess.cs:19:40:19:41 | access to parameter s1 | 2 |
268268
| ConditionalAccess.cs:19:12:19:13 | exit M6 (normal) | ConditionalAccess.cs:19:12:19:13 | exit M6 | 2 |
269-
| ConditionalAccess.cs:19:58:19:59 | access to parameter s2 | ConditionalAccess.cs:19:43:19:60 | call to method CommaJoinWith | 2 |
269+
| ConditionalAccess.cs:19:58:19:59 | access to parameter s2 | ConditionalAccess.cs:19:40:19:60 | call to method CommaJoinWith | 2 |
270270
| ConditionalAccess.cs:21:10:21:11 | enter M7 | ConditionalAccess.cs:23:18:23:29 | (...) ... | 5 |
271271
| ConditionalAccess.cs:23:13:23:38 | Nullable<Int32> j = ... | ConditionalAccess.cs:24:18:24:24 | (...) ... | 4 |
272-
| ConditionalAccess.cs:24:27:24:37 | call to method ToString | ConditionalAccess.cs:25:13:25:14 | "" | 4 |
272+
| ConditionalAccess.cs:24:17:24:37 | call to method ToString | ConditionalAccess.cs:25:13:25:14 | "" | 4 |
273273
| ConditionalAccess.cs:25:31:25:31 | access to local variable s | ConditionalAccess.cs:21:10:21:11 | exit M7 | 5 |
274274
| ConditionalAccess.cs:30:10:30:12 | enter Out | ConditionalAccess.cs:30:10:30:12 | exit Out | 5 |
275275
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:35:9:35:12 | access to property Prop | 8 |
276276
| ConditionalAccess.cs:32:10:32:11 | exit M8 (normal) | ConditionalAccess.cs:32:10:32:11 | exit M8 | 2 |
277-
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:35:14:35:24 | call to method Out | 1 |
277+
| ConditionalAccess.cs:35:9:35:24 | call to method Out | ConditionalAccess.cs:35:9:35:24 | call to method Out | 1 |
278278
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | exit CommaJoinWith | 8 |
279279
| Conditions.cs:1:7:1:16 | enter Conditions | Conditions.cs:1:7:1:16 | exit Conditions | 5 |
280280
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:5:13:5:15 | access to parameter inc | 4 |
@@ -674,8 +674,8 @@
674674
| Foreach.cs:18:10:18:11 | exit M3 (normal) | Foreach.cs:18:10:18:11 | exit M3 | 2 |
675675
| Foreach.cs:20:9:21:11 | foreach (... ... in ...) ... | Foreach.cs:20:9:21:11 | foreach (... ... in ...) ... | 1 |
676676
| Foreach.cs:20:22:20:22 | String x | Foreach.cs:21:11:21:11 | ; | 2 |
677+
| Foreach.cs:20:27:20:38 | call to method ToArray<String> | Foreach.cs:20:27:20:38 | call to method ToArray<String> | 1 |
677678
| Foreach.cs:20:27:20:68 | ... ?? ... | Foreach.cs:20:27:20:68 | ... ?? ... | 1 |
678-
| Foreach.cs:20:29:20:38 | call to method ToArray<String> | Foreach.cs:20:29:20:38 | call to method ToArray<String> | 1 |
679679
| Foreach.cs:20:43:20:68 | call to method Empty<String> | Foreach.cs:20:43:20:68 | call to method Empty<String> | 1 |
680680
| Foreach.cs:24:10:24:11 | enter M4 | Foreach.cs:26:36:26:39 | access to parameter args | 3 |
681681
| Foreach.cs:24:10:24:11 | exit M4 (normal) | Foreach.cs:24:10:24:11 | exit M4 | 2 |
@@ -1138,7 +1138,7 @@
11381138
| Switch.cs:98:16:98:20 | false | Switch.cs:98:9:98:21 | return ...; | 2 |
11391139
| Switch.cs:101:9:101:10 | enter M9 | Switch.cs:103:17:103:17 | access to parameter s | 4 |
11401140
| Switch.cs:101:9:101:10 | exit M9 (normal) | Switch.cs:101:9:101:10 | exit M9 | 2 |
1141-
| Switch.cs:103:19:103:25 | access to property Length | Switch.cs:103:19:103:25 | access to property Length | 1 |
1141+
| Switch.cs:103:17:103:25 | access to property Length | Switch.cs:103:17:103:25 | access to property Length | 1 |
11421142
| Switch.cs:105:13:105:19 | case ...: | Switch.cs:105:18:105:18 | 0 | 2 |
11431143
| Switch.cs:105:28:105:28 | 0 | Switch.cs:105:21:105:29 | return ...; | 2 |
11441144
| Switch.cs:106:13:106:19 | case ...: | Switch.cs:106:18:106:18 | 1 | 2 |
@@ -1166,6 +1166,7 @@
11661166
| Switch.cs:126:13:126:19 | return ...; | Switch.cs:126:13:126:19 | return ...; | 1 |
11671167
| Switch.cs:129:12:129:14 | enter M12 | Switch.cs:131:28:131:35 | String s | 4 |
11681168
| Switch.cs:131:9:131:67 | return ...; | Switch.cs:129:12:129:14 | exit M12 | 3 |
1169+
| Switch.cs:131:16:131:66 | call to method ToString | Switch.cs:131:16:131:66 | call to method ToString | 1 |
11691170
| Switch.cs:131:17:131:53 | [non-null] ... switch { ... } | Switch.cs:131:17:131:53 | [non-null] ... switch { ... } | 1 |
11701171
| Switch.cs:131:17:131:53 | [null] ... switch { ... } | Switch.cs:131:17:131:53 | [null] ... switch { ... } | 1 |
11711172
| Switch.cs:131:28:131:40 | [non-null] ... => ... | Switch.cs:131:28:131:40 | [non-null] ... => ... | 1 |
@@ -1174,7 +1175,6 @@
11741175
| Switch.cs:131:43:131:43 | _ | Switch.cs:131:43:131:43 | _ | 1 |
11751176
| Switch.cs:131:43:131:51 | [null] ... => ... | Switch.cs:131:43:131:51 | [null] ... => ... | 1 |
11761177
| Switch.cs:131:48:131:51 | null | Switch.cs:131:48:131:51 | null | 1 |
1177-
| Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:56:131:66 | call to method ToString | 1 |
11781178
| Switch.cs:134:9:134:11 | enter M13 | Switch.cs:139:18:139:18 | 1 | 6 |
11791179
| Switch.cs:134:9:134:11 | exit M13 (normal) | Switch.cs:134:9:134:11 | exit M13 | 2 |
11801180
| Switch.cs:138:13:138:20 | default: | Switch.cs:138:22:138:31 | return ...; | 4 |

0 commit comments

Comments
 (0)