Make the ILAst dump faithful to what the decompiler runs

The dump is only useful if it shows the same thing the real pipeline is fed and
covers everything a transform bug can hide in, so: accessor bodies are reached
through the metadata handles (ITypeDefinition.Methods hides every method that
has method semantics), the PDB reaches the ILReader (UseDebugSymbols alone is
inert without DebugInfo), and the writing options carry the same sugar as the
UI's ILAst pane.

A method whose body cannot be read, transformed or written no longer aborts the
run, and the failure now travels through the decompilation-error path, so a
crashing transform is visible in stderr and in the exit code instead of being
buried in the output a script just collected.

BlockILTransform entries name the transforms they contain: two of them ran as
identical rows before, and asking for a nested transform by name reported it as
unknown while the listing did in fact run it.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code

ICSharpCode.ILSpyCmd.Tests/ILAstOptionTests.cs

@@ -19,6 +19,7 @@
 // The --ilast options exist in debug builds only, so this fixture compiles away with them.
 #if DEBUG
 
+using System.IO;
 using System.Threading.Tasks;
 
 using NUnit.Framework;
@@ -53,6 +54,11 @@ namespace ICSharpCode.ILSpyCmd.Tests
 			Assert.That(result.Output, Does.Contain("ILFunction"));
 		}
 
+		// the loop of SumLoop is only recognised by HighLevelLoopTransform, at the very end of the
+		// pipeline: its presence tells a full run from a truncated one, while the header line
+		// (which names the requested transform count) would differ either way
+		const string structuredLoop = "BlockContainer (for)";
+
 		[Test]
 		public async Task StoppingAfterATransformYieldsDifferentILAst()
 		{
@@ -64,17 +70,20 @@ namespace ICSharpCode.ILSpyCmd.Tests
 			Assert.That(full.ExitCode, Is.EqualTo(0), full.Error);
 			Assert.That(partial.ExitCode, Is.EqualTo(0), partial.Error);
 			Assert.That(partial.Output, Does.Contain(nameof(ILAstSample.SumLoop)));
-			Assert.That(partial.Output, Is.Not.EqualTo(full.Output));
+			Assert.That(full.Output, Does.Contain(structuredLoop));
+			Assert.That(partial.Output, Does.Not.Contain(structuredLoop));
 		}
 
 		[Test]
 		public async Task TransformCanBeSelectedByIndex()
 		{
 			var partial = await RunILAstAsync("--after-transform", "1");
-			var full = await RunILAstAsync("--ilast");
 
 			Assert.That(partial.ExitCode, Is.EqualTo(0), partial.Error);
-			Assert.That(partial.Output, Is.Not.EqualTo(full.Output));
+			Assert.That(partial.Output, Does.Contain(nameof(ILAstSample.SumLoop)));
+			Assert.That(partial.Output, Does.Not.Contain(structuredLoop));
+			// AssignVariableNames is the last transform, so the locals still carry their IL names
+			Assert.That(partial.Output, Does.Contain("local V_0"));
 		}
 
 		[Test]
