Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
@@ -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; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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>
Expand Down Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)}";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)))
{
Expand All @@ -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);
}
}
Expand All @@ -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.");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading