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

Support code folding for argument lists #71064

Merged
merged 13 commits into from
Feb 17, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.Shared.Collections;
Expand Down Expand Up @@ -205,6 +206,69 @@ public void TestReverseContents([CombinatorialRange(0, 6)] int initialItems)
Assert.Equal(array[i], initialItems - 1 - i);
}

[Fact]
public void TestSort()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would take everything out related to sorting TempArray. We don't even relaly need temparray for block-structure. That's an array that's good when you commonly have <4 elements. But for block structure, we'd almost alwayshave more. So we can just pass an ArrayBuilder along instead).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or, we can keep this. i'm on the fence.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi I had the same feeling the TemporaryArray might not be very suitable for this scenario, but I preferred to limit the scope of my changes.

{
// Create arrays with different lengths, making sure to exceed the number of inline elements to test all code paths.
for (int i = 0; i <= TemporaryArray<int>.TestAccessor.InlineCapacity + 1; i++)
{
foreach (var permutation in permute(Enumerable.Range(0, i).ToArray()))
{
assertSort(permutation);
}
}

static void assertSort(ImmutableArray<int> inputArray)
{
var sortedArray = inputArray.Sort();
Assert.Equal(inputArray.Length, sortedArray.Length);
using var array = TemporaryArray<int>.Empty;
foreach (var num in inputArray)
array.Add(num);

Assert.Equal(array.Count, sortedArray.Length);
array.Sort((x, y) => x.CompareTo(y));
Assert.Equal(array.Count, sortedArray.Length);
for (int i = 0; i < array.Count; i++)
{
Assert.Equal(array[i], sortedArray[i]);
}
}

// Almost copy from ServiceHubServicesTests
static List<ImmutableArray<T>> permute<T>(T[] values)
{
var result = new List<ImmutableArray<T>>();
if (values.Length == 0)
{
result.Add(ImmutableArray<T>.Empty);
return result;
}

doPermute(0, values.Length - 1);
return result;

void doPermute(int start, int end)
{
if (start == end)
{
// We have one of our possible n! solutions,
// add it to the list.
result.Add(values.ToImmutableArray());
}
else
{
for (var i = start; i <= end; i++)
{
(values[start], values[i]) = (values[i], values[start]);
doPermute(start + 1, end);
(values[start], values[i]) = (values[i], values[start]);
}
}
}
}
}

[Theory, CombinatorialData]
public void TestRemoveLast([CombinatorialRange(0, 6)] int initialItems)
{
Expand Down
53 changes: 53 additions & 0 deletions src/Compilers/Core/Portable/Collections/TemporaryArray`1.cs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,59 @@ public void ReverseContents()
}
}

public void Sort(Comparison<T> compare)
{
if (_builder is not null)
{
_builder.Sort(compare);
return;
}

switch (_count)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically:

            Span<T> span = MemoryMarshal.CreateSpan(ref _item0, this.Count);
            span.Sort(comparison);

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not available in netstandard2.0 😕

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah. @stephentoub Any recommendations on how to best sort a span in netstandard2.0?

{
case <= 1:
return;
case 2:
if (compare(_item0, _item1) > 0)
{
(_item0, _item1) = (_item1, _item0);
}
return;
case 3:
if (compare(_item0, _item1) > 0)
(_item0, _item1) = (_item1, _item0);

if (compare(_item1, _item2) > 0)
{
(_item1, _item2) = (_item2, _item1);

if (compare(_item0, _item1) > 0)
(_item0, _item1) = (_item1, _item0);
}
return;
case 4:

if (compare(_item0, _item1) > 0)
(_item0, _item1) = (_item1, _item0);

if (compare(_item2, _item3) > 0)
(_item2, _item3) = (_item3, _item2);

if (compare(_item0, _item2) > 0)
(_item0, _item2) = (_item2, _item0);

if (compare(_item1, _item3) > 0)
(_item1, _item3) = (_item3, _item1);

if (compare(_item1, _item2) > 0)
(_item1, _item2) = (_item2, _item1);

return;
default:
throw ExceptionUtilities.Unreachable();
}
}

/// <summary>
/// Throws <see cref="IndexOutOfRangeException"/>.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp.Structure;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Structure;
using Microsoft.CodeAnalysis.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Structure;

[Trait(Traits.Feature, Traits.Features.Outlining)]
public class ArgumentListSyntaxStructureTests : AbstractCSharpSyntaxNodeStructureTests<ArgumentListSyntax>
{
internal override AbstractSyntaxStructureProvider CreateProvider() => new ArgumentListStructureProvider();

[Fact]
public async Task TestInvocationExpressionSingleLine()
{
var code = """
var x = M$$();
""";

await VerifyBlockSpansAsync(code);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test with just two args on different lines.


[Fact]
public async Task TestInvocationExpressionTwoArgumentsInTwoLines()
{
var code = """
var x = M$$("Hello",
"World");
""";

await VerifyBlockSpansAsync(code);
}

[Fact]
public async Task TestInvocationExpressionThreeLines()
{
var code = """
var x = M$${|span:(
"",
"")|};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the experience you have in the IDE if you have:

var v = M(N(
   "",
   ""));

In other words, how does nesting impact things, when the outer/inner are on the same start/end lines.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi I wasn't able to get RoslynDeployment work properly. https://www.youtube.com/watch?v=ND8dDO6oJ4U see around 38:00

""";

await VerifyBlockSpansAsync(code,
Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: false));
}

[Fact]
public async Task TestTwoInvocationExpressionsThreeLines()
{
// The inner argument list should be collapsible, but the outer one shouldn't.
// While this test shows both as collapsible, they will be deduplicated by AbstractBlockStructureProvider
// This test only tests ArgumentListStructureProvider specifically, so it doesn't show the deduplication.
// Tests in BlockStructureServiceTests show the overall behavior accurately.
var testInner = """
var x = M(M$${|span:(
"",
"")|});
""";

await VerifyBlockSpansAsync(testInner,
Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: false));

var testOuter = """
var x = M$${|span:(M(
"",
""))|};
""";

await VerifyBlockSpansAsync(testOuter,
Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: false));
}

[Fact]
public async Task TestObjectCreationSingleLine()
{
var code = """
var x = new C$$();
""";

await VerifyBlockSpansAsync(code);
}

[Fact]
public async Task TestObjectCreationThreeLines()
{
var code = """
var x = new C$${|span:(
"",
"")|};
""";

await VerifyBlockSpansAsync(code,
Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: false));
}

[Fact]
public async Task TestImplicitObjectCreationSingleLine()
{
var code = """
C x = new$$();
""";

await VerifyBlockSpansAsync(code);
}

[Fact]
public async Task TestImplicitObjectCreationThreeLines()
{
var code = """
C x = new$${|span:(
"",
"")|};
""";

await VerifyBlockSpansAsync(code,
Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ public async Task IdentifierThatLooksLikeCode()
""";

await VerifyBlockSpansAsync(code,
Region("textspan1", "hint1", CSharpStructureHelpers.Ellipsis, autoCollapse: false),
Region("textspan2", "hint2", CSharpStructureHelpers.Ellipsis, autoCollapse: false),
Region("textspan3", "/* now everything is commented (); ...", autoCollapse: true));
Region("textspan3", "/* now everything is commented (); ...", autoCollapse: true),
Region("textspan1", "hint1", CSharpStructureHelpers.Ellipsis, autoCollapse: false));
}
}
18 changes: 18 additions & 0 deletions src/EditorFeatures/Test/Structure/BlockStructureServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,24 @@ static void Goo()
Assert.Equal(4, spans.Length);
}

[Fact]
public async Task TestTwoInvocationExpressionsThreeLines()
{
// The inner argument list should be collapsible, but the outer one shouldn't.
var code = """
var x = MyMethod1(MyMethod2(
"",
"");
""";

using var workspace = TestWorkspace.CreateCSharp(code);
var spans = await GetSpansFromWorkspaceAsync(workspace);

Assert.Equal(1, spans.Length);

Assert.Equal(27, spans[0].TextSpan.Start);
}

private static async Task<ImmutableArray<BlockSpan>> GetSpansFromWorkspaceAsync(
TestWorkspace workspace)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ End Class
End Class
"
Await VerifyBlockSpansAsync(code,
Region("textspan1", "hint1", "Class C " & Ellipsis, autoCollapse:=False),
Region("textspan2", "hint2", "Public Sub " & Ellipsis, autoCollapse:=True))
Region("textspan2", "hint2", "Public Sub " & Ellipsis, autoCollapse:=True),
Region("textspan1", "hint1", "Class C " & Ellipsis, autoCollapse:=False))
End Function

End Class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ End Class|}
"

Await VerifyBlockSpansAsync(code,
Region("span1", "Class C ...", autoCollapse:=False),
Region("span2", "Something", autoCollapse:=False, isDefaultCollapsed:=True))
Region("span2", "Something", autoCollapse:=False, isDefaultCollapsed:=True),
Region("span1", "Class C ...", autoCollapse:=False))
End Function
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ private static ImmutableDictionary<Type, ImmutableArray<AbstractSyntaxStructureP
builder.Add<InterpolatedStringExpressionSyntax, InterpolatedStringExpressionStructureProvider>();
builder.Add<IfDirectiveTriviaSyntax, IfDirectiveTriviaStructureProvider>();
builder.Add<CollectionExpressionSyntax, CollectionExpressionStructureProvider>();
builder.Add<ArgumentListSyntax, ArgumentListStructureProvider>();

return builder.ToImmutable();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Structure;

namespace Microsoft.CodeAnalysis.CSharp.Structure;

internal sealed class ArgumentListStructureProvider : AbstractSyntaxNodeStructureProvider<ArgumentListSyntax>
{
protected override void CollectBlockSpans(SyntaxToken previousToken, ArgumentListSyntax node, ref TemporaryArray<BlockSpan> spans, BlockStructureOptions options, CancellationToken cancellationToken)
{
if (!IsCandidate(node, cancellationToken))
{
return;
}

spans.Add(new BlockSpan(
type: BlockTypes.Expression,
isCollapsible: true,
node.Span));
}

private static bool IsCandidate(ArgumentListSyntax node, CancellationToken cancellationToken)
{
var openToken = node.OpenParenToken;
var closeToken = node.CloseParenToken;
if (openToken.IsMissing || closeToken.IsMissing)
{
return false;
}

var text = node.SyntaxTree.GetText(cancellationToken);
var start = text.Lines.GetLinePosition(openToken.SpanStart).Line;
var end = text.Lines.GetLinePosition(closeToken.SpanStart).Line;
return end - start >= 2;
}
}
Loading