Merge pull request #5980 from tamasvajk/fix/extension-method-as-target

C#: Extract correct method symbol as target of extension method calls
This commit is contained in:
Tamás Vajk
2021-06-07 15:57:24 +02:00
committed by GitHub
7 changed files with 124 additions and 85 deletions

View File

@@ -20,26 +20,6 @@ namespace Semmle.Extraction.CSharp.Entities
IEnumerable<IParameterSymbol> parameters = Symbol.Parameters;
IEnumerable<IParameterSymbol> originalParameters = originalMethod.Symbol.Parameters;
if (IsReducedExtension)
{
if (this == originalMethod)
{
// Non-generic reduced extensions must be extracted exactly like the
// non-reduced counterparts
parameters = Symbol.ReducedFrom!.Parameters;
}
else
{
// Constructed reduced extensions are special because their non-reduced
// counterparts are not constructed. Therefore, we need to manually add
// the `this` parameter based on the type of the receiver
var originalThisParamSymbol = originalMethod.Symbol.Parameters.First();
var originalThisParam = Parameter.Create(Context, originalThisParamSymbol, originalMethod);
ConstructedExtensionParameter.Create(Context, this, originalThisParam);
originalParameters = originalParameters.Skip(1);
}
}
foreach (var p in parameters.Zip(originalParameters, (paramSymbol, originalParam) => new { paramSymbol, originalParam }))
{
var original = SymbolEqualityComparer.Default.Equals(p.paramSymbol, p.originalParam)
@@ -208,9 +188,7 @@ namespace Semmle.Extraction.CSharp.Entities
trapFile.Write('(');
var index = 0;
var @params = method.MethodKind == MethodKind.ReducedExtension
? method.ReducedFrom!.Parameters
: method.Parameters;
var @params = method.Parameters;
foreach (var param in @params)
{
@@ -272,6 +250,11 @@ namespace Semmle.Extraction.CSharp.Entities
case MethodKind.Constructor:
return Constructor.Create(cx, methodDecl);
case MethodKind.ReducedExtension:
if (SymbolEqualityComparer.Default.Equals(methodDecl, methodDecl.ConstructedFrom))
{
return OrdinaryMethod.Create(cx, methodDecl.ReducedFrom!);
}
return OrdinaryMethod.Create(cx, methodDecl.ReducedFrom!.Construct(methodDecl.TypeArguments, methodDecl.TypeArgumentNullableAnnotations));
case MethodKind.Ordinary:
case MethodKind.DelegateInvoke:
return OrdinaryMethod.Create(cx, methodDecl);
@@ -297,10 +280,7 @@ namespace Semmle.Extraction.CSharp.Entities
}
}
public Method OriginalDefinition =>
IsReducedExtension
? Create(Context, Symbol.ReducedFrom!)
: Create(Context, Symbol.OriginalDefinition);
public Method OriginalDefinition => Create(Context, Symbol.OriginalDefinition);
public override Location? FullLocation => ReportingLocation;
@@ -318,9 +298,7 @@ namespace Semmle.Extraction.CSharp.Entities
public bool IsBoundGeneric => IsGeneric && !IsUnboundGeneric;
private bool IsReducedExtension => Symbol.MethodKind == MethodKind.ReducedExtension;
protected IMethodSymbol ConstructedFromSymbol => Symbol.ConstructedFrom.ReducedFrom ?? Symbol.ConstructedFrom;
protected IMethodSymbol ConstructedFromSymbol => Symbol.ConstructedFrom;
bool IExpressionParentEntity.IsTopLevelParent => true;

View File

