roslynator icon indicating copy to clipboard operation
roslynator copied to clipboard

RFE: RR0150, RR0151 (Replace while with ...): Handle 'continue'

Open tamlin-mike opened this issue 6 years ago • 4 comments

Roslynator Refactorings 2017 up to and including 2.1.1

When refactoring, and flipping conditional code around, you can sometimes end up with something like

int i = 0;
while (i cond1 smth)
{
  if (cond2) { /* ... */ }
  ++i;
  continue; // used to be inside "if (!cond2)
}

It would be nice if these refactorings noticed the continue is a NOP, allowing these two refactorings to run, and remove the (now spurious) continue.

Coming to think of it, that could/should probably be turned into an RR of itself - removal of pointless/"dead" code.

Additionally, it could be nice when converting to for to allow (though probably not enforce) the loop condition-variable initialization to be placed as such, converting the previous into

for (int i = 0; i cond1 smth; ++i)
{
  if (cond2) { /* ... */ }
}

or perhaps that could be turned into an RR or two of itself, moving the for-loop variable initialization in/out of the for-initialization-expression. (just a brainstorming idea)

While I'm not the least familiar with the Roslynator code, a cursory glance suggest https://github.com/JosefPihrt/Roslynator/blob/d402258408dc8e17d737417e1c268f91be7b5d49/src/Refactorings/CSharp/Refactorings/ReplaceWhileWithForRefactoring.cs#L81-L82

could be the source of not allowing the refactoring? Of course, that's just the detection part...

tamlin-mike avatar May 25 '19 23:05 tamlin-mike

1) Remove continue statement

Could you clarify when do you want to remove continue (ideally please provide code before and after)?

2) Removal of pointless/dead code

There is already analyzer RCS1134 that takes care of removing unnecessary statement.

3) Make variable declaration a part of for statement.

Currently the variable declaration(s) will be made part of for statement if you select variable declaration and subsequent for statement.

But it probably make sense to to make variable declaration(s) a part of for statement if it is not referenced after for statement.

josefpihrt avatar May 29 '19 12:05 josefpihrt

  1. When continue is the last statement in a loop, i.e. preceeding the loop-body closing brace token (literally as the example - when the continue is completely redundant).

  2. I was thinking of an interactive version, for at least these two RR's, for us using only the refactoring parts of Roslynator. If it's already in RCS1134, could that code perhaps be used as-is for the two mentioned refactorings?

  3. Oh, I didn't know that! Considering I have manually moved many (probably thousands of) loop variables manually into for-loop-init-statement, not knowing about this feature, that suggests the discoverability of the feature is very low (perhaps somewhat of an understatement, considering it creates a catch 22; you would need to know about the feature to even discover it). As such, I agree it would make sense to automatically move it if possible.

tamlin-mike avatar May 29 '19 15:05 tamlin-mike

Re. 3, I simply can't get that to work. Sample:

int i = 0;
for (; i < 42; i++)
{
}

If I select from the beginning 'i' in 'int' and continue to the ')', the only Quick Action option given is "Extract Method".

If I select the two first whole lines, I'm given the additional options to wrap in region and wrap in #if.

If I select ony the identifier i, one of the options is "Inline temporary variable", which turns that code into the hilarious

int i = 0;
for (; 0 < 42; i++)
{
}

Could you please elaborate on what needs to be selected to be presented a refactoring to move that int i = 0 to become the for-initializer?

tamlin-mike avatar May 31 '19 06:05 tamlin-mike

I meant that you can select variable declaration and subsequent while statement to convert it to for statement.

josefpihrt avatar Jun 04 '19 12:06 josefpihrt