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

Add analyzer RCS0062, put expression body on its own line (#1575) #1593

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add analyzer RCS0062, put expression body on its own line (#1575) #1593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0cea640
Add analyzer RCS0062, put expression body on its own line (#1575)
cbersch Dec 6, 2024
736c1a7
Add option for breaking expression on new line #1575
cbersch Dec 9, 2024
3fc981e
Apply suggestions from code review
josefpihrt Dec 14, 2024
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
20 changes: 19 additions & 1 deletion src/Analyzers.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1629,6 +1629,23 @@ public class C
</Sample>
</Samples>
</Analyzer>
<Analyzer>
<Id>RCS0062</Id>
<Identifier>PutExpressionBodyOnItsOwnLine</Identifier>
<Title>Put expression body on its own line</Title>
<DefaultSeverity>Info</DefaultSeverity>
<IsEnabledByDefault>false</IsEnabledByDefault>
<Samples>
<Sample>
<Before><![CDATA[object Foo() => null;]]></Before>
<After><![CDATA[object Foo()
=> null;]]></After>
</Sample>
</Samples>
<ConfigOptions>
<Option Key="arrow_token_new_line" IsRequired="false" />
</ConfigOptions>
</Analyzer>
<Analyzer>
<Id>RCS1001</Id>
<Identifier>AddBracesWhenExpressionSpansOverMultipleLines</Identifier>
Expand Down Expand Up @@ -1978,6 +1995,7 @@ foreach (var item in items)
<Option Key="body_style" IsRequired="true" />
<Option Key="use_block_body_when_declaration_spans_over_multiple_lines" IsRequired="true" />
<Option Key="use_block_body_when_expression_spans_over_multiple_lines" IsRequired="true" />
<Option Key="expression_body_style_on_next_line" IsRequired="true" />
</ConfigOptions>
<Options>
<Option Identifier="ConvertExpressionBodyToBlockBodyWhenExpressionIsMultiLine">
Expand Down Expand Up @@ -7973,4 +7991,4 @@ class FooCodeFixProvider : CodeFixProvider
</Sample>
</Samples>
</Analyzer>
</Analyzers>
</Analyzers>
1 change: 1 addition & 0 deletions src/Analyzers/CSharp/Analysis/RequiredConfigOptionNotSetAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal sealed class RequiredConfigOptionNotSetAnalyzer : AbstractRequiredConfi

private static readonly ConfigOptionDescriptor[] _useBlockBodyOrExpressionBodyOptions = [
ConfigOptions.BodyStyle,
ConfigOptions.ExpressionBodyStyleOnNextLine,
ConfigOptions.UseBlockBodyWhenDeclarationSpansOverMultipleLines,
ConfigOptions.UseBlockBodyWhenExpressionSpansOverMultipleLines,
];
Expand Down
31 changes: 31 additions & 0 deletions src/Common/CSharp/Analysis/ConvertExpressionBodyAnalysis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Roslynator.CSharp.Analysis;

internal static class ConvertExpressionBodyAnalysis
{
public static bool BreakExpressionOnNewLine(SyntaxKind syntaxKind, AnalyzerConfigOptions configOptions)
{
if (!configOptions.TryGetValueAsBool(ConfigOptions.ExpressionBodyStyleOnNextLine, out bool breakOnNewLine)
|| !breakOnNewLine)
{
return false;
}

switch (syntaxKind)
{
case SyntaxKind.MethodDeclaration:
case SyntaxKind.ConstructorDeclaration:
case SyntaxKind.DestructorDeclaration:
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.IndexerDeclaration:
case SyntaxKind.OperatorDeclaration:
case SyntaxKind.ConversionOperatorDeclaration:
return true;
default:
return false;
}
}
}
1 change: 1 addition & 0 deletions src/Common/ConfigOptionKeys.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal static partial class ConfigOptionKeys
public const string EnumFlagValueStyle = "roslynator_enum_flag_value_style";
public const string EnumHasFlagStyle = "roslynator_enum_has_flag_style";
public const string EqualsTokenNewLine = "roslynator_equals_token_new_line";
public const string ExpressionBodyStyleOnNextLine = "roslynator_expression_body_style_on_next_line";
public const string InfiniteLoopStyle = "roslynator_infinite_loop_style";
public const string MaxLineLength = "roslynator_max_line_length";
public const string NewLineAtEndOfFile = "roslynator_new_line_at_end_of_file";
Expand Down
8 changes: 7 additions & 1 deletion src/Common/ConfigOptions.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ public static partial class ConfigOptions
defaultValuePlaceholder: "after|before",
description: "Place new line after/before equals sign");

public static readonly ConfigOptionDescriptor ExpressionBodyStyleOnNextLine = new(
key: ConfigOptionKeys.ExpressionBodyStyleOnNextLine,
defaultValue: null,
defaultValuePlaceholder: "true|false",
description: "Place expression body always on next/same line");

public static readonly ConfigOptionDescriptor InfiniteLoopStyle = new(
key: ConfigOptionKeys.InfiniteLoopStyle,
defaultValue: null,
Expand Down Expand Up @@ -260,7 +266,7 @@ private static IEnumerable<KeyValuePair<string, string>> GetRequiredOptions()
yield return new KeyValuePair<string, string>("RCS0060", JoinOptionKeys(ConfigOptionKeys.BlankLineAfterFileScopedNamespaceDeclaration));
yield return new KeyValuePair<string, string>("RCS0061", JoinOptionKeys(ConfigOptionKeys.BlankLineBetweenSwitchSections));
yield return new KeyValuePair<string, string>("RCS1014", JoinOptionKeys(ConfigOptionKeys.ArrayCreationTypeStyle));
yield return new KeyValuePair<string, string>("RCS1016", JoinOptionKeys(ConfigOptionKeys.BodyStyle, ConfigOptionKeys.UseBlockBodyWhenDeclarationSpansOverMultipleLines, ConfigOptionKeys.UseBlockBodyWhenExpressionSpansOverMultipleLines));
yield return new KeyValuePair<string, string>("RCS1016", JoinOptionKeys(ConfigOptionKeys.BodyStyle, ConfigOptionKeys.UseBlockBodyWhenDeclarationSpansOverMultipleLines, ConfigOptionKeys.UseBlockBodyWhenExpressionSpansOverMultipleLines, ConfigOptionKeys.ExpressionBodyStyleOnNextLine));
yield return new KeyValuePair<string, string>("RCS1018", JoinOptionKeys(ConfigOptionKeys.AccessibilityModifiers));
yield return new KeyValuePair<string, string>("RCS1050", JoinOptionKeys(ConfigOptionKeys.ObjectCreationParenthesesStyle));
yield return new KeyValuePair<string, string>("RCS1051", JoinOptionKeys(ConfigOptionKeys.ConditionalOperatorConditionParenthesesStyle));
Expand Down
4 changes: 4 additions & 0 deletions src/ConfigOptions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@
<Value>before</Value>
</Values>
</Option>
<Option Id="ExpressionBodyStyleOnNextLine">
<ValuePlaceholder>true|false</ValuePlaceholder>
<Description>Place expression body always on next/same line</Description>
</Option>
Comment on lines +122 to +125
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain why did you add this option? I think that this option is basically the same as checking if analyzer is enabled or not. If you want to decide whether to wrap line before => in RCS1016 (as an example) just check if RCS0062 is effective or not.

See extensions methods IsEffective for DiagnosticDescriptor.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically you are right, but I wouldn't know how to use that information in the CodeFixProvider.
I found usings of DiagnosticDescriptor.IsEffective only in analyzers. The respective information is then unambiguous via the created diagnostic.

But configuring the behavior of a single diagnostic (RCS1016) in the CodeFixProvider is always done via AnalyzerConfigOptions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can definitely get the information whether an analyzer is effective from CodeFixProvider. You need a SyntaxTree, which is property of a node and CompilationOptions which is a property of a Project:

DiagnosticRules.ConfigureAwait.IsEffective(
    node.SyntaxTree,
    context.Document.Project.CompilationOptions,
    cancellationToken);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'm still making my way through to understand what to get from where.

So that would work, but now I would have dependency problems:
Currently, my attempt for the (not yet working) line breaking is in Workspaces.Common.ConvertBlockBodyToExpressionBodyRefactoring, but the DiagnosticRules are defined in Formatting.Analyzers.

To know, how to refactor that, I would need to know, how the actual new line is inserted. My approach doesn't work, yet.
Probably I would need to call the SyntaxNode ConvertBlockBodyToExpressionBodyRefactoring.Refactor(SyntaxNode, AnalyzerConfigOptions) from somewhere else, then manipulated the new syntax node, and finally replace the node in the document.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you call IsEffective from code fix provider and then pass it to ConvertBlockBodyToExpressionBodyRefactoring as a parameter? Something like bool wrapLineBeforeArrowToken?

Copy link
Author

@cbersch cbersch Dec 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That also doesn't work, because the RCS0062 DiagnosticRule is defined in Formatting.Analyzers, but the caller of ConvertBlockBodyToExpressionBodyRefactoring.RefactorAsync, which would then have to evaluate the bool, like UseBlockBodyOrExpressionBodyCodeFixProvider (RCS1016) or ConstructorDeclarationRefactoring, can't reference that project.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move DiagnosticRules to Common project.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbersch I moved DiagnosticRules and DiagnosticIdentifiers to Roslynator.Common so now it's accessible from everywhere. Please re-run generate_code.ps1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grest, thanks. I'll rework when I'm back from vacaton

<Option Id="MaxLineLength">
<DefaultValue>140</DefaultValue>
<ValuePlaceholder>&lt;NUM&gt;</ValuePlaceholder>
Expand Down
8 changes: 7 additions & 1 deletion src/Formatting.Analyzers.CodeFixes/CSharp/SyntaxTokenCodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public override ImmutableArray<string> FixableDiagnosticIds
DiagnosticIdentifiers.PlaceNewLineAfterOrBeforeArrowToken,
DiagnosticIdentifiers.PlaceNewLineAfterOrBeforeEqualsToken,
DiagnosticIdentifiers.PutAttributeListOnItsOwnLine,
DiagnosticIdentifiers.AddOrRemoveNewLineBeforeWhileInDoStatement);
DiagnosticIdentifiers.AddOrRemoveNewLineBeforeWhileInDoStatement,
DiagnosticIdentifiers.PutExpressionBodyOnItsOwnLine);
}
}

