roslyn: Decompilation should put new lines between blocks

We should layout methods a little better by introducing new lines to make a little more readable (Reflector did this well)

Create Console App (.NET Framework)
In the following, CTRL+Click int

    class Program
    {
        static void Main(string[] args)
        {
            int foo;
        }
    }

When prompted say Yes to decompile
Scroll down to CompareTo:

Expected:

		public int CompareTo(object value)
		{
			if (value == null)
			{
				return 1;
			}

			if (value is int)
			{
				int num = (int)value;
				if (this < num)
				{
					return -1;
				}

				if (this > num)
				{
					return 1;
				}

				return 0;
			}

			throw new ArgumentException(Environment.GetResourceString("Arg_MustBeInt32"));
		}

Actual:

		public int CompareTo(object value)
		{
			if (value == null)
			{
				return 1;
			}
			if (value is int)
			{
				int num = (int)value;
				if (this < num)
				{
					return -1;
				}
				if (this > num)
				{
					return 1;
				}
				return 0;
			}
			throw new ArgumentException(Environment.GetResourceString("Arg_MustBeInt32"));
		}

About this issue

Original URL
State: open
Created 6 years ago
Comments: 16 (13 by maintainers)

Most upvoted comments

Honestly, what might make a lot of sense would be to have Roslyn attempt to parse the decompiled source, and if that does not produce parse errors then light up normal roslyn services on the file (like formatting it and simplifying it and whatnot). That way the decompiler doesn’t have to go try to match everything in Roslyn.

The reason we didn’t do that initially was because it was unclear if we could get good trees all the time out of the decompiler. It’s clear there are cases where we don’t, but it seems like we will much of the time. In that case, it would be rather simple to just get this improvement in Roslyn itself.

CyrusNajmabadi on May 5, 2018

Changing the code in ILSpy itself would be the shortest path to a solution here.

@sharwell @Candace-Williford currently we do not implement many of the formatting options in CSharpFormattingOptions because many of them were implemented at a different level in NRefactory. We only took the parts needed for a decompiler (i.e. type system, semantic analysis, AST and output visitor). The CSharpFormattingOptions were applied in the FormattingVisitor which changed the locations of the tokens in the AST.

However, this solution does not work anymore, because we insert the AST locations in the final step, when actually printing the AST to the output. Before the output step there are no tokens in the AST that could be modified, therefore the code that handles the formatting options would have to be moved to/implemented in CSharpOutputVisitor. This is a bigger task. At the end of this we could add a new formatting option that allows you to add new lines between block statements.

If you are interested in contributing this, I will be happy to help you. I think the easiest way to discuss this would be either opening an issue in our repo and/or contacting us on gitter.

Just one thought: Why not use the Roslyn Formatter/Simplifier? This would solve #25251 as well.

@sharwell I have done some changes to the ILSpy integration in Roslyn, I will soon provide a PR.

siegfriedpammer on Mar 11, 2018