Skip to content

Commit

Permalink
Merge pull request #19489 from alrz/bug-19175
Browse files Browse the repository at this point in the history
Fix bug when caret is at the end of an argument
  • Loading branch information
sharwell authored May 17, 2017
2 parents 3fb20b5 + f907560 commit f9f2b17
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,36 @@ class C
{
void M(Action arg1, int arg2)
=> M([|1 + 2|], 3);
}");
}

[WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)]
public async Task TestCaretPositionAtTheEnd1()
{
await TestInRegularAndScriptAsync(
@"class C
{
void M(int arg1) => M(arg1[||]);
}",
@"class C
{
void M(int arg1) => M(arg1: arg1);
}");
}

[WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)]
public async Task TestCaretPositionAtTheEnd2()
{
await TestInRegularAndScriptAsync(
@"class C
{
void M(int arg1, int arg2) => M(arg1[||], arg2);
}",
@"class C
{
void M(int arg1, int arg2) => M(arg1: arg1, arg2: arg2);
}");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,71 @@ Class C
Inherits System.Attribute
Public Sub New(arg As Integer)
End Sub
End Class")
End Function

<WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")>
<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)>
Public Async Function TestCaretPositionAtTheEnd1() As Task
Await TestInRegularAndScriptAsync(
"Class C
Sub M(arg1 As Integer)
M(arg1[||])
End Sub
End Class",
"Class C
Sub M(arg1 As Integer)
M(arg1:=arg1)
End Sub
End Class")
End Function

<WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")>
<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)>
Public Async Function TestCaretPositionAtTheEnd2() As Task
Await TestInRegularAndScriptAsync(
"Class C
Sub M(arg1 As Integer, arg2 As Integer)
M(arg1[||], arg2)
End Sub
End Class",
"Class C
Sub M(arg1 As Integer, arg2 As Integer)
M(arg1:=arg1, arg2:=arg2)
End Sub
End Class")
End Function

<WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")>
<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)>
Public Async Function TestCaretPositionAtTheEnd3() As Task
Await TestMissingInRegularAndScriptAsync(
"Class C
Sub M(arg1 As Integer, optional arg2 As Integer=1, optional arg3 as Integer=1)
M(1,[||],3)
End Sub
End Class")
End Function

<WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")>
<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)>
Public Async Function TestCaretPositionAtTheEnd4() As Task
Await TestMissingInRegularAndScriptAsync(
"Class C
Sub M(arg1 As Integer, optional arg2 As Integer=1, optional arg3 as Integer=1)
M(1[||],,3)
End Sub
End Class")
End Function

<WorkItem(19175, "https://github.com/dotnet/roslyn/issues/19175")>
<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseNamedArguments)>
Public Async Function TestCaretPositionAtTheEnd5() As Task
Await TestMissingInRegularAndScriptAsync(
"Class C
Function M(arg1 As Integer, optional arg2 As Integer=1, optional arg3 as Integer=1) As Integer
M(1, M(1,[||], 3))
End Function
End Class")
End Function
End Class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ private abstract class BaseAnalyzer<TSyntax, TSyntaxList> : Analyzer<TSyntax, TS
where TSyntax : SyntaxNode
where TSyntaxList : SyntaxNode
{
protected override SyntaxNode GetReceiver(SyntaxNode argument)
protected sealed override SyntaxNode GetReceiver(SyntaxNode argument)
=> argument.Parent.Parent;

protected override bool IsLegalToAddNamedArguments(ImmutableArray<IParameterSymbol> parameters, int argumentCount)
protected sealed override bool IsLegalToAddNamedArguments(ImmutableArray<IParameterSymbol> parameters, int argumentCount)
=> !parameters.Last().IsParams || parameters.Length >= argumentCount;

protected sealed override bool IsCloseParenOrComma(SyntaxToken token)
=> token.IsKind(SyntaxKind.CloseParenToken, SyntaxKind.CommaToken);
}

private class ArgumentAnalyzer :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,19 @@ public async Task ComputeRefactoringsAsync(
return;
}

var argument = root.FindNode(context.Span).FirstAncestorOrSelf<TBaseArgumentSyntax>() as TArgumentSyntax;
var position = context.Span.Start;
var token = root.FindToken(position);
if (token.Span.Start == position &&
IsCloseParenOrComma(token))
{
token = token.GetPreviousToken();
if (token.Span.End != position)
{
return;
}
}

var argument = root.FindNode(token.Span).FirstAncestorOrSelfUntil<TBaseArgumentSyntax>(node => node is TArgumentListSyntax) as TArgumentSyntax;
if (argument == null)
{
return;
Expand All @@ -51,7 +63,7 @@ public async Task ComputeRefactoringsAsync(
var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);

var argumentStartLine = sourceText.Lines.GetLineFromPosition(argument.Span.Start).LineNumber;
var caretLine = sourceText.Lines.GetLineFromPosition(context.Span.Start).LineNumber;
var caretLine = sourceText.Lines.GetLineFromPosition(position).LineNumber;

if (argumentStartLine != caretLine)
{
Expand Down Expand Up @@ -141,6 +153,7 @@ private TArgumentListSyntax GetOrSynthesizeNamedArguments(
protected abstract TArgumentListSyntax WithArguments(
TArgumentListSyntax argumentList, IEnumerable<TBaseArgumentSyntax> namedArguments, IEnumerable<SyntaxToken> separators);

protected abstract bool IsCloseParenOrComma(SyntaxToken token);
protected abstract bool IsLegalToAddNamedArguments(ImmutableArray<IParameterSymbol> parameters, int argumentCount);
protected abstract TArgumentSyntax WithName(TArgumentSyntax argument, string name);
protected abstract bool IsPositionalArgument(TArgumentSyntax argument);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeRefactorings.UseNamedArguments
Protected Overrides Function IsLegalToAddNamedArguments(parameters As ImmutableArray(Of IParameterSymbol), argumentCount As Integer) As Boolean
Return Not parameters.LastOrDefault().IsParams OrElse parameters.Length > argumentCount
End Function

Protected Overrides Function IsCloseParenOrComma(token As SyntaxToken) As Boolean
Return token.IsKind(SyntaxKind.CloseParenToken, SyntaxKind.CommaToken)
End Function
End Class

Public Sub New()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

Expand All @@ -26,9 +25,7 @@ public static IEnumerable<SyntaxNode> GetAncestors(this SyntaxNode node)
{
yield return current;

current = current is IStructuredTriviaSyntax
? ((IStructuredTriviaSyntax)current).ParentTrivia.Token.Parent
: current.Parent;
current = current.GetParent();
}
}

Expand All @@ -43,32 +40,20 @@ public static IEnumerable<TNode> GetAncestors<TNode>(this SyntaxNode node)
yield return tNode;
}

current = current is IStructuredTriviaSyntax
? ((IStructuredTriviaSyntax)current).ParentTrivia.Token.Parent
: current.Parent;
current = current.GetParent();
}
}

public static TNode GetAncestor<TNode>(this SyntaxNode node)
where TNode : SyntaxNode
{
if (node == null)
{
return default(TNode);
}

return node.GetAncestors<TNode>().FirstOrDefault();
return node?.GetAncestors<TNode>().FirstOrDefault();
}

public static TNode GetAncestorOrThis<TNode>(this SyntaxNode node)
where TNode : SyntaxNode
{
if (node == null)
{
return default(TNode);
}

return node.GetAncestorsOrThis<TNode>().FirstOrDefault();
return node?.GetAncestorsOrThis<TNode>().FirstOrDefault();
}

public static IEnumerable<TNode> GetAncestorsOrThis<TNode>(this SyntaxNode node)
Expand All @@ -82,9 +67,7 @@ public static IEnumerable<TNode> GetAncestorsOrThis<TNode>(this SyntaxNode node)
yield return tNode;
}

