Skip to content

Commit 3f33f2d

Browse files
tamasvajktamasvajk
authored
Merge pull request github#5980 from tamasvajk/fix/extension-method-as-target
C#: Extract correct method symbol as target of extension method calls
2 parents 4963a8f + f98781d commit 3f33f2d

File tree

7 files changed

+124
-85
lines changed

7 files changed

+124
-85
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,26 +20,6 @@ protected void PopulateParameters()
2020
IEnumerable<IParameterSymbol> parameters = Symbol.Parameters;
2121
IEnumerable<IParameterSymbol> originalParameters = originalMethod.Symbol.Parameters;
2222

23-
if (IsReducedExtension)
24-
{
25-
if (this == originalMethod)
26-
{
27-
// Non-generic reduced extensions must be extracted exactly like the
28-
// non-reduced counterparts
29-
parameters = Symbol.ReducedFrom!.Parameters;
30-
}
31-
else
32-
{
33-
// Constructed reduced extensions are special because their non-reduced
34-
// counterparts are not constructed. Therefore, we need to manually add
35-
// the `this` parameter based on the type of the receiver
36-
var originalThisParamSymbol = originalMethod.Symbol.Parameters.First();
37-
var originalThisParam = Parameter.Create(Context, originalThisParamSymbol, originalMethod);
38-
ConstructedExtensionParameter.Create(Context, this, originalThisParam);
39-
originalParameters = originalParameters.Skip(1);
40-
}
41-
}
42-
4323
foreach (var p in parameters.Zip(originalParameters, (paramSymbol, originalParam) => new { paramSymbol, originalParam }))
4424
{
4525
var original = SymbolEqualityComparer.Default.Equals(p.paramSymbol, p.originalParam)
@@ -208,9 +188,7 @@ protected static void AddParametersToId(Context cx, EscapingTextWriter trapFile,
208188
trapFile.Write('(');
209189
var index = 0;
210190

211-
var @params = method.MethodKind == MethodKind.ReducedExtension
212-
? method.ReducedFrom!.Parameters
213-
: method.Parameters;
191+
var @params = method.Parameters;
214192

215193
foreach (var param in @params)
216194
{
@@ -272,6 +250,11 @@ public static void AddExplicitInterfaceQualifierToId(Context cx, EscapingTextWri
272250
case MethodKind.Constructor:
273251
return Constructor.Create(cx, methodDecl);
274252
case MethodKind.ReducedExtension:
253+
if (SymbolEqualityComparer.Default.Equals(methodDecl, methodDecl.ConstructedFrom))
254+
{
255+
return OrdinaryMethod.Create(cx, methodDecl.ReducedFrom!);
256+
}
257+
return OrdinaryMethod.Create(cx, methodDecl.ReducedFrom!.Construct(methodDecl.TypeArguments, methodDecl.TypeArgumentNullableAnnotations));
275258
case MethodKind.Ordinary:
276259
case MethodKind.DelegateInvoke:
277260
return OrdinaryMethod.Create(cx, methodDecl);
@@ -297,10 +280,7 @@ public static void AddExplicitInterfaceQualifierToId(Context cx, EscapingTextWri
297280
}
298281
}
299282

300-
public Method OriginalDefinition =>
301-
IsReducedExtension
302-
? Create(Context, Symbol.ReducedFrom!)
303-
: Create(Context, Symbol.OriginalDefinition);
283+
public Method OriginalDefinition => Create(Context, Symbol.OriginalDefinition);
304284

305285
public override Location? FullLocation => ReportingLocation;
306286

@@ -318,9 +298,7 @@ public static void AddExplicitInterfaceQualifierToId(Context cx, EscapingTextWri
318298

319299
public bool IsBoundGeneric => IsGeneric && !IsUnboundGeneric;
320300

321-
private bool IsReducedExtension => Symbol.MethodKind == MethodKind.ReducedExtension;
322-
323-
protected IMethodSymbol ConstructedFromSymbol => Symbol.ConstructedFrom.ReducedFrom ?? Symbol.ConstructedFrom;
301+
protected IMethodSymbol ConstructedFromSymbol => Symbol.ConstructedFrom;
324302

325303
bool IExpressionParentEntity.IsTopLevelParent => true;
326304

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,7 @@ private OrdinaryMethod(Context cx, IMethodSymbol init)
1515

1616
protected override IMethodSymbol BodyDeclaringSymbol => Symbol.PartialImplementationPart ?? Symbol;
1717

18-
public IMethodSymbol SourceDeclaration
19-
{
20-
get
21-
{
22-
var reducedFrom = Symbol.ReducedFrom ?? Symbol;
23-
return reducedFrom.OriginalDefinition;
24-
}
25-
}
18+
public IMethodSymbol SourceDeclaration => Symbol.OriginalDefinition;
2619

2720
public override Microsoft.CodeAnalysis.Location ReportingLocation => Symbol.GetSymbolLocation();
2821

@@ -53,7 +46,15 @@ public override void Populate(TextWriter trapFile)
5346
ExtractCompilerGenerated(trapFile);
5447
}
5548

56-
public static new OrdinaryMethod Create(Context cx, IMethodSymbol method) => OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method);
49+
public static new OrdinaryMethod Create(Context cx, IMethodSymbol method)
50+
{
51+
if (method.MethodKind == MethodKind.ReducedExtension)
52+
{
53+
cx.Extractor.Logger.Log(Util.Logging.Severity.Warning, "Reduced extension method symbols should not be directly extracted.");
54+
}
55+
56+
return OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method);
57+
}
5758

5859
private class OrdinaryMethodFactory : CachedEntityFactory<IMethodSymbol, OrdinaryMethod>
5960
{

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs

Lines changed: 1 addition & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,7 @@ public enum Kind
2727
None, Ref, Out, Params, This, In
2828
}
2929

30-
protected virtual int Ordinal
31-
{
32-
get
33-
{
34-
// For some reason, methods of kind ReducedExtension
35-
// omit the "this" parameter, so the parameters are
36-
// actually numbered from 1.
37-
// This is to be consistent from the original (unreduced) extension method.
38-
var isReducedExtension =
39-
Symbol.ContainingSymbol is IMethodSymbol method &&
40-
method.MethodKind == MethodKind.ReducedExtension;
41-
return Symbol.Ordinal + (isReducedExtension ? 1 : 0);
42-
}
43-
}
30+
protected virtual int Ordinal => Symbol.Ordinal;
4431

4532
private Kind ParamKind
4633
{
@@ -270,33 +257,4 @@ private class VarargsParamFactory : CachedEntityFactory<Method, VarargsParam>
270257
public override VarargsParam Create(Context cx, Method init) => new VarargsParam(cx, init);
271258
}
272259
}
273-
274-
internal class ConstructedExtensionParameter : Parameter
275-
{
276-
private readonly ITypeSymbol constructedType;
277-
278-
private ConstructedExtensionParameter(Context cx, Method method, Parameter original)
279-
: base(cx, original.Symbol, method, original)
280-
{
281-
constructedType = method.Symbol.ReceiverType!;
282-
}
283-
284-
public override void Populate(TextWriter trapFile)
285-
{
286-
var typeKey = Type.Create(Context, constructedType);
287-
trapFile.@params(this, Original.Symbol.Name, typeKey.TypeRef, 0, Kind.This, Parent!, Original);
288-
trapFile.param_location(this, Original.Location);
289-
}
290-
291-
public static ConstructedExtensionParameter Create(Context cx, Method method, Parameter parameter) =>
292-
ExtensionParamFactory.Instance.CreateEntity(cx, (new SymbolEqualityWrapper(parameter.Symbol), new SymbolEqualityWrapper(method.Symbol.ReceiverType!)), (method, parameter));
293-
294-
private class ExtensionParamFactory : CachedEntityFactory<(Method, Parameter), ConstructedExtensionParameter>
295-
{
296-
public static ExtensionParamFactory Instance { get; } = new ExtensionParamFactory();
297-
298-
public override ConstructedExtensionParameter Create(Context cx, (Method, Parameter) init) =>
299-
new ConstructedExtensionParameter(cx, init.Item1, init.Item2);
300-
}
301-
}
302260
}

