Merge pull request #5 from JetBrains/bug-fixes

Improve disassembly reliability and error messages

Author: smolchanovsky

CHANGELOG.md

@@ -8,8 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## 0.1.1
 
 ### Fixed
-- Fixed statistics collection
-- Updated plugin description
+- Minor fixes and improvements
+- Improved plugin description
 
 ## 0.1.0
 

protocol/src/main/kotlin/model/rider/AsmViewerModel.kt

@@ -7,6 +7,12 @@ import com.jetbrains.rider.model.nova.ide.SolutionModel
 @Suppress("unused")
 class AsmViewerModel : Ext(SolutionModel.Solution) {
 
+    private val CompilerType = enum {
+        +"Clrjit"
+        +"Crossgen2"
+        +"Ilc"
+    }
+
     private val ErrorCode = enum {
         // PSI/Navigation errors
         +"SourceFileNotFound"
@@ -29,6 +35,7 @@ class AsmViewerModel : Ext(SolutionModel.Solution) {
         +"ProjectPathNotFound"
         +"DotnetBuildFailed"
         +"DotnetPublishFailed"
+        +"EmptyDisassembly"
 
         // Runtime/Path errors
         +"DotNetCliNotFound"
@@ -56,7 +63,7 @@ class AsmViewerModel : Ext(SolutionModel.Solution) {
         field("useDotnetPublishForReload", bool)
         field("useDotnetBuildForReload", bool)
         field("targetFrameworkOverride", string.nullable)
-        field("selectedCustomJit", string.nullable)
+        field("selectedCompiler", CompilerType)
         field("disassemblyTimeoutSeconds", int)
     }
 

src/dotnet/ReSharperPlugin.DotNetDisassembler.Tests/FusValidationPatternsTests.cs

@@ -0,0 +1,97 @@
+using NUnit.Framework;
+
+namespace ReSharperPlugin.DotNetDisassembler.Tests;
+
+[TestFixture]
+public class FusValidationPatternsTests
+{
+    [TestFixture]
+    public class SdkTypeTests
+    {
+        [TestCase("Microsoft.NET.Sdk", Description = "Base SDK")]
+        [TestCase("Microsoft.NET.Sdk.Web", Description = "Web SDK")]
+        [TestCase("Microsoft.NET.Sdk.Worker", Description = "Worker SDK")]
+        [TestCase("Microsoft.NET.Sdk.Razor", Description = "Razor SDK")]
+        [TestCase("Microsoft.NET.Sdk.BlazorWebAssembly", Description = "Blazor WebAssembly SDK")]
+        [TestCase("Microsoft.NET.Sdk.WindowsDesktop", Description = "Windows Desktop SDK (.NET Core 3.x)")]
+        public void ShouldMatchValidSdkTypes(string sdkType)
+        {
+            var match = FusValidationPatterns.SdkTypeRegex.Match(sdkType);
+
+            Assert.That(match.Success, Is.True, $"Should match SDK type: {sdkType}");
+            Assert.That(match.Value, Is.EqualTo(sdkType), "Should match the entire string");
+        }
+
+        [TestCase("Microsoft.NET", Description = "Incomplete SDK name")]
+        [TestCase("Microsoft.NET.Sdk.123Number", Description = "SDK extension starting with number")]
+        [TestCase("MyCustom.NET.Sdk", Description = "Non-Microsoft SDK")]
+        [TestCase("microsoft.net.sdk", Description = "Lowercase")]
+        [TestCase("", Description = "Empty string")]
+        public void ShouldNotMatchInvalidSdkTypes(string sdkType)
+        {
+            var match = FusValidationPatterns.SdkTypeRegex.Match(sdkType);
+
+            Assert.That(match.Success && match.Value == sdkType, Is.False,
+                $"Should not match invalid SDK type: {sdkType}");
+        }
+    }
+
+    [TestFixture]
+    public class TargetFrameworkTests
+    {
+        [TestCase("net8.0", Description = ".NET 8")]
+        [TestCase("net9.0", Description = ".NET 9")]
+        [TestCase("net6.0", Description = ".NET 6")]
+        [TestCase("net5.0", Description = ".NET 5")]
+        [TestCase("net8.0-windows", Description = ".NET 8 for Windows")]
+        [TestCase("net6.0-android", Description = ".NET 6 for Android")]
+        [TestCase("net7.0-ios", Description = ".NET 7 for iOS")]
+        [TestCase("netcoreapp3.1", Description = ".NET Core 3.1")]
+        [TestCase("netcoreapp2.1", Description = ".NET Core 2.1")]
+        [TestCase("netcoreapp1.0", Description = ".NET Core 1.0")]
+        [TestCase("netstandard2.1", Description = ".NET Standard 2.1")]
+        [TestCase("netstandard2.0", Description = ".NET Standard 2.0")]
+        [TestCase("netstandard1.6", Description = ".NET Standard 1.6")]
+        [TestCase("netstandard1.0", Description = ".NET Standard 1.0")]
+        [TestCase("net472", Description = ".NET Framework 4.7.2")]
+        [TestCase("net48", Description = ".NET Framework 4.8")]
+        [TestCase("net481", Description = ".NET Framework 4.8.1")]
+        [TestCase("net462", Description = ".NET Framework 4.6.2")]
+        public void ShouldMatchValidTargetFrameworks(string tfm)
+        {
+            var match = FusValidationPatterns.TargetFrameworkRegex.Match(tfm);
+
+            Assert.That(match.Success, Is.True, $"Should match TFM: {tfm}");
+            Assert.That(match.Value, Is.EqualTo(tfm), "Should match the entire string");
+        }
+
+        [TestCase("netframework4.8", Description = "Invalid format")]
+        [TestCase("dotnet8.0", Description = "Wrong prefix")]
+        [TestCase("NET8.0", Description = "Uppercase")]
+        [TestCase("net8", Description = "Missing minor version")]
+        [TestCase("", Description = "Empty string")]
+        [TestCase("net8.0.0", Description = "Too many version components")]
+        public void ShouldNotMatchInvalidTargetFrameworks(string tfm)
+        {
+            var match = FusValidationPatterns.TargetFrameworkRegex.Match(tfm);
+
+            Assert.That(match.Success && match.Value == tfm, Is.False,
+                $"Should not match invalid TFM: {tfm}");
+        }
+
+        [TestCase("net8.0-windows10.0.19041", Description = "Windows with build number")]
+        [TestCase("net6.0-android31.0", Description = "Android with API level")]
+        [TestCase("net8.0-ios17.2", Description = "iOS with version (from official docs)")]
+        [TestCase("net8.0-android34.0", Description = "Android API level 34 (from official docs)")]
+        [TestCase("net6.0-ios15.0", Description = "iOS with default version")]
+        [TestCase("net8.0-maccatalyst", Description = "Mac Catalyst")]
+        [TestCase("net8.0-tvos", Description = "tvOS")]
+        public void ShouldMatchTargetFrameworksWithExtendedPlatformInfo(string tfm)
+        {
+            var match = FusValidationPatterns.TargetFrameworkRegex.Match(tfm);
+
+            Assert.That(match.Success, Is.True, $"Should match TFM with platform info: {tfm}");
+            Assert.That(match.Value, Is.EqualTo(tfm), "Should match the entire string");
+        }
+    }
+}

src/dotnet/ReSharperPlugin.DotNetDisassembler.Tests/JitCodegenProviderTests.cs

@@ -96,7 +96,7 @@ public async Task GetJitCodegen_WhenConfigurationInvalid_ShouldReturnError()
 
         var config = new JitDisasmConfiguration
         {
-            SelectedCustomJit = JitDisasmConfiguration.Crossgen,
+            SelectedCustomJit = JitCompilerTypes.Crossgen,
             UsePgo = true
         };
 
@@ -158,8 +158,8 @@ public async Task GetJitCodegen_WhenCustomRuntimeWithOldFramework_ShouldReturnEr
     }
 
     [Test]
-    [TestCase(JitDisasmConfiguration.Crossgen, true, AsmViewerErrorCode.RunModeNotSupportedForAot)]
-    [TestCase(JitDisasmConfiguration.Ilc, true, AsmViewerErrorCode.RunModeNotSupportedForAot)]
+    [TestCase(JitCompilerTypes.Crossgen, true, AsmViewerErrorCode.RunModeNotSupportedForAot)]
+    [TestCase(JitCompilerTypes.Ilc, true, AsmViewerErrorCode.RunModeNotSupportedForAot)]
     public async Task GetJitCodegen_WithRunAppMode_ShouldReturnError(
         string customJit, bool runAppMode, AsmViewerErrorCode expectedError)
     {
@@ -181,8 +181,8 @@ public async Task GetJitCodegen_WithRunAppMode_ShouldReturnError(
     }
 
     [Test]
-    [TestCase(JitDisasmConfiguration.Crossgen, true, AsmViewerErrorCode.TieredJitNotSupportedForAot)]
-    [TestCase(JitDisasmConfiguration.Ilc, true, AsmViewerErrorCode.TieredJitNotSupportedForAot)]
+    [TestCase(JitCompilerTypes.Crossgen, true, AsmViewerErrorCode.TieredJitNotSupportedForAot)]
+    [TestCase(JitCompilerTypes.Ilc, true, AsmViewerErrorCode.TieredJitNotSupportedForAot)]
     public async Task GetJitCodegen_WithTieredJit_ShouldReturnError(
         string customJit, bool useTieredJit, AsmViewerErrorCode expectedError)
     {
@@ -211,7 +211,7 @@ public async Task GetJitCodegen_WithFlowgraphsAndAot_ShouldReturnError()
 
         var config = new JitDisasmConfiguration
         {
-            SelectedCustomJit = JitDisasmConfiguration.Crossgen,
+            SelectedCustomJit = JitCompilerTypes.Crossgen,
             FgEnable = true
         };
 

src/dotnet/ReSharperPlugin.DotNetDisassembler/AsmCompilationService.cs

@@ -36,6 +36,9 @@ public async Task<Result<string, Error>> CompileAsync(
         if (!result.Succeed || result.Value == null)
             return Result.FailWithValue(result.FailValue);
 
+        if (string.IsNullOrWhiteSpace(result.Value.Result))
+            return Result.FailWithValue(new Error(AsmViewerErrorCode.EmptyDisassembly));
+
         return Result.Success(result.Value.Result);
     }
 }

src/dotnet/ReSharperPlugin.DotNetDisassembler/AsmViewerErrors.cs

@@ -33,6 +33,7 @@ public enum AsmViewerErrorCode
     ProjectPathNotFound,
     DotnetBuildFailed,
     DotnetPublishFailed,
+    EmptyDisassembly,
 
     // Runtime/Path errors
     DotNetCliNotFound,

src/dotnet/ReSharperPlugin.DotNetDisassembler/AsmViewerHost.cs

@@ -134,12 +134,12 @@ private void SubscribeToTextControlChanges(Lifetime lifetime, ITextControlManage
                 return;
             }
 
-            // IL Viewer and similar tools open read-only editors with synchronized carets.
+            // IL Viewer, ASM Viewer and similar tools open read-only editors with synchronized carets.
             // We skip these and keep tracking the source editor - caret sync ensures our subscription still works.
             var projectFile = _documentManager.TryGetProjectFile(textControl.Document);
-            if (projectFile != null && _fileLocationsBlacklist.Contains(projectFile.Location))
+            if (projectFile == null || _fileLocationsBlacklist.Contains(projectFile.Location))
             {
-                _logger.Verbose("Ignoring blacklisted file location: {0}", textControl.Document.Moniker);
+                _logger.Verbose("Ignoring text control (virtual document or blacklisted): {0}", textControl.Document.Moniker);
                 return;
             }
 
@@ -174,9 +174,10 @@ private async Task RequestCompilationAsync(SequentialLifetimes compilationLifeti
         }
 
         var textControl = _currentTextControl;
-        if (textControl == null)
+        if (textControl == null || !textControl.Lifetime.IsAlive)
         {
-            _logger.Verbose("No text control, skipping");
+            _logger.Verbose("No text control or text control is no longer alive, clearing UI");
+            _model.SendResult(new CompilationResult(null, new ErrorInfo(ErrorCode.SourceFileNotFound, null)));
             return;
         }
 
@@ -191,6 +192,12 @@ private async Task RequestCompilationAsync(SequentialLifetimes compilationLifeti
             return;
         }
 
+        if (!textControl.Lifetime.IsAlive)
+        {
+            _logger.Verbose("Text control closed during compilation, discarding result");
+            return;
+        }
+
         if (!result.Succeed)
         {
             _logger.Warn("Compilation failed: {0} - {1}", result.FailValue.Code, result.FailValue.Details);

src/dotnet/ReSharperPlugin.DotNetDisassembler/AsmViewerModel.Generated.cs

@@ -112,7 +112,7 @@ internal AsmViewerModel (
 
 
 
-    protected override long SerializationHash => -8113457424028594267L;
+    protected override long SerializationHash => -8744739113612796705L;
 
     protected override Action<ISerializers> Register => RegisterDeclaredTypesSerializers;
     public static void RegisterDeclaredTypesSerializers(ISerializers serializers)
@@ -161,7 +161,7 @@ public static AsmViewerModel GetAsmViewerModel(this Solution solution)
 
 
   /// <summary>
-  /// <p>Generated from: AsmViewerModel.kt:63</p>
+  /// <p>Generated from: AsmViewerModel.kt:70</p>
   /// </summary>
   public sealed class CompilationResult : IPrintable, IEquatable<CompilationResult>
   {
@@ -258,6 +258,16 @@ public override string ToString()
   /// <summary>
   /// <p>Generated from: AsmViewerModel.kt:10</p>
   /// </summary>
+  public enum CompilerType {
+    Clrjit,
+    Crossgen2,
+    Ilc
+  }
+  
+  
+  /// <summary>
+  /// <p>Generated from: AsmViewerModel.kt:16</p>
+  /// </summary>
   public enum ErrorCode {
     SourceFileNotFound,
     PsiSourceFileUnavailable,
@@ -275,6 +285,7 @@ public enum ErrorCode {
     ProjectPathNotFound,
     DotnetBuildFailed,
     DotnetPublishFailed,
+    EmptyDisassembly,
     DotNetCliNotFound,
     RuntimePackNotFound,
     CoreClrCheckedNotFound,
@@ -285,7 +296,7 @@ public enum ErrorCode {
 
 
   /// <summary>
-  /// <p>Generated from: AsmViewerModel.kt:44</p>
+  /// <p>Generated from: AsmViewerModel.kt:51</p>
   /// </summary>
   public sealed class ErrorInfo : IPrintable, IEquatable<ErrorInfo>
   {
@@ -378,7 +389,7 @@ public override string ToString()
 
 
   /// <summary>
-  /// <p>Generated from: AsmViewerModel.kt:49</p>
+  /// <p>Generated from: AsmViewerModel.kt:56</p>
   /// </summary>
   public sealed class JitConfiguration : IPrintable, IEquatable<JitConfiguration>
   {
@@ -393,7 +404,7 @@ public sealed class JitConfiguration : IPrintable, IEquatable<JitConfiguration>
     public bool UseDotnetPublishForReload {get; private set;}
     public bool UseDotnetBuildForReload {get; private set;}
     [CanBeNull] public string TargetFrameworkOverride {get; private set;}
-    [CanBeNull] public string SelectedCustomJit {get; private set;}
+    public CompilerType SelectedCompiler {get; private set;}
     public int DisassemblyTimeoutSeconds {get; private set;}
 
     //private fields
@@ -408,7 +419,7 @@ public JitConfiguration(
       bool useDotnetPublishForReload,
       bool useDotnetBuildForReload,
       [CanBeNull] string targetFrameworkOverride,
-      [CanBeNull] string selectedCustomJit,
+      CompilerType selectedCompiler,
       int disassemblyTimeoutSeconds
     )
     {
@@ -421,12 +432,12 @@ int disassemblyTimeoutSeconds
       UseDotnetPublishForReload = useDotnetPublishForReload;
       UseDotnetBuildForReload = useDotnetBuildForReload;
       TargetFrameworkOverride = targetFrameworkOverride;
-      SelectedCustomJit = selectedCustomJit;
+      SelectedCompiler = selectedCompiler;
       DisassemblyTimeoutSeconds = disassemblyTimeoutSeconds;
     }
     //secondary constructor
     //deconstruct trait
-    public void Deconstruct(out bool showAsmComments, out bool diffable, out bool useTieredJit, out bool usePGO, out bool runAppMode, out bool useNoRestoreFlag, out bool useDotnetPublishForReload, out bool useDotnetBuildForReload, [CanBeNull] out string targetFrameworkOverride, [CanBeNull] out string selectedCustomJit, out int disassemblyTimeoutSeconds)
+    public void Deconstruct(out bool showAsmComments, out bool diffable, out bool useTieredJit, out bool usePGO, out bool runAppMode, out bool useNoRestoreFlag, out bool useDotnetPublishForReload, out bool useDotnetBuildForReload, [CanBeNull] out string targetFrameworkOverride, out CompilerType selectedCompiler, out int disassemblyTimeoutSeconds)
     {
       showAsmComments = ShowAsmComments;
       diffable = Diffable;
@@ -437,7 +448,7 @@ public void Deconstruct(out bool showAsmComments, out bool diffable, out bool us
       useDotnetPublishForReload = UseDotnetPublishForReload;
       useDotnetBuildForReload = UseDotnetBuildForReload;
       targetFrameworkOverride = TargetFrameworkOverride;
-      selectedCustomJit = SelectedCustomJit;
+      selectedCompiler = SelectedCompiler;
       disassemblyTimeoutSeconds = DisassemblyTimeoutSeconds;
     }
     //statics
@@ -453,9 +464,9 @@ public void Deconstruct(out bool showAsmComments, out bool diffable, out bool us
       var useDotnetPublishForReload = reader.ReadBool();
       var useDotnetBuildForReload = reader.ReadBool();
       var targetFrameworkOverride = ReadStringNullable(ctx, reader);
-      var selectedCustomJit = ReadStringNullable(ctx, reader);
+      var selectedCompiler = (CompilerType)reader.ReadInt();
       var disassemblyTimeoutSeconds = reader.ReadInt();
-      var _result = new JitConfiguration(showAsmComments, diffable, useTieredJit, usePGO, runAppMode, useNoRestoreFlag, useDotnetPublishForReload, useDotnetBuildForReload, targetFrameworkOverride, selectedCustomJit, disassemblyTimeoutSeconds);
+      var _result = new JitConfiguration(showAsmComments, diffable, useTieredJit, usePGO, runAppMode, useNoRestoreFlag, useDotnetPublishForReload, useDotnetBuildForReload, targetFrameworkOverride, selectedCompiler, disassemblyTimeoutSeconds);
       return _result;
     };
     public static CtxReadDelegate<string> ReadStringNullable = JetBrains.Rd.Impl.Serializers.ReadString.NullableClass();
@@ -471,7 +482,7 @@ public void Deconstruct(out bool showAsmComments, out bool diffable, out bool us
       writer.Write(value.UseDotnetPublishForReload);
       writer.Write(value.UseDotnetBuildForReload);
       WriteStringNullable(ctx, writer, value.TargetFrameworkOverride);
-      WriteStringNullable(ctx, writer, value.SelectedCustomJit);
+      writer.Write((int)value.SelectedCompiler);
       writer.Write(value.DisassemblyTimeoutSeconds);
     };
     public static  CtxWriteDelegate<string> WriteStringNullable = JetBrains.Rd.Impl.Serializers.WriteString.NullableClass();
@@ -492,7 +503,7 @@ public bool Equals(JitConfiguration other)
     {
       if (ReferenceEquals(null, other)) return false;
       if (ReferenceEquals(this, other)) return true;
-      return ShowAsmComments == other.ShowAsmComments && Diffable == other.Diffable && UseTieredJit == other.UseTieredJit && UsePGO == other.UsePGO && RunAppMode == other.RunAppMode && UseNoRestoreFlag == other.UseNoRestoreFlag && UseDotnetPublishForReload == other.UseDotnetPublishForReload && UseDotnetBuildForReload == other.UseDotnetBuildForReload && Equals(TargetFrameworkOverride, other.TargetFrameworkOverride) && Equals(SelectedCustomJit, other.SelectedCustomJit) && DisassemblyTimeoutSeconds == other.DisassemblyTimeoutSeconds;
+      return ShowAsmComments == other.ShowAsmComments && Diffable == other.Diffable && UseTieredJit == other.UseTieredJit && UsePGO == other.UsePGO && RunAppMode == other.RunAppMode && UseNoRestoreFlag == other.UseNoRestoreFlag && UseDotnetPublishForReload == other.UseDotnetPublishForReload && UseDotnetBuildForReload == other.UseDotnetBuildForReload && Equals(TargetFrameworkOverride, other.TargetFrameworkOverride) && SelectedCompiler == other.SelectedCompiler && DisassemblyTimeoutSeconds == other.DisassemblyTimeoutSeconds;
     }
     //hash code trait
     public override int GetHashCode()
@@ -508,7 +519,7 @@ public override int GetHashCode()
         hash = hash * 31 + UseDotnetPublishForReload.GetHashCode();
         hash = hash * 31 + UseDotnetBuildForReload.GetHashCode();
         hash = hash * 31 + (TargetFrameworkOverride != null ? TargetFrameworkOverride.GetHashCode() : 0);
-        hash = hash * 31 + (SelectedCustomJit != null ? SelectedCustomJit.GetHashCode() : 0);
+        hash = hash * 31 + (int) SelectedCompiler;
         hash = hash * 31 + DisassemblyTimeoutSeconds.GetHashCode();
         return hash;
       }
@@ -527,7 +538,7 @@ public void Print(PrettyPrinter printer)
         printer.Print("useDotnetPublishForReload = "); UseDotnetPublishForReload.PrintEx(printer); printer.Println();
         printer.Print("useDotnetBuildForReload = "); UseDotnetBuildForReload.PrintEx(printer); printer.Println();
         printer.Print("targetFrameworkOverride = "); TargetFrameworkOverride.PrintEx(printer); printer.Println();
-        printer.Print("selectedCustomJit = "); SelectedCustomJit.PrintEx(printer); printer.Println();
+        printer.Print("selectedCompiler = "); SelectedCompiler.PrintEx(printer); printer.Println();
         printer.Print("disassemblyTimeoutSeconds = "); DisassemblyTimeoutSeconds.PrintEx(printer); printer.Println();
       }
       printer.Print(")");