Skip to content

Commit

Permalink
EnC: Allow reordering members (#75257)
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat authored Oct 1, 2024
1 parent 8d8276d commit 6af5ab6
Show file tree
Hide file tree
Showing 26 changed files with 1,491 additions and 534 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1166,22 +1166,18 @@ newContainingMemberOrType is IPropertySymbol newPropertySymbol &&
case EditKind.Reorder:
Contract.ThrowIfNull(oldNode);

// When global statements are reordered, we issue an update edit for the synthesized main method, which is what
// oldSymbol and newSymbol will point to.
if (IsGlobalStatement(oldNode))
{
// When global statements are reordered, we issue an update edit for the synthesized main method, which is what
// oldSymbol and newSymbol will point to
result.Add((oldSymbol, newSymbol, EditKind.Update));
return;
}

// Otherwise, we don't do any semantic checks for reordering
// and we don't need to report them to the compiler either.
// Consider: Currently symbol ordering changes are not reflected in metadata (Reflection will report original order).
// Reordering of data members is only allowed if the layout of the type doesn't change.
// Reordering of other members is a no-op, although the new order won't be reflected in metadata (Reflection will report original order).
result.Add((oldSymbol, newSymbol, EditKind.Reorder));

// Consider: Reordering of fields is not allowed since it changes the layout of the type.
// This ordering should however not matter unless the type has explicit layout so we might want to allow it.
// We do not check changes to the order if they occur across multiple documents (the containing type is partial).
Debug.Assert(!IsDeclarationWithInitializer(oldNode!) && !IsDeclarationWithInitializer(newNode!));
return;

case EditKind.Update:
Expand Down Expand Up @@ -2391,14 +2387,6 @@ private void ClassifyReorder(SyntaxNode newNode)

switch (newNode.Kind())
{
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.FieldDeclaration:
case SyntaxKind.EventFieldDeclaration:
case SyntaxKind.VariableDeclarator:
// Maybe we could allow changing order of field declarations unless the containing type layout is sequential.
ReportError(RudeEditKind.Move);
return;

case SyntaxKind.EnumMemberDeclaration:
// To allow this change we would need to check that values of all fields of the enum
// are preserved, or make sure we can update all method bodies that accessed those that changed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3126,7 +3126,6 @@ public C() {}
var active = GetActiveStatements(src1, src2);

edits.VerifySemanticDiagnostics(active,
Diagnostic(RudeEditKind.Move, "int c", GetResource("field")),
Diagnostic(RudeEditKind.DeleteActiveStatement, "class C", GetResource("field", "C.a")),
Diagnostic(RudeEditKind.Delete, "class C", GetResource("field", "a")));
}
Expand Down
59 changes: 36 additions & 23 deletions src/Features/CSharpTest/EditAndContinue/LineEditTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ public void Method_Reorder1()
var src1 = @"
class C
{
static void Goo()
static void G()
{
Console.ReadLine(1);
}
static void Bar()
static void F()
{
Console.ReadLine(2);
}
Expand All @@ -83,76 +83,83 @@ static void Bar()
var src2 = @"
class C
{
static void Bar()
static void F()
{
Console.ReadLine(2);
}
static void Goo()
static void G()
{
Console.ReadLine(1);
}
}";
var edits = GetTopEdits(src1, src2);

// Consider: we could detect that the body of the method hasn't changed and avoid creating an update.
edits.VerifyLineEdits(
new[]
{
new SourceLineUpdate(4, 9),
new SourceLineUpdate(7, 7),
new SourceLineUpdate(9, 4)
});
},
semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.F"))]);
}

[Fact]
public void Method_Reorder2()
{
var src1 = @"
class Program
class C
{
static void Main()
{
Goo();
Bar();
F();
G();
}
static int Goo()
static int G()
{
return 1;
}
static int Bar()
static int F()
{
return 2;
}
}";
var src2 = @"
class Program
class C
{
static int Goo()
static int F()
{
return 1;
}
static void Main()
{
Goo();
Bar();
F();
G();
}
static int Bar()
static int G()
{
return 2;
}
}";
var edits = GetTopEdits(src1, src2);

// Consider: we could detect that the body of the method hasn't changed and create line edits instead of an update.
edits.VerifyLineEdits(
new[]
{
new SourceLineUpdate(4, 9),
new SourceLineUpdate(8, 8),
new SourceLineUpdate(10, 4),
new SourceLineUpdate(13, 13),
});
},
semanticEdits:
[
SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.F")),
SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.G"))
]);
}

[Fact]
Expand Down Expand Up @@ -758,13 +765,19 @@ public C(int a)
}
}";
var edits = GetTopEdits(src1, src2);

// Consider: we could detect that the body of the method hasn't changed and avoid creating an update.
edits.VerifyLineEdits(
new[]
{
new SourceLineUpdate(3, 7),
new SourceLineUpdate(6, 6),
new SourceLineUpdate(7, 3)
});
},
semanticEdits:
[
SemanticEdit(SemanticEditKind.Update, c => c.GetMember<INamedTypeSymbol>("C").Constructors.Single(c => c.Parameters is [{ Type.SpecialType: SpecialType.System_Boolean }]), preserveLocalVariables: true)
]);
}

[Fact]
Expand Down Expand Up @@ -1357,13 +1370,13 @@ class C
var src2 = @"
class C
{
static int Bar = 2;
static int Goo = 1;
static int Bar = 1;
static int Goo = 2;
}";
var edits = GetTopEdits(src1, src2);
edits.VerifyLineEdits(
Array.Empty<SequencePointUpdates>(),
diagnostics: [Diagnostic(RudeEditKind.Move, "static int Bar = 2", FeaturesResources.field)]);
semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember<INamedTypeSymbol>("C").StaticConstructors.Single(), preserveLocalVariables: true)]);
}

[Fact]
Expand Down
Loading

0 comments on commit 6af5ab6

Please sign in to comment.