Expand Down Expand Up @@ -61,6 +62,11 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
await CodeActionFactory.RegisterCodeActionForNewLineAsync(context).ConfigureAwait(false);
break;
}
case DiagnosticIdentifiers.PutExpressionBodyOnItsOwnLine:
{
await CodeActionFactory.RegisterCodeActionForNewLineAsync(context, increaseIndentation: true).ConfigureAwait(false);
break;
}
}
}
}
1 change: 1 addition & 0 deletions src/Formatting.Analyzers/CSharp/DiagnosticIdentifiers.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,6 @@ public static partial class DiagnosticIdentifiers
public const string PlaceNewLineAfterOrBeforeNullConditionalOperator = "RCS0059";
public const string BlankLineAfterFileScopedNamespaceDeclaration = "RCS0060";
public const string BlankLineBetweenSwitchSections = "RCS0061";
public const string PutExpressionBodyOnItsOwnLine = "RCS0062";
}
}
12 changes: 12 additions & 0 deletions src/Formatting.Analyzers/CSharp/DiagnosticRules.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,5 +645,17 @@ public static partial class DiagnosticRules
helpLinkUri: DiagnosticIdentifiers.BlankLineBetweenSwitchSections,
customTags: Array.Empty<string>());

/// <summary>RCS0062</summary>
public static readonly DiagnosticDescriptor PutExpressionBodyOnItsOwnLine = DiagnosticDescriptorFactory.Create(
id: DiagnosticIdentifiers.PutExpressionBodyOnItsOwnLine,
title: "Put expression body on its own line",
messageFormat: "Put expression body on its own line",
category: DiagnosticCategories.Roslynator,
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: false,
description: null,
helpLinkUri: DiagnosticIdentifiers.PutExpressionBodyOnItsOwnLine,
customTags: Array.Empty<string>());

}
}
76 changes: 76 additions & 0 deletions src/Formatting.Analyzers/CSharp/PutExpressionBodyOnItsOwnLineAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Reflection.Metadata;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Roslynator.CSharp;
using Roslynator.CSharp.Analysis;
using Roslynator.CSharp.CodeStyle;

