Skip to content

Commit 8e60fe0

Browse files
authored
Fixed crash in CSharpConvertToRecordRefactoringProvider (dotnet#78665)
This PR fixes the crash in `CSharpConvertToRecordRefactoringProvider` described in dotnet#78664 while maintaining the behaviors for `Clone`, `GetHashCode`, and `Equals`.
2 parents e082af4 + 732ec3f commit 8e60fe0

File tree

2 files changed

+64
-11
lines changed

2 files changed

+64
-11
lines changed

src/Analyzers/CSharp/CodeFixes/ConvertToRecord/ConvertToRecordEngine.cs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -218,24 +218,28 @@ await RefactorInitializersAsync(type, solutionEditor, propertiesToAssign, cancel
218218

219219
foreach (var method in typeDeclaration.Members.OfType<MethodDeclarationSyntax>())
220220
{
221-
var methodSymbol = (IMethodSymbol)semanticModel.GetRequiredDeclaredSymbol(method, cancellationToken);
222-
var operation = (IMethodBodyOperation)semanticModel.GetRequiredOperation(method, cancellationToken);
221+
var methodSymbol = semanticModel.GetRequiredDeclaredSymbol(method, cancellationToken);
223222

224223
if (methodSymbol.Name == "Clone")
225224
{
226225
// remove clone method as clone is a reserved method name in records
227226
documentEditor.RemoveNode(method);
228227
}
229-
else if (ConvertToRecordHelpers.IsSimpleHashCodeMethod(
230-
semanticModel.Compilation, methodSymbol, operation, expectedFields))
228+
else if (method is { Body: not null })
231229
{
232-
documentEditor.RemoveNode(method);
233-
}
234-
else if (ConvertToRecordHelpers.IsSimpleEqualsMethod(
235-
semanticModel.Compilation, methodSymbol, operation, expectedFields))
236-
{
237-
// the Equals method implementation is fundamentally equivalent to the generated one
238-
documentEditor.RemoveNode(method);
230+
var operation = (IMethodBodyOperation)semanticModel.GetRequiredOperation(method, cancellationToken);
231+
232+
if (ConvertToRecordHelpers.IsSimpleHashCodeMethod(
233+
semanticModel.Compilation, methodSymbol, operation, expectedFields))
234+
{
235+
documentEditor.RemoveNode(method);
236+
}
237+
else if (ConvertToRecordHelpers.IsSimpleEqualsMethod(
238+
semanticModel.Compilation, methodSymbol, operation, expectedFields))
239+
{
240+
// the Equals method implementation is fundamentally equivalent to the generated one
241+
documentEditor.RemoveNode(method);
242+
}
239243
}
240244
}
241245

src/Features/CSharpTest/ConvertToRecord/ConvertToRecordCodeRefactoringTests.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4515,6 +4515,55 @@ public record C(int P, bool B);
45154515
}.RunAsync();
45164516
}
45174517

4518+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78664")]
4519+
public async Task TestDoesNotCrashOnAbstractMethod()
4520+
{
4521+
var initialMarkup = """
4522+
namespace N
4523+
{
4524+
public abstract class [|C|]
4525+
{
4526+
public string? S { get; set; }
4527+
4528+
public abstract System.Guid F();
4529+
}
4530+
}
4531+
""";
4532+
var changedMarkup = """
4533+
namespace N
4534+
{
4535+
public abstract record C(string? S)
4536+
{
4537+
public abstract System.Guid F();
4538+
}
4539+
}
4540+
""";
4541+
await TestRefactoringAsync(initialMarkup, changedMarkup);
4542+
}
4543+
4544+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78664")]
4545+
public async Task TestDoesNotCrashOnAbstractCloneMethod()
4546+
{
4547+
var initialMarkup = """
4548+
namespace N
4549+
{
4550+
public abstract class [|C|]
4551+
{
4552+
public string? S { get; set; }
4553+
4554+
public abstract object Clone();
4555+
}
4556+
}
4557+
""";
4558+
var changedMarkup = """
4559+
namespace N
4560+
{
4561+
public abstract record C(string? S);
4562+
}
4563+
""";
4564+
await TestRefactoringAsync(initialMarkup, changedMarkup);
4565+
}
4566+
45184567
#pragma warning disable RS1042 // Do not implement
45194568
private sealed class ConvertToRecordTestGenerator : ISourceGenerator
45204569
#pragma warning restore RS1042 // Do not implement

0 commit comments

Comments
 (0)