Fix #1990: Empty default ctor with XML comment discarded from decompilation

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -180,7 +180,7 @@ namespace ICSharpCode.Decompiler.CSharp
 				new IntroduceUnsafeModifier(),
 				new AddCheckedBlocks(),
 				new DeclareVariables(), // should run after most transforms that modify statements
-				new ConvertConstructorCallIntoInitializer(), // must run after DeclareVariables
+				new TransformFieldAndConstructorInitializers(), // must run after DeclareVariables
 				new DecimalConstantTransform(),
 				new PrettifyAssignments(), // must run after DeclareVariables
 				new IntroduceUsingDeclarations(),

ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs

@@ -17,7 +17,6 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Linq;
 using ICSharpCode.Decompiler.CSharp.Syntax;
 using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching;
@@ -27,35 +26,31 @@ using SRM = System.Reflection.Metadata;
 namespace ICSharpCode.Decompiler.CSharp.Transforms
 {
 	/// <summary>
-	/// If the first element of a constructor is a chained constructor call, convert it into a constructor initializer.
+	/// This transform moves field initializers at the start of constructors to their respective field declarations
+	/// and transforms this-/base-ctor calls in constructors to constructor initializers.
 	/// </summary>
-	public class ConvertConstructorCallIntoInitializer : IAstTransform
+	public class TransformFieldAndConstructorInitializers : DepthFirstAstVisitor, IAstTransform
 	{
-		public void Run(AstNode node, TransformContext context)
-		{
-			var visitor = new ConvertConstructorCallIntoInitializerVisitor(context);
-			
-			// If we're viewing some set of members (fields are direct children of SyntaxTree),
-			// we also need to handle those:
-			visitor.HandleInstanceFieldInitializers(node.Children);
-			visitor.HandleStaticFieldInitializers(node.Children);
-			
-			node.AcceptVisitor(visitor);
+		TransformContext context;
 
-			visitor.RemoveSingleEmptyConstructor(node.Children, context.CurrentTypeDefinition);
-		}
-	}
-	
-	sealed class ConvertConstructorCallIntoInitializerVisitor : DepthFirstAstVisitor
-	{
-		readonly TransformContext context;
-		
-		public ConvertConstructorCallIntoInitializerVisitor(TransformContext context)
+		public void Run(AstNode node, TransformContext context)
 		{
-			Debug.Assert(context != null);
 			this.context = context;
+
+			try {
+				// If we're viewing some set of members (fields are direct children of SyntaxTree),
+				// we also need to handle those:
+				HandleInstanceFieldInitializers(node.Children);
+				HandleStaticFieldInitializers(node.Children);
+
+				node.AcceptVisitor(this);
+
+				RemoveSingleEmptyConstructor(node.Children, context.CurrentTypeDefinition);
+			} finally {
+				this.context = null;
+			}
 		}
-		
+
 		public override void VisitConstructorDeclaration(ConstructorDeclaration constructorDeclaration)
 		{
 			if (!(constructorDeclaration.Body.Statements.FirstOrDefault() is ExpressionStatement stmt))
@@ -109,7 +104,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 					break;
 			}
 		}
-		
+
 		static readonly ExpressionStatement fieldInitializerPattern = new ExpressionStatement {
 			Expression = new AssignmentExpression {
 				Left = new Choice {
@@ -123,25 +118,25 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 				Right = new AnyNode("initializer")
 			}
 		};
-		
+
 		static readonly AstNode thisCallPattern = new ExpressionStatement(new InvocationExpression(new MemberReferenceExpression(new ThisReferenceExpression(), ".ctor"), new Repeat(new AnyNode())));
-		
+
 		public override void VisitTypeDeclaration(TypeDeclaration typeDeclaration)
 		{
 			// Handle initializers on instance fields
 			HandleInstanceFieldInitializers(typeDeclaration.Members);
-			
+
 			// Now convert base constructor calls to initializers:
 			base.VisitTypeDeclaration(typeDeclaration);
-			
+
 			// Remove single empty constructor:
 			RemoveSingleEmptyConstructor(typeDeclaration.Members, (ITypeDefinition)typeDeclaration.GetSymbol());
-			
+
 			// Handle initializers on static fields:
 			HandleStaticFieldInitializers(typeDeclaration.Members);
 		}
-		
-		internal void HandleInstanceFieldInitializers(IEnumerable<AstNode> members)
+
+		void HandleInstanceFieldInitializers(IEnumerable<AstNode> members)
 		{
 			var instanceCtors = members.OfType<ConstructorDeclaration>().Where(c => (c.Modifiers & Modifiers.Static) == 0).ToArray();
 			var instanceCtorsNotChainingWithThis = instanceCtors.Where(ctor => !thisCallPattern.IsMatch(ctor.Body.Statements.FirstOrDefault())).ToArray();
@@ -151,7 +146,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 					return;
 
 				bool ctorIsUnsafe = instanceCtorsNotChainingWithThis.All(c => c.HasModifier(Modifiers.Unsafe));
-				
+
 				// Recognize field or property initializers:
 				// Translate first statement in all ctors (if all ctors have the same statement) into an initializer.
 				bool allSame;
@@ -170,7 +165,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 					// 'this'/'base' cannot be used in initializers
 					if (initializer.DescendantsAndSelf.Any(n => n is ThisReferenceExpression || n is BaseReferenceExpression))
 						break;
-					
+
 					allSame = true;
 					for (int i = 1; i < instanceCtorsNotChainingWithThis.Length; i++) {
 						var otherMatch = fieldInitializerPattern.Match(instanceCtorsNotChainingWithThis[i].Body.FirstOrDefault());
@@ -200,22 +195,34 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			}
 		}
 
-		internal void RemoveSingleEmptyConstructor(IEnumerable<AstNode> members, ITypeDefinition contextTypeDefinition)
+		void RemoveSingleEmptyConstructor(IEnumerable<AstNode> members, ITypeDefinition contextTypeDefinition)
 		{
+			// if we're outside of a type definition skip this altogether
 			if (contextTypeDefinition == null) return;
+			// first get non-static constructor declarations from the AST
 			var instanceCtors = members.OfType<ConstructorDeclaration>().Where(c => (c.Modifiers & Modifiers.Static) == 0).ToArray();
-			if (instanceCtors.Length == 1 && (members.Skip(1).Any() || instanceCtors[0].Parent is TypeDeclaration)) {
-				ConstructorDeclaration emptyCtor = new ConstructorDeclaration();
-				emptyCtor.Modifiers = contextTypeDefinition.IsAbstract ? Modifiers.Protected : Modifiers.Public;
-				if (instanceCtors[0].HasModifier(Modifiers.Unsafe))
-					emptyCtor.Modifiers |= Modifiers.Unsafe;
-				emptyCtor.Body = new BlockStatement();
-				if (emptyCtor.IsMatch(instanceCtors[0]))
-					instanceCtors[0].Remove();
+			// if there's exactly one ctor and it's part of a type declaration or there's more than one member in the current selection
+			// we can remove the constructor. (We do not want to hide the constructor if the user explicitly selected it in the tree view.) 
+			if (instanceCtors.Length == 1 && (instanceCtors[0].Parent is TypeDeclaration || members.Skip(1).Any())) {
+				var ctor = instanceCtors[0];
+				// dynamically create a pattern of an empty ctor
+				ConstructorDeclaration emptyCtorPattern = new ConstructorDeclaration();
+				emptyCtorPattern.Modifiers = contextTypeDefinition.IsAbstract ? Modifiers.Protected : Modifiers.Public;
+				if (ctor.HasModifier(Modifiers.Unsafe))
+					emptyCtorPattern.Modifiers |= Modifiers.Unsafe;
+				emptyCtorPattern.Body = new BlockStatement();
+
+				if (emptyCtorPattern.IsMatch(ctor)) {
+					bool retainBecauseOfDocumentation = ctor.GetSymbol() is IMethod ctorMethod
+						&& context.Settings.ShowXmlDocumentation
+						&& context.DecompileRun.DocumentationProvider?.GetDocumentation(ctorMethod) != null;
+					if (!retainBecauseOfDocumentation)
+						ctor.Remove();
+				}
 			}
 		}
-		
-		internal void HandleStaticFieldInitializers(IEnumerable<AstNode> members)
+
+		void HandleStaticFieldInitializers(IEnumerable<AstNode> members)
 		{
 			// Translate static constructor into field initializers if the class is BeforeFieldInit
 			var staticCtor = members.OfType<ConstructorDeclaration>().FirstOrDefault(c => (c.Modifiers & Modifiers.Static) == Modifiers.Static);

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -271,7 +271,7 @@
     <Compile Include="CSharp\ExpressionBuilder.cs" />
     <Compile Include="CSharp\StatementBuilder.cs" />
     <Compile Include="CSharp\Transforms\AddCheckedBlocks.cs" />
-    <Compile Include="CSharp\Transforms\ConvertConstructorCallIntoInitializer.cs" />
+    <Compile Include="CSharp\Transforms\TransformFieldAndConstructorInitializers.cs" />
     <Compile Include="CSharp\Transforms\CustomPatterns.cs" />
     <Compile Include="CSharp\Transforms\IAstTransform.cs" />
     <Compile Include="CSharp\Transforms\IntroduceUnsafeModifier.cs" />