Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate BreakpointSpans and fix NREs #59846

Merged
merged 2 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,36 @@ class C
}");
}

[Fact]
public void GetBreakpointSequence_InstanceContructor_NoBody()
{
VerifyAllSpansInDeclaration<ConstructorDeclarationSyntax>(@"
class Class
{
[|Cl$$ass()|]
}");
}

[Fact]
public void GetBreakpointSequence_StaticContructor_NoBody()
{
VerifyAllSpansInDeclaration<ConstructorDeclarationSyntax>(@"
class Class
{
static Cl$$ass()
}");
}

[Fact]
public void GetBreakpointSequence_Method_NoBody()
{
VerifyAllSpansInDeclaration<MethodDeclarationSyntax>(@"
class Class
{
int F$$unction()
}");
}

#region Switch Expression

[Fact]
Expand Down Expand Up @@ -4272,6 +4302,16 @@ public void InstanceConstructor_NoInitializer_BlockBody()
}");
}

[Fact]
public void InstanceConstructor_NoBody()
{
TestSpan(
@"class Class
{
[|Cla$$ss()|]
}");
}

[Fact]
public void InstanceConstructor_NoInitializer_ExpressionBody_All()
{
Expand Down Expand Up @@ -4510,6 +4550,16 @@ public void StaticConstructor_ExpressionBody()
}");
}

[Fact]
public void StaticConstructor_NoBody()
{
TestMissing(
@"class Class
{
static Cla$$ss()
}");
}

[Fact]
public void InstanceConstructorInitializer()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +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.

#nullable disable

using System;
using System.Collections.Generic;
using System.Composition;
Expand All @@ -19,7 +17,7 @@
namespace Microsoft.CodeAnalysis.CSharp.Debugging
{
[ExportLanguageService(typeof(IBreakpointResolutionService), LanguageNames.CSharp), Shared]
internal partial class CSharpBreakpointResolutionService : IBreakpointResolutionService
internal sealed class CSharpBreakpointResolutionService : IBreakpointResolutionService
{
[ImportingConstructor]
[SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
Expand All @@ -30,12 +28,12 @@ public CSharpBreakpointResolutionService()
/// <summary>
/// Returns null if a breakpoint can't be placed at the specified position.
/// </summary>
public async Task<BreakpointResolutionResult> ResolveBreakpointAsync(Document document, TextSpan textSpan, CancellationToken cancellationToken)
public async Task<BreakpointResolutionResult?> ResolveBreakpointAsync(Document document, TextSpan textSpan, CancellationToken cancellationToken)
{
try
{
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
if (!BreakpointSpans.TryGetBreakpointSpan(tree, textSpan.Start, cancellationToken, out var span))
if (tree == null || !BreakpointSpans.TryGetBreakpointSpan(tree, textSpan.Start, cancellationToken, out var span))
{
return null;
}
Expand Down
57 changes: 36 additions & 21 deletions src/Features/CSharp/Portable/EditAndContinue/BreakpointSpans.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Diagnostics;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.EditAndContinue
{
Expand Down Expand Up @@ -131,7 +130,7 @@ private static int GetEndPosition(SyntaxNodeOrToken nodeOrToken)
}
else
{
return nodeOrToken.AsNode().GetLastToken().Span.End;
return nodeOrToken.AsNode()!.GetLastToken().Span.End;
}
}

Expand Down Expand Up @@ -243,6 +242,9 @@ node is SwitchExpressionSyntax switchExpression &&
return property.Initializer.Value.Span;
}

// properties without expression body have accessor list:
Contract.ThrowIfNull(property.AccessorList);

// int P { get [|{|] ... } set { ... } }
// int P { [|get;|] [|set;|] }
return CreateSpanForAccessors(property.AccessorList.Accessors, position);
Expand All @@ -255,6 +257,9 @@ node is SwitchExpressionSyntax switchExpression &&
return indexer.ExpressionBody.Expression.Span;
}

// indexers without expression body have accessor list:
Contract.ThrowIfNull(indexer.AccessorList);

// int this[args] { get [|{|] ... } set { ... } }
return CreateSpanForAccessors(indexer.AccessorList.Accessors, position);

Expand Down Expand Up @@ -308,7 +313,7 @@ node is SwitchExpressionSyntax switchExpression &&
var localFunction = (LocalFunctionStatementSyntax)node;
return (localFunction.Body != null) ?
TryCreateSpanForNode(localFunction.Body, position) :
TryCreateSpanForNode(localFunction.ExpressionBody.Expression, position);
TryCreateSpanForNode(localFunction.ExpressionBody!.Expression, position);

default:
if (node is ExpressionSyntax expression)
Expand All @@ -325,7 +330,7 @@ node is SwitchExpressionSyntax switchExpression &&
}
}

private static TextSpan CreateSpanForConstructorDeclaration(ConstructorDeclarationSyntax constructorSyntax, int position)
private static TextSpan? CreateSpanForConstructorDeclaration(ConstructorDeclarationSyntax constructorSyntax, int position)
{
if (constructorSyntax.ExpressionBody != null &&
position > constructorSyntax.ExpressionBody.ArrowToken.Span.Start)
Expand All @@ -346,7 +351,12 @@ private static TextSpan CreateSpanForConstructorDeclaration(ConstructorDeclarati
return constructorSyntax.ExpressionBody.Expression.Span;
}

return CreateSpan(constructorSyntax.Body.OpenBraceToken);
if (constructorSyntax.Body != null)
{
return CreateSpan(constructorSyntax.Body.OpenBraceToken);
}

return null;
}

// the declaration is the span of the implicit initializer
Expand Down Expand Up @@ -579,18 +589,13 @@ private static SyntaxToken LastNotMissing(SyntaxToken token1, SyntaxToken token2
=> token2.IsMissing ? token1 : token2;

private static TextSpan? TryCreateSpanForVariableDeclaration(VariableDeclarationSyntax declaration, int position)
{
switch (declaration.Parent.Kind())
=> declaration.Parent!.Kind() switch
{
case SyntaxKind.LocalDeclarationStatement:
case SyntaxKind.EventFieldDeclaration:
case SyntaxKind.FieldDeclaration:
// parent node will handle:
return null;
}
// parent node will handle:
SyntaxKind.LocalDeclarationStatement or SyntaxKind.EventFieldDeclaration or SyntaxKind.FieldDeclaration => null,

return TryCreateSpanForVariableDeclaration(declaration, default, default, position);
}
_ => TryCreateSpanForVariableDeclaration(declaration, modifiersOpt: default, semicolonOpt: default, position),
};

private static TextSpan? TryCreateSpanForVariableDeclaration(
VariableDeclarationSyntax variableDeclaration,
Expand Down Expand Up @@ -653,7 +658,7 @@ private static TextSpan CreateSpanForVariableDeclarator(
return default;
}

var variableDeclaration = (VariableDeclarationSyntax)variableDeclarator.Parent;
var variableDeclaration = (VariableDeclarationSyntax)variableDeclarator.Parent!;
if (variableDeclaration.Variables.Count == 1)
{
return CreateSpan(modifiersOpt, variableDeclaration, semicolonOpt);
Expand All @@ -667,7 +672,7 @@ private static TextSpan CreateSpanForVariableDeclarator(
return CreateSpan(variableDeclarator);
}

private static VariableDeclaratorSyntax FindClosestDeclaratorWithInitializer(SeparatedSyntaxList<VariableDeclaratorSyntax> declarators, int position)
private static VariableDeclaratorSyntax? FindClosestDeclaratorWithInitializer(SeparatedSyntaxList<VariableDeclaratorSyntax> declarators, int position)
{
var d = GetItemIndexByPosition(declarators, position);
var i = 0;
Expand Down Expand Up @@ -789,16 +794,26 @@ private static bool IsBreakableExpression(ExpressionSyntax expression)
/// </summary>
internal static TextSpan GetEnvelope(SyntaxNode declaration)
{
if (declaration is VariableDeclaratorSyntax { Parent: { Parent: BaseFieldDeclarationSyntax fieldDeclaration } } variableDeclarator)
if (declaration is VariableDeclaratorSyntax { Parent.Parent: BaseFieldDeclarationSyntax fieldDeclaration } variableDeclarator)
{
return CreateSpanForVariableDeclarator(variableDeclarator, fieldDeclaration.Modifiers, fieldDeclaration.SemicolonToken);
}

if (declaration is ConstructorDeclarationSyntax constructorDeclaration)
{
var firstSpan = CreateSpanForConstructorDeclaration(constructorDeclaration, constructorDeclaration.Identifier.SpanStart);
var lastSpan = ((SyntaxNode)constructorDeclaration.ExpressionBody ?? constructorDeclaration.Body).Span;
return TextSpan.FromBounds(firstSpan.Start, lastSpan.End);
if (firstSpan == null)
{
return default;
}

var constructorBody = (SyntaxNode?)constructorDeclaration.ExpressionBody ?? constructorDeclaration.Body;
if (constructorBody == null)
{
return firstSpan.Value;
}

return TextSpan.FromBounds(firstSpan.Value.Start, constructorBody.Span.End);
}

if (declaration is CompilationUnitSyntax unit && unit.ContainsGlobalStatements())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Namespace Microsoft.CodeAnalysis.VisualBasic.Debugging

<ExportLanguageService(GetType(IBreakpointResolutionService), LanguageNames.VisualBasic), [Shared]>
Partial Friend Class VisualBasicBreakpointResolutionService
Friend NotInheritable Class VisualBasicBreakpointResolutionService
Implements IBreakpointResolutionService

<ImportingConstructor>
Expand All @@ -26,6 +26,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Debugging
Friend Shared Async Function GetBreakpointAsync(document As Document, position As Integer, length As Integer, cancellationToken As CancellationToken) As Task(Of BreakpointResolutionResult)
Try
Dim tree = Await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(False)
If tree Is Nothing Then
Return Nothing
End If

' Non-zero length means that the span is passed by the debugger and we may need validate it.
' In a rare VB case, this span may contain multiple methods, e.g.,
Expand All @@ -42,15 +45,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Debugging
' which happens to have a breakpoint on its head. In this situation, we should attempt to validate the span of the existing method,
' not that of a newly-prepended method.

Dim descendIntoChildren As Func(Of SyntaxNode, Boolean) =
Function(n)
Return (Not n.IsKind(SyntaxKind.ConstructorBlock)) _
AndAlso (Not n.IsKind(SyntaxKind.SubBlock))
End Function

If length > 0 Then
Dim root = Await tree.GetRootAsync(cancellationToken).ConfigureAwait(False)
Dim item = root.DescendantNodes(New TextSpan(position, length), descendIntoChildren:=descendIntoChildren).OfType(Of MethodBlockSyntax).Skip(1).LastOrDefault()

Dim item = root.DescendantNodes(
span:=New TextSpan(position, length),
descendIntoChildren:=Function(n) Not n.IsKind(SyntaxKind.ConstructorBlock, SyntaxKind.SubBlock)).
OfType(Of MethodBlockSyntax).Skip(1).LastOrDefault()

If item IsNot Nothing Then
position = item.SpanStart
End If
Expand Down
2 changes: 0 additions & 2 deletions src/Tools/ExternalAccess/Razor/RazorBreakpointSpans.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +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.

#nullable enable

using System.Threading;
using Microsoft.CodeAnalysis.CSharp.EditAndContinue;
using Microsoft.CodeAnalysis.Text;
Expand Down