Skip to content

Commit 63c55df

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Fix crash when determining if on header of invalid if-statement (#78757)
2 parents ffb4dcc + 2731268 commit 63c55df

File tree

5 files changed

+81
-141
lines changed

5 files changed

+81
-141
lines changed

‎src/EditorFeatures/CSharpTest/RefactoringHelpers/RefactoringHelpersTests.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,6 +1722,32 @@ void Goo()
17221722
""");
17231723
}
17241724

1725+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78749")]
1726+
public async Task TestMalformedIfBlock1()
1727+
{
1728+
await TestAsync<IfStatementSyntax>(
1729+
"""
1730+
{
1731+
{|result:[||]if (devsBad)
1732+
[crash]
1733+
else return;|}
1734+
}
1735+
""");
1736+
}
1737+
1738+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78749")]
1739+
public async Task TestMalformedIfBlock2()
1740+
{
1741+
await TestAsync<IfStatementSyntax>(
1742+
"""
1743+
{
1744+
if (devsBad)
1745+
{|result:[|[crash]
1746+
else return;|]|}
1747+
}
1748+
""");
1749+
}
1750+
17251751
#endregion
17261752

17271753
#region Test Deep in expression

‎src/EditorFeatures/TestUtilities/RefactoringHelpers/RefactoringHelpersTestBase.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ protected async Task TestAsync<TNode>(string text, Func<TNode, bool> predicate,
4141
protected async Task TestUnderselectedAsync<TNode>(string text) where TNode : SyntaxNode
4242
{
4343
text = GetSelectionSpan(text, out var selection);
44-
var resultNode = await GetNodeForSelectionAsync<TNode>(text, selection, Functions<TNode>.True).ConfigureAwait(false);
44+
var resultNode = await GetNodeForSelectionAsync(text, selection, Functions<TNode>.True).ConfigureAwait(false);
4545

4646
Assert.NotNull(resultNode);
4747
Assert.True(CodeRefactoringHelpers.IsNodeUnderselected(resultNode, selection));
@@ -50,7 +50,7 @@ protected async Task TestUnderselectedAsync<TNode>(string text) where TNode : Sy
5050
protected async Task TestNotUnderselectedAsync<TNode>(string text) where TNode : SyntaxNode
5151
{
5252
text = GetSelectionAndResultSpans(text, out var selection, out var result);
53-
var resultNode = await GetNodeForSelectionAsync<TNode>(text, selection, Functions<TNode>.True).ConfigureAwait(false);
53+
var resultNode = await GetNodeForSelectionAsync(text, selection, Functions<TNode>.True).ConfigureAwait(false);
5454

5555
Assert.NotNull(resultNode);
5656
Assert.Equal(result, resultNode!.Span);
@@ -64,7 +64,7 @@ protected async Task TestMissingAsync<TNode>(string text, Func<TNode, bool> pred
6464
{
6565
text = GetSelectionSpan(text, out var selection);
6666

67-
var resultNode = await GetNodeForSelectionAsync<TNode>(text, selection, predicate, allowEmptyNodes).ConfigureAwait(false);
67+
var resultNode = await GetNodeForSelectionAsync(text, selection, predicate, allowEmptyNodes).ConfigureAwait(false);
6868
Assert.Null(resultNode);
6969
}
7070

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpHeaderFacts.cs‎

Lines changed: 19 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,8 @@ protected CSharpHeaderFacts()
2323

2424
public override bool IsOnTypeHeader(SyntaxNode root, int position, bool fullHeader, [NotNullWhen(true)] out SyntaxNode? typeDeclaration)
2525
{
26-
var node = TryGetAncestorForLocation<BaseTypeDeclarationSyntax>(root, position);
27-
typeDeclaration = node;
28-
if (node == null)
29-
return false;
30-
31-
return IsOnHeader(root, position, node, GetLastToken());
26+
var node = TryGetAncestorForLocation<BaseTypeDeclarationSyntax>(root, position, out typeDeclaration);
27+
return node != null && IsOnHeader(root, position, node, GetLastToken());
3228

3329
SyntaxToken GetLastToken()
3430
{
@@ -53,107 +49,51 @@ SyntaxToken GetLastToken()
5349

5450
public override bool IsOnPropertyDeclarationHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? propertyDeclaration)
5551
{
56-
var node = TryGetAncestorForLocation<PropertyDeclarationSyntax>(root, position);
57-
propertyDeclaration = node;
58-
if (propertyDeclaration == null)
59-
{
60-
return false;
61-
}
62-
63-
RoslynDebug.AssertNotNull(node);
64-
return IsOnHeader(root, position, node, node.Identifier);
52+
var node = TryGetAncestorForLocation<PropertyDeclarationSyntax>(root, position, out propertyDeclaration);
53+
return node != null && IsOnHeader(root, position, node, node.Identifier);
6554
}
6655

6756
public override bool IsOnParameterHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? parameter)
6857
{
69-
var node = TryGetAncestorForLocation<ParameterSyntax>(root, position);
70-
parameter = node;
71-
if (parameter == null)
72-
{
73-
return false;
74-
}
75-
76-
RoslynDebug.AssertNotNull(node);
77-
return IsOnHeader(root, position, node, node);
58+
var node = TryGetAncestorForLocation<ParameterSyntax>(root, position, out parameter);
59+
return node != null && IsOnHeader(root, position, node, node);
7860
}
7961

8062
public override bool IsOnMethodHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? method)
8163
{
82-
var node = TryGetAncestorForLocation<MethodDeclarationSyntax>(root, position);
83-
method = node;
84-
if (method == null)
85-
{
86-
return false;
87-
}
88-
89-
RoslynDebug.AssertNotNull(node);
90-
return IsOnHeader(root, position, node, node.ParameterList);
64+
var node = TryGetAncestorForLocation<MethodDeclarationSyntax>(root, position, out method);
65+
return node != null && IsOnHeader(root, position, node, node.ParameterList);
9166
}
9267

9368
public override bool IsOnLocalFunctionHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? localFunction)
9469
{
95-
var node = TryGetAncestorForLocation<LocalFunctionStatementSyntax>(root, position);
96-
localFunction = node;
97-
if (localFunction == null)
98-
{
99-
return false;
100-
}
101-
102-
RoslynDebug.AssertNotNull(node);
103-
return IsOnHeader(root, position, node, node.ParameterList);
70+
var node = TryGetAncestorForLocation<LocalFunctionStatementSyntax>(root, position, out localFunction);
71+
return node != null && IsOnHeader(root, position, node, node.ParameterList);
10472
}
10573

10674
public override bool IsOnLocalDeclarationHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? localDeclaration)
10775
{
108-
var node = TryGetAncestorForLocation<LocalDeclarationStatementSyntax>(root, position);
109-
localDeclaration = node;
110-
if (localDeclaration == null)
111-
{
112-
return false;
113-
}
114-
115-
var initializersExpressions = node!.Declaration.Variables
76+
var node = TryGetAncestorForLocation<LocalDeclarationStatementSyntax>(root, position, out localDeclaration);
77+
return node != null && IsOnHeader(root, position, node, node, holes: node.Declaration.Variables
11678
.Where(v => v.Initializer != null)
117-
.SelectAsArray(initializedV => initializedV.Initializer!.Value);
118-
return IsOnHeader(root, position, node, node, holes: initializersExpressions);
79+
.SelectAsArray(initializedV => initializedV.Initializer!.Value));
11980
}
12081

12182
public override bool IsOnIfStatementHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? ifStatement)
12283
{
123-
var node = TryGetAncestorForLocation<IfStatementSyntax>(root, position);
124-
ifStatement = node;
125-
if (ifStatement == null)
126-
{
127-
return false;
128-
}
129-
130-
RoslynDebug.AssertNotNull(node);
131-
return IsOnHeader(root, position, node, node.CloseParenToken);
84+
var node = TryGetAncestorForLocation<IfStatementSyntax>(root, position, out ifStatement);
85+
return node != null && IsOnHeader(root, position, node, node.CloseParenToken);
13286
}
13387

13488
public override bool IsOnWhileStatementHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? whileStatement)
13589
{
136-
var node = TryGetAncestorForLocation<WhileStatementSyntax>(root, position);
137-
whileStatement = node;
138-
if (whileStatement == null)
139-
{
140-
return false;
141-
}
142-
143-
RoslynDebug.AssertNotNull(node);
144-
return IsOnHeader(root, position, node, node.CloseParenToken);
90+
var node = TryGetAncestorForLocation<WhileStatementSyntax>(root, position, out whileStatement);
91+
return node != null && IsOnHeader(root, position, node, node.CloseParenToken);
14592
}
14693

14794
public override bool IsOnForeachHeader(SyntaxNode root, int position, [NotNullWhen(true)] out SyntaxNode? foreachStatement)
14895
{
149-
var node = TryGetAncestorForLocation<ForEachStatementSyntax>(root, position);
150-
foreachStatement = node;
151-
if (foreachStatement == null)
152-
{
153-
return false;
154-
}
155-
156-
RoslynDebug.AssertNotNull(node);
157-
return IsOnHeader(root, position, node, node.CloseParenToken);
96+
var node = TryGetAncestorForLocation<ForEachStatementSyntax>(root, position, out foreachStatement);
97+
return node != null && IsOnHeader(root, position, node, node.CloseParenToken);
15898
}
15999
}

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/HeaderFacts/AbstractHeaderFacts.cs‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,17 @@ public bool IsOnHeader<THoleSyntax>(
3838
{
3939
Debug.Assert(ownerOfHeader.FullSpan.Contains(lastTokenOrNodeOfHeader.Span));
4040

41-
var headerSpan = TextSpan.FromBounds(
42-
start: GetStartOfNodeExcludingAttributes(root, ownerOfHeader),
43-
end: lastTokenOrNodeOfHeader.FullSpan.End);
41+
// In error cases, we may have a full missing header, followed by attributes. For example:
42+
//
43+
// [X] else { }
44+
//
45+
// This will be an if-statement where the `if(...)` part is entirely missing. In that case, just bail out
46+
// as we aren't likely to produce a reasonable result here.
47+
var startAfterAttributes = GetStartOfNodeExcludingAttributes(root, ownerOfHeader);
48+
if (startAfterAttributes > lastTokenOrNodeOfHeader.FullSpan.End)
49+
return false;
50+
51+
var headerSpan = TextSpan.FromBounds(startAfterAttributes, lastTokenOrNodeOfHeader.FullSpan.End);
4452

4553
// Is in header check is inclusive, being on the end edge of an header still counts
4654
if (!headerSpan.IntersectsWith(position))
@@ -63,22 +71,26 @@ public bool IsOnHeader<THoleSyntax>(
6371
/// Tries to get an ancestor of a Token on current position or of Token directly to left:
6472
/// e.g.: tokenWithWantedAncestor[||]tokenWithoutWantedAncestor
6573
/// </summary>
66-
protected TNode? TryGetAncestorForLocation<TNode>(SyntaxNode root, int position) where TNode : SyntaxNode
74+
protected TNode? TryGetAncestorForLocation<TNode>(SyntaxNode root, int position, out SyntaxNode? untypedResult) where TNode : SyntaxNode
6775
{
6876
var tokenToRightOrIn = root.FindToken(position);
6977
var nodeToRightOrIn = tokenToRightOrIn.GetAncestor<TNode>();
7078
if (nodeToRightOrIn != null)
7179
{
80+
untypedResult = nodeToRightOrIn;
7281
return nodeToRightOrIn;
7382
}
7483

7584
// not at the beginning of a Token -> no (different) token to the left
7685
if (tokenToRightOrIn.FullSpan.Start != position && tokenToRightOrIn.RawKind != SyntaxFacts.SyntaxKinds.EndOfFileToken)
7786
{
87+
untypedResult = null;
7888
return null;
7989
}
8090

81-
return tokenToRightOrIn.GetPreviousToken().GetAncestor<TNode>();
91+
var result = tokenToRightOrIn.GetPreviousToken().GetAncestor<TNode>();
92+
untypedResult = result;
93+
return result;
8294
}
8395

8496
protected int GetStartOfNodeExcludingAttributes(SyntaxNode root, SyntaxNode node)

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicHeaderFacts.vb‎

Lines changed: 16 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
2222
position As Integer,
2323
fullHeader As Boolean,
2424
ByRef typeDeclaration As SyntaxNode) As Boolean
25-
Dim typeBlock = TryGetAncestorForLocation(Of TypeBlockSyntax)(root, position)
25+
Dim typeBlock = TryGetAncestorForLocation(Of TypeBlockSyntax)(root, position, typeDeclaration)
2626
If typeBlock Is Nothing Then
2727
Return Nothing
2828
End If
2929

3030
Dim typeStatement = typeBlock.BlockStatement
31-
typeDeclaration = typeBlock
3231

3332
Dim lastToken = If(typeStatement.TypeParameterList?.GetLastToken(), typeStatement.Identifier)
3433
If fullHeader Then
@@ -41,35 +40,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
4140
End Function
4241

4342
Public Overrides Function IsOnPropertyDeclarationHeader(root As SyntaxNode, position As Integer, ByRef propertyDeclaration As SyntaxNode) As Boolean
44-
Dim node = TryGetAncestorForLocation(Of PropertyStatementSyntax)(root, position)
45-
propertyDeclaration = node
43+
Dim node = TryGetAncestorForLocation(Of PropertyStatementSyntax)(root, position, propertyDeclaration)
4644

47-
If propertyDeclaration Is Nothing Then
48-
Return False
49-
End If
50-
51-
If node.AsClause IsNot Nothing Then
45+
If node?.AsClause IsNot Nothing Then
5246
Return IsOnHeader(root, position, node, node.AsClause)
5347
End If
5448

55-
Return IsOnHeader(root, position, node, node.Identifier)
49+
Return node IsNot Nothing AndAlso IsOnHeader(root, position, node, node.Identifier)
5650
End Function
5751

5852
Public Overrides Function IsOnParameterHeader(root As SyntaxNode, position As Integer, ByRef parameter As SyntaxNode) As Boolean
59-
Dim node = TryGetAncestorForLocation(Of ParameterSyntax)(root, position)
60-
parameter = node
61-
62-
If parameter Is Nothing Then
63-
Return False
64-
End If
65-
66-
Return IsOnHeader(root, position, node, node)
53+
Dim node = TryGetAncestorForLocation(Of ParameterSyntax)(root, position, parameter)
54+
Return node IsNot Nothing AndAlso IsOnHeader(root, position, node, node)
6755
End Function
6856

6957
Public Overrides Function IsOnMethodHeader(root As SyntaxNode, position As Integer, ByRef method As SyntaxNode) As Boolean
70-
Dim node = TryGetAncestorForLocation(Of MethodStatementSyntax)(root, position)
71-
method = node
72-
58+
Dim node = TryGetAncestorForLocation(Of MethodStatementSyntax)(root, position, method)
7359
If method Is Nothing Then
7460
Return False
7561
End If
@@ -91,58 +77,34 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
9177
End Function
9278

9379
Public Overrides Function IsOnLocalDeclarationHeader(root As SyntaxNode, position As Integer, ByRef localDeclaration As SyntaxNode) As Boolean
94-
Dim node = TryGetAncestorForLocation(Of LocalDeclarationStatementSyntax)(root, position)
95-
localDeclaration = node
96-
97-
If localDeclaration Is Nothing Then
98-
Return False
99-
End If
100-
101-
Dim initializersExpressions = node.Declarators.
80+
Dim node = TryGetAncestorForLocation(Of LocalDeclarationStatementSyntax)(root, position, localDeclaration)
81+
Return node IsNot Nothing AndAlso IsOnHeader(root, position, node, node, node.Declarators.
10282
Where(Function(d) d.Initializer IsNot Nothing).
103-
SelectAsArray(Function(initialized) initialized.Initializer.Value)
104-
Return IsOnHeader(root, position, node, node, initializersExpressions)
83+
SelectAsArray(Function(initialized) initialized.Initializer.Value))
10584
End Function
10685

10786
Public Overrides Function IsOnIfStatementHeader(root As SyntaxNode, position As Integer, ByRef ifStatement As SyntaxNode) As Boolean
108-
ifStatement = Nothing
109-
110-
Dim multipleLineNode = TryGetAncestorForLocation(Of MultiLineIfBlockSyntax)(root, position)
87+
Dim multipleLineNode = TryGetAncestorForLocation(Of MultiLineIfBlockSyntax)(root, position, ifStatement)
11188
If multipleLineNode IsNot Nothing Then
112-
ifStatement = multipleLineNode
11389
Return IsOnHeader(root, position, multipleLineNode.IfStatement, multipleLineNode.IfStatement)
11490
End If
11591

116-
Dim singleLineNode = TryGetAncestorForLocation(Of SingleLineIfStatementSyntax)(root, position)
92+
Dim singleLineNode = TryGetAncestorForLocation(Of SingleLineIfStatementSyntax)(root, position, ifStatement)
11793
If singleLineNode IsNot Nothing Then
118-
ifStatement = singleLineNode
11994
Return IsOnHeader(root, position, singleLineNode, singleLineNode.Condition)
12095
End If
12196

12297
Return False
12398
End Function
12499

125100
Public Overrides Function IsOnWhileStatementHeader(root As SyntaxNode, position As Integer, ByRef whileStatement As SyntaxNode) As Boolean
126-
whileStatement = Nothing
127-
128-
Dim whileBlock = TryGetAncestorForLocation(Of WhileBlockSyntax)(root, position)
129-
If whileBlock IsNot Nothing Then
130-
whileStatement = whileBlock
131-
Return IsOnHeader(root, position, whileBlock.WhileStatement, whileBlock.WhileStatement)
132-
End If
133-
134-
Return False
101+
Dim whileBlock = TryGetAncestorForLocation(Of WhileBlockSyntax)(root, position, whileStatement)
102+
Return whileBlock IsNot Nothing AndAlso IsOnHeader(root, position, whileBlock.WhileStatement, whileBlock.WhileStatement)
135103
End Function
136104

137105
Public Overrides Function IsOnForeachHeader(root As SyntaxNode, position As Integer, ByRef foreachStatement As SyntaxNode) As Boolean
138-
Dim node = TryGetAncestorForLocation(Of ForEachBlockSyntax)(root, position)
139-
foreachStatement = node
140-
141-
If foreachStatement Is Nothing Then
142-
Return False
143-
End If
144-
145-
Return IsOnHeader(root, position, node, node.ForEachStatement)
106+
Dim node = TryGetAncestorForLocation(Of ForEachBlockSyntax)(root, position, foreachStatement)
107+
Return node IsNot Nothing AndAlso IsOnHeader(root, position, node, node.ForEachStatement)
146108
End Function
147109
End Class
148110
End Namespace

0 commit comments

Comments
 (0)