Skip to content

Commit

Permalink
Update analyzer to support more deletions
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidwengier
davidwengier committed Jun 27, 2022
1 parent 3307cb1 commit 56487f2
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 14 deletions.
5 changes: 3 additions & 2 deletions src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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;
Expand Down
58 changes: 48 additions & 10 deletions src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ protected virtual string GetSuspensionPointDisplayName(SyntaxNode node, EditKind
/// - a method, an indexer or a type (delegate) if the <paramref name="node"/> is a parameter,
/// - a method or an type if the <paramref name="node"/> is a type parameter.
/// </summary>
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);

Expand Down Expand Up @@ -2788,7 +2788,7 @@ private async Task<ImmutableArray<SemanticEditInfo>> 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))
{
Expand All @@ -2806,14 +2806,43 @@ private async Task<ImmutableArray<SemanticEditInfo>> 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;
}
}
}

Expand Down Expand Up @@ -2961,7 +2990,7 @@ private async Task<ImmutableArray<SemanticEditInfo>> 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
Expand Down Expand Up @@ -3315,6 +3344,15 @@ private async Task<ImmutableArray<SemanticEditInfo>> AnalyzeSemanticsAsync(
}
}

private static void AddDeleteSemanticEdit(ArrayBuilder<SemanticEditInfo> semanticEdits, ISymbol? symbol, SymbolKey containingSymbolKey, Func<SyntaxNode, SyntaxNode?>? 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,
Expand Down
5 changes: 3 additions & 2 deletions src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, <Out> ByRef declaration As SyntaxNode) As Boolean
Friend Overrides Function TryGetAssociatedMemberDeclaration(node As SyntaxNode, editKind As EditKind, <Out> 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
Expand Down

0 comments on commit 56487f2

Please sign in to comment.