From 2f6768efa3f5575bae411524bb11364b2208c2c4 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Sat, 21 Aug 2021 03:30:31 +1000 Subject: [PATCH] EnC - Allow changing names of previously synthesized record member parameters (#55739) --- .../EditAndContinue/TopLevelEditingTests.cs | 13 +++++++++++-- .../CSharpEditAndContinueAnalyzer.cs | 9 +++++---- .../AbstractEditAndContinueAnalyzer.cs | 13 +++++++------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs index 57a6fcab4e69d..08c4e5828835d 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs @@ -2625,8 +2625,6 @@ private readonly bool PrintMembers(System.Text.StringBuilder builder) [Fact] public void Record_ImplementSynthesized_WrongParameterName() { - // TODO: Remove this requirement with https://github.com/dotnet/roslyn/issues/52563 - var src1 = "record C { }"; var src2 = @" record C @@ -2652,6 +2650,17 @@ protected C(C other) Diagnostic(RudeEditKind.ExplicitRecordMethodParameterNamesMustMatch, "protected virtual bool PrintMembers(System.Text.StringBuilder sb)", "PrintMembers(System.Text.StringBuilder builder)"), Diagnostic(RudeEditKind.ExplicitRecordMethodParameterNamesMustMatch, "public virtual bool Equals(C rhs)", "Equals(C other)"), Diagnostic(RudeEditKind.ExplicitRecordMethodParameterNamesMustMatch, "protected C(C other)", "C(C original)")); + + edits.VerifySemantics( + ActiveStatementsDescription.Empty, + new[] + { + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.PrintMembers")), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C").GetMembers("Equals").OfType().First(m => SymbolEqualityComparer.Default.Equals(m.Parameters[0].Type, m.ContainingType))), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C").Constructors.Single(c => c.Parameters.FirstOrDefault()?.Type.ToDisplayString() == "C"), preserveLocalVariables: true), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C").Constructors.Single(c => c.Parameters.Length == 0), preserveLocalVariables: true), + }, + EditAndContinueTestHelpers.Net6RuntimeCapabilities); } [Fact] diff --git a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs index d3cc64f70a418..21ea560bc52c9 100644 --- a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs +++ b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs @@ -655,7 +655,7 @@ private static IEnumerable GetChildNodes(SyntaxNode root, SyntaxNode } } - internal override void ReportDeclarationInsertDeleteRudeEdits(ArrayBuilder diagnostics, SyntaxNode oldNode, SyntaxNode newNode, ISymbol oldSymbol, ISymbol newSymbol, CancellationToken cancellationToken) + internal override void ReportDeclarationInsertDeleteRudeEdits(ArrayBuilder diagnostics, SyntaxNode oldNode, SyntaxNode newNode, ISymbol oldSymbol, ISymbol newSymbol, EditAndContinueCapabilities capabilities, CancellationToken cancellationToken) { // Global statements have a declaring syntax reference to the compilation unit itself, which we can just ignore // for the purposes of declaration rude edits @@ -675,7 +675,7 @@ internal override void ReportDeclarationInsertDeleteRudeEdits(ArrayBuilder p.Name).SequenceEqual(newSymbol.GetParameters().Select(p => p.Name))) { - // TODO: Remove this requirement with https://github.com/dotnet/roslyn/issues/52563 // Explicitly implemented methods must have parameter names that match the compiler generated versions - // exactly otherwise symbol matching won't work for them. + // exactly if the runtime doesn't support updating parameters, otherwise the debugger would show incorrect + // parameter names. // We don't need to worry about parameter types, because if they were different then we wouldn't get here // as this wouldn't be the explicit implementation of a known method. // We don't need to worry about access modifiers because the symbol matching still works, and most of the diff --git a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs index 3c8dd2242cafa..804f14fe9e98b 100644 --- a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs +++ b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs @@ -700,7 +700,7 @@ private void ReportTopLevelSyntacticRudeEdits(ArrayBuilder d /// or . /// The scenarios include moving a type declaration from one file to another and moving a member of a partial type from one partial declaration to another. /// - internal virtual void ReportDeclarationInsertDeleteRudeEdits(ArrayBuilder diagnostics, SyntaxNode oldNode, SyntaxNode newNode, ISymbol oldSymbol, ISymbol newSymbol, CancellationToken cancellationToken) + internal virtual void ReportDeclarationInsertDeleteRudeEdits(ArrayBuilder diagnostics, SyntaxNode oldNode, SyntaxNode newNode, ISymbol oldSymbol, ISymbol newSymbol, EditAndContinueCapabilities capabilities, CancellationToken cancellationToken) { // When a method is moved to a different declaration and its parameters are changed at the same time // the new method symbol key will not resolve to the old one since the parameters are different. @@ -2787,7 +2787,7 @@ private async Task> AnalyzeSemanticsAsync( if (oldSymbol.DeclaringSyntaxReferences.Length == 1) { Contract.ThrowIfNull(oldDeclaration); - ReportDeclarationInsertDeleteRudeEdits(diagnostics, oldDeclaration, newDeclaration, oldSymbol, newSymbol, cancellationToken); + ReportDeclarationInsertDeleteRudeEdits(diagnostics, oldDeclaration, newDeclaration, oldSymbol, newSymbol, capabilities, cancellationToken); if (IsPropertyAccessorDeclarationMatchingPrimaryConstructorParameter(newDeclaration, newContainingType, out var isFirst)) { @@ -2822,7 +2822,7 @@ private async Task> AnalyzeSemanticsAsync( // Compare the old declaration syntax of the symbol with its new declaration and report rude edits // if it changed in any way that's not allowed. - ReportDeclarationInsertDeleteRudeEdits(diagnostics, oldDeclaration, newDeclaration, oldSymbol, newSymbol, cancellationToken); + ReportDeclarationInsertDeleteRudeEdits(diagnostics, oldDeclaration, newDeclaration, oldSymbol, newSymbol, capabilities, cancellationToken); var oldBody = TryGetDeclarationBody(oldDeclaration); if (oldBody != null) @@ -4395,9 +4395,10 @@ private void AddConstructorEdits( ReportMemberBodyUpdateRudeEdits(diagnostics, newDeclaration, firstSpan); } - // When explicitly implementing the copy constructor of a record the parameter name must match for symbol matching to work - // TODO: Remove this requirement with https://github.com/dotnet/roslyn/issues/52563 - if (oldCtor != null && + // When explicitly implementing the copy constructor of a record the parameter name if the runtime doesn't support + // updating parameters, otherwise the debugger would show the incorrect name in the autos/locals/watch window + if (!capabilities.HasFlag(EditAndContinueCapabilities.UpdateParameters) && + oldCtor != null && !isPrimaryRecordConstructor && oldCtor.DeclaringSyntaxReferences.Length == 0 && newCtor.Parameters.Length == 1 &&