current = current is IStructuredTriviaSyntax
? ((IStructuredTriviaSyntax)current).ParentTrivia.Token.Parent
: current.Parent;
current = current.GetParent();
}
}

Expand Down Expand Up @@ -122,12 +105,7 @@ public static IEnumerable<TSyntaxNode> Traverse<TSyntaxNode>(

public static bool CheckParent<T>(this SyntaxNode node, Func<T, bool> valueChecker) where T : SyntaxNode
{
if (node == null)
{
return false;
}

var parentNode = node.Parent as T;
var parentNode = node?.Parent as T;
if (parentNode == null)
{
return false;
Expand Down Expand Up @@ -208,7 +186,7 @@ public static SyntaxNode FindInnermostCommonNode(
: blocks.Intersect(node.AncestorsAndSelf().Where(predicate));
}

return blocks == null ? null : blocks.First();
return blocks?.First();
}

public static TSyntaxNode FindInnermostCommonNode<TSyntaxNode>(this IEnumerable<SyntaxNode> nodes)
Expand Down Expand Up @@ -276,9 +254,7 @@ public static IEnumerable<TextSpan> GetContiguousSpans(
}
else
{
var lastToken = getLastToken == null
? lastNode.GetLastToken()
: getLastToken(lastNode);
var lastToken = getLastToken?.Invoke(lastNode) ?? lastNode.GetLastToken();
if (lastToken.GetNextToken(includeDirectives: true) == node.GetFirstToken())
{
// Expand the span
Expand Down Expand Up @@ -695,8 +671,6 @@ public static SyntaxToken FindTokenOnLeftOfPosition(
return token;
}



public static T WithPrependedLeadingTrivia<T>(
this T node,
params SyntaxTrivia[] trivia) where T : SyntaxNode
Expand Down Expand Up @@ -772,5 +746,29 @@ public static T With<T>(
{
return node.WithLeadingTrivia(leadingTrivia).WithTrailingTrivia(trailingTrivia);
}

private static SyntaxNode GetParent(this SyntaxNode node)
{
return node is IStructuredTriviaSyntax trivia ? trivia.ParentTrivia.Token.Parent : node.Parent;
}

public static TNode FirstAncestorOrSelfUntil<TNode>(this SyntaxNode node, Func<SyntaxNode, bool> predicate)
where TNode : SyntaxNode
{
for (var current = node; current != null; current = current.GetParent())
{
if (current is TNode tnode)
{
return tnode;
}

if (predicate(current))
{
break;
}
}

return default(TNode);
}
}
}

0 comments on commit f9f2b17

Please sign in to comment.