Skip to content

Commit

Permalink
Require AddExplicitInterfaceImplementation for adding a type that imp…
Browse files Browse the repository at this point in the history
…lements member explicitly (#73265)
  • Loading branch information
tmat authored May 1, 2024
1 parent f223c7a commit 0d75f2a
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 81 deletions.
10 changes: 5 additions & 5 deletions src/Features/CSharpTest/EditAndContinue/ActiveStatementTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10453,7 +10453,7 @@ static IEnumerable<int> F()
// should not contain RUDE_EDIT_INSERT_AROUND
edits.VerifySemanticDiagnostics(
active,
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -10484,7 +10484,7 @@ static IEnumerable<int> F()

edits.VerifySemanticDiagnostics(
active,
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -10667,7 +10667,7 @@ static async Task<int> F()
// should not contain RUDE_EDIT_INSERT_AROUND
edits.VerifySemanticDiagnostics(
active,
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -10758,7 +10758,7 @@ static async Task<int> F()

edits.VerifySemanticDiagnostics(
active,
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -10786,7 +10786,7 @@ static async void F()
var active = GetActiveStatements(src1, src2);

edits.VerifySemanticDiagnostics(active,
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down
12 changes: 6 additions & 6 deletions src/Features/CSharpTest/EditAndContinue/StatementEditingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6569,7 +6569,7 @@ public void F()
capabilities: EditAndContinueCapabilities.Baseline);

edits.VerifySemanticDiagnostics(
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

#endregion
Expand Down Expand Up @@ -9615,7 +9615,7 @@ async Task<int> WaitAsync()
capabilities: EditAndContinueCapabilities.Baseline);

edits.VerifySemanticDiagnostics(
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -9649,7 +9649,7 @@ IEnumerable<int> Iter()
capabilities: EditAndContinueCapabilities.Baseline);

edits.VerifySemanticDiagnostics(
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -11484,7 +11484,7 @@ static IEnumerable<int> F()

edits.VerifySemanticDiagnostics(
targetFrameworks: [TargetFramework.Mscorlib40AndSystemCore],
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

#endregion
Expand Down Expand Up @@ -12079,7 +12079,7 @@ class C
var edits = GetTopEdits(src1, src2);

edits.VerifySemanticDiagnostics(
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down Expand Up @@ -12431,7 +12431,7 @@ static async Task<int> F()

edits.VerifySemanticDiagnostics(
targetFrameworks: [TargetFramework.MinimalAsync],
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

[Fact]
Expand Down
185 changes: 132 additions & 53 deletions src/Features/CSharpTest/EditAndContinue/TopLevelEditingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1968,70 +1968,132 @@ public void Struct_Modifiers_Partial_InsertDelete(string modifier)
}

[Fact]
public void Class_ImplementingInterface_Add()
public void Class_ImplementingInterface_Add_Implicit_NonVirtual()
{
var src1 = @"
using System;
var src1 = """
interface I
{
void F();
}
""";

public interface ISample
{
string Get();
}
var src2 = """
interface I
{
void F();
}

public interface IConflict
{
string Get();
}
class C : I
{
public void F() {}
}
""";

public class BaseClass : ISample
{
public virtual string Get() => string.Empty;
}
";
var src2 = @"
using System;
var edits = GetTopEdits(src1, src2);
edits.VerifySemantics(
[SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C"))],
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
}

public interface ISample
{
string Get();
}
[Fact]
public void Class_ImplementingInterface_Add_Implicit_Virtual()
{
var src1 = """
interface I
{
void F();
}
""";

public interface IConflict
{
string Get();
}
var src2 = """
interface I
{
void F();
}

public class BaseClass : ISample
{
public virtual string Get() => string.Empty;
}
class C : I
{
public virtual void F() {}
}
""";

public class SubClass : BaseClass, IConflict
{
public override string Get() => string.Empty;
var edits = GetTopEdits(src1, src2);
edits.VerifySemantics(
[SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C"))],
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
}

string IConflict.Get() => String.Empty;
}
";
[Fact]
public void Class_ImplementingInterface_Add_Implicit_Override()
{
var src1 = """
interface I
{
void F();
}

class C : I
{
public virtual void F() {}
}
""";

var src2 = """
interface I
{
void F();
}

class C : I
{
public virtual void F() {}
}

class D : C
{
public override void F() {}
}
""";

var edits = GetTopEdits(src1, src2);
edits.VerifySemantics(
[SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("D"))],
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);
}

edits.VerifyEdits(
@"Insert [public class SubClass : BaseClass, IConflict
{
public override string Get() => string.Empty;
[Theory]
[InlineData("void F();", "void I.F() {}")]
[InlineData("int F { get; }", "int I.F { get; }")]
[InlineData("event System.Action F;", "event System.Action I.F { add {} remove {} }")]
public void Class_ImplementingInterface_Add_Explicit_NonVirtual(string memberDef, string explicitImpl)
{
var src1 = $$"""
interface I
{
{{memberDef}}
}
""";

string IConflict.Get() => String.Empty;
}]@219",
"Insert [: BaseClass, IConflict]@241",
"Insert [public override string Get() => string.Empty;]@272",
"Insert [string IConflict.Get() => String.Empty;]@325",
"Insert [()]@298",
"Insert [()]@345");
var src2 = $$"""
interface I
{
{{memberDef}}
}

class C<T> : I
{
{{explicitImpl}}
}
""";

var edits = GetTopEdits(src1, src2);

edits.VerifySemantics(
[SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C"))],
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);

// Here we add a class implementing an interface and a method inside it with explicit interface specifier.
// We want to be sure that adding the method will not tirgger a rude edit as it happens if adding a single method with explicit interface specifier.
edits.VerifySemanticDiagnostics(
[Diagnostic(RudeEditKind.InsertNotSupportedByRuntime, "class C<T>", GetResource("class"))],
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
}

Expand Down Expand Up @@ -2312,15 +2374,28 @@ void F() {}
public void Type_Generic_InsertMembers_Reloadable()
{
var src1 = ReloadableAttributeSrc + @"
interface IExplicit
{
void F() {}
}

[CreateNewOnMetadataUpdate]
class C<T>
class C<T> : IExplicit
{
void IExplicit.F() {}
}
";
var src2 = ReloadableAttributeSrc + @"
interface IExplicit
{
void F() {}
}

[CreateNewOnMetadataUpdate]
class C<T>
class C<T> : IExplicit
{
void IExplicit.F() {}

void M() {}
int P1 { get; set; }
int P2 { get => 1; set {} }
Expand All @@ -2337,6 +2412,10 @@ class D {}
var edits = GetTopEdits(src1, src2);
edits.VerifySemantics(
[SemanticEdit(SemanticEditKind.Replace, c => c.GetMember("C"))],
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);

edits.VerifySemanticDiagnostics(
[Diagnostic(RudeEditKind.ChangingReloadableTypeNotSupportedByRuntime, "void M()", "CreateNewOnMetadataUpdateAttribute")],
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
}

Expand Down Expand Up @@ -7534,7 +7613,7 @@ public async Task<int> WaitAsync()
}";
var edits = GetTopEdits(src1, src2);
edits.VerifySemanticDiagnostics(
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);

VerifyPreserveLocalVariables(edits, preserveLocalVariables: false);
}
Expand Down Expand Up @@ -9667,7 +9746,7 @@ public void MethodUpdate_AddYieldReturn()
var edits = GetTopEdits(src1, src2);

edits.VerifySemanticDiagnostics(
capabilities: EditAndContinueCapabilities.NewTypeDefinition);
capabilities: EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation);

VerifyPreserveLocalVariables(edits, preserveLocalVariables: false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2567,7 +2567,7 @@ private async Task<ImmutableArray<SemanticEditInfo>> AnalyzeSemanticsAsync(
{
if (processedSymbols.Add(newContainingType))
{
if (capabilities.Grant(EditAndContinueCapabilities.NewTypeDefinition))
if (capabilities.GrantNewTypeDefinition(containingType))
{
semanticEdits.Add(SemanticEditInfo.CreateReplace(containingTypeSymbolKey,
IsPartialTypeEdit(oldContainingType, newContainingType, oldTree, newTree) ? containingTypeSymbolKey : null));
Expand Down Expand Up @@ -2601,7 +2601,7 @@ private async Task<ImmutableArray<SemanticEditInfo>> AnalyzeSemanticsAsync(
// https://github.com/dotnet/roslyn/issues/54881
diagnosticContext.Report(RudeEditKind.ChangingTypeParameters, cancellationToken);
}
else if (!capabilities.Grant(EditAndContinueCapabilities.NewTypeDefinition))
else if (!capabilities.GrantNewTypeDefinition(oldType))
{
diagnosticContext.Report(RudeEditKind.ChangingReloadableTypeNotSupportedByRuntime, cancellationToken);
}
Expand Down Expand Up @@ -2883,7 +2883,7 @@ newSymbol is IPropertySymbol newProperty &&
// therefore inserting the <Program>$ type
Contract.ThrowIfFalse(newSymbol is INamedTypeSymbol || IsGlobalMain(newSymbol));

if (!capabilities.Grant(EditAndContinueCapabilities.NewTypeDefinition))
if (!capabilities.GrantNewTypeDefinition((newSymbol as INamedTypeSymbol) ?? newSymbol.ContainingType))
{
diagnostics.Add(new RudeEditDiagnostic(
RudeEditKind.InsertNotSupportedByRuntime,
Expand Down Expand Up @@ -3205,7 +3205,7 @@ IFieldSymbol or
{
if (processedSymbols.Add(newContainingType))
{
if (capabilities.Grant(EditAndContinueCapabilities.NewTypeDefinition))
if (capabilities.GrantNewTypeDefinition(newContainingType))
{
var oldContainingTypeKey = SymbolKey.Create(oldContainingType, cancellationToken);
semanticEdits.Add(SemanticEditInfo.CreateReplace(oldContainingTypeKey,
Expand Down Expand Up @@ -3889,7 +3889,7 @@ private void ReportMemberOrLambdaBodyUpdateRudeEdits(

if (!oldStateMachineInfo.IsStateMachine &&
newStateMachineInfo.IsStateMachine &&
!capabilities.Grant(EditAndContinueCapabilities.NewTypeDefinition))
!capabilities.Grant(EditAndContinueCapabilities.NewTypeDefinition | EditAndContinueCapabilities.AddExplicitInterfaceImplementation))
{
// Adding a state machine, either for async or iterator, will require creating a new helper class
// so is a rude edit if the runtime doesn't support it
Expand Down Expand Up @@ -5684,9 +5684,11 @@ bool CanAddNewLambda(SyntaxNode newLambda, LambdaBody newLambdaBody1, LambdaBody
}
}

// If the old verison of the method had any lambdas the nwe know a closure type exists and a new one isn't needed.
// If the old version of the method had any lambdas then we know a closure type exists and a new one isn't needed.
// We also know that adding a local function won't create a new closure type.
// Otherwise, we assume a new type is needed.
// We also assume that the closure type does not implement an interface explicitly,
// so we do not need AddExplicitInterfaceImplementation capability.

if (!oldHasLambdas && !isLocalFunction)
{
Expand Down
Loading

0 comments on commit 0d75f2a

Please sign in to comment.