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

Analyzer that checks the breaking behavior of expression bodies #1575

Open
cbersch opened this issue Nov 5, 2024 · 9 comments · May be fixed by #1593
Open

Analyzer that checks the breaking behavior of expression bodies #1575

cbersch opened this issue Nov 5, 2024 · 9 comments · May be fixed by #1593

Comments

@cbersch
Copy link

cbersch commented Nov 5, 2024

In our code we prefer having single line methods and properties as expression bodies with

roslynator_body_style = expression
roslynator_use_block_body_when_declaration_spans_over_multiple_lines = true
roslynator_use_block_body_when_expression_spans_over_multiple_lines = true

However, we also like to break before => to easier see the actual body.

For that I would like to request an analyzer and fix.

Example

public int Property
    => 12;

public int DoSomething()
    => SingleLineBreaksBeforeArray();

What do you think of it?

It could be an option like

roslynator_expression_body_new_line = never|after_arrow|before_arrow
@josefpihrt
Copy link
Collaborator

@cbersch I think you are looking for this one: https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0032

@cbersch
Copy link
Author

cbersch commented Nov 6, 2024

@josefpihrt Awesome, this is indeed, what I was looking for. Thank you!

@cbersch cbersch closed this as completed Nov 6, 2024
@cbersch
Copy link
Author

cbersch commented Nov 6, 2024

@josefpihrt Maybe I was too fast to close this issue:
Applying RCS0032 doesn't work properly in my opinion. Consider the two methods

public int[] Allocate(int size) => new int[size];

public int[] Allocate2(int size) =>
    new int[size];

With settings

roslynator_arrow_token_new_line = before
roslynator_body_style = expression

Expected

public int[] Allocate(int size)
    => new int[size];

public int[] Allocate2(int size)
    => new int[size];

Actual

public int[] Allocate(int size) => new int[size];

public int[] Allocate2(int size)
    => new int[size];

@cbersch cbersch reopened this Nov 6, 2024
@josefpihrt
Copy link
Collaborator

Ok, so first of all, RCS0032 (and also RCS0027, RCS0028, RCS0052) are designed to change the position of an operator (e.g. =>) only if it's at the beginning/end of line.

What you want is something like: always put expression-body (of method, property etc.) on the next line. Right?

It could be something like this (just a proposal):

roslynator_expression_body_style = always_on_next_line

or

roslynator_body_style = block|expression|expression_on_next_line
roslynator_body_style = block|expression|expression_on_own_line

Position of arrow token would still be determined by RCS0032.

@cbersch
Copy link
Author

cbersch commented Nov 8, 2024

What you want is something like: always put expression-body (of method, property etc.) on the next line. Right?

Yes.

It could be something like this (just a proposal):

roslynator_expression_body_style = always_on_next_line

or

roslynator_body_style = block|expression|expression_on_next_line
roslynator_body_style = block|expression|expression_on_own_line

Yes, both should work. I can't judge, which one fits best in the naming scheme, yet.

Position of arrow token would still be determined by RCS0032.

That's also, what I would expect.

@cbersch
Copy link
Author

cbersch commented Nov 15, 2024

@josefpihrt To get started I implemented an analyzer for this.
That was quite straightforward, thanks to the amazing current code structure!

I'm not yet sure, how different analyzers are supposed to interact. As existing analyzers for this situation we have

  1. RCS0032: Place new line after/before arrow token
    Handles break position around arrow token which already has a new line somewhere, which might be changed
  2. RCS1016: Use block body or expression body
    For a method body, this creates an expression body, which is on the same line as the method declaration.

Now I implemented an additional analyzer (see https://github.com/cbersch/roslynator/tree/put-expression-body-on-its-own-line)
"Put expression body on its own line"
which breaks an existing expression body in its own line like
Before

object Foo() => null;

After

object Foo()
    => null;

The break position also depends on the option roslynator_arrow_token_new_line of RCS0032.

Is that a valid approach for an additional analyzer? What do you think?

If that's fine, I'll proceed.

@josefpihrt
Copy link
Collaborator

Hi @cbersch,

sorry for late response, I was busy at work.

Please proceed with the PR 👍 . I'll review it and add comments after it's created.

From what I've seen I think it would be also great to update code fixer for RCS1016 and https://josefpihrt.github.io/docs/roslynator/refactorings/RR0169

@cbersch
Copy link
Author

cbersch commented Dec 6, 2024

@josefpihrt
I opened a PR for the new analyzer.
I haven't changed RCS1016 and RR0169, yet, because I didn't manage adding a new line to the created ArrowExpressionClauseSyntax.
Tried things like

arrowExpressionClause = arrowExpressionClause.PrependToLeadingTrivia(CSharpFactory.NewLine());

which had no effect.
I also tried adding it to the ArrowExpressionClauseSyntax.ArrowToken itself, but again, no effect.

Can you give me a hint, how to add a new line in these cases?

@josefpihrt
Copy link
Collaborator

@cbersch From this one line of code I'm not sure where is the problem. Could you do the changes directly in the MR (even if it does not work) and we can discuss it there.

cbersch added a commit to cbersch/roslynator that referenced this issue Dec 9, 2024
cbersch added a commit to cbersch/roslynator that referenced this issue Dec 9, 2024
* Add option roslynator_expression_body_style_on_next_line to enable breaking of expression bodies on new line.
  The line position is determined by option roslynator_arrow_token_new_line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants