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

Omnisharp VS Code Issue 1814 Fix #1007

Merged
merged 15 commits into from
Nov 9, 2017
Merged
34 changes: 32 additions & 2 deletions tests/OmniSharp.Roslyn.CSharp.Tests/SignatureHelpFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ private int Foo(int one, int two, int three)
}

[Fact]
public async Task SignatureHelpforAttCtorSingleArg()
public async Task SignatureHelpforAttCtorSingleParam()
Copy link

Choose a reason for hiding this comment

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

I would prefer that we not abbreviate "attribute".

{
const string source =
@"using System;
Expand Down Expand Up @@ -192,7 +192,7 @@ public MyTestAttribute(int value)
Assert.Equal("int value", signature.Parameters.ElementAt(0).Label);
}
[Fact]
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing blank line before method. Should be between close brace and [Fact]

public async Task SignatureHelpforAttCtorMultipleArg()
public async Task SignatureHelpforAttCtorMultipleParam()
{
const string source =
@"using System;
Copy link

Choose a reason for hiding this comment

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

Does this pass? It doesn't look like there's a "$$" anywhere in the markup?

Copy link

Choose a reason for hiding this comment

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

My bad, I see it. Anyway, it seems like the test is called "MultipleParam", but you're only testing in the first parameter position.

Expand Down Expand Up @@ -225,6 +225,36 @@ public MyTestAttribute(int value1,double value2)
Assert.Equal("double value2", signature.Parameters.ElementAt(1).Label);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary blank line

[Fact]
public async Task SignatureHelpforAttCtorNoParam()
{
const string source =
@"using System;
[MyTest($$)]
public class TestClass
{
public static void Main()
{
}
}
public class MyTestAttribute : Attribute
{
int value1;
double value2;
public MyTestAttribute()
{
this.value1 = 1;
Copy link

Choose a reason for hiding this comment

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

You can delete all the "value1" and "value2" related stuff since this test doesn't use them.

this.value2 = 2;
}
}";
var actual = await GetSignatureHelp(source);
Assert.Single(actual.Signatures);
Assert.Equal(0, actual.ActiveParameter);
Assert.Equal(0, actual.ActiveSignature);
Assert.Equal("MyTestAttribute", actual.Signatures.ElementAt(0).Name);
Assert.Empty(actual.Signatures.ElementAt(0).Parameters);
}

[Fact]
public async Task ActiveParameterIsBasedOnComma2()
{
Expand Down