@@ -91,12 +100,39 @@ namespace ICSharpCode.ILSpyCmd.Tests
 		[Test]
 		public async Task AmbiguousTransformNameReportsItsOccurrences()
 		{
-			// SplitVariables runs three times; the name alone cannot identify a stop point
+			// SplitVariables runs more than once; the name alone cannot identify a stop point
 			var result = await RunILAstAsync("--after-transform", "SplitVariables");
 
 			Assert.That(result.ExitCode, Is.EqualTo(ProgramExitCodes.EX_USAGE));
-			Assert.That(result.Error, Does.Contain("SplitVariables"));
-			Assert.That(result.Error, Does.Contain("2"));
+			// the message has to be the ambiguity one, not the unknown-name listing, which
+			// mentions every transform of the pipeline as well
+			Assert.That(result.Error, Does.Contain("'SplitVariables' runs"));
+			Assert.That(result.Error, Does.Contain("times, at index"));
+			Assert.That(result.Error, Does.Not.Contain("Unknown transform"));
+		}
+
+		[Test]
+		public async Task NestedTransformNamesTheEntryThatRunsIt()
+		{
+			// LoopDetection runs inside a BlockILTransform, so it has no stop point of its own;
+			// the pipeline listing has to show where it runs instead of hiding it
+			var result = await RunILAstAsync("--after-transform", "LoopDetection");
+
+			Assert.That(result.ExitCode, Is.EqualTo(ProgramExitCodes.EX_USAGE));
+			Assert.That(result.Error, Does.Contain("runs inside the transform at index"));
+			Assert.That(result.Error, Does.Contain("BlockILTransform (LoopDetection"));
+		}
+
+		[Test]
+		public async Task DebugSymbolsProvideLocalVariableNames()
+		{
+			var withPdb = await RunILAstAsync("--ilast", "-usepdb");
+			var withoutPdb = await RunILAstAsync("--ilast");
+
+			Assert.That(withPdb.ExitCode, Is.EqualTo(0), withPdb.Error);
+			// the PDB's name for the accumulator, instead of the generated 'num'
+			Assert.That(withPdb.Output, Does.Contain("local sum"));
+			Assert.That(withoutPdb.Output, Does.Not.Contain("local sum"));
 		}
 
 		[Test]
@@ -108,6 +144,39 @@ namespace ICSharpCode.ILSpyCmd.Tests
 			Assert.That(result.ExitCode, Is.EqualTo(0), result.Error);
 			Assert.That(result.Output, Does.Contain(nameof(ILAstSample.SumLoop)));
 			Assert.That(result.Output, Does.Contain(nameof(ILAstSample.Identity)));
+			// accessors are methods with bodies too, and the type system's Methods hides them
+			Assert.That(result.Output, Does.Contain("get_" + nameof(ILAstSample.Counter)));
+		}
+
+		[Test]
+		public async Task OutputDirWritesEveryAssemblyCompletely()
+		{
+			// Two input assemblies, so the per-file output writer is swapped between files;
+			// a writer that is replaced without being flushed truncates the earlier file.
+			string ilspyCmdAssemblyPath = typeof(ILSpyCmdProgram).Assembly.Location;
+			string outputDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+			Directory.CreateDirectory(outputDir);
+			try
+			{
+				// one transform only: this dumps every method of both assemblies, and the
+				// truncation it guards against does not depend on the pipeline length
+				var result = await RunAsync(testAssemblyPath, ilspyCmdAssemblyPath,
+					"--disable-updatecheck", "--after-transform", "1", "-o", outputDir);
+
+				Assert.That(result.ExitCode, Is.EqualTo(0), result.Error);
+				foreach (string assemblyPath in new[] { testAssemblyPath, ilspyCmdAssemblyPath })
+				{
+					string outputFile = Path.Combine(outputDir, Path.GetFileNameWithoutExtension(assemblyPath) + ".ilast");
+					Assert.That(File.Exists(outputFile), Is.True, outputFile);
+					// every function ends with its closing brace; a truncated file breaks off
+					// wherever the writer's buffer happened to end
+					Assert.That(File.ReadAllText(outputFile).TrimEnd(), Does.EndWith("}"), outputFile);
+				}
+			}
+			finally
+			{
+				Directory.Delete(outputDir, recursive: true);
+			}
 		}
 	}
 
@@ -124,6 +193,8 @@ namespace ICSharpCode.ILSpyCmd.Tests
 		}
 
 		public static string Identity(string value) => value;
+
+		public static int Counter { get; set; }
 	}
 }
 

ICSharpCode.ILSpyCmd/ILAstDumper.cs

@@ -41,20 +41,53 @@ namespace ICSharpCode.ILSpyCmd
 	/// </summary>
 	class ILAstDumper
 	{
-		readonly IReadOnlyList<IILTransform> transforms = CSharpDecompiler.GetILTransforms();
-		readonly ILAstWritingOptions writingOptions = new ILAstWritingOptions();
+		static readonly IReadOnlyList<IILTransform> transforms = CSharpDecompiler.GetILTransforms();
+
+		readonly ILAstWritingOptions writingOptions = new ILAstWritingOptions {
+			// same sugar as the UI's ILAst pane, so its output and this one are diffable
+			UseFieldSugar = true,
+			UseLogicOperationSugar = true,
+		};
 
 		/// <summary>
-		/// Names of the IL transforms, in the order they run. Transforms nested inside a
-		/// BlockILTransform (LoopDetection, ConditionDetection, the statement transforms, ...)
-		/// are not listed: they run as part of their containing entry and cannot be stopped
-		/// after individually.
+		/// Names of the IL transforms, in the order they run, as <c>--after-transform</c> accepts
+		/// them. Transforms nested inside a BlockILTransform (LoopDetection, ConditionDetection,
+		/// the statement transforms, ...) have no name of their own here: they run as part of
+		/// their containing entry and cannot be stopped after individually.
 		/// </summary>
 		public static IReadOnlyList<string> TransformNames { get; } =
-			CSharpDecompiler.GetILTransforms().Select(t => t.GetType().Name).ToArray();
+			transforms.Select(t => t.GetType().Name).ToArray();
 
 		public static int TransformCount => TransformNames.Count;
 
+		/// <summary>
+		/// How a pipeline entry is displayed: a BlockILTransform also names the transforms it
+		/// runs, which are otherwise invisible - and two BlockILTransform entries would be
+		/// indistinguishable.
+		/// </summary>
+		static string Describe(int index)
+		{
+			return transforms[index] is BlockILTransform block ? block.ToString() : TransformNames[index];
+		}
+
+		/// <summary>
+		/// The 1-based index of the pipeline entry running <paramref name="name"/> as one of its
+		/// nested transforms, or 0 if no entry does.
+		/// </summary>
+		static int FindContainingEntry(string name)
+		{
+			for (int i = 0; i < transforms.Count; i++)
+			{
+				if (transforms[i] is BlockILTransform block
+					&& block.PreOrderTransforms.Concat(block.PostOrderTransforms)
+						.Any(t => string.Equals(t.GetType().Name, name, StringComparison.OrdinalIgnoreCase)))
+				{
+					return i + 1;
+				}
+			}
+			return 0;
+		}
+
 		/// <summary>
 		/// The pipeline as displayed to the user: one transform per line, prefixed by the
 		/// 1-based index that <c>--after-transform</c> accepts.
@@ -62,7 +95,7 @@ namespace ICSharpCode.ILSpyCmd
 		public static string DescribePipeline()
 		{
 			return string.Join(Environment.NewLine,
-				TransformNames.Select((name, index) => $"  {index + 1,3}  {name}"));
+				Enumerable.Range(0, TransformCount).Select(index => $"  {index + 1,3}  {Describe(index)}"));
 		}
 
 		/// <summary>
@@ -96,7 +129,10 @@ namespace ICSharpCode.ILSpyCmd
 
 			if (occurrences.Length == 0)
 			{
-				error = $"Unknown transform '{trimmed}'. Pass one of these names, or its index:{Environment.NewLine}{DescribePipeline()}";
+				int containingEntry = FindContainingEntry(trimmed);
+				error = containingEntry > 0
+					? $"'{trimmed}' runs inside the transform at index {containingEntry} and cannot be stopped after on its own. Pass that index to stop after the whole entry:{Environment.NewLine}{DescribePipeline()}"
+					: $"Unknown transform '{trimmed}'. Pass one of these names, or its index:{Environment.NewLine}{DescribePipeline()}";
 				return false;
 			}
 			if (occurrences.Length > 1)
@@ -111,32 +147,38 @@ namespace ICSharpCode.ILSpyCmd
 
 		/// <summary>
 		/// Writes the ILAst of a single method, or nothing at all if the method has no body
