Skip to content

Commit

Permalink
EnC: Allow reordering members
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat committed Sep 26, 2024
1 parent bdebb57 commit 0af607b
Show file tree
Hide file tree
Showing 8 changed files with 579 additions and 143 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 0af607b

Please sign in to comment.