Skip to content

Conversation

@michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Jan 26, 2026

In this PR we introduce support for extension types (blocks). The feature is explained in detail here.
The feature generalises extensions to operators and properties.

Here is a small example of an declaration and how it can be invoked.

public static class MyExtensions
{
    extension(string s)
    {
        public bool IsValid() { return s is not null; }
    }
}

public class A
{
    public void M()
    {
        var s = "Hello World";
        s.IsValid(); // Call to extension method.
        MyExtensions.IsValid(s) // Call to compiler generated static method.
    }
}

It turns out that Roslyn generates a static method corresponding to the extension method. To avoid extracting multiple methods with identical bodies (which would further complicate the QL implementation) we

  • Only extract the IsValid() method, which has the qualified name MyExtensions.extension(string).IsValid.
  • Add a parameter s to IsValid() corresponding to the receiver parameter s. This needs to be synthesised as we can't re-use the parameter from the extension declaration.
  • Replace invocations of MyExtensions.IsValid with MyExtensions.extensions(string).IsValid.

DCA looks good.

  • Performance appears to be un-affected.
  • Nice reduction in the number of missing call targets for a range of projects (and an increase in the number of alerts).
  • There are some "new" extraction errors for the dotnet/runtime project. These extraction errors are because some of the extensions (that we now extract) adds user-defined compound assignment operators (described here), which are not supported yet.

@github-actions github-actions bot added the C# label Jan 26, 2026
@michaelnebel michaelnebel changed the title [WIP] C# 14: Support extension blocks. C# 14: Support extension types. Feb 5, 2026
@michaelnebel
Copy link
Contributor Author

michaelnebel commented Feb 5, 2026

@hvitved : The PR is ready for review. However, I haven't added any upgrade/downgrade scripts yet in case the review leads to a change in the DB scheme. Review on commit by commit basis is encouraged.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressive work! Some mostly minor comments

| @xmllocatable | @asp_element | @namespace | @preprocessor_directive;

@declaration = @callable | @generic | @assignable | @namespace;
@declaration = @callable | @generic | @assignable | @namespace | @extension_type;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? I would think that @extension_type is part of @type, which is part of @generic.

{
/// <summary>
/// Synthetic parameter for extension methods declared using the extension syntax.
/// That is, we add a synthetic parameter s to IsValid in the following example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backticks around s and IsValid makes this easier to parse.

Comment on lines 37 to 47
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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we share this logic with the logic in Parameter.cs?

/// Returns true if this method is a compiler-generated extension method,
/// and outputs the original extension method declaration.
/// </summary>
public static bool TryGetExtensionMethod(this IMethodSymbol method, out IMethodSymbol? declaration)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply public static IMethodSymbol? TryGetExtensionMethod(this IMethodSymbol method)?

* Either an extension method (`ExtensionMethod`), an extension operator
* (`ExtensionOperator`) or an extension accessor (`ExtensionAccessor`).
*/
abstract class ExtensionCallable extends Callable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to expose abstract classes, so better to (1) rename it to ExtensionCallableImpl, (2) move it inside internal/Callable.qll, (3) update all existing references to ExtensionCallableImpl, and (4) add here final class ExtensionCallable = ExtensionCallableImpl.

Call getACall() { this = result.getTarget() }

/** Holds if this callable is declared in an extension type. */
predicate isInExtension() { this.getDeclaringType() instanceof ExtensionType }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you move this inside the Declaration class, it can be used also in the charpreds of ExtensionProperty and ExtensionAccessor.

* Either a classic extension method (`ClassicExtensionMethod`) or an extension
* type extension method (`ExtensionTypeExtensionMethod`).
*/
abstract class ExtensionMethod extends ExtensionCallable, Method {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

* ```
*/
class SyntheticExtensionParameterAccess extends ParameterAccess {
private Parameter p;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have this as a field.

- ["My.Qltest", "K", false, "GetMyFieldOnSyntheticField", "()", "", "Argument[this].SyntheticField[My.Qltest.K.MySyntheticField2].Field[My.Qltest.K.MyField]", "ReturnValue", "value", "manual"]
- ["My.Qltest", "Library", false, "SetValue", "(System.Object)", "", "Argument[0]", "Argument[this].SyntheticField[X]", "value", "dfc-generated"]
- ["My.Qltest", "Library", false, "GetValue", "()", "", "Argument[this].SyntheticField[X]", "ReturnValue", "value", "dfc-generated"]
- ["My.Qltest", "TestExtensions+extension(Object)", false, "Method1", "(System.Object)", "", "Argument[0]", "ReturnValue", "value", "manual"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ought to use the FQN of the type being extended, i.e. TestExtensions+extension(System.Object)

Comment on lines +693 to +697
catch
{
// If anything goes wrong, fall back to the unbound declaration.
return unboundDeclaration;
}

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
Comment on lines +840 to +846
if (parameter.Ordinal == 0)
{
if (parameter.ContainingSymbol is IMethodSymbol method && method.IsExtensionMethod)
{
return Parameter.Kind.This;
}
}

Check notice

Code scanning / CodeQL

Nested 'if' statements can be combined Note

These 'if' statements can be combined.
@michaelnebel michaelnebel requested a review from hvitved February 9, 2026 16:25
@michaelnebel michaelnebel marked this pull request as ready for review February 9, 2026 16:27
@michaelnebel michaelnebel requested review from a team as code owners February 9, 2026 16:27
Copilot AI review requested due to automatic review settings February 9, 2026 16:27
@michaelnebel
Copy link
Contributor Author

michaelnebel commented Feb 9, 2026

@hvitved Added some commits which addresses your comments, added DB upgrade/downgrade scripts (not intending to make the DB downgrade script delete all the extension members and types from the DB). Also made a minor modification to one of the QL4QL queries.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants