Bug 10727 - Auto-Fix regarding control flow: "Invert if and simplify else block"
Summary: Auto-Fix regarding control flow: "Invert if and simplify else block"
Status: RESOLVED FIXED
Alias: None
Product: Xamarin Studio
Classification: Desktop
Component: C# Binding ()
Version: 4.0
Hardware: PC Linux
: Normal enhancement
Target Milestone: ---
Assignee: Mike Krüger
URL:
Depends on:
Blocks:
 
Reported: 2013-02-27 07:54 UTC by Ciprian Khlud
Modified: 2013-03-27 06:34 UTC (History)
1 user (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
80% of coding for thi (1.54 KB, application/zip)
2013-02-27 16:09 UTC, Ciprian Khlud
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Ciprian Khlud 2013-02-27 07:54:45 UTC
Reference to bug #10717 comment 1
Add an Auto-Fix that simplify blocks of code:

Steps to reproduce:
Write a code that matches this pattern:

$if (condition)
{
   instr1;...; instrN;//a block with one or more instructions
}
else
{//block curly braces can be or can not be here
  continue|break|return;
}

Expected result:
Should be an Auto-Fix solution that simplify the expression by rewriting it in this form:
if(!condition)
  continue|break|return;
instr1;...; instrN;

Note: all pieces seem to be in place, it needs to be a composite of: Invert If,
Remove Redundant Else; Remove Redundant Block
Comment 1 Ciprian Khlud 2013-02-27 16:09:08 UTC
Created attachment 3504 [details]
80% of coding for thi

I did the coding as much as I was aware how to do it... I got blocked as it crashes when I do: InsertAfter (you will see when you run the unit tests)
Comment 2 Ciprian Khlud 2013-02-27 16:09:52 UTC
Added a "patch" that runs most of the code. It crashes though when I write the nodes themselves.
Comment 3 Ciprian Khlud 2013-03-26 20:31:45 UTC
I don't know how to make fully the "pull request" but I succeeded to merge with Xamarin Studio the Git repo after I clonned into a GitHub clone I've created.

It seems that Xamarin Studio gave a too big diff file (I think is because of file endings) in case of csproj files.

The initial commit is this:

https://github.com/ciplogic/NRefactory/commit/0a3fe688d89bc92a5890507de93e3055a6134c62

I've extended the initial first test so it makes sure that it simplify the first block correctly, even it has more instructions:
https://github.com/ciplogic/NRefactory/commit/a673e4472dc5e4999a6d8ae989c3130bb1ed66ed
Comment 4 Mikayla Hutchinson [MSFT] 2013-03-27 00:03:06 UTC
Looks like line ending changes, check you have autocrlf set up correctly and your git tool respects it. What OS and git tools are you using?
Comment 5 Ciprian Khlud 2013-03-27 05:34:52 UTC
Hi Michael, Thank you for prompt review.

Sorry but I'm a n00b into Git. (See this for details: http://xamarin.uservoice.com/forums/144858-xamarin-suggestions/suggestions/2698087-xamarin-studio-should-support-mercurial-as-version )  The OS is Windows and the Git tools are the ones inside Xamarin Studio. I also installed Git Extension but I wasn't able to understand it (it integrates with VS, but I commited with XS).
Comment 6 Mike Krüger 2013-03-27 05:58:51 UTC
It's the git installation - when installing git it asks about crlf settings.

Choose the option check out windows / check in unix line endings when installing git - that should work.
Comment 7 Mike Krüger 2013-03-27 06:34:13 UTC
Thanks - I've pushed your action to NRefactory:

https://github.com/icsharpcode/NRefactory/commit/772b66e109f1f215a1744c0449d92864516371d6

just revert your commits in your repository with

git reset --hard 106bfc49db31b8e76449acecfe249d324f4a6005

I recommend the command line for git :) - the github client installs all that and makes it very easy to work with github repositories.

I like this action :) - I only found a little issue:

blockStatement.Statements.First();

That would crash with an empty else block (if (true) { Something (); } else { }).
I try to avoid the use of .First () or .Last (). 

Then a more general thing (one of your unit test has shown it). You're potentially destroying code that is in the else block. Even if the code is unreachable it shouldn't be cut. And keep in mind that there may be comments :

if (cond) {
   DoSomething ();
} else {
   // Bailout because of something has happened
   return;
}

Should preserve the comment :). 

See: https://github.com/icsharpcode/NRefactory/commit/a96c0b75cf456680a33ce7f493b59b23d6aace1e

It's easy to write actions that destroy comments, therefore we need to be careful about stuff like that. At least well written comments should be preserved - if a user tries to write:

if (cond) Something (); else return /* comment */ ;  

The user plans that this comment gets destroyed by a code action :)

Writing code actions can make a lot of fun - if you have any comments/ideas on how to improve the API let me know :) 

Thanks for this action - I often see code where this action could help :)