‎csharp/ql/test/library-tests/extension-method-call/ExtensionMethodCalls.expected

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
methodCallTargets
2+
| methods.cs:14:60:14:73 | call to method Ext3 | methods.cs:14:28:14:34 | Ext3 | Ext3<T>(T, int) |
3+
| methods.cs:16:60:16:74 | call to method Ext4 | methods.cs:16:28:16:34 | Ext4 | Ext4<T>(T, int) |
4+
| methods.cs:23:13:23:22 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
5+
| methods.cs:24:13:24:27 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
6+
| methods.cs:25:13:25:30 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<double>(string, double) |
7+
| methods.cs:26:13:26:33 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<object>(string, object) |
8+
| methods.cs:27:13:27:34 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
9+
| methods.cs:28:13:28:39 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
10+
| methods.cs:29:13:29:42 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<double>(string, double) |
11+
| methods.cs:30:13:30:45 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<object>(string, object) |
12+
| methods.cs:32:13:32:22 | call to method Ext1 | methods.cs:10:28:10:31 | Ext1 | Ext1(string, int) |
13+
| methods.cs:33:13:33:34 | call to method Ext1 | methods.cs:10:28:10:31 | Ext1 | Ext1(string, int) |
14+
| methods.cs:35:13:35:21 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
15+
| methods.cs:36:13:36:26 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
16+
| methods.cs:37:13:37:22 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
17+
| methods.cs:38:13:38:30 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
18+
| methods.cs:39:13:39:30 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<object>(object, int) |
19+
| methods.cs:40:13:40:33 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
20+
| methods.cs:41:13:41:38 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
21+
| methods.cs:42:13:42:34 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
22+
| methods.cs:43:13:43:42 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
23+
| methods.cs:44:13:44:42 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<object>(object, int) |
24+
genericMethodCallTargets
25+
| methods.cs:23:13:23:22 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
26+
| methods.cs:24:13:24:27 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
27+
| methods.cs:25:13:25:30 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<double>(string, double) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
28+
| methods.cs:26:13:26:33 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<object>(string, object) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
29+
| methods.cs:27:13:27:34 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
30+
| methods.cs:28:13:28:39 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
31+
| methods.cs:29:13:29:42 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<double>(string, double) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
32+
| methods.cs:30:13:30:45 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<object>(string, object) | methods.cs:8:28:8:34 | Ext0 | Ext0<T>(string, T) |
33+
| methods.cs:35:13:35:21 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
34+
| methods.cs:36:13:36:26 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
35+
| methods.cs:37:13:37:22 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
36+
| methods.cs:38:13:38:30 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
37+
| methods.cs:39:13:39:30 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<object>(object, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
38+
| methods.cs:40:13:40:33 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
39+
| methods.cs:41:13:41:38 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
40+
| methods.cs:42:13:42:34 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
41+
| methods.cs:43:13:43:42 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |
42+
| methods.cs:44:13:44:42 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<object>(object, int) | methods.cs:12:28:12:34 | Ext2 | Ext2<T>(T, int) |

‎csharp/ql/test/library-tests/extension-method-call/ExtensionMethodCalls.ql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import csharp
2+
3+
query predicate methodCallTargets(MethodCall mc, Method m, string sig) {
4+
m = mc.getTarget() and sig = m.toStringWithTypes()
5+
}
6+
7+
query predicate genericMethodCallTargets(
8+
MethodCall mc, ConstructedMethod cm, string sig1, UnboundGenericMethod ugm, string sig2
9+
) {
10+
cm = mc.getTarget() and
11+
sig1 = cm.toStringWithTypes() and
12+
ugm = cm.getUnboundGeneric() and
13+
sig2 = ugm.toStringWithTypes()
14+
}

‎csharp/ql/test/library-tests/extension-method-call/methods.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
using System;
2+
3+
namespace Test
4+
{
5+
6+
public static class Extensions
7+
{
8+
public static void Ext0<T>(this string self, T arg) { }
9+
10+
public static void Ext1(this string self, int arg) { }
11+
12+
public static void Ext2<T>(this T self, int arg) { }
13+
14+
public static void Ext3<T>(this T self, int arg) { self.Ext3(arg); }
15+
16+
public static void Ext4<T>(this T self, int arg) { Ext4(self, arg); }
17+
}
18+
19+
public class Program
20+
{
21+
public static void M()
22+
{
23+
"".Ext0(1);
24+
"".Ext0<int>(1);
25+
"".Ext0<double>(1);
26+
"".Ext0<object>(null);
27+
Extensions.Ext0("", 1);
28+
Extensions.Ext0<int>("", 1);
29+
Extensions.Ext0<double>("", 1);
30+
Extensions.Ext0<object>("", null);
31+
32+
"".Ext1(1);
33+
Extensions.Ext1("", 1);
34+
35+
1.Ext2(1);
36+
1.Ext2<int>(1);
37+
"".Ext2(1);
38+
"".Ext2<string>(1);
39+
"".Ext2<object>(1);
40+
Extensions.Ext2(1, 1);
41+
Extensions.Ext2<int>(1, 1);
42+
Extensions.Ext2("", 1);
43+
Extensions.Ext2<string>("", 1);
44+
Extensions.Ext2<object>("", 1);
45+
}
46+
}
47+
}

‎csharp/ql/test/library-tests/methods/Parameters8.expected

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@
1616
| methods.cs:73:21:73:24 | F | methods.cs:73:28:73:28 | x |
1717
| methods.cs:78:21:78:21 | F | methods.cs:78:30:78:30 | x |
1818
| methods.cs:78:21:78:21 | F | methods.cs:78:40:78:40 | y |
19-
| methods.cs:100:27:100:33 | ToInt32 | methods.cs:100:35:100:47 | s |
2019
| methods.cs:100:27:100:33 | ToInt32 | methods.cs:100:47:100:47 | s |
2120
| methods.cs:105:28:105:33 | ToBool | methods.cs:105:47:105:47 | s |
2221
| methods.cs:105:28:105:33 | ToBool | methods.cs:105:69:105:69 | f |
23-
| methods.cs:110:27:110:34 | Slice | methods.cs:110:36:110:50 | source |
22+
| methods.cs:110:27:110:34 | Slice | methods.cs:110:45:110:50 | source |
2423
| methods.cs:110:27:110:34 | Slice | methods.cs:110:45:110:50 | source |
2524
| methods.cs:110:27:110:34 | Slice | methods.cs:110:57:110:61 | index |
2625
| methods.cs:110:27:110:34 | Slice | methods.cs:110:57:110:61 | index |
@@ -35,7 +34,7 @@
3534
| methods.cs:146:14:146:20 | Method2 | methods.cs:146:65:146:65 | e |
3635
| methods.cs:169:27:169:30 | Plus | methods.cs:169:41:169:44 | left |
3736
| methods.cs:169:27:169:30 | Plus | methods.cs:169:51:169:55 | right |
38-
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:76:174:126 | list |
37+
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:123:174:126 | list |
3938
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:123:174:126 | list |
4039
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:133:174:133 | i |
4140
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:133:174:133 | i |

0 commit comments

Comments
 (0)