Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EnC: Allow reordering members #75257

Merged
merged 3 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading