Skip to content

Commit fe94b3b

Browse files
michaelnebelmichaelnebel
committed
C#: Address review comments.
1 parent 02e4a8b commit fe94b3b

File tree

10 files changed

+92
-105
lines changed

10 files changed

+92
-105
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -647,15 +647,13 @@ public static bool IsSourceDeclaration(this IParameterSymbol parameter)
647647
/// Return true if this method is a compiler-generated extension method.
648648
/// </summary>
649649
public static bool IsCompilerGeneratedExtensionMethod(this IMethodSymbol method) =>
650-
method.TryGetExtensionMethod(out _);
650+
method.TryGetExtensionMethod() is not null;
651651

652652
/// <summary>
653-
/// Returns true if this method is a compiler-generated extension method,
654-
/// and outputs the original extension method declaration.
653+
/// Returns the extension method corresponding to this compiler-generated extension method, if it exists.
655654
/// </summary>
656-
public static bool TryGetExtensionMethod(this IMethodSymbol method, out IMethodSymbol? declaration)
655+
public static IMethodSymbol? TryGetExtensionMethod(this IMethodSymbol method)
657656
{
658-
declaration = null;
659657
if (method.IsImplicitlyDeclared && method.ContainingSymbol is INamedTypeSymbol containingType)
660658
{
661659
// Extension types are declared within the same type as the generated
@@ -688,23 +686,22 @@ public static bool TryGetExtensionMethod(this IMethodSymbol method, out IMethodS
688686
.First(c => SymbolEqualityComparer.Default.Equals(c.OriginalDefinition, unboundDeclaration));
689687

690688
// If the extension declaration is unbound apply the remaning type arguments and construct it.
691-
declaration = extensionDeclaration.IsUnboundGenericMethod()
689+
return extensionDeclaration.IsUnboundGenericMethod()
692690
? extensionDeclaration.Construct(extensionMethodArguments.ToArray())
693691
: extensionDeclaration;
694692
}
695693
catch
696694
{
697695
// If anything goes wrong, fall back to the unbound declaration.
698-
declaration = unboundDeclaration;
696+
return unboundDeclaration;
699697
}
700698
}
701699
else
702700
{
703-
declaration = unboundDeclaration;
701+
return unboundDeclaration;
704702
}
705-
706703
}
707-
return declaration is not null;
704+
return null;
708705
}
709706

710707
/// <summary>
@@ -820,5 +817,35 @@ public static bool ShouldExtractSymbol(this ISymbol symbol)
820817
/// </summary>
821818
public static IEnumerable<T> ExtractionCandidates<T>(this IEnumerable<T> symbols) where T : ISymbol =>
822819
symbols.Where(symbol => symbol.ShouldExtractSymbol());
820+
821+
/// <summary>
822+
/// Returns the parameter kind for this parameter symbol, e.g. `ref`, `out`, `params`, etc.
823+
/// </summary>
824+
public static Parameter.Kind GetParameterKind(this IParameterSymbol parameter)
825+
{
826+
switch (parameter.RefKind)
827+
{
828+
case RefKind.Out:
829+
return Parameter.Kind.Out;
830+
case RefKind.Ref:
831+
return Parameter.Kind.Ref;
832+
case RefKind.In:
833+
return Parameter.Kind.In;
834+
case RefKind.RefReadOnlyParameter:
835+
return Parameter.Kind.RefReadOnly;
836+
default:
837+
if (parameter.IsParams)
838+
return Parameter.Kind.Params;
839+
840+
if (parameter.Ordinal == 0)
841+
{
842+
if (parameter.ContainingSymbol is IMethodSymbol method && method.IsExtensionMethod)
843+
{
844+
return Parameter.Kind.This;
845+
}
846+
}
847+
return Parameter.Kind.None;
848+
}
849+
}
823850
}
824851
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,7 @@ private static bool IsDynamicCall(ExpressionNodeInfo info)
126126
private static bool IsOperatorLikeCall(ExpressionNodeInfo info)
127127
{
128128
return info.SymbolInfo.Symbol is IMethodSymbol method &&
129-
method.TryGetExtensionMethod(out var original) &&
130-
original!.MethodKind == MethodKind.UserDefinedOperator;
129+
method.TryGetExtensionMethod()?.MethodKind == MethodKind.UserDefinedOperator;
131130
}
132131

