From 3821d39d6cdc46b64b8b97e1a873e9a69b3c3296 Mon Sep 17 00:00:00 2001 From: Justin Goshi Date: Thu, 26 Jan 2017 12:53:29 -0800 Subject: [PATCH] Address PR comments --- .../ProjectReader.cs | 30 +++++++-------- .../ProjectMigrator.cs | 38 +++++++++++++------ .../commands/dotnet-migrate/MigrateCommand.cs | 17 +++++++++ .../GivenAProjectMigrator.cs | 26 +++++++++++++ ...venThatIWantToMigrateDeprecatedProjects.cs | 8 ++-- 5 files changed, 88 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.DotNet.ProjectJsonMigration/Microsoft.DotNet.Internal.ProjectModel/ProjectReader.cs b/src/Microsoft.DotNet.ProjectJsonMigration/Microsoft.DotNet.Internal.ProjectModel/ProjectReader.cs index 1b20a58af..7a2778d9c 100644 --- a/src/Microsoft.DotNet.ProjectJsonMigration/Microsoft.DotNet.Internal.ProjectModel/ProjectReader.cs +++ b/src/Microsoft.DotNet.ProjectJsonMigration/Microsoft.DotNet.Internal.ProjectModel/ProjectReader.cs @@ -955,6 +955,21 @@ namespace Microsoft.DotNet.Internal.ProjectModel return (rawProject.SelectToken(jpathToNewFormatObject) != null); } + private static void ConvertFromDeprecatedFormat( + JObject rawProject, + string jpathToObject, + string deprecatedKey, + string newKey + ) + { + var deprecatedValue = rawProject.Value(deprecatedKey); + if (deprecatedValue != null) + { + var objectNode = GetOrCreateObjectHierarchy(rawProject, jpathToObject); + objectNode[newKey] = deprecatedValue.DeepClone(); + } + } + private static JObject GetOrCreateObjectHierarchy(JObject rawProject, string jpath) { var currentObject = rawProject as JObject; @@ -973,21 +988,6 @@ namespace Microsoft.DotNet.Internal.ProjectModel return currentObject; } - - private static void ConvertFromDeprecatedFormat( - JObject rawProject, - string jpathToObject, - string deprecatedKey, - string newKey - ) - { - var deprecatedValue = rawProject.Value(deprecatedKey); - if (deprecatedValue != null) - { - var objectNode = GetOrCreateObjectHierarchy(rawProject, jpathToObject); - objectNode[newKey] = deprecatedValue.DeepClone(); - } - } private static bool TryGetStringEnumerable(JToken token, out IEnumerable result) { diff --git a/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs b/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs index 37cc20b50..2554ee9c8 100644 --- a/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs +++ b/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs @@ -42,10 +42,11 @@ namespace Microsoft.DotNet.ProjectJsonMigration IEnumerable projectDependencies = null; var projectMigrationReports = new List(); + List warnings = null; try { // Verify up front so we can prefer these errors over an unresolved project dependency - VerifyInputs(rootInputs, rootSettings); + VerifyInputs(rootInputs, rootSettings, out warnings); projectMigrationReports.Add(MigrateProject(rootSettings)); @@ -68,7 +69,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration rootSettings.ProjectDirectory, rootInputs?.DefaultProjectContext?.GetProjectName(), new List {e.Error}, - null) + warnings) }); } @@ -144,6 +145,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration var projectName = migrationRuleInputs.DefaultProjectContext.GetProjectName(); var outputProject = Path.Combine(migrationSettings.OutputDirectory, projectName + ".csproj"); + List warnings = null; try { if (File.Exists(outputProject)) @@ -166,7 +168,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration } } - VerifyInputs(migrationRuleInputs, migrationSettings); + VerifyInputs(migrationRuleInputs, migrationSettings, out warnings); SetupOutputDirectory(migrationSettings.ProjectDirectory, migrationSettings.OutputDirectory); @@ -179,7 +181,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration exc.Error }; - return new ProjectMigrationReport(migrationSettings.ProjectDirectory, projectName, error, null); + return new ProjectMigrationReport(migrationSettings.ProjectDirectory, projectName, error, warnings); } List csprojDependencies = null; @@ -208,7 +210,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration projectName, outputProject, null, - null, + warnings, csprojDependencies); } @@ -235,13 +237,22 @@ namespace Microsoft.DotNet.ProjectJsonMigration return new MigrationRuleInputs(projectContexts, templateMSBuildProject, itemGroup, propertyGroup, xproj); } - private void VerifyInputs(MigrationRuleInputs migrationRuleInputs, MigrationSettings migrationSettings) + private void VerifyInputs( + MigrationRuleInputs migrationRuleInputs, + MigrationSettings migrationSettings, + out List warningMessages + ) { - VerifyProject(migrationRuleInputs.ProjectContexts, migrationSettings.ProjectDirectory); + VerifyProject(migrationRuleInputs.ProjectContexts, migrationSettings.ProjectDirectory, out warningMessages); } - private void VerifyProject(IEnumerable projectContexts, string projectDirectory) + private void VerifyProject( + IEnumerable projectContexts, + string projectDirectory, + out List warningMessages) { + warningMessages = null; + if (!projectContexts.Any()) { MigrationErrorCodes.MIGRATE1013(String.Format(LocalizableStrings.MIGRATE1013Arg, projectDirectory)).Throw(); @@ -255,11 +266,14 @@ namespace Microsoft.DotNet.ProjectJsonMigration var warnings = diagnostics.Where(d => d.Severity == DiagnosticMessageSeverity.Warning); if (warnings.Any()) { - var deprecatedProjectJsonWarnings = string.Join( + var migrationError = MigrationErrorCodes.MIGRATE1011(String.Format( + "{0}{1}{2}", + projectDirectory, Environment.NewLine, - diagnostics.Select(d => FormatDiagnosticMessage(d))); - var warningMessage = $"{projectDirectory}{Environment.NewLine}{deprecatedProjectJsonWarnings}"; - Reporter.Output.WriteLine(warningMessage.Yellow()); + string.Join(Environment.NewLine, diagnostics.Select(d => FormatDiagnosticMessage(d))))); + + warningMessages = new List(); + warningMessages.Add(migrationError.GetFormattedErrorMessage()); } var errors = diagnostics.Where(d => d.Severity == DiagnosticMessageSeverity.Error); diff --git a/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs b/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs index b50aca8d1..196339c1f 100644 --- a/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs +++ b/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs @@ -262,6 +262,8 @@ namespace Microsoft.DotNet.Tools.Migrate { var errorContent = GetProjectReportErrorContent(projectMigrationReport, colored: true); var successContent = GetProjectReportSuccessContent(projectMigrationReport, colored: true); + var warningContent = GetProjectReportWarningContent(projectMigrationReport, colored: true); + Reporter.Output.WriteLine(warningContent); if (!string.IsNullOrEmpty(errorContent)) { Reporter.Error.WriteLine(errorContent); @@ -290,6 +292,8 @@ namespace Microsoft.DotNet.Tools.Migrate { var errorContent = GetProjectReportErrorContent(projectMigrationReport, colored: colored); var successContent = GetProjectReportSuccessContent(projectMigrationReport, colored: colored); + var warningContent = GetProjectReportWarningContent(projectMigrationReport, colored: colored); + sb.AppendLine(warningContent); if (!string.IsNullOrEmpty(errorContent)) { sb.AppendLine(errorContent); @@ -331,6 +335,19 @@ namespace Microsoft.DotNet.Tools.Migrate projectMigrationReport.ProjectDirectory)); } + private string GetProjectReportWarningContent(ProjectMigrationReport projectMigrationReport, bool colored) + { + StringBuilder sb = new StringBuilder(); + Func YellowIfColored = (str) => colored ? str.Yellow() : str; + + foreach (var warning in projectMigrationReport.Warnings) + { + sb.AppendLine(YellowIfColored(warning)); + } + + return sb.ToString(); + } + private string GetProjectReportErrorContent(ProjectMigrationReport projectMigrationReport, bool colored) { StringBuilder sb = new StringBuilder(); diff --git a/test/Microsoft.DotNet.ProjectJsonMigration.Tests/GivenAProjectMigrator.cs b/test/Microsoft.DotNet.ProjectJsonMigration.Tests/GivenAProjectMigrator.cs index e0111f385..e6ca62262 100644 --- a/test/Microsoft.DotNet.ProjectJsonMigration.Tests/GivenAProjectMigrator.cs +++ b/test/Microsoft.DotNet.ProjectJsonMigration.Tests/GivenAProjectMigrator.cs @@ -39,6 +39,32 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests } } + [Fact] + public void ItHasWarningWhenMigratingADeprecatedProjectJson() + { + var testProjectDirectory = TestAssets + .GetProjectJson(TestAssetKinds.NonRestoredTestProjects, "PJAppWithDeprecatedCompileOptions") + .CreateInstance() + .WithSourceFiles() + .Root + .GetDirectory("project") + .FullName; + + var mockProj = ProjectRootElement.Create(); + var testSettings = MigrationSettings.CreateMigrationSettingsTestHook( + testProjectDirectory, + testProjectDirectory, + mockProj); + + var projectMigrator = new ProjectMigrator(new FakeEmptyMigrationRule()); + var report = projectMigrator.Migrate(testSettings); + + var projectReport = report.ProjectMigrationReports.First(); + var warningMessage = projectReport.Warnings.First(); + warningMessage.Should().Contain("MIGRATE1011::Deprecated Project:"); + warningMessage.Should().Contain("The 'compile' option is deprecated. Use 'compile' in 'buildOptions' instead. (line: 3, file:"); + } + [Fact] public void ItHasErrorWhenMigratingADeprecatedNamedResourceOptionProjectJson() { diff --git a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateDeprecatedProjects.cs b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateDeprecatedProjects.cs index 419f50f29..3229a333e 100644 --- a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateDeprecatedProjects.cs +++ b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateDeprecatedProjects.cs @@ -72,12 +72,12 @@ namespace Microsoft.DotNet.Migration.Tests new DotnetCommand() .WithWorkingDirectory(projectDirectory) - .Execute("build") + .Execute("build -c Debug") .Should().Pass(); new DotnetCommand() .WithWorkingDirectory(projectDirectory) - .Execute("pack") + .Execute("pack -c Debug") .Should().Pass(); var outputDir = projectDirectory.GetDirectory("bin", "Debug"); @@ -212,12 +212,12 @@ namespace Microsoft.DotNet.Migration.Tests new DotnetCommand() .WithWorkingDirectory(projectDirectory) - .Execute("build") + .Execute("build -c Debug") .Should().Pass(); new DotnetCommand() .WithWorkingDirectory(projectDirectory) - .Execute("publish") + .Execute("publish -c Debug") .Should().Pass(); var outputDir = projectDirectory.GetDirectory("bin", "Debug", "netcoreapp1.0");