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

Fix: fix all for Use Auto Property #75788

Merged
merged 11 commits into from
Nov 7, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Shared.Extensions;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.CSharp.UseAutoProperty;
using Microsoft.CodeAnalysis.Diagnostics;
Expand Down Expand Up @@ -1877,7 +1878,7 @@ class Class

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/23216")]
[WorkItem("https://github.com/dotnet/roslyn/issues/23215")]
public async Task TestFixAllInDocument()
public async Task TestFixAllInDocument1()
{
await TestInRegularAndScript1Async(
"""
Expand Down Expand Up @@ -1914,6 +1915,53 @@ class Class
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/26527")]
public async Task TestFixAllInDocument2()
{
await TestInRegularAndScript1Async(
"""
internal struct StringFormat
{
private readonly object {|FixAllInDocument:_argument1|};
private readonly object _argument2;
private readonly object _argument3;
private readonly object[] _arguments;

public object Argument1
{
get { return _argument1; }
}

public object Argument2
{
get { return _argument2; }
}

public object Argument3
{
get { return _argument3; }
}

public object[] Arguments
{
get { return _arguments; }
}
}
""",
"""
internal struct StringFormat
{
public object Argument1 { get; }

public object Argument2 { get; }

public object Argument3 { get; }

public object[] Arguments { get; }
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/23735")]
public async Task ExplicitInterfaceImplementationGetterOnly()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Shared.Extensions;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;
Expand Down Expand Up @@ -68,6 +69,38 @@ string P
""", parseOptions: Preview);
}

[Fact]
public async Task TestFieldWithInitializer()
{
await TestInRegularAndScriptAsync(
"""
class Class
{
[|string s = ""|];

string P
{
get
{
return s.Trim();
}
}
}
""",
"""
class Class
{
string P
{
get
{
return field.Trim();
}
} = "";
}
""", parseOptions: Preview);
}

[Fact]
public async Task TestFieldAccessOffOfThis()
{
Expand Down Expand Up @@ -1471,4 +1504,39 @@ void M(ref int i) { }
}
""", parseOptions: Preview);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/26527")]
public async Task TestFixAllInDocument3()
{
await TestInRegularAndScript1Async(
"""
using System;

public sealed class SomeViewModel
{
private bool {|FixAllInDocument:a|} = true;
public bool A { get => a; set => Set(ref a, value); }

private bool b = true;
public bool B { get => b; set => Set(ref b, value); }

private bool c = true;
public bool C { get => c; set => Set(ref c, value); }

private void Set<T>(ref T field, T value) => throw new NotImplementedException();
}
""",
"""
using System;

public sealed class SomeViewModel
{
public bool A { get; set => Set(ref field, value); } = true;
public bool B { get; set => Set(ref field, value); } = true;
public bool C { get; set => Set(ref field, value); } = true;

private void Set<T>(ref T field, T value) => throw new NotImplementedException();
}
""", new TestParameters(parseOptions: Preview));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3147,7 +3147,7 @@ Class C

End Function

<WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/29938")>
<ConditionalWpfFact(GetType(IsEnglishLocal), Skip:="https://github.com/dotnet/roslyn/issues/29938"), WorkItem("https://github.com/dotnet/roslyn/issues/29938")>
Public Async Function TestMatchWithTurkishIWorkaround9() As Task
Using New CultureContext(New Globalization.CultureInfo("tr-TR", useUserOverride:=False))
Using state = TestStateFactory.CreateVisualBasicTestState(
Expand All @@ -3165,13 +3165,16 @@ Class C
$$]]></Document>)
state.SendTypeChars("IF")
Await state.WaitForAsynchronousOperationsAsync()
Await state.AssertSelectedCompletionItem("If")

' ICustomFormatter is better than `if` as the user used full-caps, which makes it more likely that
' they wanted to camel case match
Await state.AssertSelectedCompletionItem("ICustomFormatter")
End Using
End Using

End Function

<WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/29938")>
<ConditionalWpfFact(GetType(IsEnglishLocal), Skip:="https://github.com/dotnet/roslyn/issues/29938"), WorkItem("https://github.com/dotnet/roslyn/issues/29938")>
Public Async Function TestMatchWithTurkishIWorkaround10() As Task
Using New CultureContext(New Globalization.CultureInfo("tr-TR", useUserOverride:=False))
Using state = TestStateFactory.CreateVisualBasicTestState(
Expand All @@ -3190,7 +3193,10 @@ Class C
]]></Document>)
state.SendTypeChars("IF")
Await state.WaitForAsynchronousOperationsAsync()
Await state.AssertSelectedCompletionItem("If")

