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

Add OutVariableArgumentProvider for 'out' parameters #53049

Merged
merged 4 commits into from
Jan 14, 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 @@ -2,14 +2,24 @@
// 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.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Test.Utilities.Completion;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Completion.ArgumentProviders
{
public abstract class AbstractCSharpArgumentProviderTests<TWorkspaceFixture>
: AbstractArgumentProviderTests<TWorkspaceFixture>
where TWorkspaceFixture : TestWorkspaceFixture, new()
{
protected override (SyntaxNode argumentList, ImmutableArray<SyntaxNode> arguments) GetArgumentList(SyntaxToken token)
{
var argumentList = token.GetRequiredParent().GetAncestorsOrThis<BaseArgumentListSyntax>().First();
return (argumentList, argumentList.Arguments.ToImmutableArray<SyntaxNode>());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public void TestArgumentProviderOrder()

// Built-in providers
typeof(ContextVariableArgumentProvider),
typeof(OutVariableArgumentProvider),
typeof(DefaultArgumentProvider),

// Marker for end of built-in argument providers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,37 @@ void Target({type} arg)
await VerifyDefaultValueAsync(markup, expectedDefaultValue: null, previousDefaultValue: "prior");
}

[Theory]
[CombinatorialData]
public async Task TestOutVariable(
[CombinatorialValues("string", "bool", "int?")] string type,
[CombinatorialValues("out", "ref", "in")] string modifier)
{
var markup = $@"
class C
{{
void Method()
{{
{type} arg;
this.Target($$);
}}

void Target({modifier} {type} arg)
{{
}}
}}
";

var generatedModifier = modifier switch
{
"in" => "",
_ => $"{modifier} ",
};

await VerifyDefaultValueAsync(markup, $"{generatedModifier}arg");
await VerifyDefaultValueAsync(markup, expectedDefaultValue: null, previousDefaultValue: "prior");
}

[Theory]
[InlineData("string")]
[InlineData("bool")]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// 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;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Completion.Providers;
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
using Microsoft.CodeAnalysis.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Completion.ArgumentProviders
{
[Trait(Traits.Feature, Traits.Features.Completion)]
public class OutVariableArgumentProviderTests : AbstractCSharpArgumentProviderTests
{
private static readonly OptionsCollection s_useExplicitTypeOptions = new(LanguageNames.CSharp)
{
{ CSharpCodeStyleOptions.VarForBuiltInTypes, false },
{ CSharpCodeStyleOptions.VarWhenTypeIsApparent, false },
{ CSharpCodeStyleOptions.VarElsewhere, false },
};

private static readonly OptionsCollection s_useExplicitMetadataTypeOptions = new(LanguageNames.CSharp)
{
{ CSharpCodeStyleOptions.VarForBuiltInTypes, false },
{ CSharpCodeStyleOptions.VarWhenTypeIsApparent, false },
{ CSharpCodeStyleOptions.VarElsewhere, false },
{ CodeStyleOptions2.PreferIntrinsicPredefinedTypeKeywordInDeclaration, false },
};

private static readonly OptionsCollection s_useImplicitTypeOptions = new(LanguageNames.CSharp)
{
{ CSharpCodeStyleOptions.VarForBuiltInTypes, true },
{ CSharpCodeStyleOptions.VarWhenTypeIsApparent, true },
{ CSharpCodeStyleOptions.VarElsewhere, true },
};

internal override Type GetArgumentProviderType()
=> typeof(OutVariableArgumentProvider);

[Theory]
[InlineData("")]
[InlineData("ref")]
[InlineData("in")]
public async Task TestUnsupportedModifiers(string modifier)
{
var markup = $@"
class C
{{
void Method()
{{
TryParse($$)
}}

bool TryParse({modifier} int value) => throw null;
}}
";

await VerifyDefaultValueAsync(markup, expectedDefaultValue: null);
}

[Fact]
public async Task TestDeclareVariable()
{
var markup = $@"
class C
{{
void Method()
{{
int.TryParse(""x"", $$)
}}
}}
";

await VerifyDefaultValueAsync(markup, "out var result");
await VerifyDefaultValueAsync(markup, expectedDefaultValue: null, previousDefaultValue: "prior");
}

[Theory(Skip = "https://github.com/dotnet/roslyn/issues/53056")]
[CombinatorialData]
public async Task TestDeclareVariableBuiltInType(bool preferVar, bool preferBuiltInType)
{
var markup = $@"
using System;
class C
{{
void Method()
{{
int.TryParse(""x"", $$)
}}
}}
";

var expected = (preferVar, preferBuiltInType) switch
{
(true, _) => "out var result",
(false, true) => "out int result",
(false, false) => "out Int32 result",
};

var options = (preferVar, preferBuiltInType) switch
{
(true, _) => s_useImplicitTypeOptions,
(false, true) => s_useExplicitTypeOptions,
(false, false) => s_useExplicitMetadataTypeOptions,
};

await VerifyDefaultValueAsync(markup, expected, options: options);
}

[Theory]
[InlineData("string")]
[InlineData("bool")]
[InlineData("int?")]
public async Task TestDeclareVariableEscapedIdentifier(string type)
{
var markup = $@"
class C
{{
void Method()
{{
this.Target($$);
}}

void Target(out {type} @void)
{{
@void = default;
}}
}}
";

await VerifyDefaultValueAsync(markup, "out var @void");
await VerifyDefaultValueAsync(markup, expectedDefaultValue: null, previousDefaultValue: "prior");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,21 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.VisualStudio.Composition;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
using Xunit;
using Microsoft.CodeAnalysis.Shared.Extensions;

namespace Microsoft.CodeAnalysis.Test.Utilities.Completion
{
Expand All @@ -39,38 +42,90 @@ private protected ReferenceCountedDisposable<TWorkspaceFixture> GetOrCreateWorks

internal abstract Type GetArgumentProviderType();

protected abstract (SyntaxNode argumentList, ImmutableArray<SyntaxNode> arguments) GetArgumentList(SyntaxToken token);

protected virtual OptionSet WithChangedOptions(OptionSet options) => options;

private protected async Task VerifyDefaultValueAsync(
string markup,
string? expectedDefaultValue,
string? previousDefaultValue = null)
string? previousDefaultValue = null,
OptionsCollection? options = null)
{
using var workspaceFixture = GetOrCreateWorkspaceFixture();

var workspace = workspaceFixture.Target.GetWorkspace(markup, ExportProvider);
var code = workspaceFixture.Target.Code;
var position = workspaceFixture.Target.Position;

workspace.SetOptions(WithChangedOptions(workspace.Options));
var changedOptions = WithChangedOptions(workspace.Options);
if (options is not null)
{
foreach (var option in options)
changedOptions = changedOptions.WithChangedOption(option.Key, option.Value);
}

workspace.SetOptions(changedOptions);

var document = workspaceFixture.Target.UpdateDocument(code, SourceCodeKind.Regular);

var provider = workspace.ExportProvider.GetExportedValues<ArgumentProvider>().Single();
Assert.IsType(GetArgumentProviderType(), provider);

var root = await document.GetRequiredSyntaxRootAsync(CancellationToken.None);
var token = root.FindToken(position - 2);
var documentOptions = await document.GetOptionsAsync(CancellationToken.None);
var semanticModel = await document.GetRequiredSemanticModelAsync(CancellationToken.None);
var symbolInfo = semanticModel.GetSymbolInfo(token.GetRequiredParent(), CancellationToken.None);
var target = symbolInfo.Symbol ?? symbolInfo.CandidateSymbols.Single();
Contract.ThrowIfNull(target);
var parameter = GetParameterSymbolInfo(workspace, semanticModel, root, position, CancellationToken.None);
Contract.ThrowIfNull(parameter);

var parameter = target.GetParameters().Single();
var context = new ArgumentContext(provider, semanticModel, position, parameter, previousDefaultValue, CancellationToken.None);
var context = new ArgumentContext(provider, documentOptions, semanticModel, position, parameter, previousDefaultValue, CancellationToken.None);
await provider.ProvideArgumentAsync(context);

Assert.Equal(expectedDefaultValue, context.DefaultValue);
}

private IParameterSymbol GetParameterSymbolInfo(Workspace workspace, SemanticModel semanticModel, SyntaxNode root, int position, CancellationToken cancellationToken)
{
var token = root.FindToken(position);
var (argumentList, arguments) = GetArgumentList(token);
var symbols = semanticModel.GetSymbolInfo(argumentList.GetRequiredParent(), cancellationToken).GetAllSymbols();

// if more than one symbol is found, filter to only include symbols with a matching number of arguments
if (symbols.Length > 1)
{
symbols = symbols.WhereAsArray(
symbol =>
{
var parameters = symbol.GetParameters();
if (arguments.Length < GetMinimumArgumentCount(parameters))
return false;

if (arguments.Length > GetMaximumArgumentCount(parameters))
return false;

return true;
});
}

var symbol = symbols.Single();
Copy link
Member

Choose a reason for hiding this comment

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

Is the filtering above guaranteed to limit us to a single result? or would an exception here mean the test should be rewritten to remove ambiguity?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still possible to construct a test that the framework doesn't support. We'd have to modify it at that time to support the specifics of the new test.

var parameters = symbol.GetParameters();

var syntaxFacts = workspace.Services.GetLanguageServices(root.Language).GetRequiredService<ISyntaxFactsService>();
Contract.ThrowIfTrue(arguments.Any(argument => syntaxFacts.IsNamedArgument(argument)), "Named arguments are not currently supported by this test.");
Contract.ThrowIfTrue(parameters.Any(parameter => parameter.IsParams), "'params' parameters are not currently supported by this test.");

var index = arguments.Any()
? arguments.IndexOf(arguments.Single(argument => argument.FullSpan.Start <= position && argument.FullSpan.End >= position))
: 0;

return parameters[index];

// Local functions
static int GetMinimumArgumentCount(ImmutableArray<IParameterSymbol> parameters)
=> parameters.Count(parameter => !parameter.IsOptional && !parameter.IsParams);

static int GetMaximumArgumentCount(ImmutableArray<IParameterSymbol> parameters)
=> parameters.Any(parameter => parameter.IsParams) ? int.MaxValue : parameters.Length;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Test.Utilities.Completion
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Completion.ArgumentProviders
Public MustInherit Class AbstractVisualBasicArgumentProviderTests
Inherits AbstractArgumentProviderTests(Of VisualBasicTestWorkspaceFixture)

Protected Overrides Function GetArgumentList(token As SyntaxToken) As (argumentList As SyntaxNode, arguments As ImmutableArray(Of SyntaxNode))
Dim argumentList = token.GetRequiredParent().GetAncestorsOrThis(Of ArgumentListSyntax)().First()
Return (argumentList, argumentList.Arguments.Cast(Of SyntaxNode)().ToImmutableArray())
End Function
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Composition;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.Host.Mef;

Expand All @@ -19,5 +20,28 @@ internal sealed class ContextVariableArgumentProvider : AbstractContextVariableA
public ContextVariableArgumentProvider()
{
}

public override async Task ProvideArgumentAsync(ArgumentContext context)
{
await base.ProvideArgumentAsync(context).ConfigureAwait(false);
if (context.DefaultValue is not null)
{
switch (context.Parameter.RefKind)
{
case RefKind.Ref:
context.DefaultValue = "ref " + context.DefaultValue;
break;

case RefKind.Out:
context.DefaultValue = "out " + context.DefaultValue;
break;
sharwell marked this conversation as resolved.
Show resolved Hide resolved

case RefKind.In:
case RefKind.None:
default:
break;
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace Microsoft.CodeAnalysis.CSharp.Completion.Providers
{
[ExportArgumentProvider(nameof(DefaultArgumentProvider), LanguageNames.CSharp)]
[ExtensionOrder(After = nameof(ContextVariableArgumentProvider))]
[ExtensionOrder(After = nameof(OutVariableArgumentProvider))]
[Shared]
internal sealed class DefaultArgumentProvider : AbstractDefaultArgumentProvider
{
Expand Down
Loading