Skip to content

Commit

Permalink
Merge pull request #73383 from CyrusNajmabadi/cleanupSyntax
Browse files Browse the repository at this point in the history 
Do syntactic cleanup during the initial pass of producing fixed documents in fix-all.
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored May 8, 2024
2 parents a1e7312 + e8a21ae commit c0a3973
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 106 deletions.
16 changes: 9 additions & 7 deletions ...rs/CSharp/CodeFixes/MakeMethodAsynchronous/CSharpMakeMethodAsynchronousCodeFixProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ internal class CSharpMakeMethodAsynchronousCodeFixProvider : AbstractMakeMethodA
private const string CS4034 = nameof(CS4034); // The 'await' operator can only be used within an async lambda expression. Consider marking this method with the 'async' modifier.
private const string CS0246 = nameof(CS0246); // The type or namespace name 'await' could not be found

private static readonly SyntaxToken s_asyncKeywordWithSpace = AsyncKeyword.WithoutTrivia().WithTrailingTrivia(Space);

[ImportingConstructor]
[SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
public CSharpMakeMethodAsynchronousCodeFixProvider()
Expand Down Expand Up @@ -92,7 +94,7 @@ private static MethodDeclarationSyntax FixMethod(
CancellationToken cancellationToken)
{
var newReturnType = FixMethodReturnType(keepVoid, methodSymbol, method.ReturnType, knownTypes, cancellationToken);
var newModifiers = AddAsyncModifierWithCorrectedTrivia(method.Modifiers, ref newReturnType);
(var newModifiers, newReturnType) = AddAsyncModifierWithCorrectedTrivia(method.Modifiers, newReturnType);
return method.WithReturnType(newReturnType).WithModifiers(newModifiers);
}

Expand All @@ -104,7 +106,7 @@ private static LocalFunctionStatementSyntax FixLocalFunction(
CancellationToken cancellationToken)
{
var newReturnType = FixMethodReturnType(keepVoid, methodSymbol, localFunction.ReturnType, knownTypes, cancellationToken);
var newModifiers = AddAsyncModifierWithCorrectedTrivia(localFunction.Modifiers, ref newReturnType);
(var newModifiers, newReturnType) = AddAsyncModifierWithCorrectedTrivia(localFunction.Modifiers, newReturnType);
return localFunction.WithReturnType(newReturnType).WithModifiers(newModifiers);
}

Expand Down Expand Up @@ -176,15 +178,15 @@ private static bool IsIEnumerable(ITypeSymbol returnType, KnownTaskTypes knownTy
private static bool IsIEnumerator(ITypeSymbol returnType, KnownTaskTypes knownTypes)
=> returnType.OriginalDefinition.Equals(knownTypes.IEnumeratorOfTType);

private static SyntaxTokenList AddAsyncModifierWithCorrectedTrivia(SyntaxTokenList modifiers, ref TypeSyntax newReturnType)
private static (SyntaxTokenList newModifiers, TypeSyntax newReturnType) AddAsyncModifierWithCorrectedTrivia(SyntaxTokenList modifiers, TypeSyntax returnType)
{
if (modifiers.Any())
return modifiers.Add(AsyncKeyword);
return (modifiers.Add(s_asyncKeywordWithSpace), returnType);

// Move the leading trivia from the return type to the new modifiers list.
var result = TokenList(AsyncKeyword.WithLeadingTrivia(newReturnType.GetLeadingTrivia()));
newReturnType = newReturnType.WithoutLeadingTrivia();
return result;
var newModifiers = TokenList(s_asyncKeywordWithSpace.WithLeadingTrivia(returnType.GetLeadingTrivia()));
var newReturnType = returnType.WithoutLeadingTrivia();
return (newModifiers, newReturnType);
}

private static AnonymousFunctionExpressionSyntax FixAnonymousFunction(AnonymousFunctionExpressionSyntax anonymous)
Expand Down
6 changes: 0 additions & 6 deletions ...vokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessTests_FixAllTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class C
void Goo()
{
a?.Invoke();
a?.Invoke();
}
}
Expand Down Expand Up @@ -86,7 +85,6 @@ class C
void Goo()
{
a?.Invoke();
a?.Invoke();
}
}
Expand Down Expand Up @@ -126,7 +124,6 @@ class C
void Goo()
{
a?.Invoke();
a?.Invoke();
}
}
Expand Down Expand Up @@ -166,7 +163,6 @@ class C
void Goo()
{
a?.Invoke();
a?.Invoke();
}
}
Expand Down Expand Up @@ -206,7 +202,6 @@ class C
void Goo()
{
a?.Invoke();
a?.Invoke();
}
}
Expand Down Expand Up @@ -246,7 +241,6 @@ class C
void Goo()
{
a?.Invoke();
a?.Invoke();
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/Analyzers/CSharp/Tests/RemoveAsyncModifier/RemoveAsyncModifierTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,8 @@ class C
{
public void M1()
{
Func<int, Task> foo = x => {
Func<int, Task> foo = x =>
{
if (System.DateTime.Now.Ticks > 0)
{
return Task.CompletedTask;
Expand Down Expand Up @@ -1040,7 +1041,8 @@ class C
{
public void M1()
{
Func<Task> foo = () => {
Func<Task> foo = () =>
{
if (System.DateTime.Now.Ticks > 0)
{
return Task.CompletedTask;
Expand Down
8 changes: 4 additions & 4 deletions ...sts/UseCoalesceExpression/UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class C
{
void M(int? x, int? y)
{
var z = x ?? y ;
var z = x ?? y;
}
}
""");
Expand Down Expand Up @@ -105,7 +105,7 @@ class C
{
void M(int? x, int? y)
{
var z = (x + y) ?? y ;
var z = (x + y) ?? y;
}
}
""");
Expand Down Expand Up @@ -163,7 +163,7 @@ class C
void M(int? x, int? y)
{
var z1 = x ?? y;
var z2 = x ?? y ;
var z2 = x ?? y;
}
}
""");
Expand Down Expand Up @@ -249,7 +249,7 @@ class C
{
void M(int? x, int? y)
{
Expression<Func<int>> e = () => {|Warning:x ?? y|} ;
Expression<Func<int>> e = () => {|Warning:x ?? y|};
}
}
""");
Expand Down
6 changes: 3 additions & 3 deletions src/EditorFeatures/VisualBasicTest/CodeActions/InlineMethod/VisualBasicInlineMethodTests.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,8 @@ Imports System
Public Class TestClass
Public Sub Caller(i As Integer, j As Integer)
Dim x = Function()
Return i * j
End Function()
Return i * j
End Function()
End Sub
##
Private Function Callee(i As Integer, j As Integer) as Func(Of Integer)
Expand Down Expand Up @@ -1288,7 +1288,7 @@ Public Class TestClass
End Class", "
Public Class TestClass
Public Sub Caller(i As Integer)
Dim y = If (true, 10, 100)
Dim y = If(true, 10, 100)
End Sub
##
Private Function Callee(a As Boolean) As Integer
Expand Down
30 changes: 16 additions & 14 deletions src/Features/CSharpTest/ConvertLinq/ConvertForEachToLinqQueryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4156,10 +4156,10 @@ List<int> M(IEnumerable<int> nums)
return /*30*/ /* 1 *//* 2 *//* 3 *//* 4 */// 5
/*31*//* 6 */
(from/* 8 *//* 7 *//* 9 */x /* 10 */ in/* 11 */nums/* 12 */// 13
/* 14 */// 15
/* 16 *//* 17 */
let y /* 18 */ = /* 19 */ x + 1/* 20 *///21
select y)/* 24 *//*27*///28
/* 14 */// 15
/* 16 *//* 17 */
let y /* 18 */ = /* 19 */ x + 1/* 20 *///21
select y)/* 24 *//*27*///28
.ToList()/* 22 *//* 23 *//* 25 *///26
; //32
}
Expand Down Expand Up @@ -4205,7 +4205,7 @@ List<int> M(IEnumerable<int> nums)
/*25*//* 14 */// 15
/* 6 */
(from/* 8 *//* 7 *//* 9 */x /* 10 */ in/* 11 */nums/* 12 */// 13
select x + 1)/* 18 *//*21*///22
select x + 1)/* 18 *//*21*///22
.ToList()/* 16 *//* 17 *//* 19 *///20
; //26
}
Expand All @@ -4223,7 +4223,8 @@ List<int> M(IEnumerable<int> nums)
{
/*23*/
return /*24*/ /* 1 *//* 2 *//* 3 *//* 4 */// 5
/*25*/nums /* 12 */.Select(
/*25*/
nums /* 12 */.Select(
/* 6 *//* 7 *//* 14 */// 15
/* 9 */x /* 10 */ => x + 1/* 18 *//*21*///22
/* 8 *//* 11 */// 13
Expand Down Expand Up @@ -4268,7 +4269,7 @@ int M(IEnumerable<int> nums)
/*23*//* 14 */// 15
/* 6 */
(from/* 8 *//* 7 *//* 9 */x /* 10 */ in/* 11 */nums/* 12 */// 13
select x)/* 10 *//*19*///20
select x)/* 10 *//*19*///20
.Count()/* 16 *//* 17 *///18
; //24
}
Expand All @@ -4286,7 +4287,8 @@ int M(IEnumerable<int> nums)
{
/*21*/
return /*22*/ /* 1 *//* 2 *//* 3 *//* 4 */// 5
/*23*/nums /* 12 *//* 6 *//* 7 *//* 14 */// 15
/*23*/
nums /* 12 *//* 6 *//* 7 *//* 14 */// 15
/* 9 *//* 10 *//* 10 *//*19*///20
/* 8 *//* 11 */// 13
.Count()/* 16 *//* 17 *///18
Expand Down Expand Up @@ -4328,11 +4330,11 @@ void M(IEnumerable<int> nums)
{
foreach (var (a /* 12 */ , b /*16*/ ) in
/* 1 */from/* 2 */int /* 3 */ n1 /* 4 */in/* 5 */nums/* 6 */// 7
/* 8*/// 9
/* 10 *//* 11 */
let a /* 12 */ = /* 13 */ n1 + n1/* 14*//* 15 */
let b /*16*/ = /*17*/ n1 * n1/*18*///19
select (a /* 12 */ , b /*16*/ )/*22*//*23*/)
/* 8*/// 9
/* 10 *//* 11 */
let a /* 12 */ = /* 13 */ n1 + n1/* 14*//* 15 */
let b /*16*/ = /*17*/ n1 * n1/*18*///19
select (a /* 12 */ , b /*16*/ )/*22*//*23*/)
{
/*20*/
Console.WriteLine(a + b);//21
Expand Down Expand Up @@ -4384,7 +4386,7 @@ void M(IEnumerable<int> nums)
/* 10 */
where/* 11 *//* 12 */n1 /* 13 */ > /* 14 */ 0/* 15 */// 16
select n1/* 4 *//* 21 */// 22
/*23*//*24*/
/*23*//*24*/
)
{
/*19*/
Expand Down
24 changes: 15 additions & 9 deletions src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.CodeRefactorings.InlineTemporary;
using Microsoft.CodeAnalysis.CSharp.Shared.Extensions;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.UnitTests;
Expand All @@ -16,7 +15,7 @@
namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.InlineTemporary
{
[Trait(Traits.Feature, Traits.Features.CodeActionsInlineTemporary)]
public class InlineTemporaryTests : AbstractCSharpCodeActionTest_NoEditor
public sealed class InlineTemporaryTests : AbstractCSharpCodeActionTest_NoEditor
{
protected override CodeRefactoringProvider CreateCodeRefactoringProvider(TestWorkspace workspace, TestParameters parameters)
=> new CSharpInlineTemporaryCodeRefactoringProvider();
Expand Down Expand Up @@ -253,16 +252,23 @@ public void M()
[Fact]
public async Task Conversion_NoConversion()
{
await TestFixOneAsync(
await TestAsync(
"""
{ int [||]x = 3;
class C
{
void F(){ int [||]x = 3;
x.ToString(); }
}
""",
"""
class C
{
void F(){
3.ToString(); }
}
""",
"""
{
3.ToString(); }
""");
CSharpParseOptions.Default);
}

[Fact]
Expand Down Expand Up @@ -690,7 +696,7 @@ class Program
static void Main()
{
int x = 2;
Bar(x < x, x > 1+2);
Bar(x < x, x > 1 + 2);
}
static void Bar(object a, object b)
Expand Down
2 changes: 1 addition & 1 deletion src/Features/CSharpTest/InvertIf/InvertIfTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@ public async Task TestEmptyIf()
{
await TestInRegularAndScriptAsync(
@"class C { void M(string s){ [||]if (s == ""a""){}else{ s = ""b""}}}",
@"class C { void M(string s){ if (s != ""a""){ s = ""b""}}}");
@"class C { void M(string s){ if (s != ""a"") { s = ""b""}}}");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43224")]
Expand Down
7 changes: 6 additions & 1 deletion src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.Shared.Extensions;
Expand Down Expand Up @@ -552,10 +553,14 @@ private async Task<SyntaxNode> GetChangedCallerAsync(Document document,
// After:
// void Caller() { var x = ((Func<int>)(() => 1))(); }
// Func<int> Callee() { return () => 1; }
//
// Also, ensure that the node is formatted properly at the destination location. This is needed as the
// location of the destination node might be very different (indentation/nesting wise) from the original
// method where the inlined code is coming from.
inlineExpression = (TExpressionSyntax)syntaxGenerator.AddParentheses(
syntaxGenerator.CastExpression(
GenerateTypeSyntax(calleeMethodSymbol.ReturnType, allowVar: false),
syntaxGenerator.AddParentheses(inlineMethodContext.InlineExpression)));
syntaxGenerator.AddParentheses(inlineMethodContext.InlineExpression.WithAdditionalAnnotations(Formatter.Annotation))));

}

Expand Down
36 changes: 26 additions & 10 deletions src/Workspaces/Core/Portable/CodeActions/CodeAction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,23 +518,39 @@ protected virtual async Task<Document> PostProcessChangesAsync(Document document
return document;
}

internal static async Task<Document> CleanupDocumentAsync(
Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
internal static async Task<Document> CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
document = await ImportAdder.AddImportsFromSymbolAnnotationAsync(
document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken).ConfigureAwait(false);

document = await Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken).ConfigureAwait(false);
// First, do a syntax pass. Ensuring that things are formatted correctly based on the original nodes and
// tokens. We want to do this prior to cleaning semantics as semantic cleanup can change the shape of the tree
// (for example, by removing tokens), which can then cause formatting to not work as expected.
var document1 = await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false);

// Now, do the semantic cleaning pass.
var document2 = await CleanupSemanticsAsync(document1, options, cancellationToken).ConfigureAwait(false);
return document2;
}

internal static async Task<Document> CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
// format any node with explicit formatter annotation
document = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);

// format any elastic whitespace
document = await Formatter.FormatAsync(document, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
return document2;
}

document = await CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false);
internal static async Task<Document> CleanupSemanticsAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
var document1 = await ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken).ConfigureAwait(false);
var document2 = await Simplifier.ReduceAsync(document1, Simplifier.Annotation, options.SimplifierOptions, cancellationToken).ConfigureAwait(false);
var document3 = await CaseCorrector.CaseCorrectAsync(document2, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false);

return document;
// After doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a good state.
// The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be cleaned.
var document4 = await CleanupSyntaxAsync(document3, options, cancellationToken).ConfigureAwait(false);

return document4;
}

#region Factories for standard code actions
Expand Down
1 change: 0 additions & 1 deletion src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/DocumentBasedFixAllProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixesAndRefactorings;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
Expand Down
Loading

0 comments on commit c0a3973

Please sign in to comment.