-		/// (abstract, extern or a runtime-provided implementation).
+		/// (abstract, extern or a runtime-provided implementation). Returns the failure if the
+		/// body could not be read, transformed or written, so that one broken method neither
+		/// aborts the dump nor makes the run look successful.
 		/// </summary>
-		public void WriteMethod(CSharpDecompiler decompiler, DecompilerSettings settings, IMethod method,
+		public DecompilerException WriteMethod(CSharpDecompiler decompiler, DecompilerSettings settings, IMethod method,
 			int transformCount, ITextOutput output, CancellationToken cancellationToken)
 		{
-			if (method.MetadataToken.IsNil || method.MetadataToken.Kind != HandleKind.MethodDefinition)
-				return;
-			var metadataFile = decompiler.TypeSystem.MainModule.MetadataFile;
+			if (method == null || method.MetadataToken.IsNil || method.MetadataToken.Kind != HandleKind.MethodDefinition)
+				return null;
+			var module = decompiler.TypeSystem.MainModule;
+			var metadataFile = module.MetadataFile;
 			var handle = (MethodDefinitionHandle)method.MetadataToken;
 			var methodDefinition = metadataFile.Metadata.GetMethodDefinition(handle);
 			if (!methodDefinition.HasBody())
-				return;
+				return null;
 
 			output.WriteLine($"// {method.FullName}");
-			output.WriteLine($"// ILAst after {transformCount} of {TransformCount} transforms ({TransformNames[transformCount - 1]})");
-
-			var reader = new ILReader(decompiler.TypeSystem.MainModule) {
-				UseDebugSymbols = settings.UseDebugSymbols,
-				UseRefLocalsForAccurateOrderOfEvaluation = settings.UseRefLocalsForAccurateOrderOfEvaluation,
-			};
-			var body = metadataFile.GetMethodBody(methodDefinition.RelativeVirtualAddress);
-			ILFunction function = reader.ReadIL(handle, body, kind: ILFunctionKind.TopLevelFunction,
-				cancellationToken: cancellationToken);
-			ILTransformContext context = decompiler.CreateILTransformContext(function);
+			output.WriteLine($"// ILAst after {transformCount} of {TransformCount} transforms ({Describe(transformCount - 1)})");
+
+			DecompilerException error = null;
+			ILFunction function = null;
 			try
 			{
+				var reader = new ILReader(module) {
+					UseDebugSymbols = settings.UseDebugSymbols,
+					UseRefLocalsForAccurateOrderOfEvaluation = settings.UseRefLocalsForAccurateOrderOfEvaluation,
+					DebugInfo = decompiler.DebugInfoProvider,
+				};
+				var body = metadataFile.GetMethodBody(methodDefinition.RelativeVirtualAddress);
+				function = reader.ReadIL(handle, body, kind: ILFunctionKind.TopLevelFunction,
+					cancellationToken: cancellationToken);
+				ILTransformContext context = decompiler.CreateILTransformContext(function);
 				function.RunTransforms(transforms.Take(transformCount), context);
 			}
 			catch (Exception ex)
@@ -146,10 +188,20 @@ namespace ICSharpCode.ILSpyCmd
 				// rather than aborting the whole dump.
 				output.WriteLine(ex.ToString());
 				output.WriteLine("// ILAst after the crash:");
+				error = new DecompilerException(module, method, ex);
+			}
+			try
+			{
+				function?.WriteTo(output, writingOptions);
+			}
+			catch (Exception ex)
+			{
+				output.WriteLine(ex.ToString());
+				error ??= new DecompilerException(module, method, ex);
 			}
-			function.WriteTo(output, writingOptions);
 			output.WriteLine();
 			output.WriteLine();
+			return error;
 		}
 	}
 }

ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs

