Do syntactic cleanup during the initial pass of producing fixed documents in fix-all. #73383
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -163,7 +163,7 @@ class C | |||
| void M(int? x, int? y) | |||
| { | |||
| var z1 = x ?? y; | |||
| var z2 = x ?? y ; | |||
| var z2 = x ?? y; | |||
There was a problem hiding this comment.
This fixed an issue here where we had errant whitespace.
| @@ -46,7 +46,6 @@ class C | |||
| void Goo() | |||
| { | |||
| a?.Invoke(); | |||
|
|
|||
| a?.Invoke(); | |||
There was a problem hiding this comment.
these changed because the code that was generated was (a)?.Invoke(); with elastic trivia on the parens. But when the parens were removed in the simplification pass, we lost that trivia. So we weren't formatting properly. the new formatting follows the expected formatting rules now.
| @@ -93,15 +92,7 @@ public sealed override IEnumerable<FixAllScope> GetSupportedFixAllScopes() | |||
| return; | |||
|
|
|||
| var newDocument = await this.FixAllAsync(fixAllContext, document, documentDiagnostics).ConfigureAwait(false); | |||
| if (newDocument == null || newDocument == document) | |||
There was a problem hiding this comment.
this code was duplicated in the CodeFix FixAllProvider, and the CodeRefactoring FixAllProvider. It was been extracted and is run when this code calls into onDocumentFixed.
| @@ -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); | |||
There was a problem hiding this comment.
fixing up the feature so it consistently emits async without messing with other trivia.
| @@ -902,7 +902,8 @@ class C | |||
| { | |||
| public void M1() | |||
| { | |||
| Func<int, Task> foo = x => { | |||
| Func<int, Task> foo = x => | |||
| { | |||
There was a problem hiding this comment.
this is the correct formatting ew want. the previous results were undesirable.
| @@ -49,7 +49,7 @@ class C | |||
| { | |||
| void M(int? x, int? y) | |||
| { | |||
| var z = x ?? y ; | |||
| var z = x ?? y; | |||
There was a problem hiding this comment.
same. the old results were undesirable.
| /* 14 */// 15 | ||
| /* 16 *//* 17 */ | ||
| let y /* 18 */ = /* 19 */ x + 1/* 20 *///21 | ||
| select y)/* 24 *//*27*///28 |
There was a problem hiding this comment.
these results are expected. the other clauses align with the from now.
There was a problem hiding this comment.
aside; these tests are insane. they don't reflect any sort of real code, and it's nigh impossible to reason about them.
| { | ||
| 3.ToString(); } | ||
| """); | ||
| CSharpParseOptions.Default); |
There was a problem hiding this comment.
this test was impossible to understand in its original form (due to how it concats strings). The updated test shows the full code sample, and shows the desired behavior (we don't touch the { on the method when inlining here.
| @@ -690,7 +696,7 @@ class Program | |||
| static void Main() | |||
| { | |||
| int x = 2; | |||
| Bar(x < x, x > 1+2); | |||
| Bar(x < x, x > 1 + 2); | |||
There was a problem hiding this comment.
formatting this now because it's parsed as an arithmetic + binexpr.
|
@ToddGrun ptal |
|
@ToddGrun this one is ready. |
|
LGTM (I'll trust you on the behavior changes being desired, they seem good to me) |
|
@jasonmalinowski For review when you get back. |
Followup to #73385.