' ICustomFormatter is better than `if` as the user used full-caps, which makes it more likely that
' they wanted to camel case match
Await state.AssertSelectedCompletionItem("ICustomFormatter")
End Using
End Using
End Function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UseAutoProperty;
[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
internal sealed partial class CSharpUseAutoPropertyCodeFixProvider()
: AbstractUseAutoPropertyCodeFixProvider<
CSharpUseAutoPropertyCodeFixProvider,
TypeDeclarationSyntax,
PropertyDeclarationSyntax,
VariableDeclaratorSyntax,
Expand Down Expand Up @@ -130,8 +131,8 @@ protected override Task<SyntaxNode> UpdatePropertyAsync(
// Move any field initializer over to the property as well.
if (fieldInitializer != null)
{
updatedProperty = updatedProperty
.WithInitializer(EqualsValueClause(fieldInitializer))
updatedProperty = updatedProperty.WithoutTrailingTrivia()
.WithInitializer(EqualsValueClause(EqualsToken.WithLeadingTrivia(Space).WithTrailingTrivia(Space), fieldInitializer))
.WithSemicolonToken(SemicolonToken);
}

Expand Down
33 changes: 17 additions & 16 deletions src/Features/Core/Portable/Completion/CompletionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,13 @@ private static int CompareItems(
return expandedDiff;
}

// Then see how the two items compare in a case insensitive fashion. Matches that
// are strictly better (ignoring case) should prioritize the item. i.e. if we have
// a prefix match, that should always be better than a substring match.
// Then see how the two items compare in a case insensitive fashion. Matches that are strictly better (ignoring
// case) should prioritize the item. i.e. if we have a prefix match, that should always be better than a
// substring match.
//
// The reason we ignore case is that it's very common for people to type expecting
// completion to fix up their casing. i.e. 'false' will be written with the
// expectation that it will get fixed by the completion list to 'False'.
// The reason we ignore case is that it's very common for people to type expecting completion to fix up their
// casing. i.e. 'false' will be written with the expectation that it will get fixed by the completion list to
// 'False'.
var caseInsensitiveComparison = match1.CompareTo(match2, ignoreCase: true);
if (caseInsensitiveComparison != 0)
{
Expand All @@ -139,17 +139,18 @@ private static int CompareItems(

// Now we have two items match in case-insensitive manner,
//
// 1. if we are in a case-insensitive language, we'd first check if either item has the MatchPriority set to one of
// the two special values ("Preselect" and "Deprioritize"). If so and these two items have different MatchPriority,
// then we'd select the one of "Preselect", or the one that's not of "Deprioritize". Otherwise we will prefer the one
// matches case-sensitively. This is to make sure common items in VB like "True" and "False" are prioritized for selection
// when user types "t" and "f" (see https://github.com/dotnet/roslyn/issues/4892)
// 1. if we are in a case-insensitive language, we'd first check if either item has the MatchPriority set to one
// of the two special values ("Preselect" and "Deprioritize"). If so and these two items have different
// MatchPriority, then we'd select the one of "Preselect", or the one that's not of "Deprioritize". Otherwise
// we will prefer the one matches case-sensitively. This is to make sure common items in VB like "True" and
// "False" are prioritized for selection when user types "t" and "f" (see
// https://github.com/dotnet/roslyn/issues/4892)
//
// 2. or similarly, if the filter text contains only lowercase letters, we want to relax our filtering standard a tiny
// bit to account for the sceanrio that users expect completion to fix the casing. This only happens if one of the item's
// MatchPriority is "Deprioritize". Otherwise we will always prefer the one matches case-sensitively.
// This is to make sure uncommon items like conversion "(short)" are not selected over `Should` when user types `sho`
// (see https://github.com/dotnet/roslyn/issues/55546)
// 2. or similarly, if the filter text contains only lowercase letters, we want to relax our filtering standard
// a tiny bit to account for the scenario that users expect completion to fix the casing. This only happens
// if one of the item's MatchPriority is "Deprioritize". Otherwise we will always prefer the one matches
// case-sensitively. This is to make sure uncommon items like conversion "(short)" are not selected over
// `Should` when user types `sho` (see https://github.com/dotnet/roslyn/issues/55546)

var specialMatchPriorityValuesDiff = 0;
if (!isCaseSensitive)
Expand Down
3 changes: 3 additions & 0 deletions src/Features/Core/Portable/Completion/MatchResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,7 @@ public int Compare(MatchResult x, MatchResult y)
return x.IndexInOriginalSortedOrder - y.IndexInOriginalSortedOrder;
}
}

public override string ToString()
=> this.CompletionItem.ToString();
}
Loading
Loading