Fix #3618: Extend validation of declared members and ctor parameters

ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs

@@ -18,6 +18,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 
 		public record Derived(int B) : Base(B.ToString());
 
+		public record BaseRecordWithObject(object Id);
+
+		public record DerivedRecordWithString : BaseRecordWithObject
+		{
+			public DerivedRecordWithString(string Id)
+				: base(Id)
+			{
+			}
+		}
+
 		public record Empty;
 
 		public record Fields
@@ -400,6 +410,25 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 				this.A = int.Parse(A);
 			}
 		}
+
+		public record struct RecordCtorChain(int A, string B)
+		{
+#if EXPECTED_OUTPUT
+			public double C = 0.0;
+#else
+			public double C;
+#endif
+			public RecordCtorChain(int A)
+				: this(A, "default")
+			{
+				C = 3.14;
+			}
+			public RecordCtorChain(string B)
+				: this(42, B)
+			{
+				C = 1.41;
+			}
+		}
 	}
 #endif
 }

ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs

@@ -167,6 +167,14 @@ namespace ICSharpCode.Decompiler.CSharp
 
 			var subst = recordTypeDef.AsParameterizedType().GetSubstitution();
 
+			Dictionary<string, IMember> nameToMemberMap = new Dictionary<string, IMember>(StringComparer.Ordinal);
+			Dictionary<IMethod, IMethod> ctorChainMap = new Dictionary<IMethod, IMethod>();
+
+			foreach (IMember m in recordTypeDef.GetMembers(m => m.SymbolKind is SymbolKind.Property or SymbolKind.Field, GetMemberOptions.ReturnMemberDefinitions))
+			{
+				nameToMemberMap[m.Name] = m;
+			}
+
 			IMethod guessedPrimaryCtor = null;
 
 			foreach (var method in recordTypeDef.Methods)
@@ -178,8 +186,21 @@ namespace ICSharpCode.Decompiler.CSharp
 				{
 					continue;
 				}
+				var body = DecompileBody(method);
+				if (body == null)
+				{
+					continue;
+				}
+				IMethod chainedCtor = (IMethod)FindChainedCtor(body)?.MemberDefinition;
+				ctorChainMap[method] = chainedCtor;
+
+				if (chainedCtor != null && chainedCtor.DeclaringTypeDefinition.Equals(recordTypeDef))
+				{
+					continue;
+				}
+
 				var m = method.Specialize(subst);
-				if (IsPrimaryConstructor(m, method))
+				if (IsPrimaryConstructor(body, m, method))
 				{
 					if (guessedPrimaryCtor == null)
 					{
@@ -194,6 +215,26 @@ namespace ICSharpCode.Decompiler.CSharp
 				}
 			}
 