@@ -15,14 +15,7 @@ namespace Semmle.Extraction.CSharp.Entities
protected override IMethodSymbol BodyDeclaringSymbol => Symbol.PartialImplementationPart ?? Symbol;
public IMethodSymbol SourceDeclaration
{
get
{
var reducedFrom = Symbol.ReducedFrom ?? Symbol;
return reducedFrom.OriginalDefinition;
}
}
public IMethodSymbol SourceDeclaration => Symbol.OriginalDefinition;
public override Microsoft.CodeAnalysis.Location ReportingLocation => Symbol.GetSymbolLocation();
@@ -53,7 +46,15 @@ namespace Semmle.Extraction.CSharp.Entities
ExtractCompilerGenerated(trapFile);
}
public static new OrdinaryMethod Create(Context cx, IMethodSymbol method) => OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method);
public static new OrdinaryMethod Create(Context cx, IMethodSymbol method)
{
if (method.MethodKind == MethodKind.ReducedExtension)
{
cx.Extractor.Logger.Log(Util.Logging.Severity.Warning, "Reduced extension method symbols should not be directly extracted.");
}
return OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method);
}
private class OrdinaryMethodFactory : CachedEntityFactory<IMethodSymbol, OrdinaryMethod>
{

View File

@@ -27,20 +27,7 @@ namespace Semmle.Extraction.CSharp.Entities
None, Ref, Out, Params, This, In
}
protected virtual int Ordinal
{
get
{
// For some reason, methods of kind ReducedExtension
// omit the "this" parameter, so the parameters are
// actually numbered from 1.
// This is to be consistent from the original (unreduced) extension method.
var isReducedExtension =
Symbol.ContainingSymbol is IMethodSymbol method &&
method.MethodKind == MethodKind.ReducedExtension;
return Symbol.Ordinal + (isReducedExtension ? 1 : 0);
}
}
protected virtual int Ordinal => Symbol.Ordinal;
private Kind ParamKind
{
@@ -270,33 +257,4 @@ namespace Semmle.Extraction.CSharp.Entities
public override VarargsParam Create(Context cx, Method init) => new VarargsParam(cx, init);
}
}
internal class ConstructedExtensionParameter : Parameter
{
private readonly ITypeSymbol constructedType;
private ConstructedExtensionParameter(Context cx, Method method, Parameter original)
: base(cx, original.Symbol, method, original)
{
constructedType = method.Symbol.ReceiverType!;
}
public override void Populate(TextWriter trapFile)
{
var typeKey = Type.Create(Context, constructedType);
trapFile.@params(this, Original.Symbol.Name, typeKey.TypeRef, 0, Kind.This, Parent!, Original);
trapFile.param_location(this, Original.Location);
}
public static ConstructedExtensionParameter Create(Context cx, Method method, Parameter parameter) =>
ExtensionParamFactory.Instance.CreateEntity(cx, (new SymbolEqualityWrapper(parameter.Symbol), new SymbolEqualityWrapper(method.Symbol.ReceiverType!)), (method, parameter));
private class ExtensionParamFactory : CachedEntityFactory<(Method, Parameter), ConstructedExtensionParameter>
{
public static ExtensionParamFactory Instance { get; } = new ExtensionParamFactory();
public override ConstructedExtensionParameter Create(Context cx, (Method, Parameter) init) =>
new ConstructedExtensionParameter(cx, init.Item1, init.Item2);
}
}
}

View File

@@ -0,0 +1,42 @@
methodCallTargets
| methods.cs:14:60:14:73 | call to method Ext3 | methods.cs:14:28:14:34 | Ext3 | Ext3<T>(T, int) |
| methods.cs:16:60:16:74 | call to method Ext4 | methods.cs:16:28:16:34 | Ext4 | Ext4<T>(T, int) |
| methods.cs:23:13:23:22 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
| methods.cs:24:13:24:27 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
| methods.cs:25:13:25:30 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<double>(string, double) |
| methods.cs:26:13:26:33 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<object>(string, object) |
| methods.cs:27:13:27:34 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
| methods.cs:28:13:28:39 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<int>(string, int) |
| methods.cs:29:13:29:42 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<double>(string, double) |
| methods.cs:30:13:30:45 | call to method Ext0 | methods.cs:8:28:8:34 | Ext0 | Ext0<object>(string, object) |
| methods.cs:32:13:32:22 | call to method Ext1 | methods.cs:10:28:10:31 | Ext1 | Ext1(string, int) |
| methods.cs:33:13:33:34 | call to method Ext1 | methods.cs:10:28:10:31 | Ext1 | Ext1(string, int) |
| methods.cs:35:13:35:21 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
| methods.cs:36:13:36:26 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
| methods.cs:37:13:37:22 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
| methods.cs:38:13:38:30 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
| methods.cs:39:13:39:30 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<object>(object, int) |
| methods.cs:40:13:40:33 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
| methods.cs:41:13:41:38 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<int>(int, int) |
| methods.cs:42:13:42:34 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
| methods.cs:43:13:43:42 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<string>(string, int) |
| methods.cs:44:13:44:42 | call to method Ext2 | methods.cs:12:28:12:34 | Ext2 | Ext2<object>(object, int) |
genericMethodCallTargets
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |
| 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) |

