diff --git a/csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs index 0c0c17df125..c108a18f136 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs @@ -647,15 +647,13 @@ namespace Semmle.Extraction.CSharp /// Return true if this method is a compiler-generated extension method. /// public static bool IsCompilerGeneratedExtensionMethod(this IMethodSymbol method) => - method.TryGetExtensionMethod(out _); + method.TryGetExtensionMethod() is not null; /// - /// Returns true if this method is a compiler-generated extension method, - /// and outputs the original extension method declaration. + /// Returns the extension method corresponding to this compiler-generated extension method, if it exists. /// - public static bool TryGetExtensionMethod(this IMethodSymbol method, out IMethodSymbol? declaration) + public static IMethodSymbol? TryGetExtensionMethod(this IMethodSymbol method) { - declaration = null; if (method.IsImplicitlyDeclared && method.ContainingSymbol is INamedTypeSymbol containingType) { // Extension types are declared within the same type as the generated @@ -688,23 +686,22 @@ namespace Semmle.Extraction.CSharp .First(c => SymbolEqualityComparer.Default.Equals(c.OriginalDefinition, unboundDeclaration)); // If the extension declaration is unbound apply the remaning type arguments and construct it. - declaration = extensionDeclaration.IsUnboundGenericMethod() + return extensionDeclaration.IsUnboundGenericMethod() ? extensionDeclaration.Construct(extensionMethodArguments.ToArray()) : extensionDeclaration; } catch { // If anything goes wrong, fall back to the unbound declaration. - declaration = unboundDeclaration; + return unboundDeclaration; } } else { - declaration = unboundDeclaration; + return unboundDeclaration; } - } - return declaration is not null; + return null; } /// @@ -820,5 +817,35 @@ namespace Semmle.Extraction.CSharp /// public static IEnumerable ExtractionCandidates(this IEnumerable symbols) where T : ISymbol => symbols.Where(symbol => symbol.ShouldExtractSymbol()); + + /// + /// Returns the parameter kind for this parameter symbol, e.g. `ref`, `out`, `params`, etc. + /// + public static Parameter.Kind GetParameterKind(this IParameterSymbol parameter) + { + switch (parameter.RefKind) + { + case RefKind.Out: + return Parameter.Kind.Out; + case RefKind.Ref: + return Parameter.Kind.Ref; + case RefKind.In: + return Parameter.Kind.In; + case RefKind.RefReadOnlyParameter: + return Parameter.Kind.RefReadOnly; + default: + if (parameter.IsParams) + return Parameter.Kind.Params; + + if (parameter.Ordinal == 0) + { + if (parameter.ContainingSymbol is IMethodSymbol method && method.IsExtensionMethod) + { + return Parameter.Kind.This; + } + } + return Parameter.Kind.None; + } + } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs index 26d64339ef0..2ed7aec9955 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs @@ -126,8 +126,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions private static bool IsOperatorLikeCall(ExpressionNodeInfo info) { return info.SymbolInfo.Symbol is IMethodSymbol method && - method.TryGetExtensionMethod(out var original) && - original!.MethodKind == MethodKind.UserDefinedOperator; + method.TryGetExtensionMethod()?.MethodKind == MethodKind.UserDefinedOperator; } public IMethodSymbol? TargetSymbol @@ -140,13 +139,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions { var method = symbol as IMethodSymbol; // Case for compiler-generated extension methods. - if (method is not null && - method.TryGetExtensionMethod(out var original)) - { - return original; - } - - return method; + return method?.TryGetExtensionMethod() ?? method; } if (si.CandidateReason == CandidateReason.OverloadResolutionFailure) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs index 9cca0683f00..dbb410382f9 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs @@ -41,36 +41,6 @@ namespace Semmle.Extraction.CSharp.Entities protected virtual int Ordinal => Symbol.Ordinal + PositionOffset; - private Kind ParamKind - { - get - { - switch (Symbol.RefKind) - { - case RefKind.Out: - return Kind.Out; - case RefKind.Ref: - return Kind.Ref; - case RefKind.In: - return Kind.In; - case RefKind.RefReadOnlyParameter: - return Kind.RefReadOnly; - default: - if (Symbol.IsParams) - return Kind.Params; - - if (Ordinal == 0) - { - if (Symbol.ContainingSymbol is IMethodSymbol method && method.IsExtensionMethod) - { - return Kind.This; - } - } - return Kind.None; - } - } - } - public static Parameter Create(Context cx, IParameterSymbol param, IEntity parent, Parameter? original = null, int positionOffset = 0) { var cachedSymbol = cx.GetPossiblyCachedParameterSymbol(param); @@ -125,7 +95,8 @@ namespace Semmle.Extraction.CSharp.Entities Context.ModelError(Symbol, "Inconsistent parameter declaration"); var type = Type.Create(Context, Symbol.Type); - trapFile.@params(this, Name, type.TypeRef, Ordinal, ParamKind, Parent!, Original); + var kind = Symbol.GetParameterKind(); + trapFile.@params(this, Name, type.TypeRef, Ordinal, kind, Parent!, Original); if (Context.OnlyScaffold) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/SyntheticExtensionParameter.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/SyntheticExtensionParameter.cs index d417af6eedc..81d58507a10 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/SyntheticExtensionParameter.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/SyntheticExtensionParameter.cs @@ -7,7 +7,7 @@ namespace Semmle.Extraction.CSharp.Entities { /// /// Synthetic parameter for extension methods declared using the extension syntax. - /// That is, we add a synthetic parameter s to IsValid in the following example: + /// That is, we add a synthetic parameter `s` to `IsValid` in the following example: /// extension(string s) { /// public bool IsValid() { ... } /// } @@ -30,24 +30,6 @@ namespace Semmle.Extraction.CSharp.Entities private static int Ordinal => 0; - private Parameter.Kind ParamKind - { - get - { - switch (ExtensionParameter.RefKind) - { - case RefKind.Ref: - return Parameter.Kind.Ref; - case RefKind.In: - return Parameter.Kind.In; - case RefKind.RefReadOnlyParameter: - return Parameter.Kind.RefReadOnly; - default: - return Parameter.Kind.None; - } - } - } - private string Name => ExtensionParameter.Name; private bool IsSourceDeclaration => ExtensionMethod.Symbol.IsSourceDeclaration(); @@ -58,7 +40,8 @@ namespace Semmle.Extraction.CSharp.Entities PopulateRefKind(trapFile, ExtensionParameter.RefKind); var type = Type.Create(Context, ExtensionParameter.Type); - trapFile.@params(this, Name, type.TypeRef, Ordinal, ParamKind, ExtensionMethod, Original); + var kind = ExtensionParameter.GetParameterKind(); + trapFile.@params(this, Name, type.TypeRef, Ordinal, kind, ExtensionMethod, Original); if (Context.OnlyScaffold) { diff --git a/csharp/ql/lib/semmle/code/csharp/Callable.qll b/csharp/ql/lib/semmle/code/csharp/Callable.qll index 1bdfb008144..f8346cfe01e 100644 --- a/csharp/ql/lib/semmle/code/csharp/Callable.qll +++ b/csharp/ql/lib/semmle/code/csharp/Callable.qll @@ -10,6 +10,7 @@ import exprs.Call private import commons.QualifiedName private import commons.Collections private import semmle.code.csharp.ExprOrStmtParent +private import semmle.code.csharp.internal.Callable private import semmle.code.csharp.metrics.Complexity private import TypeRef @@ -221,24 +222,9 @@ class Callable extends Parameterizable, ExprOrStmtParent, @callable { /** Gets a `Call` that has this callable as a target. */ Call getACall() { this = result.getTarget() } - - /** Holds if this callable is declared in an extension type. */ - predicate isInExtension() { this.getDeclaringType() instanceof ExtensionType } } -/** - * A callable that is declared as an extension. - * - * Either an extension method (`ExtensionMethod`), an extension operator - * (`ExtensionOperator`) or an extension accessor (`ExtensionAccessor`). - */ -abstract class ExtensionCallable extends Callable { - /** Gets the type being extended by this method. */ - pragma[noinline] - Type getExtendedType() { result = this.getDeclaringType().(ExtensionType).getExtendedType() } - - override string getAPrimaryQlClass() { result = "ExtensionCallable" } -} +final class ExtensionCallable = ExtensionCallableImpl; /** * A method, for example @@ -315,15 +301,7 @@ class Method extends Callable, Virtualizable, Attributable, @method { override string getAPrimaryQlClass() { result = "Method" } } -/** - * An extension method. - * - * Either a classic extension method (`ClassicExtensionMethod`) or an extension - * type extension method (`ExtensionTypeExtensionMethod`). - */ -abstract class ExtensionMethod extends ExtensionCallable, Method { - override string getAPrimaryQlClass() { result = "ExtensionMethod" } -} +final class ExtensionMethod = ExtensionMethodImpl; /** * An extension method, for example @@ -334,7 +312,7 @@ abstract class ExtensionMethod extends ExtensionCallable, Method { * } * ``` */ -class ClassicExtensionMethod extends ExtensionMethod { +class ClassicExtensionMethod extends ExtensionMethodImpl { ClassicExtensionMethod() { this.isClassicExtensionMethod() } pragma[noinline] @@ -354,7 +332,7 @@ class ClassicExtensionMethod extends ExtensionMethod { * } * ``` */ -class ExtensionTypeExtensionMethod extends ExtensionMethod { +class ExtensionTypeExtensionMethod extends ExtensionMethodImpl { ExtensionTypeExtensionMethod() { this.isInExtension() } } @@ -589,7 +567,7 @@ class RecordCloneMethod extends Method { * } * ``` */ -class ExtensionOperator extends ExtensionCallable, Operator { +class ExtensionOperator extends ExtensionCallableImpl, Operator { ExtensionOperator() { this.isInExtension() } } diff --git a/csharp/ql/lib/semmle/code/csharp/Member.qll b/csharp/ql/lib/semmle/code/csharp/Member.qll index 529ce8b8b7e..b64f408af64 100644 --- a/csharp/ql/lib/semmle/code/csharp/Member.qll +++ b/csharp/ql/lib/semmle/code/csharp/Member.qll @@ -102,6 +102,9 @@ class Declaration extends NamedElement, @declaration { * implicit constructors or accessors. */ predicate isCompilerGenerated() { compiler_generated(this) } + + /** Holds if this declaration is in an extension type. */ + predicate isInExtension() { this.getDeclaringType() instanceof ExtensionType } } /** A declaration that can have a modifier. */ diff --git a/csharp/ql/lib/semmle/code/csharp/Property.qll b/csharp/ql/lib/semmle/code/csharp/Property.qll index d3e65def671..88665280d5b 100644 --- a/csharp/ql/lib/semmle/code/csharp/Property.qll +++ b/csharp/ql/lib/semmle/code/csharp/Property.qll @@ -6,6 +6,7 @@ import Member import Stmt import Type private import semmle.code.csharp.ExprOrStmtParent +private import semmle.code.csharp.internal.Callable private import TypeRef /** @@ -272,7 +273,7 @@ class Property extends DeclarationWithGetSetAccessors, @property { * ``` */ class ExtensionProperty extends Property { - ExtensionProperty() { this.getDeclaringType() instanceof ExtensionType } + ExtensionProperty() { this.isInExtension() } } /** @@ -440,8 +441,8 @@ class Accessor extends Callable, Modifiable, Attributable, Overridable, @callabl * } * ``` */ -class ExtensionAccessor extends ExtensionCallable, Accessor { - ExtensionAccessor() { this.getDeclaringType() instanceof ExtensionType } +class ExtensionAccessor extends ExtensionCallableImpl, Accessor { + ExtensionAccessor() { this.isInExtension() } } /** diff --git a/csharp/ql/lib/semmle/code/csharp/exprs/Access.qll b/csharp/ql/lib/semmle/code/csharp/exprs/Access.qll index 71a25ace8d2..84375bc7013 100644 --- a/csharp/ql/lib/semmle/code/csharp/exprs/Access.qll +++ b/csharp/ql/lib/semmle/code/csharp/exprs/Access.qll @@ -236,10 +236,8 @@ class ParameterAccess extends LocalScopeVariableAccess, @parameter_access_expr { * ``` */ class SyntheticExtensionParameterAccess extends ParameterAccess { - private Parameter p; - SyntheticExtensionParameterAccess() { - exists(ExtensionType et | + exists(ExtensionType et, Parameter p | p = et.getReceiverParameter() and expr_access(this, p) ) diff --git a/csharp/ql/lib/semmle/code/csharp/internal/Callable.qll b/csharp/ql/lib/semmle/code/csharp/internal/Callable.qll new file mode 100644 index 00000000000..533bf31c074 --- /dev/null +++ b/csharp/ql/lib/semmle/code/csharp/internal/Callable.qll @@ -0,0 +1,33 @@ +/** + * INTERNAL: Do not use. + * + * Provides `Callable` classes, which are things that can be called + * such as methods and operators. + */ + +private import semmle.code.csharp.Callable +private import semmle.code.csharp.Property + +/** + * A callable that is declared as an extension. + * + * Either an extension method (`ExtensionMethod`), an extension operator + * (`ExtensionOperator`) or an extension accessor (`ExtensionAccessor`). + */ +abstract class ExtensionCallableImpl extends Callable { + /** Gets the type being extended by this method. */ + pragma[noinline] + Type getExtendedType() { result = this.getDeclaringType().(ExtensionType).getExtendedType() } + + override string getAPrimaryQlClass() { result = "ExtensionCallable" } +} + +/** + * An extension method. + * + * Either a classic extension method (`ClassicExtensionMethod`) or an extension + * type extension method (`ExtensionTypeExtensionMethod`). + */ +abstract class ExtensionMethodImpl extends ExtensionCallableImpl, Method { + override string getAPrimaryQlClass() { result = "ExtensionMethod" } +} diff --git a/csharp/ql/lib/semmlecode.csharp.dbscheme b/csharp/ql/lib/semmlecode.csharp.dbscheme index a39a96d1f33..178a7e6cf33 100644 --- a/csharp/ql/lib/semmlecode.csharp.dbscheme +++ b/csharp/ql/lib/semmlecode.csharp.dbscheme @@ -222,7 +222,7 @@ overlayChangedFiles( | @using_directive | @type_parameter_constraints | @externalDataElement | @xmllocatable | @asp_element | @namespace | @preprocessor_directive; -@declaration = @callable | @generic | @assignable | @namespace | @extension_type; +@declaration = @callable | @generic | @assignable | @namespace; @named_element = @namespace | @declaration;