133132
public IMethodSymbol? TargetSymbol
@@ -140,13 +139,7 @@ public IMethodSymbol? TargetSymbol
140139
{
141140
var method = symbol as IMethodSymbol;
142141
// Case for compiler-generated extension methods.
143-
if (method is not null &&
144-
method.TryGetExtensionMethod(out var original))
145-
{
146-
return original;
147-
}
148-
149-
return method;
142+
return method?.TryGetExtensionMethod() ?? method;
150143
}
151144

152145
if (si.CandidateReason == CandidateReason.OverloadResolutionFailure)

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

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -41,36 +41,6 @@ public enum Kind
4141

4242
protected virtual int Ordinal => Symbol.Ordinal + PositionOffset;
4343

44-
private Kind ParamKind
45-
{
46-
get
47-
{
48-
switch (Symbol.RefKind)
49-
{
50-
case RefKind.Out:
51-
return Kind.Out;
52-
case RefKind.Ref:
53-
return Kind.Ref;
54-
case RefKind.In:
55-
return Kind.In;
56-
case RefKind.RefReadOnlyParameter:
57-
return Kind.RefReadOnly;
58-
default:
59-
if (Symbol.IsParams)
60-
return Kind.Params;
61-
62-
if (Ordinal == 0)
63-
{
64-
if (Symbol.ContainingSymbol is IMethodSymbol method && method.IsExtensionMethod)
65-
{
66-
return Kind.This;
67-
}
68-
}
69-
return Kind.None;
70-
}
71-
}
72-
}
73-
7444
public static Parameter Create(Context cx, IParameterSymbol param, IEntity parent, Parameter? original = null, int positionOffset = 0)
7545
{
7646
var cachedSymbol = cx.GetPossiblyCachedParameterSymbol(param);
@@ -125,7 +95,8 @@ public override void Populate(TextWriter trapFile)
12595
Context.ModelError(Symbol, "Inconsistent parameter declaration");
12696

12797
var type = Type.Create(Context, Symbol.Type);
128-
trapFile.@params(this, Name, type.TypeRef, Ordinal, ParamKind, Parent!, Original);
98+
var kind = Symbol.GetParameterKind();
99+
trapFile.@params(this, Name, type.TypeRef, Ordinal, kind, Parent!, Original);
129100

130101
if (Context.OnlyScaffold)
131102
{

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

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace Semmle.Extraction.CSharp.Entities
77
{
88
/// <summary>
99
/// Synthetic parameter for extension methods declared using the extension syntax.
10-
/// That is, we add a synthetic parameter s to IsValid in the following example:
10+
/// That is, we add a synthetic parameter `s` to `IsValid` in the following example:
1111
/// extension(string s) {
1212
/// public bool IsValid() { ... }
1313
/// }
@@ -30,24 +30,6 @@ private SyntheticExtensionParameter(Context cx, Method method, IParameterSymbol
3030

3131
private static int Ordinal => 0;
3232

33-
private Parameter.Kind ParamKind
34-
{
35-
get
36-
{
37-
switch (ExtensionParameter.RefKind)
38-
{
39-
case RefKind.Ref:
40-
return Parameter.Kind.Ref;
41-
case RefKind.In:
42-
return Parameter.Kind.In;
43-
case RefKind.RefReadOnlyParameter:
44-
return Parameter.Kind.RefReadOnly;
45-
default:
46-
return Parameter.Kind.None;
47-
}
48-
}
49-
}
50-
5133
private string Name => ExtensionParameter.Name;
5234

5335
private bool IsSourceDeclaration => ExtensionMethod.Symbol.IsSourceDeclaration();
@@ -58,7 +40,8 @@ protected override void Populate(TextWriter trapFile)
5840
PopulateRefKind(trapFile, ExtensionParameter.RefKind);
5941

6042
var type = Type.Create(Context, ExtensionParameter.Type);
61-
trapFile.@params(this, Name, type.TypeRef, Ordinal, ParamKind, ExtensionMethod, Original);
43+
var kind = ExtensionParameter.GetParameterKind();
44+
trapFile.@params(this, Name, type.TypeRef, Ordinal, kind, ExtensionMethod, Original);
6245

6346
if (Context.OnlyScaffold)
6447
{

‎csharp/ql/lib/semmle/code/csharp/Callable.qll‎

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import exprs.Call
1010
private import commons.QualifiedName
1111
private import commons.Collections
1212
private import semmle.code.csharp.ExprOrStmtParent
13+
private import semmle.code.csharp.internal.Callable
1314
private import semmle.code.csharp.metrics.Complexity
1415
private import TypeRef
1516

@@ -221,24 +222,9 @@ class Callable extends Parameterizable, ExprOrStmtParent, @callable {
221222

222223
/** Gets a `Call` that has this callable as a target. */
223224
Call getACall() { this = result.getTarget() }
224-
225-
/** Holds if this callable is declared in an extension type. */
226-
predicate isInExtension() { this.getDeclaringType() instanceof ExtensionType }
227225
}
228226

229-
/**
230-
* A callable that is declared as an extension.
231-
*
232-
* Either an extension method (`ExtensionMethod`), an extension operator
233-
* (`ExtensionOperator`) or an extension accessor (`ExtensionAccessor`).
234-
*/
235-
abstract class ExtensionCallable extends Callable {
236-
/** Gets the type being extended by this method. */
237-
pragma[noinline]
238-
Type getExtendedType() { result = this.getDeclaringType().(ExtensionType).getExtendedType() }
239-
240-
override string getAPrimaryQlClass() { result = "ExtensionCallable" }
241-
}
227+
final class ExtensionCallable = ExtensionCallableImpl;
242228

243229
/**
244230
* A method, for example
@@ -315,15 +301,7 @@ class Method extends Callable, Virtualizable, Attributable, @method {
315301
override string getAPrimaryQlClass() { result = "Method" }
316302
}
317303

318-
/**
319-
* An extension method.
320-
*
321-
* Either a classic extension method (`ClassicExtensionMethod`) or an extension
322-
* type extension method (`ExtensionTypeExtensionMethod`).
323-
*/
324-
abstract class ExtensionMethod extends ExtensionCallable, Method {
325-
override string getAPrimaryQlClass() { result = "ExtensionMethod" }
326-
}
304+
final class ExtensionMethod = ExtensionMethodImpl;
327305

328306
/**
329307
* An extension method, for example
@@ -334,7 +312,7 @@ abstract class ExtensionMethod extends ExtensionCallable, Method {
334312
* }
335313
* ```
336314
*/
337-
class ClassicExtensionMethod extends ExtensionMethod {
315+
class ClassicExtensionMethod extends ExtensionMethodImpl {
338316
ClassicExtensionMethod() { this.isClassicExtensionMethod() }
339317

340318
pragma[noinline]
@@ -354,7 +332,7 @@ class ClassicExtensionMethod extends ExtensionMethod {
354332
* }
355333
* ```
356334
*/
357-
class ExtensionTypeExtensionMethod extends ExtensionMethod {
335+
class ExtensionTypeExtensionMethod extends ExtensionMethodImpl {
358336
ExtensionTypeExtensionMethod() { this.isInExtension() }
359337
}
360338

@@ -589,7 +567,7 @@ class RecordCloneMethod extends Method {
589567
* }
590568
* ```
591569
*/
592-
class ExtensionOperator extends ExtensionCallable, Operator {
570+
class ExtensionOperator extends ExtensionCallableImpl, Operator {
593571
ExtensionOperator() { this.isInExtension() }
594572
}
595573

‎csharp/ql/lib/semmle/code/csharp/Member.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ class Declaration extends NamedElement, @declaration {
102102
* implicit constructors or accessors.
103103
*/
104104
predicate isCompilerGenerated() { compiler_generated(this) }
105+
106+
/** Holds if this declaration is in an extension type. */
107+
predicate isInExtension() { this.getDeclaringType() instanceof ExtensionType }
105108
}
106109

107110
/** A declaration that can have a modifier. */

‎csharp/ql/lib/semmle/code/csharp/Property.qll‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import Member
66
import Stmt
77
import Type
88
private import semmle.code.csharp.ExprOrStmtParent
9+
private import semmle.code.csharp.internal.Callable
910
private import TypeRef
1011

1112
/**
@@ -272,7 +273,7 @@ class Property extends DeclarationWithGetSetAccessors, @property {
272273
* ```
273274
*/
274275
class ExtensionProperty extends Property {
275-
ExtensionProperty() { this.getDeclaringType() instanceof ExtensionType }
276+
ExtensionProperty() { this.isInExtension() }
276277
}
277278

278279
/**
@@ -440,8 +441,8 @@ class Accessor extends Callable, Modifiable, Attributable, Overridable, @callabl
440441
* }
441442
* ```
442443
*/
443-
class ExtensionAccessor extends ExtensionCallable, Accessor {
444-
ExtensionAccessor() { this.getDeclaringType() instanceof ExtensionType }
444+
class ExtensionAccessor extends ExtensionCallableImpl, Accessor {
445+
ExtensionAccessor() { this.isInExtension() }
445446
}
446447

447448
/**

‎csharp/ql/lib/semmle/code/csharp/exprs/Access.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,8 @@ class ParameterAccess extends LocalScopeVariableAccess, @parameter_access_expr {
236236
* ```
237237
*/
238238
class SyntheticExtensionParameterAccess extends ParameterAccess {
239-
private Parameter p;
240-
241239
SyntheticExtensionParameterAccess() {
242-
exists(ExtensionType et |
240+
exists(ExtensionType et, Parameter p |
243241
p = et.getReceiverParameter() and
244242
expr_access(this, p)
245243
)

‎csharp/ql/lib/semmle/code/csharp/internal/Callable.qll‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* INTERNAL: Do not use.
3+
*
4+
* Provides `Callable` classes, which are things that can be called
5+
* such as methods and operators.
6+
*/
7+
8+
private import semmle.code.csharp.Callable
9+
private import semmle.code.csharp.Property
10+
11+
/**
12+
* A callable that is declared as an extension.
13+
*
14+
* Either an extension method (`ExtensionMethod`), an extension operator
15+
* (`ExtensionOperator`) or an extension accessor (`ExtensionAccessor`).
16+
*/
17+
abstract class ExtensionCallableImpl extends Callable {
18+
/** Gets the type being extended by this method. */
19+
pragma[noinline]
20+
Type getExtendedType() { result = this.getDeclaringType().(ExtensionType).getExtendedType() }
21+
22+
override string getAPrimaryQlClass() { result = "ExtensionCallable" }
23+
}
24+
25+
/**
26+
* An extension method.
27+
*
28+
* Either a classic extension method (`ClassicExtensionMethod`) or an extension
29+
* type extension method (`ExtensionTypeExtensionMethod`).
30+
*/
31+
abstract class ExtensionMethodImpl extends ExtensionCallableImpl, Method {
32+
override string getAPrimaryQlClass() { result = "ExtensionMethod" }
33+
}

‎csharp/ql/lib/semmlecode.csharp.dbscheme‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ overlayChangedFiles(
222222
| @using_directive | @type_parameter_constraints | @externalDataElement
223223
| @xmllocatable | @asp_element | @namespace | @preprocessor_directive;
224224

225-
@declaration = @callable | @generic | @assignable | @namespace | @extension_type;
225+
@declaration = @callable | @generic | @assignable | @namespace;
226226

227227
@named_element = @namespace | @declaration;
228228

0 commit comments

Comments
 (0)