From 56487f2d7eb562c95363daa4a6d045fd64ea5ac3 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Mon, 27 Jun 2022 17:59:13 +1000 Subject: [PATCH] Update analyzer to support more deletions --- .../CSharpEditAndContinueAnalyzer.cs | 5 +- .../AbstractEditAndContinueAnalyzer.cs | 58 +++++++++++++++---- .../VisualBasicEditAndContinueAnalyzer.vb | 5 +- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs index bdb4e90debbca..9c208127637c4 100644 --- a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs +++ b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs @@ -1160,7 +1160,7 @@ internal override bool HasBackingField(SyntaxNode propertyOrIndexerDeclaration) => propertyOrIndexerDeclaration.IsKind(SyntaxKind.PropertyDeclaration, out PropertyDeclarationSyntax? propertyDecl) && SyntaxUtilities.HasBackingField(propertyDecl); - internal override bool TryGetAssociatedMemberDeclaration(SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? declaration) + internal override bool TryGetAssociatedMemberDeclaration(SyntaxNode node, EditKind editKind, [NotNullWhen(true)] out SyntaxNode? declaration) { if (node.IsKind(SyntaxKind.Parameter, SyntaxKind.TypeParameter)) { @@ -1169,7 +1169,8 @@ internal override bool TryGetAssociatedMemberDeclaration(SyntaxNode node, [NotNu return true; } - if (node.Parent.IsParentKind(SyntaxKind.PropertyDeclaration, SyntaxKind.IndexerDeclaration, SyntaxKind.EventDeclaration)) + // For deletes, we don't associate accessors with their parents, as deleting accessors is allowed + if (editKind != EditKind.Delete && node.Parent.IsParentKind(SyntaxKind.PropertyDeclaration, SyntaxKind.IndexerDeclaration, SyntaxKind.EventDeclaration)) { declaration = node.Parent.Parent!; return true; diff --git a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs index 777a8ee5ba8b6..6c3f05a325b5e 100644 --- a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs +++ b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs @@ -465,7 +465,7 @@ protected virtual string GetSuspensionPointDisplayName(SyntaxNode node, EditKind /// - a method, an indexer or a type (delegate) if the is a parameter, /// - a method or an type if the is a type parameter. /// - internal abstract bool TryGetAssociatedMemberDeclaration(SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? declaration); + internal abstract bool TryGetAssociatedMemberDeclaration(SyntaxNode node, EditKind editKind, [NotNullWhen(true)] out SyntaxNode? declaration); internal abstract bool HasBackingField(SyntaxNode propertyDeclaration); @@ -2788,7 +2788,7 @@ private async Task> AnalyzeSemanticsAsync( // Associated member declarations must be in the same document as the symbol, so we don't need to resolve their symbol. // In some cases the symbol even can't be resolved unambiguously. Consider e.g. resolving a method with its parameter deleted - // we wouldn't know which overload to resolve to. - if (TryGetAssociatedMemberDeclaration(oldDeclaration, out var oldAssociatedMemberDeclaration)) + if (TryGetAssociatedMemberDeclaration(oldDeclaration, EditKind.Delete, out var oldAssociatedMemberDeclaration)) { if (HasEdit(editMap, oldAssociatedMemberDeclaration, EditKind.Delete)) { @@ -2806,14 +2806,43 @@ private async Task> AnalyzeSemanticsAsync( continue; } - // Deleting an ordinary method is allowed, and we store the newContainingSymbol in NewSymbol for later use - // We don't currently allow deleting virtual or abstract methods, because if those are in the middle of - // an inheritance chain then throwing a missing method exception is not expected - if (oldSymbol is IMethodSymbol { MethodKind: MethodKind.Ordinary, IsExtern: false, ContainingType.TypeKind: TypeKind.Class or TypeKind.Struct } && - oldSymbol.GetSymbolModifiers() is { IsVirtual: false, IsAbstract: false, IsOverride: false }) + if (oldSymbol.GetSymbolModifiers() is { IsVirtual: false, IsAbstract: false, IsOverride: false } && + oldSymbol.ContainingType is { TypeKind: TypeKind.Class or TypeKind.Struct } && + !oldSymbol.IsExtern) { - semanticEdits.Add(new SemanticEditInfo(editKind, symbolKey, syntaxMap, syntaxMapTree: null, partialType: null, deletedSymbolContainer: containingSymbolKey)); - continue; + // Deleting an ordinary method is allowed, and we store the newContainingSymbol in NewSymbol for later use + // We don't currently allow deleting virtual or abstract methods, because if those are in the middle of + // an inheritance chain then throwing a missing method exception is not expected + if (oldSymbol is IMethodSymbol + { + MethodKind: + MethodKind.Ordinary or + MethodKind.Constructor or + MethodKind.EventAdd or + MethodKind.EventRemove or + MethodKind.EventRaise or + MethodKind.Conversion or + MethodKind.UserDefinedOperator or + MethodKind.PropertyGet or + MethodKind.PropertySet + }) + { + AddDeleteSemanticEdit(semanticEdits, oldSymbol, containingSymbolKey, syntaxMap, cancellationToken); + continue; + } + else if (oldSymbol is IPropertySymbol propertySymbol) + { + AddDeleteSemanticEdit(semanticEdits, propertySymbol.GetMethod, containingSymbolKey, syntaxMap, cancellationToken); + AddDeleteSemanticEdit(semanticEdits, propertySymbol.SetMethod, containingSymbolKey, syntaxMap, cancellationToken); + continue; + } + else if (oldSymbol is IEventSymbol eventSymbol) + { + AddDeleteSemanticEdit(semanticEdits, eventSymbol.AddMethod, containingSymbolKey, syntaxMap, cancellationToken); + AddDeleteSemanticEdit(semanticEdits, eventSymbol.RemoveMethod, containingSymbolKey, syntaxMap, cancellationToken); + AddDeleteSemanticEdit(semanticEdits, eventSymbol.RaiseMethod, containingSymbolKey, syntaxMap, cancellationToken); + continue; + } } } @@ -2961,7 +2990,7 @@ private async Task> AnalyzeSemanticsAsync( editKind = SemanticEditKind.Update; } } - else if (TryGetAssociatedMemberDeclaration(newDeclaration, out var newAssociatedMemberDeclaration) && + else if (TryGetAssociatedMemberDeclaration(newDeclaration, EditKind.Insert, out var newAssociatedMemberDeclaration) && HasEdit(editMap, newAssociatedMemberDeclaration, EditKind.Insert)) { // If the symbol is an accessor and the containing property/indexer/event declaration has also been inserted skip @@ -3315,6 +3344,15 @@ private async Task> AnalyzeSemanticsAsync( } } + private static void AddDeleteSemanticEdit(ArrayBuilder semanticEdits, ISymbol? symbol, SymbolKey containingSymbolKey, Func? syntaxMap, CancellationToken cancellationToken) + { + if (symbol is null) + return; + + var symbolKey = SymbolKey.Create(symbol, cancellationToken); + semanticEdits.Add(new SemanticEditInfo(SemanticEditKind.Delete, symbolKey, syntaxMap, syntaxMapTree: null, partialType: null, deletedSymbolContainer: containingSymbolKey)); + } + private ImmutableArray<(ISymbol? oldSymbol, ISymbol? newSymbol, EditKind editKind)> GetNamespaceSymbolEdits( SemanticModel oldModel, SemanticModel newModel, diff --git a/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb b/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb index d0d54f9c54fe5..d11b1e16a4a32 100644 --- a/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb +++ b/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb @@ -1043,14 +1043,15 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EditAndContinue Return node.Parent.FirstAncestorOrSelf(Of TypeBlockSyntax)() ' TODO: EnbumBlock? End Function - Friend Overrides Function TryGetAssociatedMemberDeclaration(node As SyntaxNode, ByRef declaration As SyntaxNode) As Boolean + Friend Overrides Function TryGetAssociatedMemberDeclaration(node As SyntaxNode, editKind As EditKind, ByRef declaration As SyntaxNode) As Boolean If node.IsKind(SyntaxKind.Parameter, SyntaxKind.TypeParameter) Then Contract.ThrowIfFalse(node.IsParentKind(SyntaxKind.ParameterList, SyntaxKind.TypeParameterList)) declaration = node.Parent.Parent Return True End If - If node.IsParentKind(SyntaxKind.PropertyBlock, SyntaxKind.EventBlock) Then + ' We allow deleting event and property accessors, so don't associate them + If editKind <> EditKind.Delete AndAlso node.IsParentKind(SyntaxKind.PropertyBlock, SyntaxKind.EventBlock) Then declaration = node.Parent Return True End If