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.

Sign up for GitHub

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

Jump to bottom

Fix crash, and offer "declare as nullable" for tuple fields #46437

Merged
merged 4 commits into from
Jul 30, 2020
Merged

Fix crash, and offer "declare as nullable" for tuple fields #46437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
21258de
Don't assume fields have declared syntax references
davidwengier Jul 30, 2020
0e00451
Support fixing tuple fields, whether they're implicitly defined or not
davidwengier Jul 30, 2020
8347ae5
Align attributes
davidwengier Jul 30, 2020
80af46c
Move comments
davidwengier Jul 30, 2020
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
118 changes: 90 additions & 28 deletions src/EditorFeatures/CSharpTest/Diagnostics/Nullable/CSharpDeclareAsNullableCodeFixTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ class Program
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(26639, "https://github.com/dotnet/roslyn/issues/26639")]
[Fact, WorkItem(26639, "https://github.com/dotnet/roslyn/issues/26639")]
public async Task FixReturnType_LocalFunction_ArrowBody()
{
await TestMissingInRegularAndScriptAsync(
Expand All @@ -182,8 +181,7 @@ static void M()
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(26639, "https://github.com/dotnet/roslyn/issues/26639")]
[Fact, WorkItem(26639, "https://github.com/dotnet/roslyn/issues/26639")]
public async Task FixLocalFunctionReturnType()
{
await TestInRegularAndScript1Async(
Expand Down Expand Up @@ -225,8 +223,7 @@ class Program
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(26628, "https://github.com/dotnet/roslyn/issues/26628")]
[Fact, WorkItem(26628, "https://github.com/dotnet/roslyn/issues/26628")]
public async Task FixField()
{
await TestInRegularAndScript1Async(
Expand Down Expand Up @@ -417,8 +414,7 @@ static void M()
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(26628, "https://github.com/dotnet/roslyn/issues/26628")]
[Fact, WorkItem(26628, "https://github.com/dotnet/roslyn/issues/26628")]
public async Task FixPropertyDeclaration()
{
await TestInRegularAndScript1Async(
Expand Down Expand Up @@ -606,8 +602,7 @@ void M()
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(39422, "https://github.com/dotnet/roslyn/issues/39422")]
[Fact, WorkItem(39422, "https://github.com/dotnet/roslyn/issues/39422")]
public async Task FixReturnType_ConditionalOperator_Function()
{
await TestInRegularAndScript1Async(
Expand All @@ -629,8 +624,7 @@ class Program
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(39422, "https://github.com/dotnet/roslyn/issues/39422")]
[Fact, WorkItem(39422, "https://github.com/dotnet/roslyn/issues/39422")]
public async Task FixAllReturnType_ConditionalOperator_Function()
{
await TestInRegularAndScript1Async(
Expand Down Expand Up @@ -682,8 +676,7 @@ string Test2(bool? value)
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(39422, "https://github.com/dotnet/roslyn/issues/39422")]
[Fact, WorkItem(39422, "https://github.com/dotnet/roslyn/issues/39422")]
public async Task FixAllReturnType_Invocation()
{
await TestInRegularAndScript1Async(
Expand Down Expand Up @@ -739,8 +732,7 @@ string Test2(bool? value)
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(39420, "https://github.com/dotnet/roslyn/issues/39420")]
[Fact, WorkItem(39420, "https://github.com/dotnet/roslyn/issues/39420")]
public async Task FixReturnType_TernaryExpression_Function()
{
await TestInRegularAndScript1Async(
Expand All @@ -762,8 +754,7 @@ class Program
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(39423, "https://github.com/dotnet/roslyn/issues/39423")]
[Fact, WorkItem(39423, "https://github.com/dotnet/roslyn/issues/39423")]
public async Task FixReturnType_Default()
{
await TestInRegularAndScript1Async(
Expand All @@ -785,8 +776,7 @@ class Program
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(39423, "https://github.com/dotnet/roslyn/issues/39423")]
[Fact, WorkItem(39423, "https://github.com/dotnet/roslyn/issues/39423")]
public async Task FixReturnType_DefaultWithNullableType()
{
await TestInRegularAndScript1Async(
Expand Down Expand Up @@ -832,8 +822,7 @@ void M2(string? x) { }
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(44338, "https://github.com/dotnet/roslyn/issues/44338")]
[Fact, WorkItem(44338, "https://github.com/dotnet/roslyn/issues/44338")]
public async Task NoFixInvocationOfExternalMethod_NamedArgument()
{
await TestMissingInRegularAndScriptAsync(
Expand Down Expand Up @@ -872,8 +861,7 @@ void M2(int i, string? x) { }
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(44338, "https://github.com/dotnet/roslyn/issues/44338")]
[Fact, WorkItem(44338, "https://github.com/dotnet/roslyn/issues/44338")]
public async Task NoFixInvocationOfExternalMethod_NamedArgument_OutOfOrder()
{
await TestMissingInRegularAndScriptAsync(
Expand Down Expand Up @@ -928,8 +916,7 @@ void M2(string? x) { }
}", parameters: s_nullableFeature);
}

[Fact]
[WorkItem(44338, "https://github.com/dotnet/roslyn/issues/44338")]
[Fact, WorkItem(44338, "https://github.com/dotnet/roslyn/issues/44338")]
public async Task NoFixInvocationOfExternalMethod_PositionArgument()
{
await TestMissingInRegularAndScriptAsync(
Expand Down Expand Up @@ -1037,15 +1024,90 @@ class C
parameters: s_nullableFeature);
}

[Fact]
[WorkItem(44983, "https://github.com/dotnet/roslyn/issues/44983")]
[Fact, WorkItem(44983, "https://github.com/dotnet/roslyn/issues/44983")]
public async Task MultipleDeclarator_NoDiagnostic()
{
await TestMissingInRegularAndScriptAsync(
@"#nullable enable
class Program
{
string [|s|], s2 = ""hello"";
}", parameters: s_nullableFeature);
}

[Fact, WorkItem(46354, "https://github.com/dotnet/roslyn/issues/46354")]
public async Task FixTupleFieldAssignment()
{
await TestInRegularAndScript1Async(
@"#nullable enable
class Program
{
static void F1((string, string?) t)
{
if (t.Item2 == null) return;
t.Item1 = [|null|];
}
}",
@"#nullable enable
class Program
{
static void F1((string?, string?) t)
{
if (t.Item2 == null) return;
t.Item1 = null;
}
}", parameters: s_nullableFeature);
}

[Fact, WorkItem(46354, "https://github.com/dotnet/roslyn/issues/46354")]
public async Task FixTupleNamedFieldAssignment()
{
await TestInRegularAndScript1Async(
@"#nullable enable
class Program
{
static void F1((string Foo, string? Bar) t)
{
if (t.Bar == null) return;
t.Foo = [|null|];
}
}",
@"#nullable enable
class Program
{
static void F1((string? Foo, string? Bar) t)
{
if (t.Bar == null) return;
t.Foo = null;
}
}", parameters: s_nullableFeature);
}

[Fact, WorkItem(46354, "https://github.com/dotnet/roslyn/issues/46354")]
public async Task FixTupleGenericFieldAssignment()
{
await TestInRegularAndScript1Async(
@"#nullable enable
class Program
{
static void F1<T>((T, T?) t) where T : class
{
if (t.Item2 == null) return;
t.Item1 = [|null|];

var (a, b) = t;
}
}",
@"#nullable enable
class Program
{
static void F1<T>((T?, T?) t) where T : class
{
if (t.Item2 == null) return;
t.Item1 = null;

var (a, b) = t;
}
}", parameters: s_nullableFeature);
}
}
Expand Down
21 changes: 20 additions & 1 deletion src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,34 @@ declarator.Parent is VariableDeclarationSyntax declaration &&
{
return TryGetParameterTypeSyntax(parameter);
}
else if (symbol is IFieldSymbol field)
else if (symbol is IFieldSymbol { IsImplicitlyDeclared: false } field)
{
// implicitly declared fields don't have DeclaringSyntaxReferences so filter them out
var syntax = field.DeclaringSyntaxReferences[0].GetSyntax();
if (syntax is VariableDeclaratorSyntax declarator &&
declarator.Parent is VariableDeclarationSyntax declaration &&
declaration.Variables.Count == 1)
{
return declaration.Type;
}
else if (syntax is TupleElementSyntax tupleElement)
{
return tupleElement.Type;
}
}
else if (symbol is IFieldSymbol { CorrespondingTupleField: IFieldSymbol tupleField })
{
// Assigning a tuple field, eg. foo.Item1 = null
// The tupleField won't have DeclaringSyntaxReferences because it's implicitly declared, otherwise it
// would have fallen into the branch above. We can use the Locations instead, if there is one and it's in source
if (tupleField.Locations is { Length: 1 } &&
tupleField.Locations[0] is { IsInSource: true } location)
{
if (location.FindNode(default) is TupleElementSyntax tupleElement)
{
return tupleElement.Type;
}
}
}
else if (symbol is IPropertySymbol property)
{
Expand Down