From cd2e10040f61efc03b4528d642995fd9b0380e65 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 19 Jan 2025 22:48:02 +0100 Subject: [PATCH] Fix line break when converting block to expression body --- .../RCS1016UseExpressionBodyTests.cs | 30 +++++- ...ertBlockBodyToExpressionBodyRefactoring.cs | 92 ++++++++++--------- 2 files changed, 75 insertions(+), 47 deletions(-) diff --git a/src/Tests/Analyzers.Tests/RCS1016UseExpressionBodyTests.cs b/src/Tests/Analyzers.Tests/RCS1016UseExpressionBodyTests.cs index 50d044f657..186c7f64b6 100644 --- a/src/Tests/Analyzers.Tests/RCS1016UseExpressionBodyTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1016UseExpressionBodyTests.cs @@ -62,8 +62,34 @@ public C() void M() { } } ", - options: Options.AddConfigOption(ConfigOptionKeys.ArrowTokenNewLine, ConfigOptionValues.ArrowTokenNewLine_Before), - additionalDiagnostics: new DiagnosticDescriptor[] { DiagnosticRules.PutExpressionBodyOnItsOwnLine }); + options: Options.EnableDiagnostic(DiagnosticRules.PutExpressionBodyOnItsOwnLine) + .AddConfigOption(ConfigOptionKeys.ArrowTokenNewLine, ConfigOptionValues.ArrowTokenNewLine_Before)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseBlockBodyOrExpressionBody)] + public async Task Test_Constructor_BreakAfterArrow() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + public C() + [|{ + M(); + }|] + + void M() { } +} +", @" +class C +{ + public C() => + M(); + + void M() { } +} +", + options: Options.EnableDiagnostic(DiagnosticRules.PutExpressionBodyOnItsOwnLine) + .AddConfigOption(ConfigOptionKeys.ArrowTokenNewLine, ConfigOptionValues.ArrowTokenNewLine_After)); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseBlockBodyOrExpressionBody)] diff --git a/src/Workspaces.Common/CSharp/Refactorings/ConvertBlockBodyToExpressionBodyRefactoring.cs b/src/Workspaces.Common/CSharp/Refactorings/ConvertBlockBodyToExpressionBodyRefactoring.cs index 86c93487b5..a8cb6c6e66 100644 --- a/src/Workspaces.Common/CSharp/Refactorings/ConvertBlockBodyToExpressionBodyRefactoring.cs +++ b/src/Workspaces.Common/CSharp/Refactorings/ConvertBlockBodyToExpressionBodyRefactoring.cs @@ -166,9 +166,9 @@ public static Task RefactorAsync( if (CanRefactor(member)) { AnalyzerConfigOptions configOptions = document.GetConfigOptions(selectedMembers.Parent.SyntaxTree); - bool wrapLineBeforeArrowToken = DiagnosticRules.PutExpressionBodyOnItsOwnLine.IsEffective(member.SyntaxTree, document.Project.CompilationOptions); + NewLinePosition newLinePosition = GetNewLinePosition(document, member, configOptions, cancellationToken); - var newMember = (MemberDeclarationSyntax)Refactor(member, configOptions, wrapLineBeforeArrowToken); + var newMember = (MemberDeclarationSyntax)Refactor(member, configOptions, newLinePosition); return newMember .WithTrailingTrivia(member.GetTrailingTrivia()) @@ -181,20 +181,43 @@ public static Task RefactorAsync( return document.ReplaceMembersAsync(SyntaxInfo.MemberDeclarationListInfo(selectedMembers.Parent), newMembers, cancellationToken); } - public static Task RefactorAsync( + public static async Task RefactorAsync( Document document, SyntaxNode node, CancellationToken cancellationToken = default) { AnalyzerConfigOptions configOptions = document.GetConfigOptions(node.SyntaxTree); - bool wrapLineBeforeArrowToken = DiagnosticRules.PutExpressionBodyOnItsOwnLine.IsEffective(node.SyntaxTree, document.Project.CompilationOptions, cancellationToken); + NewLinePosition newLinePosition = GetNewLinePosition(document, node, configOptions, cancellationToken); + + SyntaxNode newNode = Refactor(node, configOptions, newLinePosition).WithFormatterAnnotation(); - SyntaxNode newNode = Refactor(node, configOptions, wrapLineBeforeArrowToken).WithFormatterAnnotation(); + if (newLinePosition == NewLinePosition.After) + { + var arrowToken = CSharpUtility.GetExpressionBody(newNode).ArrowToken; + var annotation = new SyntaxAnnotation(); + newNode = newNode.ReplaceToken(arrowToken, arrowToken.WithAdditionalAnnotations(annotation)); + document = await document.ReplaceNodeAsync(node, newNode, cancellationToken).ConfigureAwait(false); + SyntaxNode root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + arrowToken = root.GetAnnotatedTokens(annotation).Single(); + var textChange = new TextChange(TextSpan.FromBounds(arrowToken.GetPreviousToken().Span.End, arrowToken.SpanStart), " "); + return await document.WithTextChangeAsync(textChange, cancellationToken).ConfigureAwait(false); + } - return document.ReplaceNodeAsync(node, newNode, cancellationToken); + return await document.ReplaceNodeAsync(node, newNode, cancellationToken).ConfigureAwait(false); } - private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions configOptions, bool wrapLineBeforeArrowToken) + private static NewLinePosition GetNewLinePosition(Document document, SyntaxNode node, AnalyzerConfigOptions configOptions, CancellationToken cancellationToken) + { + if (DiagnosticRules.PutExpressionBodyOnItsOwnLine.IsEffective(node.SyntaxTree, document.Project.CompilationOptions, cancellationToken) + && ConvertExpressionBodyAnalysis.AllowPutExpressionBodyOnItsOwnLine(node.Kind())) + { + return configOptions.GetArrowTokenNewLinePosition(); + } + + return NewLinePosition.None; + } + + private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions configOptions, NewLinePosition newLinePosition) { switch (node.Kind()) { @@ -204,7 +227,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(methodDeclaration.Body); return methodDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, methodDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, methodDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(methodDeclaration.Body, analysis)) .WithBody(null); } @@ -214,7 +237,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(constructorDeclaration.Body); return constructorDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, constructorDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, constructorDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(constructorDeclaration.Body, analysis)) .WithBody(null); } @@ -224,7 +247,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(destructorDeclaration.Body); return destructorDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, destructorDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, destructorDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(destructorDeclaration.Body, analysis)) .WithBody(null); } @@ -234,7 +257,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(localFunction.Body); return localFunction - .WithExpressionBody(CreateExpressionBody(analysis, localFunction, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, localFunction, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(localFunction.Body, analysis)) .WithBody(null); } @@ -244,7 +267,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(operatorDeclaration.Body); return operatorDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, operatorDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, operatorDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(operatorDeclaration.Body, analysis)) .WithBody(null); } @@ -254,7 +277,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(operatorDeclaration.Body); return operatorDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, operatorDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, operatorDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(operatorDeclaration.Body, analysis)) .WithBody(null); } @@ -264,7 +287,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(propertyDeclaration.AccessorList); return propertyDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, propertyDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, propertyDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(analysis.Block, analysis)) .WithAccessorList(null); } @@ -274,7 +297,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(indexerDeclaration.AccessorList); return indexerDeclaration - .WithExpressionBody(CreateExpressionBody(analysis, indexerDeclaration, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, indexerDeclaration, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(analysis.Block, analysis)) .WithAccessorList(null); } @@ -288,7 +311,7 @@ private static SyntaxNode Refactor(SyntaxNode node, AnalyzerConfigOptions config BlockExpressionAnalysis analysis = BlockExpressionAnalysis.Create(accessor); return accessor - .WithExpressionBody(CreateExpressionBody(analysis, accessor, configOptions, wrapLineBeforeArrowToken)) + .WithExpressionBody(CreateExpressionBody(analysis, accessor, configOptions, newLinePosition)) .WithSemicolonToken(CreateSemicolonToken(analysis.Block, analysis)) .WithBody(null); } @@ -304,7 +327,7 @@ private static ArrowExpressionClauseSyntax CreateExpressionBody( BlockExpressionAnalysis analysis, SyntaxNode declaration, AnalyzerConfigOptions configOptions, - bool wrapLineBeforeArrowToken) + NewLinePosition newLinePosition) { SyntaxToken arrowToken = Token(SyntaxKind.EqualsGreaterThanToken); @@ -335,42 +358,21 @@ private static ArrowExpressionClauseSyntax CreateExpressionBody( } } - expression = SyntaxTriviaAnalysis.SetIndentation(expression, declaration, configOptions); - - if (wrapLineBeforeArrowToken) - { - NewLinePosition newLinePosition = GetArrowNewLinePosition(declaration.Kind(), configOptions); - return CreateArrayExpression(arrowToken, expression, newLinePosition); - } - else - { - return ArrowExpressionClause(arrowToken, expression); - } - } - - private static NewLinePosition GetArrowNewLinePosition(SyntaxKind syntaxKind, AnalyzerConfigOptions configOptions) - { - if (ConvertExpressionBodyAnalysis.AllowPutExpressionBodyOnItsOwnLine(syntaxKind)) - { - return configOptions.GetArrowTokenNewLinePosition(); - } - else - { - return NewLinePosition.None; - } - } - - private static ArrowExpressionClauseSyntax CreateArrayExpression(SyntaxToken arrowToken, ExpressionSyntax expression, NewLinePosition newLinePosition) - { switch (newLinePosition) { case NewLinePosition.After: arrowToken = arrowToken.AppendToTrailingTrivia(CSharpFactory.NewLine()); + expression = SyntaxTriviaAnalysis.SetIndentation(expression, declaration, configOptions); break; case NewLinePosition.Before: - arrowToken = arrowToken.PrependToLeadingTrivia(CSharpFactory.NewLine()); + SyntaxTrivia trivia = SyntaxTriviaAnalysis.GetIncreasedIndentationTrivia(declaration, configOptions, CancellationToken.None); + arrowToken = arrowToken.WithLeadingTrivia(trivia); + break; + default: + expression = SyntaxTriviaAnalysis.SetIndentation(expression, declaration, configOptions); break; } + return ArrowExpressionClause(arrowToken, expression); }