From d1ffbbf78af46f425dbbb809562557516eb8f0b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 14:12:18 +0000 Subject: [PATCH 1/8] Initial plan From 86b9a8ed52fcc8e73a99caf634638c267d6bc7d6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 14:26:08 +0000 Subject: [PATCH 2/8] Update documentation for DeclaringSyntaxReferences and Locations to clarify partial members behavior Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> --- .../Test/Symbol/Symbols/Source/MethodTests.cs | 53 +++++++++++++++++++ .../Core/Portable/Symbols/ISymbol.cs | 16 ++++-- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs index 7a04fd7df3dd7..8425c9b8e025f 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs @@ -2576,5 +2576,58 @@ public partial void M() { } Assert.False(partialImpl.IsPartialDefinition); Assert.False(partialImplConstructed.IsPartialDefinition); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/13456")] + public void PartialMethodsLocationsAndSyntaxReferences() + { + // This test documents the behavior described in https://github.com/dotnet/roslyn/issues/13456 + // For partial methods, Locations and DeclaringSyntaxReferences return only one location, + // not both the definition and implementation parts. To get all locations, you must use + // PartialDefinitionPart and PartialImplementationPart properties. + + var source = @" +namespace N1 +{ + partial class C1 + { + partial void PartialM(); + } +}"; + + var source2 = @" +namespace N1 +{ + partial class C1 + { + partial void PartialM() { } + } +}"; + + var comp = CreateCompilation(new[] { source, source2 }); + comp.VerifyDiagnostics(); + + var methodSymbols = comp.GetSymbolsWithName("PartialM"); + Assert.Equal(1, methodSymbols.Count()); + + var method = methodSymbols.First() as IMethodSymbol; + Assert.NotNull(method); + + // For partial methods, Locations and DeclaringSyntaxReferences contain only one location + Assert.Equal(1, method.Locations.Length); + Assert.Equal(1, method.DeclaringSyntaxReferences.Length); + + // The single location is the definition part + Assert.True(method.IsPartialDefinition); + Assert.Null(method.PartialDefinitionPart); + Assert.NotNull(method.PartialImplementationPart); + + // To get all locations, you need to use PartialImplementationPart + var implementationPart = method.PartialImplementationPart; + Assert.Equal(1, implementationPart.Locations.Length); + Assert.Equal(1, implementationPart.DeclaringSyntaxReferences.Length); + + // Verify the locations are different + Assert.NotEqual(method.Locations[0], implementationPart.Locations[0]); + } } } diff --git a/src/Compilers/Core/Portable/Symbols/ISymbol.cs b/src/Compilers/Core/Portable/Symbols/ISymbol.cs index 1c461b0825233..189a6e28c2cbb 100644 --- a/src/Compilers/Core/Portable/Symbols/ISymbol.cs +++ b/src/Compilers/Core/Portable/Symbols/ISymbol.cs @@ -165,18 +165,28 @@ public interface ISymbol : IEquatable /// /// Gets the locations where the symbol was originally defined, either in source or - /// metadata. Some symbols (for example, partial classes) may be defined in more than one - /// location. + /// metadata. Some symbols (for example, partial types) may be defined in more than one + /// location. Note that for partial members (methods, properties, events), this property returns + /// only one location. To get all locations for a partial member, use the PartialDefinitionPart and + /// PartialImplementationPart properties on , , or + /// . /// ImmutableArray Locations { get; } /// /// Get the syntax node(s) where this symbol was declared in source. Some symbols (for example, - /// partial classes) may be defined in more than one location. This property should return + /// partial types) may be defined in more than one location. This property should return /// one or more syntax nodes only if the symbol was declared in source code and also was /// not implicitly declared (see the IsImplicitlyDeclared property). /// /// + /// Note that for partial members (methods, properties, events), this property returns only one + /// syntax node. To get all syntax nodes for a partial member, use the PartialDefinitionPart and + /// PartialImplementationPart properties on , , or + /// . + /// + /// + /// /// Note that for namespace symbol, the declaring syntax might be declaring a nested namespace. /// For example, the declaring syntax node for N1 in "namespace N1.N2 {...}" is the entire /// NamespaceDeclarationSyntax for N1.N2. For the global namespace, the declaring syntax will From 7cf70f9bb6f9b09a5e70fbe54942d0aa3cb61bf6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 14:31:22 +0000 Subject: [PATCH 3/8] Make documentation more explicit about partial types Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> --- src/Compilers/Core/Portable/Symbols/ISymbol.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/Portable/Symbols/ISymbol.cs b/src/Compilers/Core/Portable/Symbols/ISymbol.cs index 189a6e28c2cbb..2ca1a5c15a05c 100644 --- a/src/Compilers/Core/Portable/Symbols/ISymbol.cs +++ b/src/Compilers/Core/Portable/Symbols/ISymbol.cs @@ -165,7 +165,7 @@ public interface ISymbol : IEquatable /// /// Gets the locations where the symbol was originally defined, either in source or - /// metadata. Some symbols (for example, partial types) may be defined in more than one + /// metadata. Some symbols (for example, partial types such as classes, structs, and interfaces) may be defined in more than one /// location. Note that for partial members (methods, properties, events), this property returns /// only one location. To get all locations for a partial member, use the PartialDefinitionPart and /// PartialImplementationPart properties on , , or @@ -175,7 +175,7 @@ public interface ISymbol : IEquatable /// /// Get the syntax node(s) where this symbol was declared in source. Some symbols (for example, - /// partial types) may be defined in more than one location. This property should return + /// partial types such as classes, structs, and interfaces) may be defined in more than one location. This property should return /// one or more syntax nodes only if the symbol was declared in source code and also was /// not implicitly declared (see the IsImplicitlyDeclared property). /// From f7560e5183a3d014abd884c3b236bc19132c28c4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 15 Oct 2025 19:22:33 +0200 Subject: [PATCH 4/8] Simplify --- .../Test/Symbol/Symbols/Source/MethodTests.cs | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs index 8425c9b8e025f..db88dbd38f5aa 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs @@ -2580,36 +2580,32 @@ public partial void M() { } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/13456")] public void PartialMethodsLocationsAndSyntaxReferences() { - // This test documents the behavior described in https://github.com/dotnet/roslyn/issues/13456 - // For partial methods, Locations and DeclaringSyntaxReferences return only one location, - // not both the definition and implementation parts. To get all locations, you must use - // PartialDefinitionPart and PartialImplementationPart properties. - - var source = @" -namespace N1 -{ - partial class C1 - { - partial void PartialM(); - } -}"; + var source = """ + namespace N1 + { + partial class C1 + { + partial void PartialM(); + } + } + """; - var source2 = @" -namespace N1 -{ - partial class C1 - { - partial void PartialM() { } - } -}"; + var source2 = """ + namespace N1 + { + partial class C1 + { + partial void PartialM() { } + } + } + """; - var comp = CreateCompilation(new[] { source, source2 }); + var comp = CreateCompilation([source, source2]); comp.VerifyDiagnostics(); var methodSymbols = comp.GetSymbolsWithName("PartialM"); - Assert.Equal(1, methodSymbols.Count()); - var method = methodSymbols.First() as IMethodSymbol; + var method = (IMethodSymbol)methodSymbols.Single(); Assert.NotNull(method); // For partial methods, Locations and DeclaringSyntaxReferences contain only one location From a2577d217543cfcf3cb203057f3512bd144fe521 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 15 Oct 2025 19:26:20 +0200 Subject: [PATCH 5/8] Simplify test --- .../CSharp/Test/Symbol/Symbols/Source/MethodTests.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs index db88dbd38f5aa..a94443f3a45f5 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs @@ -2580,7 +2580,7 @@ public partial void M() { } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/13456")] public void PartialMethodsLocationsAndSyntaxReferences() { - var source = """ + var source1 = """ namespace N1 { partial class C1 @@ -2600,7 +2600,7 @@ partial void PartialM() { } } """; - var comp = CreateCompilation([source, source2]); + var comp = CreateCompilation([(source1, "source1"), (source2, "source2")]); comp.VerifyDiagnostics(); var methodSymbols = comp.GetSymbolsWithName("PartialM"); @@ -2624,6 +2624,9 @@ partial void PartialM() { } // Verify the locations are different Assert.NotEqual(method.Locations[0], implementationPart.Locations[0]); + + Assert.Equal("source1", method.Locations[0].SourceTree.FilePath); + Assert.Equal("source2", implementationPart.Locations[0].SourceTree.FilePath); } } } From 0c0af75fc22f3b6437153d7b3584a2155d19a5ad Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Oct 2025 22:58:05 +0200 Subject: [PATCH 6/8] Update src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs --- src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs index a94443f3a45f5..7cfb2f1175351 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs @@ -2577,7 +2577,7 @@ public partial void M() { } Assert.False(partialImplConstructed.IsPartialDefinition); } - [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/13456")] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/22598")] public void PartialMethodsLocationsAndSyntaxReferences() { var source1 = """ From dc7f5953304d714f4964b8977c1bbdfefb521b5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Oct 2025 21:05:30 +0000 Subject: [PATCH 7/8] Address review feedback: fix WorkItem link, remove unnecessary assertion, inline variable, and update instructions Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> --- .github/instructions/Compiler.instructions.md | 1 + .../CSharp/Test/Symbol/Symbols/Source/MethodTests.cs | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/instructions/Compiler.instructions.md b/.github/instructions/Compiler.instructions.md index e9e40e2b3dcd3..770a07247dfb5 100644 --- a/.github/instructions/Compiler.instructions.md +++ b/.github/instructions/Compiler.instructions.md @@ -60,6 +60,7 @@ dotnet run --file eng/generate-compiler-code.cs - **Unit tests**: Test individual compiler phases (lexing, parsing) - **Compilation tests**: Create `Compilation` objects and verify symbols/diagnostics - **Cross-language patterns**: Many test patterns work for both C# and VB with minor syntax changes +- **Keep tests focused**: Avoid unnecessary assertions. Tests should do the minimal work necessary to get to the core assertions that validate the issue being addressed. For example, use `Single()` instead of checking counts and then accessing the first element. ## Debugger Integration diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs index 7cfb2f1175351..f6b4b5fa76627 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs @@ -2603,10 +2603,7 @@ partial void PartialM() { } var comp = CreateCompilation([(source1, "source1"), (source2, "source2")]); comp.VerifyDiagnostics(); - var methodSymbols = comp.GetSymbolsWithName("PartialM"); - - var method = (IMethodSymbol)methodSymbols.Single(); - Assert.NotNull(method); + var method = (IMethodSymbol)comp.GetSymbolsWithName("PartialM").Single(); // For partial methods, Locations and DeclaringSyntaxReferences contain only one location Assert.Equal(1, method.Locations.Length); From d93ff58beb7f0bd623915c5ccf402ad27665a491 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Oct 2025 23:33:17 -0700 Subject: [PATCH 8/8] Update src/Compilers/Core/Portable/Symbols/ISymbol.cs Co-authored-by: Rikki Gibson --- src/Compilers/Core/Portable/Symbols/ISymbol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Symbols/ISymbol.cs b/src/Compilers/Core/Portable/Symbols/ISymbol.cs index 2ca1a5c15a05c..b7f735d0d0b7f 100644 --- a/src/Compilers/Core/Portable/Symbols/ISymbol.cs +++ b/src/Compilers/Core/Portable/Symbols/ISymbol.cs @@ -166,7 +166,7 @@ public interface ISymbol : IEquatable /// /// Gets the locations where the symbol was originally defined, either in source or /// metadata. Some symbols (for example, partial types such as classes, structs, and interfaces) may be defined in more than one - /// location. Note that for partial members (methods, properties, events), this property returns + /// location. Note that for partial members (such as methods, properties, and events), this property returns /// only one location. To get all locations for a partial member, use the PartialDefinitionPart and /// PartialImplementationPart properties on , , or /// .