From cec60eff99664a2c19911c36fddb4321a97b0843 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Mon, 29 May 2023 17:28:13 +1000 Subject: [PATCH 1/4] Upgrade change types for containing types --- .../EditAndContinue/EditAndContinueTests.cs | 90 +++++++++++++++++++ .../Emit/EditAndContinue/SymbolChanges.cs | 12 ++- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs index ceaf874a2740a..44eb0497f404a 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs @@ -5050,6 +5050,96 @@ void ValidateReplacedType(CompilationDifference diff, MetadataReader[] readers) CheckBlobValue(readers, reader3.GetCustomAttribute(reader3.CustomAttributes.First()).Value, new byte[] { 0x01, 0x00, 0x00, 0x00 }); } + [Fact] + public void ReplaceType_UpdateNestedType() + { + using var _ = new EditAndContinueTest() + .AddBaseline( + source: $$""" + using System; + + class C + { + class D + { + void M() + { + Console.WriteLine("1"); + } + } + + void N() + { + Console.WriteLine("1"); + } + } + """ + MetadataUpdateOriginalTypeAttributeSource, + validator: g => + { + g.VerifyTypeDefNames("", "C", "MetadataUpdateOriginalTypeAttribute", "D"); + g.VerifyMethodDefNames("N", ".ctor", ".ctor", "get_OriginalType", "M", ".ctor"); + }) + + .AddGeneration( + source: """ + using System; + + class C + { + class D + { + void M() + { + Console.WriteLine("2"); + } + } + + void N() + { + Console.WriteLine("2"); + } + } + """ + MetadataUpdateOriginalTypeAttributeSource, + edits: new[] { + // Note: Nested type edit needs to be seen first to repro the bug. Real world scenario requires the nested + // class to be in a separate file. + Edit(SemanticEditKind.Update, c => c.GetMember("C.D")), + Edit(SemanticEditKind.Replace, c => null, newSymbolProvider: c => c.GetMember("C")), + }, + validator: g => + { + g.VerifyTypeDefNames("D", "C#1"); + g.VerifyMethodDefNames("N", ".ctor", "M", ".ctor"); + g.VerifyEncLogDefinitions(new[] + { + Row(4, TableIndex.TypeDef, EditAndContinueOperation.Default), + Row(5, TableIndex.TypeDef, EditAndContinueOperation.Default), + Row(5, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), + Row(7, TableIndex.MethodDef, EditAndContinueOperation.Default), + Row(5, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), + Row(8, TableIndex.MethodDef, EditAndContinueOperation.Default), + Row(4, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), + Row(9, TableIndex.MethodDef, EditAndContinueOperation.Default), + Row(4, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), + Row(10, TableIndex.MethodDef, EditAndContinueOperation.Default), + Row(8, TableIndex.CustomAttribute, EditAndContinueOperation.Default), + Row(2, TableIndex.NestedClass, EditAndContinueOperation.Default) + }); + g.VerifyEncMapDefinitions(new[] + { + Handle(4, TableIndex.TypeDef), + Handle(5, TableIndex.TypeDef), + Handle(7, TableIndex.MethodDef), + Handle(8, TableIndex.MethodDef), + Handle(9, TableIndex.MethodDef), + Handle(10, TableIndex.MethodDef), + Handle(8, TableIndex.CustomAttribute), + Handle(2, TableIndex.NestedClass) + }); + }) + .Verify(); + } + [Fact] public void EventFields_Attributes() { diff --git a/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs b/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs index 62af09ad4622e..84ef2e782a937 100644 --- a/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs +++ b/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs @@ -494,7 +494,17 @@ private static void CalculateChanges(IEnumerable edits, out IReadO } AddContainingTypesAndNamespaces(changesBuilder, member); - changesBuilder.Add(member, change); + + // If we saw an edit for a symbol that is a nested type of this symbol, we might already have a dictionary entry + // for it, flagging that it contains changes. If so we "upgrade" the change to the real one. + if (changesBuilder.TryGetValue(member, out var existingChange) && existingChange == SymbolChange.ContainsChanges) + { + changesBuilder[member] = change; + } + else + { + changesBuilder.Add(member, change); + } } changes = changesBuilder; From 38225badf2d3ed7559107b0c78d2098f4a1aa3c8 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 31 May 2023 08:59:53 +1000 Subject: [PATCH 2/4] Make test match the real world --- .../EditAndContinue/EditAndContinueTests.cs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs index 44eb0497f404a..87a0c8c2462aa 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs @@ -5103,38 +5103,33 @@ void N() edits: new[] { // Note: Nested type edit needs to be seen first to repro the bug. Real world scenario requires the nested // class to be in a separate file. - Edit(SemanticEditKind.Update, c => c.GetMember("C.D")), + Edit(SemanticEditKind.Update, c => c.GetMember("C.D.M")), Edit(SemanticEditKind.Replace, c => null, newSymbolProvider: c => c.GetMember("C")), }, validator: g => { - g.VerifyTypeDefNames("D", "C#1"); - g.VerifyMethodDefNames("N", ".ctor", "M", ".ctor"); + g.VerifyTypeDefNames("C#1"); + g.VerifyMethodDefNames("M", "N", ".ctor", ".ctor"); g.VerifyEncLogDefinitions(new[] { - Row(4, TableIndex.TypeDef, EditAndContinueOperation.Default), Row(5, TableIndex.TypeDef, EditAndContinueOperation.Default), + Row(5, TableIndex.MethodDef, EditAndContinueOperation.Default), Row(5, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), Row(7, TableIndex.MethodDef, EditAndContinueOperation.Default), Row(5, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), Row(8, TableIndex.MethodDef, EditAndContinueOperation.Default), Row(4, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), Row(9, TableIndex.MethodDef, EditAndContinueOperation.Default), - Row(4, TableIndex.TypeDef, EditAndContinueOperation.AddMethod), - Row(10, TableIndex.MethodDef, EditAndContinueOperation.Default), - Row(8, TableIndex.CustomAttribute, EditAndContinueOperation.Default), - Row(2, TableIndex.NestedClass, EditAndContinueOperation.Default) + Row(8, TableIndex.CustomAttribute, EditAndContinueOperation.Default) }); g.VerifyEncMapDefinitions(new[] { - Handle(4, TableIndex.TypeDef), Handle(5, TableIndex.TypeDef), + Handle(5, TableIndex.MethodDef), Handle(7, TableIndex.MethodDef), Handle(8, TableIndex.MethodDef), Handle(9, TableIndex.MethodDef), - Handle(10, TableIndex.MethodDef), - Handle(8, TableIndex.CustomAttribute), - Handle(2, TableIndex.NestedClass) + Handle(8, TableIndex.CustomAttribute) }); }) .Verify(); From 479a4bf258de612ff7d0129dac802ae4097bf734 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 7 Jun 2023 15:05:15 +1000 Subject: [PATCH 3/4] PR Feedback --- .../Emit/EditAndContinue/SymbolChanges.cs | 1 + .../EditAndContinue/TopLevelEditingTests.cs | 81 +++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs b/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs index 84ef2e782a937..1a4c0ae63fe6b 100644 --- a/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs +++ b/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs @@ -499,6 +499,7 @@ private static void CalculateChanges(IEnumerable edits, out IReadO // for it, flagging that it contains changes. If so we "upgrade" the change to the real one. if (changesBuilder.TryGetValue(member, out var existingChange) && existingChange == SymbolChange.ContainsChanges) { + Debug.Assert(member is INamedTypeSymbol); changesBuilder[member] = change; } else diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs index 308d5ecf2643e..52cbb2c8d0606 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs @@ -4908,6 +4908,87 @@ public void Delegate_ReadOnlyRef_ReturnType_Update() #region Nested Types + [Fact] + public void NestedType_Replace_WithUpdateInNestedType_Partial_DiffertDocument() + { + var srcA1 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { int N() => 1; }"; + var srcB1 = "partial class C { class D { int M() => 1; } }"; + var srcA2 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { int N() => 2; }"; + var srcB2 = "partial class C { class D { int M() => 2; } }"; + + var editsA = GetTopEdits(srcA1, srcA2); + var editsB = GetTopEdits(srcB1, srcB2); + + EditAndContinueValidation.VerifySemantics( + new[] { editsA, editsB }, + new[] + { + DocumentResults(semanticEdits: new[] + { + SemanticEdit(SemanticEditKind.Replace, c => c.GetMember("C"), partialType: "C") + }), + DocumentResults(semanticEdits: new[] + { + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.D.M")) + }) + }, + capabilities: EditAndContinueCapabilities.NewTypeDefinition); + } + + [Fact] + public void NestedType_Replace_WithUpdateInNestedType_Partial_SameDocument() + { + var src1 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { int N() => 1; } partial class C { class D { int M() => 1; } }"; + var src2 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { int N() => 2; } partial class C { class D { int M() => 2; } }"; + + var edits = GetTopEdits(src1, src2); + + EditAndContinueValidation.VerifySemantics( + edits, + semanticEdits: new[] + { + SemanticEdit(SemanticEditKind.Replace, c => c.GetMember("C")), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.D.M")) + }, + capabilities: EditAndContinueCapabilities.NewTypeDefinition); + } + + [Fact] + public void NestedType_Replace_WithUpdateInNestedType() + { + var src1 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]class C { int N() => 1; class D { int M() => 1; } }"; + var src2 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]class C { int N() => 2; class D { int M() => 2; } }"; + + var edits = GetTopEdits(src1, src2); + + EditAndContinueValidation.VerifySemantics( + edits, + semanticEdits: new[] + { + SemanticEdit(SemanticEditKind.Replace, c => c.GetMember("C")), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.D.M")) + }, + capabilities: EditAndContinueCapabilities.NewTypeDefinition); + } + + [Fact] + public void NestedType_Update_WithUpdateInNestedType() + { + var src1 = @"[System.Obsolete(""A"")]class C { int N() => 1; class D { int M() => 1; } }"; + var src2 = @"[System.Obsolete(""B"")]class C { int N() => 1; class D { int M() => 2; } }"; + + var edits = GetTopEdits(src1, src2); + + EditAndContinueValidation.VerifySemantics( + edits, + semanticEdits: new[] + { + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C")), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.D.M")) + }, + capabilities: EditAndContinueCapabilities.ChangeCustomAttributes); + } + [Fact] public void NestedType_Move_Sideways() { From c0ab6c8e0be70e246b3d5fa90dd54e0e1ab9037b Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 7 Jun 2023 16:05:41 +1000 Subject: [PATCH 4/4] Update src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs --- .../CSharpTest/EditAndContinue/TopLevelEditingTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs index 52cbb2c8d0606..d25bdaffc1aa2 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs @@ -4909,7 +4909,7 @@ public void Delegate_ReadOnlyRef_ReturnType_Update() #region Nested Types [Fact] - public void NestedType_Replace_WithUpdateInNestedType_Partial_DiffertDocument() + public void NestedType_Replace_WithUpdateInNestedType_Partial_DifferentDocument() { var srcA1 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { int N() => 1; }"; var srcB1 = "partial class C { class D { int M() => 1; } }";