namespace Roslynator.Formatting.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class PutExpressionBodyOnItsOwnLineAnalyzer : BaseDiagnosticAnalyzer
{
private static ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics;

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
{
get
{
if (_supportedDiagnostics.IsDefault)
Immutable.InterlockedInitialize(ref _supportedDiagnostics, DiagnosticRules.PutExpressionBodyOnItsOwnLine);

return _supportedDiagnostics;
}
}

public override void Initialize(AnalysisContext context)
{
base.Initialize(context);

context.RegisterSyntaxNodeAction(f => AnalyzeArrowExpressionClause(f), SyntaxKind.ArrowExpressionClause);
}

private static void AnalyzeArrowExpressionClause(SyntaxNodeAnalysisContext context)
{
var arrowExpressionClause = (ArrowExpressionClauseSyntax)context.Node;
AnalyzerConfigOptions configOptions = context.GetConfigOptions();

if (ConvertExpressionBodyAnalysis.BreakExpressionOnNewLine(arrowExpressionClause.Parent.Kind(), configOptions))
{
AnalyzeArrowExpressionClause(arrowExpressionClause.ArrowToken, context);
}
}

private static void AnalyzeArrowExpressionClause(SyntaxToken arrowToken, SyntaxNodeAnalysisContext context)
{
NewLinePosition newLinePosition = context.GetArrowTokenNewLinePosition();

SyntaxToken first;
SyntaxToken second;
if (newLinePosition == NewLinePosition.After)
{
first = arrowToken;
second = arrowToken.GetNextToken();
}
else
{
first = arrowToken.GetPreviousToken();
second = arrowToken;
}

TriviaBlock block = TriviaBlock.FromBetween(first, second);

if (block.Kind == TriviaBlockKind.NoNewLine)
{
DiagnosticHelpers.ReportDiagnostic(
context,
DiagnosticRules.PutExpressionBodyOnItsOwnLine,
block.GetLocation());
}
}
}
Loading
Loading