+			if (guessedPrimaryCtor != null)
+			{
+				foreach (var (source, target) in ctorChainMap)
+				{
+					if (guessedPrimaryCtor.Equals(source))
+						continue;
+					// in a record with a primary ctor every other ctor must call the primary ctor
+					// at the end of the ctor chain.
+					// if the target is null or a ctor of another type, we found a ctor that does not
+					// follow this rule.
+					// we don't have to check the full chain, because the C# compiler enforces that
+					// there are no loops in the ctor call graph.
+					if (target == null || !target.DeclaringTypeDefinition!.Equals(recordTypeDef))
+					{
+						guessedPrimaryCtor = null;
+						break;
+					}
+				}
+			}
+
 			if (guessedPrimaryCtor == null)
 			{
 				primaryCtorParameterToAutoProperty.Clear();
@@ -213,12 +254,22 @@ namespace ICSharpCode.Decompiler.CSharp
 
 			return guessedPrimaryCtor;
 
-			bool IsPrimaryConstructor(IMethod method, IMethod unspecializedMethod)
+			bool IsPrimaryConstructor(Block body, IMethod method, IMethod unspecializedMethod)
 			{
-				Debug.Assert(method.IsConstructor);
+				foreach (IParameter p in unspecializedMethod.Parameters)
-				var body = DecompileBody(method);
+				{
-				if (body == null)
+					// ref and out are not valid modifiers in this context
-					return false;
+					if (p.ReferenceKind is ReferenceKind.Ref or ReferenceKind.Out)
+						return false;
+
+					// for each positional parameter there must be a field or property of the same name and type
+					if (!nameToMemberMap.TryGetValue(p.Name, out var member))
+						return false;
+
+					var paramType = p.ReferenceKind != ReferenceKind.None ? p.Type.UnwrapByRef() : p.Type;
+					if (!NormalizeTypeVisitor.TypeErasure.EquivalentTypes(paramType, member.ReturnType))
+						return false;
+				}
 
 				if (!isStruct)
 				{
@@ -236,50 +287,73 @@ namespace ICSharpCode.Decompiler.CSharp
 				if (body.Instructions.Count < addonInst)
 					return false;
 
+				int parameterIndex = 0;
+
 				for (int i = 0; i < body.Instructions.Count - addonInst; i++)
 				{
 					if (!body.Instructions[i].MatchStFld(out var target, out var field, out var valueInst))
 						return false;
 					if (!target.MatchLdThis())
 						return false;
-					if (i < method.Parameters.Count)
+					// allow assignments to fields that are not backing fields of auto-properties
+					if (!backingFieldToAutoProperty.TryGetValue(field, out var property))
+						continue;
+					if (valueInst.MatchLdLoc(out var v))
 					{
-						if (method.Parameters[i].ReferenceKind is ReferenceKind.In or ReferenceKind.RefReadOnly)
+						if (v.Kind != VariableKind.Parameter || v.Index < parameterIndex)
-						{
-							if (!valueInst.MatchLdObj(out valueInst, out _))
-								return false;
-						}
-						if (!valueInst.MatchLdLoc(out var value))
 							return false;
-						if (!(value.Kind == VariableKind.Parameter && value.Index >= 0 && value.Index < method.Parameters.Count))
+						parameterIndex = v.Index.Value;
-							return false;
+					}
-
+					else if (valueInst.MatchLdObj(out valueInst, out _) && valueInst.MatchLdLoc(out v))
-						if (!backingFieldToAutoProperty.TryGetValue(field, out var property))
+					{
-						{
+						if (v.Kind != VariableKind.Parameter || v.Index < parameterIndex)
 							return false;
-						}
+						parameterIndex = v.Index.Value;
-
+						if (method.Parameters[parameterIndex].ReferenceKind is ReferenceKind.None)
-						IParameter parameter = unspecializedMethod.Parameters[i];
-						if (primaryCtorParameterToAutoProperty.ContainsKey(parameter))
 						{
 							return false;
 						}
+					}
+					else
+					{
+						continue;
+					}
+					IParameter parameter = unspecializedMethod.Parameters[parameterIndex];
+					if (primaryCtorParameterToAutoProperty.ContainsKey(parameter))
+					{
+						continue;
+					}
 
-						if (recordTypeDef.Kind != TypeKind.Struct)
+					if (recordTypeDef.Kind != TypeKind.Struct)
+					{
+						if (!(property.CanSet && property.Setter.IsInitOnly))
 						{
-							if (!(property.CanSet && property.Setter.IsInitOnly))
+							continue;
-							{
-								continue;
-							}
 						}
-
-						primaryCtorParameterToAutoProperty.Add(parameter, property);
 					}
+					primaryCtorParameterToAutoProperty.Add(parameter, property);
 				}
 
 				var returnInst = body.Instructions.LastOrDefault();
 				return returnInst != null && returnInst.MatchReturn(out var retVal) && retVal.MatchNop();
 			}
+
+			IMethod FindChainedCtor(Block body)
+			{
+				// look for a call instruction or assignment to a chained constructor
+				foreach (var inst in body.Instructions)
+				{
+					switch (inst)
+					{
+						case Call { Method: { IsConstructor: true } ctor }:
+							return ctor;
+						case StObj { Value: NewObj { Method: var ctor } } stObj when stObj.Target.MatchLdThis():
+							return ctor;
+					}
+				}
+
+				return null;
+			}
 		}
 
 		static List<IMember> DetectMemberOrder(ITypeDefinition recordTypeDef, Dictionary<IField, IProperty> backingFieldToAutoProperty)