Skip to content

C#: Fix issue with partial method extraction.#21351

Merged
michaelnebel merged 9 commits intogithub:mainfrom
michaelnebel:csharp/fixpartialmethod
Feb 24, 2026
Merged

C#: Fix issue with partial method extraction.#21351
michaelnebel merged 9 commits intogithub:mainfrom
michaelnebel:csharp/fixpartialmethod

Conversation

@michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Feb 20, 2026

It turns out that our current implementation for extracting partial methods in some cases might extract multiple method bodies.
An example:

partial class TwoPartClass
{
    public partial object M(object obj);
}
partial class TwoPartClass
{
    public partial object M(object obj)
    {
        return obj;
    }
}

In this case the following happens in the extractor:

  • The type TwoPartClass is visited and the members are extracted. However, only the partial definition is listed as a member (the one without the body).
  • During extraction of the symbol representing the partial definition, we also extract the "body" of the partial declaration.
  • During the extraction of the partial declaration body we encounter and extract the parameter symbol corresponding to obj (as the parameter is accessed).
  • Extracting the parameter symbol obj as a side effect also extracts its parent, which is the symbol representing the partial declaration of M.
  • Even though method extraction is done using caching, the symbol for the partial definition and partial declaration are different, which means that the symbol representing the partial declaration is fully extracted as well.
  • The extracted tuple ID for the partial implementation and declaration are identical, which means that the extraction is collapsed into a single entity.
  • Extracting the partial implementation, will yet again extract the body of the partial implementation.

As a result of the above, the body might be extracted twice.

In this PR we skip the extraction of the partial definition and only extract the partial declaration (if one exists).

Review on a commit by commit basis is recommended.

@github-actions github-actions bot added the C# label Feb 20, 2026
@michaelnebel michaelnebel force-pushed the csharp/fixpartialmethod branch 3 times, most recently from 51d3823 to b87b59d Compare February 23, 2026 12:55
@michaelnebel michaelnebel force-pushed the csharp/fixpartialmethod branch from ca855c8 to a930cbf Compare February 23, 2026 14:11
@michaelnebel michaelnebel marked this pull request as ready for review February 23, 2026 14:53
@michaelnebel michaelnebel requested a review from a team as a code owner February 23, 2026 14:53
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.

Pull request overview

Fixes a C# extractor issue where partial members (notably partial methods) could have their bodies extracted multiple times due to symbol/caching interactions between defining vs implementing declarations.

Changes:

  • Prefer extracting the body-declaring symbol for methods/properties/events (via new GetBodyDeclaringSymbol helpers) to avoid duplicate extraction for partial members.
  • Refactor body detection (Block/ExpressionBody) into cached helpers and use HasBody for accessors/event accessors.
  • Update/add library tests and expectations, including a new dataflow regression test involving a partial method body.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs Adds GetBodyDeclaringSymbol helpers (methods/properties/events).
csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs Creates ordinary method entities from the body-declaring symbol.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs Uses Symbol for NumberOfLines after symbol selection changes.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs Creates property entities from the body-declaring symbol; updates accessor extraction to use Symbol.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs Creates indexer entities from the body-declaring symbol; updates accessor extraction to use Symbol.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs Creates event entities from the body-declaring symbol; updates accessor extraction to use Symbol.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Base/CachedSymbol.cs Adds cached Block/ExpressionBody and HasBody helpers.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs Uses HasBody when deciding whether to mark accessors as compiler-generated.
csharp/extractor/Semmle.Extraction.CSharp/Entities/EventAccessor.cs Uses HasBody when deciding whether to mark event accessors as compiler-generated.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs Renames/refactors synthetic-body logic (MakeSyntheticBody) and uses HasBody.
csharp/ql/lib/change-notes/2026-02-23-partial-extraction-fix.md Adds changelog entry for the extraction fix.
csharp/ql/test/library-tests/partial/Partial.cs Extends partial-member test input with a partial method with body and parameters.
csharp/ql/test/library-tests/partial/PartialMethodBody.ql Extends test query to assert body-count is 0/1 (detect duplicates).
csharp/ql/test/library-tests/dataflow/methods/Methods.cs Adds a partial-method dataflow scenario for regression coverage.
csharp/ql/test/library-tests/dataflow/methods/MethodFlow.ql Adds a path-problem query to validate flow through a partial method body.
csharp/ql/test/library-tests/*.expected Updates expected outputs to reflect the new extraction behavior and new tests.

@michaelnebel michaelnebel marked this pull request as draft February 23, 2026 15:29
@michaelnebel michaelnebel force-pushed the csharp/fixpartialmethod branch from a930cbf to 0423882 Compare February 23, 2026 15:40
@michaelnebel michaelnebel force-pushed the csharp/fixpartialmethod branch from 0423882 to 7de476a Compare February 24, 2026 06:57
@michaelnebel michaelnebel marked this pull request as ready for review February 24, 2026 08:25
@michaelnebel
Copy link
Contributor Author

michaelnebel commented Feb 24, 2026

DCA looks good.

  • Query execution times appear to be unaffected.
  • The initial DCA run showed a performance degradation on database build time for JetBrains/resharper-rider-plugin, but an extra DCA run indicates that this was just an outlier.

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.

Very nice!

@michaelnebel michaelnebel merged commit 3e2f6e5 into github:main Feb 24, 2026
24 checks passed
@michaelnebel michaelnebel deleted the csharp/fixpartialmethod branch February 24, 2026 13:23
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.

3 participants