Merge pull request #4057 from icsharpcode/fix/lambda-parameter-syntax

Fix uncompilable lambda parameter lists

ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@@ -295,6 +295,12 @@ namespace ICSharpCode.Decompiler.Tests
 			await RunCS(options: options);
 		}
 
+		[Test]
+		public async Task LambdaOptionalAndParamsParameters([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions options)
+		{
+			await RunCS(options: options);
+		}
+
 		[Test]
 		public async Task NullPropagation([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions options)
 		{

ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@@ -824,6 +824,12 @@ namespace ICSharpCode.Decompiler.Tests
 			await RunForLibrary(cscOptions: cscOptions);
 		}
 
+		[Test]
+		public async Task LambdaOptionalAndParamsParameters([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions)
+		{
+			await RunForLibrary(cscOptions: cscOptions);
+		}
+
 		[Test]
 		public async Task RefStructInterfaces([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions)
 		{

ICSharpCode.Decompiler.Tests/TestCases/Correctness/LambdaOptionalAndParamsParameters.cs

@@ -0,0 +1,68 @@
+#pragma warning disable CS9099, CS9100
+using System;
+using System.Reflection;
+
+namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
+{
+	class LambdaOptionalAndParamsParameters
+	{
+		static void Main()
+		{
+#if CS120 && !NET40
+			ReportMetadata();
+			ReportCalls();
+#endif
+		}
+
+#if CS120 && !NET40
+		delegate int OptionalFunc(int x = 5);
+
+		delegate int PlainFunc(int x);
+
+		delegate void ParamsAction(params int[] xs);
+
+		delegate void PlainAction(int[] xs);
+
+		// A lambda's parameter list does not have to repeat what the delegate declares: it may
+		// state a different default, one the delegate does not have, or none where the delegate
+		// has one, and the same for the params modifier. Metadata records what the lambda itself
+		// declared, and reflection reports that rather than the delegate's, so the decompiled
+		// lambda has to carry the lambda's own list back.
+		static void Report(string name, Delegate d)
+		{
+			ParameterInfo p = d.Method.GetParameters()[0];
+			Console.WriteLine("{0}: hasDefault={1} default={2} params={3}",
+				name,
+				p.HasDefaultValue,
+				p.HasDefaultValue ? p.DefaultValue : "none",
+				p.IsDefined(typeof(ParamArrayAttribute), inherit: false));
+		}
+
+		static void ReportMetadata()
+		{
+			Report("DefaultOnlyInDelegate", (OptionalFunc)((int x) => x * 2));
+			Report("DefaultOnlyInLambda", (PlainFunc)((int x = 3) => x * 2));
+			Report("DefaultDiffersFromDelegate", (OptionalFunc)((int x = 7) => x * 2));
+			Report("DefaultAgreesWithDelegate", (OptionalFunc)((int x = 5) => x * 2));
+			Report("ParamsOnlyInDelegate", (ParamsAction)((int[] xs) => Console.WriteLine(xs.Length)));
+			Report("ParamsOnlyInLambda", (PlainAction)((params int[] xs) => Console.WriteLine(xs.Length)));
+		}
+
+		// The value a caller gets for an omitted argument comes from the delegate, never from the
+		// lambda, so these stay the same whatever the lambda declared.
+		static void ReportCalls()
+		{
+			OptionalFunc noDefault = (int x) => x * 2;
+			OptionalFunc otherDefault = (int x = 7) => x * 2;
+			Console.WriteLine(noDefault());
+			Console.WriteLine(otherDefault());
+			Console.WriteLine(otherDefault(1));
+
+			ParamsAction expandedByDelegate = (int[] xs) => Console.WriteLine(xs.Length);
+			expandedByDelegate(1, 2, 3);
+			PlainAction notExpanded = (params int[] xs) => Console.WriteLine(xs.Length);
+			notExpanded(new int[2]);
+		}
+#endif
+	}
+}

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

@@ -599,6 +599,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.DelegateConstruction
 				Console.WriteLine(x);
 			};
 		}
+
+		public static int LambdaWithAttributeOnAnonymousTypeParameter()
+		{
+			return new[] {
+				new {
+					X = 1
+				}
+			}.Select([My] (a) => a.X).Sum();
+		}
 #endif
 
 		public static void CallRecursiveDelegate(ref RefRecursiveDelegate d)

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

@@ -0,0 +1,99 @@
+#pragma warning disable CS9099, CS9100
+using System;
+
+namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
+{
+	internal class LambdaOptionalAndParamsParameters
+	{
+		public delegate void ParamsAction(params int[] xs);
+
+		public delegate int OptionalFunc(int x = 5);
+
+		public delegate int PlainFunc(int x);
+
+		// A lambda states 'params' and defaults on its own account: what the delegate declares
+		// is the delegate's, and reflection over the lambda's method reports only the lambda's.
+		// Roslyn records the modifier on the anonymous function's method from version 5 on, so
+		// the cases that need it back are guarded on ROSLYN5.
+
+		private int total;
+
+#if ROSLYN5
+		public ParamsAction ParamsStatementBody()
+		{
+			return (params int[] xs) => {
+				total += xs.Length;
+				Console.WriteLine(xs.Length);
+			};
+		}
+#endif
+
+#if ROSLYN5
+		public ParamsAction ParamsSingleStatementBody()
+		{
+			return (params int[] xs) => {
+				Console.WriteLine(xs.Length);
+			};
+		}
+#endif
+
+		public OptionalFunc OptionalStatementBody()
+		{
+			return (int x = 5) => {
+				total += x;
+				return x * 2;
+			};
+		}
+
+		public OptionalFunc OptionalExpressionBody()
+		{
+			return (int x = 5) => x * 2;
+		}
+
+		public ParamsAction ParamsWithoutParameterList()
+		{
+			return delegate {
+			};
+		}
+
+		public OptionalFunc OptionalWithoutParameterList()
+		{
+			return delegate {
+				return 1;
+			};
+		}
+
+		// The lambda's own defaults and params modifier can differ from the delegate's; the metadata
+		// records the lambda's, so that is what must round-trip.
+		public OptionalFunc OptionalDifferentDefault()
+		{
+			return (int x = 7) => x * 2;
+		}
+
+		public OptionalFunc OptionalOnlyInDelegate()
+		{
+			return (int x) => x * 2;
+		}
+
+		public PlainFunc OptionalOnlyInLambda()
+		{
+			return (int x = 3) => x * 2;
+		}
+
+		public ParamsAction ParamsOnlyInDelegate()
+		{
+			return (int[] xs) => {
+				Console.WriteLine(xs.Length);
+			};
+		}
+
+#if ROSLYN5
+		public Action<int[]> ParamsOnlyInLambda()
+		{
+			return (params int[] xs) => {
+				Console.WriteLine(xs.Length);
+			};
+		}
+#endif
+	}
+}

ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@@ -2627,7 +2627,6 @@ namespace ICSharpCode.Decompiler.CSharp
 				let v = ident.GetILVariable()
 				where v != null && v.Function == function && v.Kind == VariableKind.Parameter
 				select ident).Any();
-
 			bool isLambda = false;
 			if (ame.Parameters.Any(p => p.Type is null))
 			{
@@ -2650,6 +2649,33 @@ namespace ICSharpCode.Decompiler.CSharp
 				// form is compatible with any delegate signature, so it is always legal there.
 				isLambda = true;
 			}
+			// 'params' and parameter default values are only legal on the explicitly typed
+			// parameter list of a lambda, and only since C# 12; and a list that is about to be
+			// dropped cannot carry them at all. Everywhere else they are decorative - the
+			// delegate type still declares both, and that is what call sites bind against.
+			if (settings.LambdaOptionalAndParamsParameters
+				&& (isLambda || parametersAreUsed)
+				&& ame.Parameters.All(p => p.Type is not null))
+			{
+				// Only what the anonymous function's own metadata declares is written. A lambda
+				// may state a different default than its target delegate, or none where the
+				// delegate has one, and reflection over the lambda's method reports what the
+				// lambda declared - so taking either from the delegate's Invoke would change
+				// what the recompiled assembly says. The delegate type keeps declaring both,
+				// and call sites bind against it, so nothing is lost by leaving them out here.
+
+				// An anonymous method cannot declare either, in any language version.
+				if (ame.Parameters.Any(p => p.IsParams || p.DefaultExpression is not null))
+					isLambda = true;
+			}
+			else
+			{
+				foreach (var p in ame.Parameters)
+				{
+					p.IsParams = false;
+					p.DefaultExpression?.Detach();
+				}
+			}
 			// Remove the parameter list from an AnonymousMethodExpression if the parameters are not used in the method body
 			if (!isLambda && !parametersAreUsed)
 			{

ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs

@@ -1061,11 +1061,21 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor
 
 		protected bool LambdaNeedsParenthesis(LambdaExpression lambdaExpression)
 		{
+			if (lambdaExpression.Attributes.Count > 0)
+			{
+				// attributes on the lambda require a parenthesized parameter list
+				return true;
+			}
 			if (lambdaExpression.Parameters.Count != 1)
 			{
 				return true;
 			}
 			var p = lambdaExpression.Parameters.Single();
+			if (p.Attributes.Count > 0)
+			{
+				// parameter attributes have no unparenthesized form
+				return true;
+			}
 			return !(p.Type is null && p.ParameterModifier == ReferenceKind.None && !p.IsParams);
 		}
 

ICSharpCode.Decompiler/DecompilerSettings.cs

@@ -848,6 +848,14 @@ namespace ICSharpCode.Decompiler
 		[DecompilerSetting(CSharp.LanguageVersion.CSharp12_0)]
 		public partial bool InlineArrays { get; set; }
 
+		/// <summary>
+		/// Gets/sets whether lambda parameter lists may declare 'params' and parameter default
+		/// values. When disabled, these modifiers are dropped from anonymous functions instead.
+		/// </summary>
+		[Description("DecompilerSettings.LambdaOptionalAndParamsParameters")]
+		[DecompilerSetting(CSharp.LanguageVersion.CSharp12_0)]
+		public partial bool LambdaOptionalAndParamsParameters { get; set; }
+
 		/// <summary>
 		/// Gets/Sets whether C# 14.0 extension members should be transformed.
 		/// </summary>

ILSpy/Properties/Resources.Designer.cs

@@ -1361,6 +1361,15 @@ namespace ICSharpCode.ILSpy.Properties {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to &apos;params&apos; and optional parameters in lambdas.
+        /// </summary>
+        public static string DecompilerSettings_LambdaOptionalAndParamsParameters {
+            get {
+                return ResourceManager.GetString("DecompilerSettings.LambdaOptionalAndParamsParameters", resourceCulture);
+            }
+        }
+
         /// <summary>
         ///   Looks up a localized string similar to Use nint/nuint types.
         /// </summary>

ILSpy/Properties/Resources.resx

@@ -486,6 +486,9 @@ Are you sure you want to continue?</value>
   <data name="DecompilerSettings.IsUnmanagedAttributeOnTypeParametersShouldBeReplacedWithUnmanagedConstraints" xml:space="preserve">
     <value>IsUnmanagedAttribute on type parameters should be replaced with 'unmanaged' constraints</value>
   </data>
+  <data name="DecompilerSettings.LambdaOptionalAndParamsParameters" xml:space="preserve">
+    <value>'params' and optional parameters in lambdas</value>
+  </data>
   <data name="DecompilerSettings.NativeIntegers" xml:space="preserve">
     <value>Use nint/nuint types</value>
   </data>