@@ -363,8 +363,12 @@ Examples:
 				{
 					if (outputDirectory != null)
 					{
+						// per-file writer, disposed here: the shared 'output' is only closed once
+						// at the end of the run, which would lose the buffered tail of every file
+						// but the last when dumping multiple assemblies
 						string outputName = Path.GetFileNameWithoutExtension(fileName);
-						output = File.CreateText(Path.Combine(outputDirectory, outputName) + ".ilast");
+						using var ilastOutput = File.CreateText(Path.Combine(outputDirectory, outputName) + ".ilast");
+						return ShowILAst(fileName, ilastOutput, app);
 					}
 
 					return ShowILAst(fileName, output, app);
@@ -533,7 +537,9 @@ Examples:
 			return decompilerSettings;
 		}
 
-		CSharpDecompiler GetDecompiler(string assemblyFileName)
+		CSharpDecompiler GetDecompiler(string assemblyFileName) => GetDecompiler(assemblyFileName, out _);
+
+		CSharpDecompiler GetDecompiler(string assemblyFileName, out DecompilerSettings settings)
 		{
 			var module = new PEFile(assemblyFileName);
 			var resolver = new UniversalAssemblyResolver(assemblyFileName, false, module.Metadata.DetectTargetFrameworkId());
@@ -541,7 +547,8 @@ Examples:
 			{
 				resolver.AddSearchDirectory(path);
 			}
-			return new CSharpDecompiler(assemblyFileName, resolver, GetSettings(module)) {
+			settings = GetSettings(module);
+			return new CSharpDecompiler(assemblyFileName, resolver, settings) {
 				DebugInfoProvider = TryLoadPDB(module)
 			};
 		}
@@ -667,8 +674,9 @@ Examples:
 				return ProgramExitCodes.EX_USAGE;
 			}
 
-			var settings = GetSettings(new PEFile(assemblyFileName));
-			CSharpDecompiler decompiler = GetDecompiler(assemblyFileName);
+			CSharpDecompiler decompiler = GetDecompiler(assemblyFileName, out var settings);
+			var mainModule = decompiler.TypeSystem.MainModule;
+			var metadata = mainModule.MetadataFile.Metadata;
 			IEnumerable<IMethod> methods;
 
 			if (MemberIdString != null)
@@ -683,7 +691,7 @@ Examples:
 					Console.Error.WriteLine($"'{MemberIdString}' does not name a method; ILAst exists for method bodies only.");
 					return ProgramExitCodes.EX_DATAERR;
 				}
-				methods = new[] { decompiler.TypeSystem.MainModule.GetDefinition((MethodDefinitionHandle)handle) };
+				methods = new[] { mainModule.GetDefinition((MethodDefinitionHandle)handle) };
 			}
 			else if (TypeName != null)
 			{
@@ -692,19 +700,26 @@ Examples:
 					Console.Error.WriteLine(error);
 					return ProgramExitCodes.EX_DATAERR;
 				}
-				methods = typeDefinition.Methods;
+				// via the metadata handles, not ITypeDefinition.Methods: the latter drops every
+				// method that has method semantics, i.e. all property and event accessors
+				methods = metadata.GetTypeDefinition((TypeDefinitionHandle)typeDefinition.MetadataToken)
+					.GetMethods().Select(mainModule.GetDefinition);
 			}
 			else
 			{
-				methods = decompiler.TypeSystem.MainModule.TypeDefinitions.SelectMany(type => type.Methods);
+				methods = metadata.MethodDefinitions.Select(mainModule.GetDefinition);
 			}
 
 			var textOutput = new PlainTextOutput(output);
 			var dumper = new ILAstDumper();
+			var errors = new List<DecompilerException>();
 			foreach (var method in methods)
 			{
-				dumper.WriteMethod(decompiler, settings, method, transformCount, textOutput, CancellationToken.None);
+				var error = dumper.WriteMethod(decompiler, settings, method, transformCount, textOutput, CancellationToken.None);
+				if (error != null)
+					errors.Add(error);
 			}
+			ReportDecompilationErrors(assemblyFileName, errors);
 			return 0;
 		}
 #endif