View File

@@ -0,0 +1,14 @@
import csharp
query predicate methodCallTargets(MethodCall mc, Method m, string sig) {
m = mc.getTarget() and sig = m.toStringWithTypes()
}
query predicate genericMethodCallTargets(
MethodCall mc, ConstructedMethod cm, string sig1, UnboundGenericMethod ugm, string sig2
) {
cm = mc.getTarget() and
sig1 = cm.toStringWithTypes() and
ugm = cm.getUnboundGeneric() and
sig2 = ugm.toStringWithTypes()
}

View File

@@ -0,0 +1,47 @@
using System;
namespace Test
{
public static class Extensions
{
public static void Ext0<T>(this string self, T arg) { }
public static void Ext1(this string self, int arg) { }
public static void Ext2<T>(this T self, int arg) { }
public static void Ext3<T>(this T self, int arg) { self.Ext3(arg); }
public static void Ext4<T>(this T self, int arg) { Ext4(self, arg); }
}
public class Program
{
public static void M()
{
"".Ext0(1);
"".Ext0<int>(1);
"".Ext0<double>(1);
"".Ext0<object>(null);
Extensions.Ext0("", 1);
Extensions.Ext0<int>("", 1);
Extensions.Ext0<double>("", 1);
Extensions.Ext0<object>("", null);
"".Ext1(1);
Extensions.Ext1("", 1);
1.Ext2(1);
1.Ext2<int>(1);
"".Ext2(1);
"".Ext2<string>(1);
"".Ext2<object>(1);
Extensions.Ext2(1, 1);
Extensions.Ext2<int>(1, 1);
Extensions.Ext2("", 1);
Extensions.Ext2<string>("", 1);
Extensions.Ext2<object>("", 1);
}
}
}

View File

@@ -16,11 +16,10 @@
| methods.cs:73:21:73:24 | F | methods.cs:73:28:73:28 | x |
| methods.cs:78:21:78:21 | F | methods.cs:78:30:78:30 | x |
| methods.cs:78:21:78:21 | F | methods.cs:78:40:78:40 | y |
| methods.cs:100:27:100:33 | ToInt32 | methods.cs:100:35:100:47 | s |
| methods.cs:100:27:100:33 | ToInt32 | methods.cs:100:47:100:47 | s |
| methods.cs:105:28:105:33 | ToBool | methods.cs:105:47:105:47 | s |
| methods.cs:105:28:105:33 | ToBool | methods.cs:105:69:105:69 | f |
| methods.cs:110:27:110:34 | Slice | methods.cs:110:36:110:50 | source |
| methods.cs:110:27:110:34 | Slice | methods.cs:110:45:110:50 | source |
| methods.cs:110:27:110:34 | Slice | methods.cs:110:45:110:50 | source |
| methods.cs:110:27:110:34 | Slice | methods.cs:110:57:110:61 | index |
| methods.cs:110:27:110:34 | Slice | methods.cs:110:57:110:61 | index |
@@ -35,7 +34,7 @@
| methods.cs:146:14:146:20 | Method2 | methods.cs:146:65:146:65 | e |
| methods.cs:169:27:169:30 | Plus | methods.cs:169:41:169:44 | left |
| methods.cs:169:27:169:30 | Plus | methods.cs:169:51:169:55 | right |
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:76:174:126 | list |
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:123:174:126 | list |
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:123:174:126 | list |
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:133:174:133 | i |
| methods.cs:174:65:174:74 | SkipTwo | methods.cs:174:133:174:133 | i |