From 70e4931e95909c86255d4f4a2edff639aa42c21a Mon Sep 17 00:00:00 2001 From: tmat Date: Mon, 22 Jul 2024 17:57:14 -0700 Subject: [PATCH 1/4] EnC: Implement support for partial properties --- .../CSharp/Portable/Symbols/PropertySymbol.cs | 11 +- .../Symbols/Source/SourcePropertySymbol.cs | 10 +- .../EditAndContinue/EditAndContinueTests.cs | 186 +++++--- .../Emit/EditAndContinue/SymbolChanges.cs | 13 +- .../Core/Portable/Emit/SemanticEdit.cs | 9 +- .../Symbols/IPropertySymbolInternal.cs | 17 + .../SymbolKey/SymbolKeyCompilationsTests.cs | 1 - .../CSharpEditAndContinueAnalyzer.cs | 4 + .../EditAndContinue/TopLevelEditingTests.cs | 406 +++++++++++++++++- .../AbstractEditAndContinueAnalyzer.cs | 30 +- .../EditAndContinue/SemanticEditInfo.cs | 22 + .../EditAndContinue/Utilities/Extensions.cs | 33 +- .../EditAndContinueTestVerifier.cs | 9 +- 13 files changed, 658 insertions(+), 93 deletions(-) create mode 100644 src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs diff --git a/src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs index 94d59417b4c61..329beb975e807 100644 --- a/src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs @@ -8,6 +8,7 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Linq; +using Microsoft.CodeAnalysis.Symbols; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -15,7 +16,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols /// /// Represents a property or indexer. /// - internal abstract partial class PropertySymbol : Symbol + internal abstract partial class PropertySymbol : Symbol, IPropertySymbolInternal { /// /// As a performance optimization, cache parameter types and refkinds - overload resolution uses them a lot. @@ -321,6 +322,14 @@ internal virtual bool IsExplicitInterfaceImplementation /// public abstract ImmutableArray ExplicitInterfaceImplementations { get; } +#nullable enable + internal virtual PropertySymbol? PartialImplementationPart => null; + internal virtual PropertySymbol? PartialDefinitionPart => null; + + IPropertySymbolInternal? IPropertySymbolInternal.PartialImplementationPart => PartialImplementationPart; + IPropertySymbolInternal? IPropertySymbolInternal.PartialDefinitionPart => PartialDefinitionPart; +#nullable disable + /// /// Gets the kind of this symbol. /// diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index 2deeefb254519..ddfd658541916 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -179,7 +179,7 @@ public override OneOrMany> GetAttributeDeclarati // This is an error scenario (requires using a property initializer and field-targeted attributes on partial property implementation part). || this.BackingField is not null); - if (PartialImplementationPart is { } implementationPart) + if (SourcePartialImplementationPart is { } implementationPart) { return OneOrMany.Create( ((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists, @@ -191,7 +191,7 @@ public override OneOrMany> GetAttributeDeclarati } } - protected override SourcePropertySymbolBase? BoundAttributesSource => PartialDefinitionPart; + protected override SourcePropertySymbolBase? BoundAttributesSource => SourcePartialDefinitionPart; public override IAttributeTargetSymbol AttributesOwner => this; @@ -727,9 +727,11 @@ private void PartialPropertyChecks(SourcePropertySymbol implementation, BindingD internal bool IsPartialImplementation => IsPartial && (AccessorsHaveImplementation || HasExternModifier); - internal SourcePropertySymbol? PartialDefinitionPart => IsPartialImplementation ? OtherPartOfPartial : null; + internal SourcePropertySymbol? SourcePartialDefinitionPart => IsPartialImplementation ? OtherPartOfPartial : null; + internal SourcePropertySymbol? SourcePartialImplementationPart => IsPartialDefinition ? OtherPartOfPartial : null; - internal SourcePropertySymbol? PartialImplementationPart => IsPartialDefinition ? OtherPartOfPartial : null; + internal override PropertySymbol? PartialDefinitionPart => SourcePartialDefinitionPart; + internal override PropertySymbol? PartialImplementationPart => SourcePartialImplementationPart; internal static void InitializePartialPropertyParts(SourcePropertySymbol definition, SourcePropertySymbol implementation) { diff --git a/src/Compilers/CSharp/Test/Emit2/Emit/EditAndContinue/EditAndContinueTests.cs b/src/Compilers/CSharp/Test/Emit2/Emit/EditAndContinue/EditAndContinueTests.cs index 41ff3e1fd848c..ee02e97546b66 100644 --- a/src/Compilers/CSharp/Test/Emit2/Emit/EditAndContinue/EditAndContinueTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Emit/EditAndContinue/EditAndContinueTests.cs @@ -2567,56 +2567,52 @@ .locals init ([unchanged] V_0, [Fact] public void PartialMethod() { - var source = -@"partial class C -{ - static partial void M1(); - static partial void M2(); - static partial void M3(); - static partial void M1() { } - static partial void M2() { } -}"; - var compilation0 = CreateCompilation(source, parseOptions: TestOptions.Regular.WithNoRefSafetyRulesAttribute(), options: TestOptions.DebugDll); - var compilation1 = compilation0.WithSource(source); - - var bytes0 = compilation0.EmitToArray(); - using var md0 = ModuleMetadata.CreateFromImage(bytes0); - var reader0 = md0.MetadataReader; - - CheckNames(reader0, reader0.GetMethodDefNames(), "M1", "M2", ".ctor"); - - var method0 = compilation0.GetMember("C.M2").PartialImplementationPart; - var method1 = compilation1.GetMember("C.M2").PartialImplementationPart; - - var generation0 = CreateInitialBaseline(compilation0, - md0, - EmptyLocalsProvider); - - var diff1 = compilation1.EmitDifference( - generation0, - ImmutableArray.Create(SemanticEdit.Create(SemanticEditKind.Update, method0, method1))); - - var methods = diff1.TestData.GetMethodsByName(); - Assert.Equal(1, methods.Count); - Assert.True(methods.ContainsKey("C.M2()")); - - using var md1 = diff1.GetMetadata(); - var reader1 = md1.Reader; - var readers = new[] { reader0, reader1 }; - - EncValidation.VerifyModuleMvid(1, reader0, reader1); - - CheckNames(readers, reader1.GetMethodDefNames(), "M2"); + using var _ = new EditAndContinueTest(options: TestOptions.DebugDll) + .AddBaseline( + source: """ + partial class C + { + static partial void M1(); + static partial void M2(); + static partial void M3(); + static partial void M1() { } + static partial void M2() { } + } + """, + validator: v => + { + v.VerifyMethodDefNames("M1", "M2", ".ctor"); + }) + .AddGeneration( + source: """ + partial class C + { + static partial void M1(); + static partial void M2(); + static partial void M3(); + static partial void M1() { } + static partial void M2() { } + } + """, + edits: + [ + Edit(SemanticEditKind.Update, c => c.GetMember("C.M2").PartialImplementationPart), + ], + validator: v => + { + v.VerifyMethodDefNames("M2"); - CheckEncLog(reader1, - Row(2, TableIndex.AssemblyRef, EditAndContinueOperation.Default), - Row(6, TableIndex.TypeRef, EditAndContinueOperation.Default), - Row(2, TableIndex.MethodDef, EditAndContinueOperation.Default)); + v.VerifyEncLogDefinitions( + [ + Row(2, TableIndex.MethodDef, EditAndContinueOperation.Default) + ]); - CheckEncMap(reader1, - Handle(6, TableIndex.TypeRef), - Handle(2, TableIndex.MethodDef), - Handle(2, TableIndex.AssemblyRef)); + v.VerifyEncMapDefinitions( + [ + Handle(2, TableIndex.MethodDef) + ]); + }) + .Verify(); } [WorkItem(60804, "https://github.com/dotnet/roslyn/issues/60804")] @@ -2711,6 +2707,100 @@ .maxstack 8 .Verify(); } + [Fact] + public void PartialProperty() + { + using var _ = new EditAndContinueTest(options: TestOptions.DebugDll) + .AddBaseline( + source: """ + partial class C + { + partial int P { get; } + partial int P => 1; + } + """, + validator: v => + { + v.VerifyMethodDefNames("get_P", ".ctor"); + }) + .AddGeneration( + source: """ + partial class C + { + [System.Obsolete]partial int P { get; } + partial int P => 1; + } + """, + edits: + [ + Edit(SemanticEditKind.Update, c => c.GetMember("C.P").PartialImplementationPart), + ], + validator: v => + { + v.VerifyMethodDefNames(); + + v.VerifyEncLogDefinitions( + [ + Row(1, TableIndex.Property, EditAndContinueOperation.Default), + Row(4, TableIndex.CustomAttribute, EditAndContinueOperation.Default), + Row(2, TableIndex.MethodSemantics, EditAndContinueOperation.Default) + ]); + + v.VerifyEncMapDefinitions( + [ + Handle(4, TableIndex.CustomAttribute), + Handle(1, TableIndex.Property), + Handle(2, TableIndex.MethodSemantics) + ]); + }) + .Verify(); + } + + [Fact] + public void PartialProperty_Accessor() + { + using var _ = new EditAndContinueTest(options: TestOptions.DebugDll) + .AddBaseline( + source: """ + partial class C + { + partial int P { get; } + partial int P => 1; + } + """, + validator: v => + { + v.VerifyMethodDefNames("get_P", ".ctor"); + }) + .AddGeneration( + source: """ + partial class C + { + partial int P { get; } + partial int P => 2; + } + """, + edits: + [ + Edit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart), + ], + validator: v => + { + v.VerifyMethodDefNames("get_P"); + + v.VerifyEncLogDefinitions( + [ + Row(1, TableIndex.MethodDef, EditAndContinueOperation.Default) + ]); + + v.VerifyEncMapDefinitions( + [ + Handle(1, TableIndex.MethodDef) + ]); + }) + .Verify(); + } + [Fact] public void Method_WithAttributes_Add() { diff --git a/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs b/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs index 89c2cf1fd101a..a8c2777cf8d08 100644 --- a/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs +++ b/src/Compilers/Core/Portable/Emit/EditAndContinue/SymbolChanges.cs @@ -412,9 +412,8 @@ private void CalculateChanges( var newMember = GetRequiredInternalSymbol(edit.NewSymbol); - // Partial methods are supplied as implementations but recorded + // Partial methods/properties/indexers are supplied as implementations but recorded // internally as definitions since definitions are used in emit. - // https://github.com/dotnet/roslyn/issues/73772: should we also make sure to use the definition for a partial property? if (newMember.Kind == SymbolKind.Method) { var newMethod = (IMethodSymbolInternal)newMember; @@ -438,6 +437,16 @@ private void CalculateChanges( updatedMethodsPerType.Add((oldMethod.PartialDefinitionPart ?? oldMethod, (IMethodSymbolInternal)newMember)); } } + else if (newMember.Kind == SymbolKind.Property) + { + var newProperty = (IPropertySymbolInternal)newMember; + + // Partial properties should be implementations, not definitions. + Debug.Assert(newProperty.PartialImplementationPart == null); + Debug.Assert(edit.OldSymbol == null || ((IPropertySymbol)edit.OldSymbol).PartialImplementationPart == null); + + newMember = newProperty.PartialDefinitionPart ?? newMember; + } AddContainingSymbolChanges(changesBuilder, newMember); diff --git a/src/Compilers/Core/Portable/Emit/SemanticEdit.cs b/src/Compilers/Core/Portable/Emit/SemanticEdit.cs index 14cec150ebc50..1a55b7db49c21 100644 --- a/src/Compilers/Core/Portable/Emit/SemanticEdit.cs +++ b/src/Compilers/Core/Portable/Emit/SemanticEdit.cs @@ -142,15 +142,14 @@ public SemanticEdit(SemanticEditKind kind, ISymbol? oldSymbol, ISymbol? newSymbo } } - // https://github.com/dotnet/roslyn/issues/73772: should we also do this check for partial properties? - if (oldSymbol is IMethodSymbol { PartialImplementationPart: not null }) + if (oldSymbol is IMethodSymbol { PartialImplementationPart: not null } or IPropertySymbol { PartialImplementationPart: not null }) { - throw new ArgumentException("Partial method implementation required", nameof(oldSymbol)); + throw new ArgumentException("Partial member implementation required", nameof(oldSymbol)); } - if (newSymbol is IMethodSymbol { PartialImplementationPart: not null }) + if (newSymbol is IMethodSymbol { PartialImplementationPart: not null } or IPropertySymbol { PartialImplementationPart: not null }) { - throw new ArgumentException("Partial method implementation required", nameof(newSymbol)); + throw new ArgumentException("Partial member implementation required", nameof(newSymbol)); } if (kind == SemanticEditKind.Delete && oldSymbol is not (IMethodSymbol or IPropertySymbol or IEventSymbol)) diff --git a/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs b/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs new file mode 100644 index 0000000000000..3f79c4e3e88e7 --- /dev/null +++ b/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.CodeAnalysis.Symbols; + +internal interface IPropertySymbolInternal : ISymbolInternal +{ + IPropertySymbolInternal? PartialImplementationPart { get; } + IPropertySymbolInternal? PartialDefinitionPart { get; } +} diff --git a/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs b/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs index b9f1ec2e3ae6c..51430c5d8c820 100644 --- a/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs +++ b/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs @@ -160,7 +160,6 @@ public partial void M() { } var implementation = definition.PartialImplementationPart; // Assert that both the definition and implementation resolve back to themselves - // https://github.com/dotnet/roslyn/issues/73772: add a similar test for properties Assert.Equal(definition, ResolveSymbol(definition, comp, SymbolKeyComparison.None)); Assert.Equal(implementation, ResolveSymbol(implementation, comp, SymbolKeyComparison.None)); } diff --git a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs index a4952cf559764..a6dd322b453bc 100644 --- a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs +++ b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs @@ -1198,6 +1198,10 @@ newContainingMemberOrType is IPropertySymbol newPropertySymbol && // int this[...] { get => expr; } // int P => expr; // int P { get => expr; } = init + // + // Note: An update of a partial property/indexer definition can only affect its attributes. The partial definition does not have a body. + // In that case we do not need to update/analyze the accessors since attribute update has no impact on them. + // // 2) Property/indexer declarations differ in readonly keyword. // 3) Property signature changes // 4) Property name changes diff --git a/src/Features/CSharpTest/EditAndContinue/TopLevelEditingTests.cs b/src/Features/CSharpTest/EditAndContinue/TopLevelEditingTests.cs index 7cf962f9814f8..b09c53ff6a0df 100644 --- a/src/Features/CSharpTest/EditAndContinue/TopLevelEditingTests.cs +++ b/src/Features/CSharpTest/EditAndContinue/TopLevelEditingTests.cs @@ -8485,7 +8485,7 @@ void M(int a, int b, int c) } [Fact] - public void Method_Update_Parameter_Partial() + public void Method_Update_Parameter_Insert_Partial() { var src1 = @" class C @@ -10046,7 +10046,7 @@ public void Method_Partial_DeleteInsert_ImplementationPart() DocumentResults(), DocumentResults(), DocumentResults( - semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C").GetMember("F").PartialImplementationPart, partialType: "C")]), + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.F").PartialImplementationPart, partialType: "C")]), ]); } @@ -10063,9 +10063,9 @@ public void Method_Partial_Swap_ImplementationAndDefinitionParts() [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], [ DocumentResults( - semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C").GetMember("F").PartialImplementationPart, partialType: "C")]), + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.F").PartialImplementationPart, partialType: "C")]), DocumentResults( - semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C").GetMember("F").PartialImplementationPart, partialType: "C")]), + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.F").PartialImplementationPart, partialType: "C")]), ]); } @@ -18783,6 +18783,316 @@ class C(int A, int B) capabilities: EditAndContinueCapabilities.AddMethodToExistingType | EditAndContinueCapabilities.AddInstanceFieldToExistingType); } + [Fact] + public void Property_Partial_DeleteInsert_DefinitionPart() + { + var srcA1 = "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + var srcC1 = "partial class C { }"; + + var srcA2 = "partial class C { }"; + var srcB2 = "partial class C { partial int P => 1; }"; + var srcC2 = "partial class C { partial int P { get; } }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2), GetTopEdits(srcC1, srcC2)], + [ + DocumentResults(), + DocumentResults(), + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C")]), + ]); + } + + [Fact] + public void Property_Partial_DeleteInsert_ImplementationPart() + { + var srcA1 = "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + var srcC1 = "partial class C { }"; + + var srcA2 = "partial class C { partial int P { get; } }"; + var srcB2 = "partial class C { }"; + var srcC2 = "partial class C { partial int P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2), GetTopEdits(srcC1, srcC2)], + [ + DocumentResults(), + DocumentResults(), + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C")]), + ]); + } + + [Fact] + public void Property_Partial_Swap_ImplementationAndDefinitionParts() + { + var srcA1 = "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + + var srcA2 = "partial class C { partial int P => 1; }"; + var srcB2 = "partial class C { partial int P { get; } }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C")]), + DocumentResults(), + ]); + } + + [Fact] + public void Property_Partial_DeleteBoth() + { + var srcA1 = "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + + var srcA2 = "partial class C { }"; + var srcB2 = "partial class C { }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C") + ]), + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C") + ]), + ]); + } + + [Fact] + public void Property_Partial_DeleteInsertBoth() + { + var srcA1 = "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + var srcC1 = "partial class C { }"; + var srcD1 = "partial class C { }"; + + var srcA2 = "partial class C { }"; + var srcB2 = "partial class C { }"; + var srcC2 = "partial class C { partial int P { get; } }"; + var srcD2 = "partial class C { partial int P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2), GetTopEdits(srcC1, srcC2), GetTopEdits(srcD1, srcD2)], + [ + DocumentResults(), + DocumentResults(), + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C")]), + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C")]) + ]); + } + + [Fact] + public void Property_Partial_Insert() + { + var srcA1 = "partial class C { }"; + var srcB1 = "partial class C { }"; + + var srcA2 = "partial class C { partial int P { get; } }"; + var srcB2 = "partial class C { partial int P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults(), + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.P").PartialImplementationPart)]), + ], + capabilities: EditAndContinueCapabilities.AddMethodToExistingType); + } + + [Fact] + public void Property_Partial_Insert_Reloadable() + { + var srcA1 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { }"; + var srcB1 = "partial class C { }"; + + var srcA2 = ReloadableAttributeSrc + "[CreateNewOnMetadataUpdate]partial class C { partial int P { get; } }"; + var srcB2 = "partial class C { partial int P { get => 1; } }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Replace, c => c.GetMember("C"), partialType: "C")]), + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Replace, c => c.GetMember("C"), partialType: "C")]), + ], + capabilities: EditAndContinueCapabilities.NewTypeDefinition); + } + + [Fact] + public void Property_Partial_Update_Attribute_Definition() + { + var attribute = """ + public class A : System.Attribute { public A(int x) {} } + """; + + var srcA1 = attribute + + "partial class C { [A(1)]partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + + var srcA2 = attribute + + "partial class C { [A(2)]partial int P { get; } }"; + var srcB2 = "partial class C { partial int P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C")]), + DocumentResults(), + ], + capabilities: EditAndContinueCapabilities.ChangeCustomAttributes); + } + + [Fact] + public void Property_Partial_Update_Attribute_Implementation() + { + var attribute = """ + public class A : System.Attribute { public A(int x) {} } + """; + + var srcA1 = attribute + + "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { [A(1)]partial int P => 1; }"; + + var srcA2 = attribute + + "partial class C { partial int P { get; } }"; + var srcB2 = "partial class C { [A(2)]partial int P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults(), + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C"), + + // Updating the accessor is superfluous. + // It is added since we see an update to an expression bodied property. We don't distinguish between update to the body and update to an attribute. + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C") + ]), + ], + capabilities: EditAndContinueCapabilities.ChangeCustomAttributes); + } + + [Fact] + public void Property_Partial_Update_Attribute_DefinitionAndImplementation() + { + var attribute = """ + public class A : System.Attribute { public A(int x) {} } + """; + + var srcA1 = attribute + + "partial class C { [A(1)]partial int P { get; } }"; + var srcB1 = "partial class C { [A(1)]partial int P => 1; }"; + + var srcA2 = attribute + + "partial class C { [A(2)]partial int P { get; } }"; + var srcB2 = "partial class C { [A(2)]partial int P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: [SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C")]), + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C") + ]), + ], + capabilities: EditAndContinueCapabilities.ChangeCustomAttributes); + } + + [Fact] + public void Property_Partial_DeleteInsert_DefinitionWithAttributeChange() + { + var attribute = """ + public class A : System.Attribute {} + """; + + var srcA1 = attribute + + "partial class C { [A]partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + + var srcA2 = attribute + + "partial class C { }"; + var srcB2 = "partial class C { partial int P => 1; partial int P { get; } }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults(), + + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Update, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C") + ]), + ], + capabilities: EditAndContinueCapabilities.ChangeCustomAttributes); + } + + [Fact] + public void Property_Partial_Parameter_TypeChange() + { + var srcA1 = "partial class C { partial int P { get; } }"; + var srcB1 = "partial class C { partial int P => 1; }"; + + var srcA2 = "partial class C { partial long P { get; } }"; + var srcB2 = "partial class C { partial long P => 1; }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C"), + ]), + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.P").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.get_P").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.P").PartialImplementationPart, partialType: "C"), + ]), + ], + capabilities: EditAndContinueCapabilities.AddMethodToExistingType); + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + diagnostics: [Diagnostic(RudeEditKind.ChangingSignatureNotSupportedByRuntime, "partial long P", GetResource("property"))]), + DocumentResults( + diagnostics: [Diagnostic(RudeEditKind.ChangingSignatureNotSupportedByRuntime, "partial long P", GetResource("property"))]), + ], + capabilities: EditAndContinueCapabilities.Baseline); + } + #endregion #region Indexers @@ -19337,7 +19647,7 @@ public void Indexer_Update_Type_Stackalloc_WithExpressionBody() } [Fact] - public void Indexer_Parameter_Update_Type() + public void Indexer_Parameter_TypeChange() { var src1 = "class C { int this[byte a] { get => 1; set { } } }"; var src2 = "class C { int this[long a] { get => 1; set { } } }"; @@ -19356,6 +19666,52 @@ public void Indexer_Parameter_Update_Type() capabilities: EditAndContinueCapabilities.AddMethodToExistingType); } + [Fact] + public void Indexer_Partial_Parameter_TypeChange() + { + var srcA1 = "partial class C { partial int this[long x] { get; set; } }"; + var srcB1 = "partial class C { partial int this[long x] { get => 1; set { } } }"; + + var srcA2 = "partial class C { partial int this[byte x] { get; set; } }"; + var srcB2 = "partial class C { partial int this[byte x] { get => 1; set { } } }"; + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.this[]").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_Item").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.set_Item").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.this[]").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.get_Item").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.set_Item").PartialImplementationPart, partialType: "C"), + ]), + DocumentResults( + semanticEdits: + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.this[]").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_Item").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.set_Item").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.this[]").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.get_Item").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.set_Item").PartialImplementationPart, partialType: "C"), + ]), + ], + capabilities: EditAndContinueCapabilities.AddMethodToExistingType); + + EditAndContinueValidation.VerifySemantics( + [GetTopEdits(srcA1, srcA2), GetTopEdits(srcB1, srcB2)], + [ + DocumentResults( + diagnostics: [Diagnostic(RudeEditKind.ChangingSignatureNotSupportedByRuntime, "byte x", GetResource("indexer"))]), + DocumentResults( + diagnostics: [Diagnostic(RudeEditKind.ChangingSignatureNotSupportedByRuntime, "byte x", GetResource("indexer"))]), + ], + capabilities: EditAndContinueCapabilities.Baseline); + } + [Fact] public void Indexer_Parameter_Rename() { @@ -19420,6 +19776,46 @@ public void Indexer_Parameter_Insert() capabilities: EditAndContinueCapabilities.AddMethodToExistingType); } + [Fact] + public void Indexer_Parameter_Insert_Partial() + { + var src1 = """ + class C + { + partial int this[int a] { get; set; } + partial int this[int a] { get => 1; set { } } + } + """; + + var src2 = """ + class C + { + partial int this[int a, int/*1*/b, int c] { get; set; } + partial int this[int a, int/*2*/b, int c] { get => 1; set { } } + } + """; + + var edits = GetTopEdits(src1, src2); + + edits.VerifySemantics( + [ + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.this[]").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.get_Item").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.set_Item").PartialImplementationPart, deletedSymbolContainerProvider: c => c.GetMember("C"), partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.this[]").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.get_Item").PartialImplementationPart, partialType: "C"), + SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("C.set_Item").PartialImplementationPart, partialType: "C"), + ], + capabilities: EditAndContinueCapabilities.AddMethodToExistingType); + + edits.VerifySemanticDiagnostics( + [ + Diagnostic(RudeEditKind.ChangingSignatureNotSupportedByRuntime, "int/*1*/b", GetResource("indexer")), + Diagnostic(RudeEditKind.ChangingSignatureNotSupportedByRuntime, "int/*2*/b", GetResource("indexer")) + ], + capabilities: EditAndContinueCapabilities.Baseline); + } + [Fact] public void Indexer_Parameter_Delete() { diff --git a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs index 769cf5bc507d0..475f739c98a49 100644 --- a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs +++ b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs @@ -2673,9 +2673,9 @@ newSymbol is IPropertySymbol newProperty && break; } - // If a partial method definition is deleted (and not moved to another partial type declaration, which is handled above) + // If a partial method/property/indexer definition is deleted (and not moved to another partial type declaration, which is handled above) // so must be the implementation. An edit will be issued for the implementation change. - if (newSymbol is IMethodSymbol { IsPartialDefinition: true }) + if (newSymbol?.IsPartialDefinition() == true) { continue; } @@ -2823,7 +2823,7 @@ newSymbol is IPropertySymbol newProperty && // If a partial method definition is inserted (and not moved to another partial type declaration, which is handled above) // so must be the implementation. An edit will be issued for the implementation change. - if (newSymbol is IMethodSymbol { IsPartialDefinition: true }) + if (newSymbol.IsPartialDefinition()) { continue; } @@ -3185,8 +3185,7 @@ IFieldSymbol or // The partial type needs to be specified in the following cases: // 1) partial method is updated (in case both implementation and definition are updated) // 2) partial type is updated - // https://github.com/dotnet/roslyn/issues/73772: do we also need to check IPropertySymbol.PartialDefinitionPart here? - var partialType = editKind == SemanticEditKind.Update && symbol is IMethodSymbol { PartialDefinitionPart: not null } + var partialType = editKind == SemanticEditKind.Update && symbol.IsPartialImplementation() ? symbolCache.GetKey(symbol.ContainingType, cancellationToken) : IsPartialTypeEdit(oldSymbol, newSymbol, oldTree, newTree) ? symbolKey @@ -3355,8 +3354,7 @@ bool PreprocessSymbolEdit(ref ISymbol? oldSymbol, ref ISymbol? newSymbol) var result = symbolKey.Resolve(compilation, ignoreAssemblyKey: true, cancellationToken).Symbol; // If we were looking for a definition and an implementation is returned the definition does not exist. - // https://github.com/dotnet/roslyn/issues/73772: Does PartialDefinitionPart also need to be checked here? - return symbol is IMethodSymbol { PartialDefinitionPart: not null } && result is IMethodSymbol { IsPartialDefinition: true } ? null : result; + return symbol.IsPartialImplementation() && result?.IsPartialDefinition() == true ? null : result; } var symbol = newSymbol ?? oldSymbol; @@ -3627,9 +3625,7 @@ void AddUpdate(ISymbol? symbol) if (symbol is null) return; - Debug.Assert(symbol is not IMethodSymbol { IsPartialDefinition: true }); - - semanticEdits.Add(SemanticEditInfo.CreateUpdate(SymbolKey.Create(symbol, cancellationToken), syntaxMaps: default, partialType: null)); + semanticEdits.Add(SemanticEditInfo.CreateUpdate(symbol, syntaxMaps: default, cancellationToken)); } } @@ -3668,11 +3664,7 @@ void AddDelete(ISymbol? symbol) if (symbol is null) return; - // https://github.com/dotnet/roslyn/issues/73772 - Debug.Assert(symbol is not IMethodSymbol { IsPartialDefinition: true }); - - var partialType = symbol is IMethodSymbol { PartialDefinitionPart: not null } ? SymbolKey.Create(symbol.ContainingType, cancellationToken) : (SymbolKey?)null; - semanticEdits.Add(SemanticEditInfo.CreateDelete(SymbolKey.Create(symbol, cancellationToken), deletedSymbolContainer, partialType)); + semanticEdits.Add(SemanticEditInfo.CreateDelete(symbol, deletedSymbolContainer, cancellationToken)); } } @@ -3686,9 +3678,7 @@ private static void AddInsertEditsForMemberAndAccessors(ArrayBuilder new(SemanticEditKind.Insert, symbol, syntaxMaps: default, partialType, deletedSymbolContainer: null); + public static SemanticEditInfo CreateInsert(ISymbol symbol, CancellationToken cancellationToken) + { + Debug.Assert(!symbol.IsPartialDefinition()); + var partialType = symbol.IsPartialImplementation() ? SymbolKey.Create(symbol.ContainingType, cancellationToken) : (SymbolKey?)null; + return CreateInsert(SymbolKey.Create(symbol, cancellationToken), partialType); + } + public static SemanticEditInfo CreateUpdate(SymbolKey symbol, SyntaxMaps syntaxMaps, SymbolKey? partialType) => new(SemanticEditKind.Update, symbol, syntaxMaps, partialType, deletedSymbolContainer: null); + public static SemanticEditInfo CreateUpdate(ISymbol symbol, SyntaxMaps syntaxMaps, CancellationToken cancellationToken) + { + Debug.Assert(!symbol.IsPartialDefinition()); + var partialType = symbol.IsPartialImplementation() ? SymbolKey.Create(symbol.ContainingType, cancellationToken) : (SymbolKey?)null; + return CreateUpdate(SymbolKey.Create(symbol, cancellationToken), syntaxMaps, partialType); + } + public static SemanticEditInfo CreateReplace(SymbolKey symbol, SymbolKey? partialType) => new(SemanticEditKind.Replace, symbol, syntaxMaps: default, partialType, deletedSymbolContainer: null); public static SemanticEditInfo CreateDelete(SymbolKey symbol, SymbolKey deletedSymbolContainer, SymbolKey? partialType) => new(SemanticEditKind.Delete, symbol, syntaxMaps: default, partialType, deletedSymbolContainer); + public static SemanticEditInfo CreateDelete(ISymbol symbol, SymbolKey containingSymbolKey, CancellationToken cancellationToken) + { + Debug.Assert(!symbol.IsPartialDefinition()); + var partialType = symbol.IsPartialImplementation() ? containingSymbolKey : (SymbolKey?)null; + return CreateDelete(SymbolKey.Create(symbol, cancellationToken), containingSymbolKey, partialType); + } + /// /// or or . /// diff --git a/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs b/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs index 0d2226145bc2a..198e84b9c29c2 100644 --- a/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs +++ b/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs @@ -183,9 +183,38 @@ private static bool ParsePrimaryParameterBackingFieldName(string fieldName, [Not => (IMethodSymbol?)constructor.ContainingType.GetMembers(WellKnownMemberNames.DeconstructMethodName).FirstOrDefault( static (symbol, constructor) => symbol is IMethodSymbol method && HasDeconstructorSignature(method, constructor), constructor)?.PartialAsImplementation(); - // https://github.com/dotnet/roslyn/issues/73772: does this helper need to be updated to use IPropertySymbol.PartialImplementationPart? + /// + /// Returns a partial implementation part of a partial member, or the member itself if it's not partial. + /// public static ISymbol PartialAsImplementation(this ISymbol symbol) - => symbol is IMethodSymbol { PartialImplementationPart: { } impl } ? impl : symbol; + => symbol switch + { + IMethodSymbol { PartialImplementationPart: { } impl } => impl, + IPropertySymbol { PartialImplementationPart: { } impl } => impl, + _ => symbol + }; + + public static bool IsPartialDefinition(this ISymbol symbol) + => symbol is IMethodSymbol { IsPartialDefinition: true } or IPropertySymbol { IsPartialDefinition: true }; + + public static bool IsPartialImplementation(this ISymbol symbol) + => symbol is IMethodSymbol { PartialDefinitionPart: not null } or IPropertySymbol { PartialDefinitionPart: not null }; + + public static ISymbol? PartialDefinitionPart(this ISymbol symbol) + => symbol switch + { + IMethodSymbol { PartialDefinitionPart: var def } => def, + IPropertySymbol { PartialDefinitionPart: var def } => def, + _ => symbol + }; + + public static ISymbol? PartialImplementationPart(this ISymbol symbol) + => symbol switch + { + IMethodSymbol { PartialImplementationPart: var impl } => impl, + IPropertySymbol { PartialImplementationPart: var impl } => impl, + _ => symbol + }; /// /// Returns true if any member of the type implements an interface member explicitly. diff --git a/src/Features/TestUtilities/EditAndContinue/EditAndContinueTestVerifier.cs b/src/Features/TestUtilities/EditAndContinue/EditAndContinueTestVerifier.cs index 71932011549d6..6bc6fba2cb68a 100644 --- a/src/Features/TestUtilities/EditAndContinue/EditAndContinueTestVerifier.cs +++ b/src/Features/TestUtilities/EditAndContinue/EditAndContinueTestVerifier.cs @@ -322,12 +322,11 @@ SymbolKey CreateSymbolKey(SemanticEditDescription edit) // Symbol key will happily resolve to a definition part that has no implementation, so we validate that // differently - // https://github.com/dotnet/roslyn/issues/73772: what about deletion of partial property? - if (expectedOldSymbol is IMethodSymbol { IsPartialDefinition: true } && - symbolKey.Resolve(oldCompilation, ignoreAssemblyKey: true).Symbol is IMethodSymbol resolvedMethod) + if (expectedOldSymbol.IsPartialDefinition() && + symbolKey.Resolve(oldCompilation, ignoreAssemblyKey: true).Symbol is ISymbol resolvedSymbol) { - Assert.Equal(expectedOldSymbol, resolvedMethod.PartialDefinitionPart); - Assert.Equal(null, resolvedMethod.PartialImplementationPart); + Assert.Equal(expectedOldSymbol, resolvedSymbol.PartialDefinitionPart()); + Assert.Equal(null, resolvedSymbol.PartialImplementationPart()); } else { From e6748915802688a0f02bb67bb1c2d9c7aaf74e85 Mon Sep 17 00:00:00 2001 From: tmat Date: Mon, 22 Jul 2024 18:38:18 -0700 Subject: [PATCH 2/4] Fix --- .../Symbol/Symbols/PartialPropertiesTests.cs | 6 +++--- .../Symbols/IPropertySymbolInternal.cs | 6 ------ .../Portable/Symbols/PropertySymbol.vb | 20 +++++++++++++++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index d5f299f336a9c..d1a8b34d21ea1 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -924,7 +924,7 @@ .maxstack 2 Assert.True(propDefinition.IsPartialDefinition); var propImplementation = propDefinition.PartialImplementationPart!; - Assert.True(propImplementation.IsPartialImplementation); + Assert.True(propImplementation.IsPartialImplementation()); Assert.Same(propDefinition, propImplementation.PartialDefinitionPart); Assert.Null(propImplementation.PartialImplementationPart); @@ -998,7 +998,7 @@ .maxstack 1 Assert.True(propDefinition.IsPartialDefinition); var propImplementation = propDefinition.PartialImplementationPart!; - Assert.True(propImplementation.IsPartialImplementation); + Assert.True(propImplementation.IsPartialImplementation()); Assert.Same(propDefinition, propImplementation.PartialDefinitionPart); Assert.Null(propImplementation.PartialImplementationPart); @@ -1084,7 +1084,7 @@ .maxstack 1 Assert.True(propDefinition.IsPartialDefinition); var propImplementation = propDefinition.PartialImplementationPart!; - Assert.True(propImplementation.IsPartialImplementation); + Assert.True(propImplementation.IsPartialImplementation()); Assert.Same(propDefinition, propImplementation.PartialDefinitionPart); Assert.Null(propImplementation.PartialImplementationPart); diff --git a/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs b/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs index 3f79c4e3e88e7..06ca49674b661 100644 --- a/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs +++ b/src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs @@ -2,12 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - namespace Microsoft.CodeAnalysis.Symbols; internal interface IPropertySymbolInternal : ISymbolInternal diff --git a/src/Compilers/VisualBasic/Portable/Symbols/PropertySymbol.vb b/src/Compilers/VisualBasic/Portable/Symbols/PropertySymbol.vb index c3e8138ec3749..e88c3d0b985a6 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/PropertySymbol.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/PropertySymbol.vb @@ -2,12 +2,10 @@ ' The .NET Foundation licenses this file to you under the MIT license. ' See the LICENSE file in the project root for more information. -Imports System.Collections.Generic Imports System.Collections.Immutable Imports Microsoft.CodeAnalysis.PooledObjects -Imports Microsoft.CodeAnalysis.Text +Imports Microsoft.CodeAnalysis.Symbols Imports Microsoft.CodeAnalysis.VisualBasic.Symbols -Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols ''' @@ -15,7 +13,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols ''' Friend MustInherit Class PropertySymbol Inherits Symbol - Implements IPropertySymbol + Implements IPropertySymbol, IPropertySymbolInternal ' !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ' Changes to the public interface of this class should remain synchronized with the C# version. @@ -642,6 +640,20 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols End Get End Property + Public ReadOnly Property IPropertySymbolInternal_PartialImplementationPart As IPropertySymbolInternal Implements IPropertySymbolInternal.PartialImplementationPart + Get + ' Feature not supported in VB + Return Nothing + End Get + End Property + + Public ReadOnly Property IPropertySymbolInternal_PartialDefinitionPart As IPropertySymbolInternal Implements IPropertySymbolInternal.PartialDefinitionPart + Get + ' Feature not supported in VB + Return Nothing + End Get + End Property + Public Overrides Sub Accept(visitor As SymbolVisitor) visitor.VisitProperty(Me) End Sub From 398ead4c25e2658787d8de1df0230a8518552171 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 23 Jul 2024 08:28:36 -0700 Subject: [PATCH 3/4] Feedback --- .../Core/Portable/EditAndContinue/Utilities/Extensions.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs b/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs index 198e84b9c29c2..4f919e9639eb5 100644 --- a/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs +++ b/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs @@ -187,12 +187,7 @@ private static bool ParsePrimaryParameterBackingFieldName(string fieldName, [Not /// Returns a partial implementation part of a partial member, or the member itself if it's not partial. /// public static ISymbol PartialAsImplementation(this ISymbol symbol) - => symbol switch - { - IMethodSymbol { PartialImplementationPart: { } impl } => impl, - IPropertySymbol { PartialImplementationPart: { } impl } => impl, - _ => symbol - }; + => PartialImplementationPart(symbol) ?? symbol; public static bool IsPartialDefinition(this ISymbol symbol) => symbol is IMethodSymbol { IsPartialDefinition: true } or IPropertySymbol { IsPartialDefinition: true }; From 2aff403c22b1fa54bcef59d72225d6fa3d7dc9f1 Mon Sep 17 00:00:00 2001 From: tmat Date: Mon, 29 Jul 2024 08:24:10 -0700 Subject: [PATCH 4/4] Feedback --- .../CSharp/Portable/Symbols/PublicModel/PropertySymbol.cs | 4 ++-- .../Core/Portable/EditAndContinue/Utilities/Extensions.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/PublicModel/PropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/PublicModel/PropertySymbol.cs index 3072bfb1875da..4179ba6c272b9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/PublicModel/PropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/PublicModel/PropertySymbol.cs @@ -110,9 +110,9 @@ ImmutableArray IPropertySymbol.RefCustomModifiers RefKind IPropertySymbol.RefKind => _underlying.RefKind; #nullable enable - IPropertySymbol? IPropertySymbol.PartialDefinitionPart => (_underlying as SourcePropertySymbol)?.PartialDefinitionPart.GetPublicSymbol(); + IPropertySymbol? IPropertySymbol.PartialDefinitionPart => _underlying.PartialDefinitionPart.GetPublicSymbol(); - IPropertySymbol? IPropertySymbol.PartialImplementationPart => (_underlying as SourcePropertySymbol)?.PartialImplementationPart.GetPublicSymbol(); + IPropertySymbol? IPropertySymbol.PartialImplementationPart => _underlying.PartialImplementationPart.GetPublicSymbol(); bool IPropertySymbol.IsPartialDefinition => (_underlying as SourcePropertySymbol)?.IsPartialDefinition ?? false; #nullable disable diff --git a/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs b/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs index 4f919e9639eb5..6df922b2c8d55 100644 --- a/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs +++ b/src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs @@ -200,7 +200,7 @@ public static bool IsPartialImplementation(this ISymbol symbol) { IMethodSymbol { PartialDefinitionPart: var def } => def, IPropertySymbol { PartialDefinitionPart: var def } => def, - _ => symbol + _ => null }; public static ISymbol? PartialImplementationPart(this ISymbol symbol) @@ -208,7 +208,7 @@ public static bool IsPartialImplementation(this ISymbol symbol) { IMethodSymbol { PartialImplementationPart: var impl } => impl, IPropertySymbol { PartialImplementationPart: var impl } => impl, - _ => symbol + _ => null }; ///