Fixes for MAUI, logging, failed step repetitions, etc. (#1403)

* * Allow any version of .NetStandard to be migrated to MAUI
* Avoid unnecessary TFM merges for MAUI TFM SelectorFilter
* Always show 'dotnet --info' in case of workload install failure
* Added more information to log messages, such as project names
* Added MAUI step results to SARIF output
* Prevent endless failed step repetitions by adding Failded to IsDone
* Added failure level info to SARIF output using Note as default
* Added additional optional failure details to SARIF output
* Removed about:blank default which caused empty browser windows on click
* Fixed a few grammar and spelling issues

* * Restored `UpgradeStep.IsDone` behavior
* Don't return failed steps in non-interactive mode instead
* Updated tests to reflect new SARIF output
* Re-enabled Maui E2E tests
* Introduced TestOptions to skip the `MauiWorkloadUpgradeStep` during E2E tests

* * Replace the try-convert runner version with `[VERSION]` in E2E test SARIF output
* Skip the Apply part of the `MauiWorkloadUpgradeStep` but keep the rest for diagnostic output in the test

src/cli/Microsoft.DotNet.UpgradeAssistant.Cli/Commands/Upgrade/ConsoleUpgrade.cs

@@ -127,7 +127,7 @@ private async Task RunStepAsync(IUpgradeContext context, UpgradeStep step, Cance
             if (!await ExecuteAndTimeCommand(context, step, command, token))
             {
                 Console.ForegroundColor = ConsoleColor.Yellow;
-                await _io.Output.WriteAsync($"Command ({command.CommandText}) did not succeed");
+                await _io.Output.WriteLineAsync($"Command ({command.CommandText}) did not succeed");
                 Console.ResetColor();
             }
             else if (!await _input.WaitToProceedAsync(token))

src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/IUpgradeContextExtensions.cs

@@ -12,13 +12,13 @@ namespace Microsoft.DotNet.UpgradeAssistant
     public static class IUpgradeContextExtensions
     {
         public static void AddResultForStep(this IUpgradeContext context, UpgradeStep step,
-            string location, UpgradeStepStatus upgradeStepStatus, string message)
+            string location, UpgradeStepStatus upgradeStepStatus, string message, string? details = null, Uri? helpUri = null, OutputLevel? outputLevel = null)
         {
-            AddResult(context, step.Title, location, step.Id, upgradeStepStatus, message);
+            AddResult(context, step.Title, step.Description, location, step.Id, upgradeStepStatus, message, details, helpUri, outputLevel);
         }
 
-        public static void AddResult(this IUpgradeContext context, string stepName, string location, string ruleId,
-            UpgradeStepStatus upgradeStepStatus, string message)
+        public static void AddResult(this IUpgradeContext context, string stepName, string stepDescription, string location, string ruleId,
+            UpgradeStepStatus upgradeStepStatus, string message, string? details = null, Uri? helpUri = null, OutputLevel? outputLevel = null)
         {
             var status = upgradeStepStatus switch
             {
@@ -29,12 +29,23 @@ public static void AddResult(this IUpgradeContext context, string stepName, stri
                 _ => throw new ArgumentException("Invalid UpgradeStepStatus", nameof(upgradeStepStatus))
             };
 
+            var level = outputLevel ?? upgradeStepStatus switch
+            {
+                UpgradeStepStatus.Skipped => OutputLevel.Info,
+                UpgradeStepStatus.Failed => OutputLevel.Warning,
+                UpgradeStepStatus.Complete => OutputLevel.Info,
+                UpgradeStepStatus.Incomplete => OutputLevel.Info,
+                _ => throw new ArgumentException("Invalid UpgradeStepStatus", nameof(upgradeStepStatus))
+            };
+
             var result = new OutputResult()
             {
+                Level = level,
                 FileLocation = location,
                 RuleId = ruleId,
-                ResultMessage = $"{status}: {message}",
-                FullDescription = message,
+                ResultMessage = string.IsNullOrEmpty(details) ? $"{status}: {message}" : $"{status}: {message}{Environment.NewLine}{details}",
+                FullDescription = stepDescription,
+                HelpUri = helpUri
             };
 
             var outputResultDefinition = new OutputResultDefinition()

src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/OutputLevel.cs

@@ -0,0 +1,14 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+
+namespace Microsoft.DotNet.UpgradeAssistant
+{
+    public enum OutputLevel
+    {
+        Info,
+        Warning,
+        Error
+    }
+}

src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/OutputResult.cs

@@ -7,6 +7,8 @@ namespace Microsoft.DotNet.UpgradeAssistant
 {
     public record OutputResult
     {
+        public OutputLevel Level { get; init; } = OutputLevel.Info;
+
         public string RuleId { get; init; } = string.Empty;
 
         public string RuleName { get; init; } = string.Empty;
@@ -19,7 +21,7 @@ public record OutputResult
         /// <summary>
         /// Gets the link to the documentation.
         /// </summary>
-        public Uri HelpUri { get; init; } = new("about:blank");
+        public Uri? HelpUri { get; init; }
 
         public int LineNumber { get; init; }
 

src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/TestOptions.cs

@@ -0,0 +1,10 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.DotNet.UpgradeAssistant
+{
+    public class TestOptions
+    {
+        public bool IsRunningTest { get; set; }
+    }
+}

src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/UpgradeStep.cs

@@ -80,7 +80,7 @@ protected UpgradeStep(ILogger logger)
         public BuildBreakRisk Risk { get; private set; }
 
         /// <summary>
-        /// Gets a value indicating whether the upgrade is done (has completed successfully or was skipped).
+        /// Gets a value indicating whether the upgrade is done (has completed successfully, failed, or was skipped).
         /// </summary>
         public bool IsDone => Status switch
         {
@@ -97,7 +97,7 @@ protected UpgradeStep(ILogger logger)
         /// <summary>
         /// Implementers should use this method to indicate whether the upgrade step applies to a given upgrade context.
         /// Note that applicability is not about whether the step is complete or not (InitializeImplAsync should check that),
-        /// rather it is about whether it would ever make sense to run the migraiton step on the given context or not.
+        /// rather it is about whether it would ever make sense to run the migration step on the given context or not.
         /// For example, a upgrade step that acts at the project level would be inapplicable when a solution is selected
         /// rather than a project.
         /// </summary>
@@ -196,8 +196,9 @@ public virtual async Task<bool> ApplyAsync(IUpgradeContext context, Cancellation
             catch (Exception e)
 #pragma warning restore CA1031 // Do not catch general exception types
             {
-                (Status, StatusDetails) = (UpgradeStepStatus.Failed, "Unexpected error applying step.");
-                Logger.LogError(e, "Unexpected error applying step");
+                (Status, StatusDetails) = (UpgradeStepStatus.Failed, $"Unexpected error applying upgrade step '{Title}'");
+                Logger.LogError(e, "Unexpected error applying upgrade step {StepTitle}", Title);
+                context.AddResultForStep(this, context.CurrentProject?.GetFile()?.FilePath ?? string.Empty, Status, StatusDetails, details: e.ToString(), outputLevel: OutputLevel.Error);
                 return false;
             }
         }

src/components/Microsoft.DotNet.UpgradeAssistant.Analysis/SarifAnalyzeResultWriter.cs

@@ -97,13 +97,25 @@ private static ReportingDescriptor ExtractRule(IGrouping<string, OutputResult> a
 
         private static IList<Result> ExtractResults(IList<OutputResult> analyzeResults)
         {
+            FailureLevel GetFailureLevel(OutputLevel level)
+            {
+                return level switch
+                {
+                    OutputLevel.Info => FailureLevel.Note,
+                    OutputLevel.Warning => FailureLevel.Warning,
+                    OutputLevel.Error => FailureLevel.Error,
+                    _ => throw new ArgumentException($"Invalid {nameof(OutputLevel)}", nameof(level))
+                };
+            }
+
             var results = new List<Result>();
             foreach (var r in analyzeResults)
             {
                 results.Add(new()
                 {
                     RuleId = r.RuleId,
                     Message = r.ResultMessage.ToMessage(),
+                    Level = GetFailureLevel(r.Level),
                     Locations = new List<Location>()
                     {
                         new()

src/components/Microsoft.DotNet.UpgradeAssistant.MSBuild/MSBuildProject.File.cs

@@ -132,7 +132,7 @@ public void RenameFile(string filePath)
             var backupName = $"{Path.GetFileNameWithoutExtension(fileName)}.old{Path.GetExtension(fileName)}";
             var counter = 0;
 
-            while (File.Exists(backupName))
+            while (File.Exists(Path.Combine(Path.GetDirectoryName(filePath)!, backupName)))
             {
                 backupName = $"{Path.GetFileNameWithoutExtension(fileName)}.old.{counter++}{Path.GetExtension(fileName)}";
             }

src/components/Microsoft.DotNet.UpgradeAssistant/TargetFramework/DependencyMinimumTargetFrameworkSelectorFilter.cs

@@ -32,7 +32,7 @@ public void Process(ITargetFrameworkSelectorFilterState tfm)
 
             if (tfm.Components.HasFlag(ProjectComponents.MauiAndroid) || tfm.Components.HasFlag(ProjectComponents.MauiiOS) || tfm.Components.HasFlag(ProjectComponents.Maui))
             {
-                _logger.LogInformation("Skip minimum dependency check because .NET MAUI support multiple TFMs.");
+                _logger.LogInformation("Skip minimum dependency check because .NET MAUI supports multiple TFMs.");
             }
             else if (tfm.Components.HasFlag(ProjectComponents.WinUI))
             {
@@ -50,7 +50,7 @@ public void Process(ITargetFrameworkSelectorFilterState tfm)
                     {
                         if (tfm.TryUpdate(min))
                         {
-                            _logger.LogInformation("Recommending TFM {TFM} because of dependency on project {Dependency}", min, dep.GetFile().FilePath);
+                            _logger.LogInformation("Recommending TFM {TFM} for project {Name} because of dependency on project {Dependency}", min, tfm.Project, dep.GetFile().FilePath);
                         }
                     }
                 }

src/components/Microsoft.DotNet.UpgradeAssistant/TargetFramework/ExecutableTargetFrameworkSelectorFilter.cs

@@ -26,7 +26,7 @@ public void Process(ITargetFrameworkSelectorFilterState tfm)
             {
                 if (tfm.TryUpdate(tfm.AppBase))
                 {
-                    _logger.LogInformation("Recommending executable TFM {TFM} because the project builds to an executable", tfm.AppBase);
+                    _logger.LogInformation("Recommending executable TFM {TFM} for project {Name} because the project builds to an executable", tfm.AppBase, tfm.Project);
                 }
             }
         }

src/components/Microsoft.DotNet.UpgradeAssistant/UpgraderManager.cs

@@ -15,15 +15,18 @@ public class UpgraderManager
     {
         private readonly IUpgradeStepOrderer _orderer;
         private readonly ITelemetry _telemetry;
+        private readonly IUserInput _userInput;
         private readonly ILogger _logger;
 
         public UpgraderManager(
             IUpgradeStepOrderer orderer,
             ITelemetry telemetry,
+            IUserInput userInput,
             ILogger<UpgraderManager> logger)
         {
             _orderer = orderer ?? throw new ArgumentNullException(nameof(orderer));
             _telemetry = telemetry ?? throw new ArgumentNullException(nameof(telemetry));
+            _userInput = userInput ?? throw new ArgumentNullException(nameof(userInput));
             _logger = logger ?? throw new ArgumentNullException(nameof(logger));
         }
 
@@ -155,6 +158,12 @@ public async Task<IEnumerable<UpgradeStep>> InitializeAsync(IUpgradeContext cont
                     }
                 }
 
+                if (step.Status == UpgradeStepStatus.Failed && !_userInput.IsInteractive)
+                {
+                    // Don't return failed steps in non-interactive mode
+                    continue;
+                }
+
                 if (!step.IsDone)
                 {
                     return step;

src/extensions/default/Microsoft.DotNet.UpgradeAssistant.Steps.Solution/CurrentProjectSelectionStep.cs

@@ -101,8 +101,8 @@ protected override async Task<UpgradeStepInitializeResult> InitializeImplAsync(I
 
             if (allProjectsAreUpgraded)
             {
-                Logger.LogInformation("No projects need upgraded for selected entrypoint");
-                return new UpgradeStepInitializeResult(UpgradeStepStatus.Complete, "No projects need upgraded", BuildBreakRisk.None);
+                Logger.LogInformation("No projects need to be upgraded for selected entrypoint");
+                return new UpgradeStepInitializeResult(UpgradeStepStatus.Complete, "No projects need to be upgraded", BuildBreakRisk.None);
             }
 
             // If no project is selected, and at least one still needs upgraded, identify the next project to upgrade
@@ -129,7 +129,7 @@ protected override async Task<UpgradeStepInitializeResult> InitializeImplAsync(I
             {
                 if (await IsCompletedAsync(context, newCurrentProject, token).ConfigureAwait(false))
                 {
-                    Logger.LogDebug("Project {Project} does not need upgraded", newCurrentProject.FileInfo);
+                    Logger.LogDebug("Project {Project} does not need to be upgraded", newCurrentProject.FileInfo);
                 }
                 else if (!(await RunChecksAsync(newCurrentProject, token).ConfigureAwait(false)))
                 {
@@ -140,6 +140,7 @@ protected override async Task<UpgradeStepInitializeResult> InitializeImplAsync(I
                     context.SetCurrentProject(newCurrentProject);
                 }
 
+                Logger.LogInformation("Project {Name} was selected", newCurrentProject.GetRoslynProject().Name);
                 return new UpgradeStepInitializeResult(UpgradeStepStatus.Complete, $"Project {newCurrentProject.GetRoslynProject().Name} was selected", BuildBreakRisk.None);
             }
         }
@@ -160,6 +161,7 @@ protected override async Task<UpgradeStepApplyResult> ApplyImplAsync(IUpgradeCon
             else
             {
                 context.SetCurrentProject(selectedProject);
+                Logger.LogInformation("Project {Name} was selected", selectedProject.GetRoslynProject().Name);
                 return new UpgradeStepApplyResult(UpgradeStepStatus.Complete, $"Project {selectedProject.GetRoslynProject().Name} was selected.");
             }
         }

src/extensions/default/Microsoft.DotNet.UpgradeAssistant.Steps.Source/CodeFixerStep.cs

@@ -165,7 +165,7 @@ protected override async Task<UpgradeStepApplyResult> ApplyImplAsync(IUpgradeCon
 
         private void AddResultToContext(IUpgradeContext context, string diagnosticId, string location, UpgradeStepStatus status, string resultMessage)
         {
-            context.AddResult(Title, location, diagnosticId, status, resultMessage);
+            context.AddResult(Title, Description, location, diagnosticId, status, resultMessage);
         }
 
         private async Task<Solution?> TryFixDiagnosticAsync(Diagnostic diagnostic, Document document, CancellationToken token)