From 84e4da57cc5362ccb18268cd8b7c751eca89be89 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 8 Dec 2016 12:59:14 -0800 Subject: [PATCH 1/5] add compat checks to dotnet add --- src/dotnet/CommonLocalizableStrings.cs | 5 + src/dotnet/MsbuildProject.cs | 99 +++++++++++++++++-- .../dotnet-add/dotnet-add-p2p/Program.cs | 42 +++++++- .../dotnet-remove-p2p/Program.cs | 2 +- 4 files changed, 138 insertions(+), 10 deletions(-) diff --git a/src/dotnet/CommonLocalizableStrings.cs b/src/dotnet/CommonLocalizableStrings.cs index e62af8228..351ee8ee5 100644 --- a/src/dotnet/CommonLocalizableStrings.cs +++ b/src/dotnet/CommonLocalizableStrings.cs @@ -179,5 +179,10 @@ public const string SpecifiedNameExists = "Specified name {0} already exists. Please specify a different name."; public const string SpecifiedAliasExists = "Specified alias {0} already exists. Please specify a different alias."; public const string MandatoryParameterMissing = "Mandatory parameter {0} missing for template {1}. "; + + /// compatibility + public const string ProjectNotCompatibleWithFramework = "Project `{0}` is not compatible with `{1}`."; + public const string ProjectDoesNotTargetFramework = "Project `{0}` does not target `{1}`."; + public const string ProjectCouldNotBeEvaluated = "Project `{0}` could not be evaluated. Evaluation failed with following error:\n{1}"; } } diff --git a/src/dotnet/MsbuildProject.cs b/src/dotnet/MsbuildProject.cs index efa1a22b6..6fcf8525f 100644 --- a/src/dotnet/MsbuildProject.cs +++ b/src/dotnet/MsbuildProject.cs @@ -3,8 +3,10 @@ using Microsoft.Build.Construction; using Microsoft.Build.Evaluation; +using Microsoft.Build.Exceptions; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Tools.Common; +using NuGet.Frameworks; using System; using System.Collections.Generic; using System.IO; @@ -16,13 +18,16 @@ namespace Microsoft.DotNet.Tools { const string ProjectItemElementType = "ProjectReference"; - public ProjectRootElement Project { get; private set; } + public ProjectRootElement ProjectRoot { get; private set; } public string ProjectDirectory { get; private set; } + + private List _cachedTfms = null; + private Project _cachedEvaluatedProject = null; private MsbuildProject(ProjectRootElement project) { - Project = project; - ProjectDirectory = PathUtility.EnsureTrailingSlash(Project.DirectoryPath); + ProjectRoot = project; + ProjectDirectory = PathUtility.EnsureTrailingSlash(ProjectRoot.DirectoryPath); } public static MsbuildProject FromFileOrDirectory(string fileOrDirectory) @@ -101,17 +106,17 @@ namespace Microsoft.DotNet.Tools { int numberOfAddedReferences = 0; - ProjectItemGroupElement itemGroup = Project.FindUniformOrCreateItemGroupWithCondition(ProjectItemElementType, framework); + ProjectItemGroupElement itemGroup = ProjectRoot.FindUniformOrCreateItemGroupWithCondition(ProjectItemElementType, framework); foreach (var @ref in refs.Select((r) => NormalizeSlashes(r))) { - if (Project.HasExistingItemWithCondition(framework, @ref)) + if (ProjectRoot.HasExistingItemWithCondition(framework, @ref)) { Reporter.Output.WriteLine(string.Format(CommonLocalizableStrings.ProjectAlreadyHasAreference, @ref)); continue; } numberOfAddedReferences++; - itemGroup.AppendChild(Project.CreateItemElement(ProjectItemElementType, @ref)); + itemGroup.AppendChild(ProjectRoot.CreateItemElement(ProjectItemElementType, @ref)); Reporter.Output.WriteLine(string.Format(CommonLocalizableStrings.ReferenceAddedToTheProject, @ref)); } @@ -133,7 +138,7 @@ namespace Microsoft.DotNet.Tools public IEnumerable GetProjectToProjectReferences() { - return Project.GetAllItemsWithElementType(ProjectItemElementType); + return ProjectRoot.GetAllItemsWithElementType(ProjectItemElementType); } public void ConvertPathsToRelative(ref List references) @@ -166,12 +171,90 @@ namespace Microsoft.DotNet.Tools } } + public IEnumerable GetTargetFrameworks() + { + if (_cachedTfms != null) + { + return _cachedTfms; + } + + var project = GetEvaluatedProject(); + + var properties = project.AllEvaluatedProperties + .Where(p => p.Name.Equals("TargetFrameworks", StringComparison.OrdinalIgnoreCase) || + p.Name.Equals("TargetFramework", StringComparison.OrdinalIgnoreCase)) + .Select(p => p.EvaluatedValue.ToLower()).ToList(); + + var uniqueTfms = new HashSet(); + + foreach (var property in properties) + { + var tfms = property + .Split(';') + .Select((tfm) => tfm.Trim()) + .Where((tfm) => !string.IsNullOrEmpty(tfm)); + + foreach (var tfm in tfms) + { + uniqueTfms.Add(tfm); + } + } + + _cachedTfms = uniqueTfms.Select((frameworkString) => NuGetFramework.Parse(frameworkString)).ToList(); + return _cachedTfms; + } + + public bool CanWorkOnFramework(NuGetFramework framework) + { + foreach (var tfm in GetTargetFrameworks()) + { + if (DefaultCompatibilityProvider.Instance.IsCompatible(framework, tfm)) + { + return true; + } + } + + return false; + } + + public bool TargetsFramework(NuGetFramework framework) + { + foreach (var tfm in GetTargetFrameworks()) + { + if (framework.Equals(tfm)) + { + return true; + } + } + + return false; + } + + private Project GetEvaluatedProject() + { + if (_cachedEvaluatedProject != null) + { + return _cachedEvaluatedProject; + } + + try + { + _cachedEvaluatedProject = new Project(ProjectRoot); + } + catch (InvalidProjectFileException e) + { + throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectCouldNotBeEvaluated, ProjectRoot.FullPath, e.Message)); + } + + return _cachedEvaluatedProject; + } + private int RemoveProjectToProjectReferenceAlternatives(string framework, string reference) { int numberOfRemovedRefs = 0; foreach (var r in GetIncludeAlternativesForRemoval(reference)) { - foreach (var existingItem in Project.FindExistingItemsWithCondition(framework, r)) + foreach (var existingItem in ProjectRoot.FindExistingItemsWithCondition(framework, r)) { ProjectElementContainer itemGroup = existingItem.Parent; itemGroup.RemoveChild(existingItem); diff --git a/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs b/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs index 581ea1d03..02d1e1608 100644 --- a/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs +++ b/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs @@ -3,7 +3,9 @@ using Microsoft.DotNet.Cli.CommandLine; using Microsoft.DotNet.Cli.Utils; +using NuGet.Frameworks; using System.Collections.Generic; +using System.Linq; namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference { @@ -51,10 +53,43 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference throw new GracefulException(LocalizableStrings.SpecifyAtLeastOneReferenceToAdd); } + string frameworkString = frameworkOption.Value(); List references = app.RemainingArguments; if (!forceOption.HasValue()) { MsbuildProject.EnsureAllReferencesExist(references); + IEnumerable refs = references.Select((r) => MsbuildProject.FromFile(r)); + + if (frameworkString == null) + { + foreach (var tfm in msbuildProj.GetTargetFrameworks()) + { + foreach (var r in refs) + { + if (!r.CanWorkOnFramework(tfm)) + { + throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectNotCompatibleWithFramework, r.ProjectRoot.FullPath, GetFrameworkDisplayString(tfm))); + } + } + } + } + else + { + var framework = NuGetFramework.Parse(frameworkString); + if (!msbuildProj.TargetsFramework(framework)) + { + throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectDoesNotTargetFramework, msbuildProj.ProjectRoot.FullPath, GetFrameworkDisplayString(framework))); + } + + foreach (var r in refs) + { + if (!r.CanWorkOnFramework(framework)) + { + throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectNotCompatibleWithFramework, r.ProjectRoot.FullPath, GetFrameworkDisplayString(framework))); + } + } + } + msbuildProj.ConvertPathsToRelative(ref references); } @@ -64,7 +99,7 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference if (numberOfAddedReferences != 0) { - msbuildProj.Project.Save(); + msbuildProj.ProjectRoot.Save(); } return 0; @@ -81,5 +116,10 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference return 1; } } + + private static string GetFrameworkDisplayString(NuGetFramework framework) + { + return framework.GetShortFolderName(); + } } } diff --git a/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs b/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs index fcdf92fba..6ef8033d1 100644 --- a/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs +++ b/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs @@ -54,7 +54,7 @@ namespace Microsoft.DotNet.Tools.Remove.ProjectToProjectReference if (numberOfRemovedReferences != 0) { - msbuildProj.Project.Save(); + msbuildProj.ProjectRoot.Save(); } return 0; From 2d38aaa6e197f4328fb02afec9dd8ecdf6d66931 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 8 Dec 2016 14:56:31 -0800 Subject: [PATCH 2/5] fix dotnet-add-p2p tests --- .../DotnetAddP2PProjects/Lib/Lib.csproj | 2 +- .../ValidRef/ValidRef.csproj | 2 +- src/dotnet/MsbuildProject.cs | 40 ++++++++++------ .../dotnet-add/dotnet-add-p2p/Program.cs | 6 ++- .../dotnet-list/dotnet-list-p2ps/Program.cs | 3 +- .../dotnet-remove-p2p/Program.cs | 3 +- .../dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs | 48 ++++++++++++------- 7 files changed, 67 insertions(+), 37 deletions(-) diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Lib/Lib.csproj b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Lib/Lib.csproj index 7cd1371e1..f45f323b3 100644 --- a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Lib/Lib.csproj +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Lib/Lib.csproj @@ -4,7 +4,7 @@ Library - net451;netcoreapp1.0 + net451;netcoreapp1.0;netstandard1.4 diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/ValidRef/ValidRef.csproj b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/ValidRef/ValidRef.csproj index 7cd1371e1..f45f323b3 100644 --- a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/ValidRef/ValidRef.csproj +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/ValidRef/ValidRef.csproj @@ -4,7 +4,7 @@ Library - net451;netcoreapp1.0 + net451;netcoreapp1.0;netstandard1.4 diff --git a/src/dotnet/MsbuildProject.cs b/src/dotnet/MsbuildProject.cs index 6fcf8525f..cdd14143a 100644 --- a/src/dotnet/MsbuildProject.cs +++ b/src/dotnet/MsbuildProject.cs @@ -8,6 +8,7 @@ using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Tools.Common; using NuGet.Frameworks; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.Linq; @@ -20,45 +21,47 @@ namespace Microsoft.DotNet.Tools public ProjectRootElement ProjectRoot { get; private set; } public string ProjectDirectory { get; private set; } - + + private ProjectCollection _collection; private List _cachedTfms = null; private Project _cachedEvaluatedProject = null; - private MsbuildProject(ProjectRootElement project) + private MsbuildProject(ProjectCollection collection, ProjectRootElement project) { + _collection = collection; ProjectRoot = project; ProjectDirectory = PathUtility.EnsureTrailingSlash(ProjectRoot.DirectoryPath); } - public static MsbuildProject FromFileOrDirectory(string fileOrDirectory) + public static MsbuildProject FromFileOrDirectory(ProjectCollection collection, string fileOrDirectory) { if (File.Exists(fileOrDirectory)) { - return FromFile(fileOrDirectory); + return FromFile(collection, fileOrDirectory); } else { - return FromDirectory(fileOrDirectory); + return FromDirectory(collection, fileOrDirectory); } } - public static MsbuildProject FromFile(string projectPath) + public static MsbuildProject FromFile(ProjectCollection collection, string projectPath) { if (!File.Exists(projectPath)) { throw new GracefulException(CommonLocalizableStrings.ProjectDoesNotExist, projectPath); } - var project = TryOpenProject(projectPath); + var project = TryOpenProject(collection, projectPath); if (project == null) { throw new GracefulException(CommonLocalizableStrings.ProjectIsInvalid, projectPath); } - return new MsbuildProject(project); + return new MsbuildProject(collection, project); } - public static MsbuildProject FromDirectory(string projectDirectory) + public static MsbuildProject FromDirectory(ProjectCollection collection, string projectDirectory) { DirectoryInfo dir; try @@ -93,13 +96,13 @@ namespace Microsoft.DotNet.Tools throw new GracefulException(CommonLocalizableStrings.CouldNotFindAnyProjectInDirectory, projectDirectory); } - var project = TryOpenProject(projectFile.FullName); + var project = TryOpenProject(collection, projectFile.FullName); if (project == null) { throw new GracefulException(CommonLocalizableStrings.FoundInvalidProject, projectFile.FullName); } - return new MsbuildProject(project); + return new MsbuildProject(collection, project); } public int AddProjectToProjectReferences(string framework, IEnumerable refs) @@ -237,9 +240,16 @@ namespace Microsoft.DotNet.Tools return _cachedEvaluatedProject; } + var loadedProjects = _collection.GetLoadedProjects(ProjectRoot.FullPath); + if (loadedProjects.Count >= 1) + { + _cachedEvaluatedProject = loadedProjects.First(); + return _cachedEvaluatedProject; + } + try { - _cachedEvaluatedProject = new Project(ProjectRoot); + _cachedEvaluatedProject = new Project(ProjectRoot, null, null, _collection); } catch (InvalidProjectFileException e) { @@ -302,13 +312,13 @@ namespace Microsoft.DotNet.Tools // There is ProjectRootElement.TryOpen but it does not work as expected // I.e. it returns null for some valid projects - private static ProjectRootElement TryOpenProject(string filename) + private static ProjectRootElement TryOpenProject(ProjectCollection collection, string filename) { try { - return ProjectRootElement.Open(filename, new ProjectCollection(), preserveFormatting: true); + return ProjectRootElement.Open(filename, collection, preserveFormatting: true); } - catch (Microsoft.Build.Exceptions.InvalidProjectFileException) + catch (InvalidProjectFileException) { return null; } diff --git a/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs b/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs index 02d1e1608..24a5b3085 100644 --- a/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs +++ b/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.Build.Evaluation; using Microsoft.DotNet.Cli.CommandLine; using Microsoft.DotNet.Cli.Utils; using NuGet.Frameworks; @@ -46,7 +47,8 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference throw new GracefulException(CommonLocalizableStrings.RequiredArgumentNotPassed, $"<{LocalizableStrings.ProjectException}>"); } - var msbuildProj = MsbuildProject.FromFileOrDirectory(projectArgument.Value); + ProjectCollection collection = new ProjectCollection(); + var msbuildProj = MsbuildProject.FromFileOrDirectory(collection, projectArgument.Value); if (app.RemainingArguments.Count == 0) { @@ -58,7 +60,7 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference if (!forceOption.HasValue()) { MsbuildProject.EnsureAllReferencesExist(references); - IEnumerable refs = references.Select((r) => MsbuildProject.FromFile(r)); + IEnumerable refs = references.Select((r) => MsbuildProject.FromFile(collection, r)); if (frameworkString == null) { diff --git a/src/dotnet/commands/dotnet-list/dotnet-list-p2ps/Program.cs b/src/dotnet/commands/dotnet-list/dotnet-list-p2ps/Program.cs index 487959484..349c82bd4 100644 --- a/src/dotnet/commands/dotnet-list/dotnet-list-p2ps/Program.cs +++ b/src/dotnet/commands/dotnet-list/dotnet-list-p2ps/Program.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.Build.Evaluation; using Microsoft.DotNet.Cli.CommandLine; using Microsoft.DotNet.Cli.Utils; using System.Collections.Generic; @@ -31,7 +32,7 @@ namespace Microsoft.DotNet.Tools.List.ProjectToProjectReferences throw new GracefulException(CommonLocalizableStrings.RequiredArgumentNotPassed, $"<{LocalizableStrings.ProjectArgumentValueName}>"); } - var msbuildProj = MsbuildProject.FromFileOrDirectory(projectArgument.Value); + var msbuildProj = MsbuildProject.FromFileOrDirectory(new ProjectCollection(), projectArgument.Value); var p2ps = msbuildProj.GetProjectToProjectReferences(); if (p2ps.Count() == 0) diff --git a/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs b/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs index 6ef8033d1..ec646b6d2 100644 --- a/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs +++ b/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.Build.Evaluation; using Microsoft.DotNet.Cli.CommandLine; using Microsoft.DotNet.Cli.Utils; using System.Collections.Generic; @@ -39,7 +40,7 @@ namespace Microsoft.DotNet.Tools.Remove.ProjectToProjectReference throw new GracefulException(CommonLocalizableStrings.RequiredArgumentNotPassed, $"<{LocalizableStrings.ProjectException}>"); } - var msbuildProj = MsbuildProject.FromFileOrDirectory(projectArgument.Value); + var msbuildProj = MsbuildProject.FromFileOrDirectory(new ProjectCollection(), projectArgument.Value); if (app.RemainingArguments.Count == 0) { diff --git a/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs b/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs index 49bcceda7..6147c3415 100644 --- a/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs +++ b/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs @@ -7,6 +7,7 @@ using Microsoft.DotNet.Tools.Test.Utilities; using Msbuild.Tests.Utilities; using System; using System.IO; +using System.Linq; using Xunit; namespace Microsoft.DotNet.Cli.Add.P2P.Tests @@ -17,6 +18,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests const string ConditionFrameworkNet451 = "== 'net451'"; const string FrameworkNetCoreApp10Arg = "-f netcoreapp1.0"; const string ConditionFrameworkNetCoreApp10 = "== 'netcoreapp1.0'"; + static readonly string[] DefaultFrameworks = new string[] { "netcoreapp1.0", "net451" }; private TestSetup Setup([System.Runtime.CompilerServices.CallerMemberName] string callingMethod = nameof(Setup), string identifier = "") { @@ -52,6 +54,20 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests return dir; } + private static void SetTargetFrameworks(ProjDir proj, string[] frameworks) + { + var csproj = proj.CsProj(); + csproj.AddProperty("TargetFrameworks", string.Join(";", frameworks)); + csproj.Save(); + } + + private ProjDir NewLibWithFrameworks([System.Runtime.CompilerServices.CallerMemberName] string callingMethod = nameof(NewDir), string identifier = "") + { + var ret = NewLib(callingMethod: callingMethod, identifier: identifier); + SetTargetFrameworks(ret, DefaultFrameworks); + return ret; + } + [Theory] [InlineData("--help")] [InlineData("-h")] @@ -122,7 +138,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void ItAddsRefWithoutCondAndPrintsStatus() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); int noCondBefore = lib.CsProj().NumberOfItemGroupsWithoutCondition(); @@ -141,7 +157,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void ItAddsRefWithCondAndPrintsStatus() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); int condBefore = lib.CsProj().NumberOfItemGroupsWithConditionContaining(ConditionFrameworkNet451); @@ -160,7 +176,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenRefWithoutCondIsPresentItAddsDifferentRefWithoutCond() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); new AddP2PCommand() @@ -184,7 +200,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenRefWithCondIsPresentItAddsDifferentRefWithCond() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); new AddP2PCommand() @@ -208,7 +224,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenRefWithCondIsPresentItAddsRefWithDifferentCond() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); new AddP2PCommand() @@ -232,7 +248,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenRefWithConditionIsPresentItAddsDifferentRefWithoutCond() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); new AddP2PCommand() @@ -256,7 +272,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenRefWithNoCondAlreadyExistsItDoesntDuplicate() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); new AddP2PCommand() @@ -297,7 +313,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenRefWithCondOnItemGroupAlreadyExistsItDoesntDuplicate() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); new AddP2PCommand() @@ -421,7 +437,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void ItAddsMultipleRefsNoCondToTheSameItemGroup() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); int noCondBefore = lib.CsProj().NumberOfItemGroupsWithoutCondition(); @@ -440,7 +456,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void ItAddsMultipleRefsWithCondToTheSameItemGroup() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); int noCondBefore = lib.CsProj().NumberOfItemGroupsWithConditionContaining(ConditionFrameworkNet451); @@ -459,7 +475,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenProjectNameIsNotPassedItFindsItAndAddsReference() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); int noCondBefore = lib.CsProj().NumberOfItemGroupsWithoutCondition(); @@ -477,7 +493,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void ItAddsRefBetweenImports() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var cmd = new AddP2PCommand() @@ -522,7 +538,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenPassedReferenceDoesNotExistItShowsAnError() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var contentBefore = lib.CsProjContent(); var cmd = new AddP2PCommand() @@ -537,7 +553,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenPassedMultipleRefsAndOneOfthemDoesNotExistItCancelsWholeOperation() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var contentBefore = lib.CsProjContent(); @@ -554,7 +570,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenPassedReferenceDoesNotExistAndForceSwitchIsPassedItAddsIt() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); const string nonExisting = "IDoNotExist.csproj"; int noCondBefore = lib.CsProj().NumberOfItemGroupsWithoutCondition(); @@ -573,7 +589,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests [Fact] public void WhenPassedReferenceIsUsingSlashesItNormalizesItToBackslashes() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); int noCondBefore = lib.CsProj().NumberOfItemGroupsWithoutCondition(); From 21993e012931d5d670f9357ae7fc7abe8ce0cd65 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 8 Dec 2016 15:04:32 -0800 Subject: [PATCH 3/5] fix dotnet-remove-p2p tests --- .../GivenDotnetRemoveP2P.cs | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/test/dotnet-remove-p2p.Tests/GivenDotnetRemoveP2P.cs b/test/dotnet-remove-p2p.Tests/GivenDotnetRemoveP2P.cs index cbd500e5f..3c3017485 100644 --- a/test/dotnet-remove-p2p.Tests/GivenDotnetRemoveP2P.cs +++ b/test/dotnet-remove-p2p.Tests/GivenDotnetRemoveP2P.cs @@ -17,6 +17,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests const string ConditionFrameworkNet451 = "== 'net451'"; const string FrameworkNetCoreApp10Arg = "-f netcoreapp1.0"; const string ConditionFrameworkNetCoreApp10 = "== 'netcoreapp1.0'"; + static readonly string[] DefaultFrameworks = new string[] { "netcoreapp1.0", "net451" }; private TestSetup Setup([System.Runtime.CompilerServices.CallerMemberName] string callingMethod = nameof(Setup), string identifier = "") { @@ -52,6 +53,20 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests return dir; } + private static void SetTargetFrameworks(ProjDir proj, string[] frameworks) + { + var csproj = proj.CsProj(); + csproj.AddProperty("TargetFrameworks", string.Join(";", frameworks)); + csproj.Save(); + } + + private ProjDir NewLibWithFrameworks([System.Runtime.CompilerServices.CallerMemberName] string callingMethod = nameof(NewDir), string identifier = "") + { + var ret = NewLib(callingMethod: callingMethod, identifier: identifier); + SetTargetFrameworks(ret, DefaultFrameworks); + return ret; + } + private ProjDir GetLibRef(TestSetup setup) { return new ProjDir(setup.LibDir); @@ -151,7 +166,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void ItRemovesRefWithoutCondAndPrintsStatus() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = AddLibRef(setup, lib); @@ -170,7 +185,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void ItRemovesRefWithCondAndPrintsStatus() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = AddLibRef(setup, lib, FrameworkNet451Arg); @@ -189,7 +204,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenTwoDifferentRefsArePresentItDoesNotRemoveBoth() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = AddLibRef(setup, lib); var validref = AddValidRef(setup, lib); @@ -210,7 +225,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenRefWithoutCondIsNotThereItPrintsMessage() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = GetLibRef(setup); @@ -227,7 +242,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenRefWithCondIsNotThereItPrintsMessage() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = GetLibRef(setup); @@ -244,7 +259,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenRefWithAndWithoutCondArePresentAndRemovingNoCondItDoesNotRemoveOther() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var librefCond = AddLibRef(setup, lib, FrameworkNet451Arg); var librefNoCond = AddLibRef(setup, lib); @@ -269,7 +284,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenRefWithAndWithoutCondArePresentAndRemovingCondItDoesNotRemoveOther() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var librefCond = AddLibRef(setup, lib, FrameworkNet451Arg); var librefNoCond = AddLibRef(setup, lib); @@ -294,7 +309,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenRefWithDifferentCondIsPresentItDoesNotRemoveIt() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var librefCondNet451 = AddLibRef(setup, lib, FrameworkNet451Arg); var librefCondNetCoreApp10 = AddLibRef(setup, lib, FrameworkNetCoreApp10Arg); @@ -396,7 +411,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenPassingMultipleReferencesItRemovesThemAll() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = AddLibRef(setup, lib); var validref = AddValidRef(setup, lib); @@ -417,7 +432,7 @@ namespace Microsoft.DotNet.Cli.Remove.P2P.Tests [Fact] public void WhenPassingMultipleReferencesAndOneOfThemDoesNotExistItRemovesOne() { - var lib = NewLib(); + var lib = NewLibWithFrameworks(); var setup = Setup(); var libref = GetLibRef(setup); var validref = AddValidRef(setup, lib); From 9a8d158e09c0e82ee7f1f9c3254066ff74c0638f Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 8 Dec 2016 16:23:21 -0800 Subject: [PATCH 4/5] add test coverage --- .../Net452AndNetCoreApp10Lib.cs | 3 + .../Net452AndNetCoreApp10Lib.csproj | 25 +++++++ .../DotnetAddP2PProjects/Net45Lib/Net45Lib.cs | 3 + .../Net45Lib/Net45Lib.csproj | 25 +++++++ src/dotnet/CommonLocalizableStrings.cs | 1 - .../dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs | 74 +++++++++++++++++++ 6 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs create mode 100644 TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.csproj create mode 100644 TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs create mode 100644 TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.csproj diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs new file mode 100644 index 000000000..913c9e16b --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs @@ -0,0 +1,3 @@ +public class Net452AndNetCoreApp10Lib +{ +} \ No newline at end of file diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.csproj b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.csproj new file mode 100644 index 000000000..e47dc3662 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.csproj @@ -0,0 +1,25 @@ + + + + + + Library + net452;netcoreapp1.0 + + + + + + + + $(CLI_NETSDK_Version) + All + + + + + 1.0.1 + + + + diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs new file mode 100644 index 000000000..9d62cf731 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs @@ -0,0 +1,3 @@ +public class Net45Lib +{ +} \ No newline at end of file diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.csproj b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.csproj new file mode 100644 index 000000000..f8769c442 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.csproj @@ -0,0 +1,25 @@ + + + + + + Library + net45 + + + + + + + + $(CLI_NETSDK_Version) + All + + + + + 1.0.1 + + + + diff --git a/src/dotnet/CommonLocalizableStrings.cs b/src/dotnet/CommonLocalizableStrings.cs index 351ee8ee5..350be7c2e 100644 --- a/src/dotnet/CommonLocalizableStrings.cs +++ b/src/dotnet/CommonLocalizableStrings.cs @@ -180,7 +180,6 @@ public const string SpecifiedAliasExists = "Specified alias {0} already exists. Please specify a different alias."; public const string MandatoryParameterMissing = "Mandatory parameter {0} missing for template {1}. "; - /// compatibility public const string ProjectNotCompatibleWithFramework = "Project `{0}` is not compatible with `{1}`."; public const string ProjectDoesNotTargetFramework = "Project `{0}` does not target `{1}`."; public const string ProjectCouldNotBeEvaluated = "Project `{0}` could not be evaluated. Evaluation failed with following error:\n{1}"; diff --git a/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs b/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs index 6147c3415..a22706726 100644 --- a/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs +++ b/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs @@ -642,5 +642,79 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests csproj.NumberOfItemGroupsWithoutCondition().Should().Be(noCondBefore + 1); csproj.NumberOfProjectReferencesWithIncludeContaining(setup.ValidRefCsprojRelPath.Replace('/', '\\')).Should().Be(1); } + + [Fact] + public void ItCanAddRefWithCondOnCompatibleFramework() + { + var setup = Setup(); + var lib = new ProjDir(setup.LibDir); + var net45lib = new ProjDir(Path.Combine(setup.TestRoot, "Net45Lib")); + + int condBefore = lib.CsProj().NumberOfItemGroupsWithConditionContaining(ConditionFrameworkNet451); + var cmd = new AddP2PCommand() + .WithProject(lib.CsProjPath) + .Execute($"{FrameworkNet451Arg} \"{net45lib.CsProjPath}\""); + cmd.Should().Pass(); + cmd.StdOut.Should().Contain("added to the project"); + var csproj = lib.CsProj(); + csproj.NumberOfItemGroupsWithConditionContaining(ConditionFrameworkNet451).Should().Be(condBefore + 1); + csproj.NumberOfProjectReferencesWithIncludeAndConditionContaining(net45lib.CsProjName, ConditionFrameworkNet451).Should().Be(1); + } + + [Fact] + public void ItCanAddRefWithoutCondAndTargetingSupersetOfFrameworksAndOneOfReferencesCompatible() + { + var setup = Setup(); + var lib = new ProjDir(setup.LibDir); + var net452netcoreapp10lib = new ProjDir(Path.Combine(setup.TestRoot, "Net452AndNetCoreApp10Lib")); + + int noCondBefore = net452netcoreapp10lib.CsProj().NumberOfItemGroupsWithoutCondition(); + var cmd = new AddP2PCommand() + .WithProject(net452netcoreapp10lib.CsProjPath) + .Execute($"\"{lib.CsProjPath}\""); + cmd.Should().Pass(); + cmd.StdOut.Should().Contain("added to the project"); + var csproj = net452netcoreapp10lib.CsProj(); + csproj.NumberOfItemGroupsWithoutCondition().Should().Be(noCondBefore + 1); + csproj.NumberOfProjectReferencesWithIncludeContaining(lib.CsProjName).Should().Be(1); + } + + [Theory] + [InlineData("net45")] + [InlineData("net40")] + [InlineData("netcoreapp1.1")] + [InlineData("nonexistingframeworkname")] + public void WhenFrameworkSwitchIsNotMatchingAnyOfTargetedFrameworksItPrintsError(string framework) + { + var setup = Setup(); + var lib = new ProjDir(setup.LibDir); + var net45lib = new ProjDir(Path.Combine(setup.TestRoot, "Net45Lib")); + + var csProjContent = lib.CsProjContent(); + var cmd = new AddP2PCommand() + .WithProject(lib.CsProjPath) + .Execute($"-f {framework} \"{net45lib.CsProjPath}\""); + cmd.Should().Fail(); + cmd.StdErr.Should().Contain("does not target"); + lib.CsProjContent().Should().BeEquivalentTo(csProjContent); + } + + [Theory] + [InlineData("")] + [InlineData("-f net45")] + public void WhenIncompatibleFrameworkDetectedItPrintsError(string frameworkArg) + { + var setup = Setup(); + var lib = new ProjDir(setup.LibDir); + var net45lib = new ProjDir(Path.Combine(setup.TestRoot, "Net45Lib")); + + var csProjContent = net45lib.CsProjContent(); + var cmd = new AddP2PCommand() + .WithProject(net45lib.CsProjPath) + .Execute($"{frameworkArg} \"{lib.CsProjPath}\""); + cmd.Should().Fail(); + cmd.StdErr.Should().Contain("is not compatible with"); + net45lib.CsProjContent().Should().BeEquivalentTo(csProjContent); + } } } From 524fe3cb6b3fd620773f6ef72df400b741de8444 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Fri, 9 Dec 2016 12:11:11 -0800 Subject: [PATCH 5/5] apply review feedback --- .../Net452AndNetCoreApp10Lib.cs | 2 +- .../DotnetAddP2PProjects/Net45Lib/Net45Lib.cs | 2 +- src/dotnet/CommonLocalizableStrings.cs | 4 +- src/dotnet/MsbuildProject.cs | 116 ++++++++---------- src/dotnet/ProjectExtensions.cs | 36 ++++++ .../dotnet-add/dotnet-add-p2p/Program.cs | 43 ++++--- .../dotnet-remove-p2p/Program.cs | 2 +- .../dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs | 12 +- 8 files changed, 126 insertions(+), 91 deletions(-) create mode 100644 src/dotnet/ProjectExtensions.cs diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs index 913c9e16b..ead0b80b3 100644 --- a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net452AndNetCoreApp10Lib/Net452AndNetCoreApp10Lib.cs @@ -1,3 +1,3 @@ public class Net452AndNetCoreApp10Lib { -} \ No newline at end of file +} diff --git a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs index 9d62cf731..8f0ebe8b1 100644 --- a/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs +++ b/TestAssets/NonRestoredTestProjects/DotnetAddP2PProjects/Net45Lib/Net45Lib.cs @@ -1,3 +1,3 @@ public class Net45Lib { -} \ No newline at end of file +} diff --git a/src/dotnet/CommonLocalizableStrings.cs b/src/dotnet/CommonLocalizableStrings.cs index 350be7c2e..51961465d 100644 --- a/src/dotnet/CommonLocalizableStrings.cs +++ b/src/dotnet/CommonLocalizableStrings.cs @@ -180,8 +180,8 @@ public const string SpecifiedAliasExists = "Specified alias {0} already exists. Please specify a different alias."; public const string MandatoryParameterMissing = "Mandatory parameter {0} missing for template {1}. "; - public const string ProjectNotCompatibleWithFramework = "Project `{0}` is not compatible with `{1}`."; - public const string ProjectDoesNotTargetFramework = "Project `{0}` does not target `{1}`."; + public const string ProjectNotCompatibleWithFrameworks = "Project `{0}` cannot be added due to incompatible targeted frameworks between the two projects. Please review the project you are trying to add and verify that is compatible with the following targets:"; + public const string ProjectDoesNotTargetFramework = "Project `{0}` does not target framework `{1}`."; public const string ProjectCouldNotBeEvaluated = "Project `{0}` could not be evaluated. Evaluation failed with following error:\n{1}"; } } diff --git a/src/dotnet/MsbuildProject.cs b/src/dotnet/MsbuildProject.cs index cdd14143a..f53d084da 100644 --- a/src/dotnet/MsbuildProject.cs +++ b/src/dotnet/MsbuildProject.cs @@ -6,9 +6,9 @@ using Microsoft.Build.Evaluation; using Microsoft.Build.Exceptions; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Tools.Common; +using Microsoft.DotNet.Tools.ProjectExtensions; using NuGet.Frameworks; using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.Linq; @@ -19,49 +19,48 @@ namespace Microsoft.DotNet.Tools { const string ProjectItemElementType = "ProjectReference"; - public ProjectRootElement ProjectRoot { get; private set; } + public ProjectRootElement ProjectRootElement { get; private set; } public string ProjectDirectory { get; private set; } - private ProjectCollection _collection; + private ProjectCollection _projects; private List _cachedTfms = null; - private Project _cachedEvaluatedProject = null; - private MsbuildProject(ProjectCollection collection, ProjectRootElement project) + private MsbuildProject(ProjectCollection projects, ProjectRootElement project) { - _collection = collection; - ProjectRoot = project; - ProjectDirectory = PathUtility.EnsureTrailingSlash(ProjectRoot.DirectoryPath); + _projects = projects; + ProjectRootElement = project; + ProjectDirectory = PathUtility.EnsureTrailingSlash(ProjectRootElement.DirectoryPath); } - public static MsbuildProject FromFileOrDirectory(ProjectCollection collection, string fileOrDirectory) + public static MsbuildProject FromFileOrDirectory(ProjectCollection projects, string fileOrDirectory) { if (File.Exists(fileOrDirectory)) { - return FromFile(collection, fileOrDirectory); + return FromFile(projects, fileOrDirectory); } else { - return FromDirectory(collection, fileOrDirectory); + return FromDirectory(projects, fileOrDirectory); } } - public static MsbuildProject FromFile(ProjectCollection collection, string projectPath) + public static MsbuildProject FromFile(ProjectCollection projects, string projectPath) { if (!File.Exists(projectPath)) { throw new GracefulException(CommonLocalizableStrings.ProjectDoesNotExist, projectPath); } - var project = TryOpenProject(collection, projectPath); + var project = TryOpenProject(projects, projectPath); if (project == null) { throw new GracefulException(CommonLocalizableStrings.ProjectIsInvalid, projectPath); } - return new MsbuildProject(collection, project); + return new MsbuildProject(projects, project); } - public static MsbuildProject FromDirectory(ProjectCollection collection, string projectDirectory) + public static MsbuildProject FromDirectory(ProjectCollection projects, string projectDirectory) { DirectoryInfo dir; try @@ -81,7 +80,9 @@ namespace Microsoft.DotNet.Tools FileInfo[] files = dir.GetFiles("*proj"); if (files.Length == 0) { - throw new GracefulException(CommonLocalizableStrings.CouldNotFindAnyProjectInDirectory, projectDirectory); + throw new GracefulException( + CommonLocalizableStrings.CouldNotFindAnyProjectInDirectory, + projectDirectory); } if (files.Length > 1) @@ -93,33 +94,39 @@ namespace Microsoft.DotNet.Tools if (!projectFile.Exists) { - throw new GracefulException(CommonLocalizableStrings.CouldNotFindAnyProjectInDirectory, projectDirectory); + throw new GracefulException( + CommonLocalizableStrings.CouldNotFindAnyProjectInDirectory, + projectDirectory); } - var project = TryOpenProject(collection, projectFile.FullName); + var project = TryOpenProject(projects, projectFile.FullName); if (project == null) { throw new GracefulException(CommonLocalizableStrings.FoundInvalidProject, projectFile.FullName); } - return new MsbuildProject(collection, project); + return new MsbuildProject(projects, project); } public int AddProjectToProjectReferences(string framework, IEnumerable refs) { int numberOfAddedReferences = 0; - ProjectItemGroupElement itemGroup = ProjectRoot.FindUniformOrCreateItemGroupWithCondition(ProjectItemElementType, framework); + ProjectItemGroupElement itemGroup = ProjectRootElement.FindUniformOrCreateItemGroupWithCondition( + ProjectItemElementType, + framework); foreach (var @ref in refs.Select((r) => NormalizeSlashes(r))) { - if (ProjectRoot.HasExistingItemWithCondition(framework, @ref)) + if (ProjectRootElement.HasExistingItemWithCondition(framework, @ref)) { - Reporter.Output.WriteLine(string.Format(CommonLocalizableStrings.ProjectAlreadyHasAreference, @ref)); + Reporter.Output.WriteLine(string.Format( + CommonLocalizableStrings.ProjectAlreadyHasAreference, + @ref)); continue; } numberOfAddedReferences++; - itemGroup.AppendChild(ProjectRoot.CreateItemElement(ProjectItemElementType, @ref)); + itemGroup.AppendChild(ProjectRootElement.CreateItemElement(ProjectItemElementType, @ref)); Reporter.Output.WriteLine(string.Format(CommonLocalizableStrings.ReferenceAddedToTheProject, @ref)); } @@ -141,12 +148,16 @@ namespace Microsoft.DotNet.Tools public IEnumerable GetProjectToProjectReferences() { - return ProjectRoot.GetAllItemsWithElementType(ProjectItemElementType); + return ProjectRootElement.GetAllItemsWithElementType(ProjectItemElementType); } public void ConvertPathsToRelative(ref List references) { - references = references.Select((r) => PathUtility.GetRelativePath(ProjectDirectory, Path.GetFullPath(r))).ToList(); + references = references.Select((r) => + PathUtility.GetRelativePath( + ProjectDirectory, + Path.GetFullPath(r))) + .ToList(); } public static string NormalizeSlashes(string path) @@ -182,28 +193,7 @@ namespace Microsoft.DotNet.Tools } var project = GetEvaluatedProject(); - - var properties = project.AllEvaluatedProperties - .Where(p => p.Name.Equals("TargetFrameworks", StringComparison.OrdinalIgnoreCase) || - p.Name.Equals("TargetFramework", StringComparison.OrdinalIgnoreCase)) - .Select(p => p.EvaluatedValue.ToLower()).ToList(); - - var uniqueTfms = new HashSet(); - - foreach (var property in properties) - { - var tfms = property - .Split(';') - .Select((tfm) => tfm.Trim()) - .Where((tfm) => !string.IsNullOrEmpty(tfm)); - - foreach (var tfm in tfms) - { - uniqueTfms.Add(tfm); - } - } - - _cachedTfms = uniqueTfms.Select((frameworkString) => NuGetFramework.Parse(frameworkString)).ToList(); + _cachedTfms = project.GetTargetFrameworks().ToList(); return _cachedTfms; } @@ -220,7 +210,7 @@ namespace Microsoft.DotNet.Tools return false; } - public bool TargetsFramework(NuGetFramework framework) + public bool IsTargettingFramework(NuGetFramework framework) { foreach (var tfm in GetTargetFrameworks()) { @@ -235,28 +225,16 @@ namespace Microsoft.DotNet.Tools private Project GetEvaluatedProject() { - if (_cachedEvaluatedProject != null) - { - return _cachedEvaluatedProject; - } - - var loadedProjects = _collection.GetLoadedProjects(ProjectRoot.FullPath); - if (loadedProjects.Count >= 1) - { - _cachedEvaluatedProject = loadedProjects.First(); - return _cachedEvaluatedProject; - } - try { - _cachedEvaluatedProject = new Project(ProjectRoot, null, null, _collection); + return _projects.LoadProject(ProjectRootElement.FullPath); } catch (InvalidProjectFileException e) { - throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectCouldNotBeEvaluated, ProjectRoot.FullPath, e.Message)); + throw new GracefulException(string.Format( + CommonLocalizableStrings.ProjectCouldNotBeEvaluated, + ProjectRootElement.FullPath, e.Message)); } - - return _cachedEvaluatedProject; } private int RemoveProjectToProjectReferenceAlternatives(string framework, string reference) @@ -264,7 +242,7 @@ namespace Microsoft.DotNet.Tools int numberOfRemovedRefs = 0; foreach (var r in GetIncludeAlternativesForRemoval(reference)) { - foreach (var existingItem in ProjectRoot.FindExistingItemsWithCondition(framework, r)) + foreach (var existingItem in ProjectRootElement.FindExistingItemsWithCondition(framework, r)) { ProjectElementContainer itemGroup = existingItem.Parent; itemGroup.RemoveChild(existingItem); @@ -280,7 +258,9 @@ namespace Microsoft.DotNet.Tools if (numberOfRemovedRefs == 0) { - Reporter.Output.WriteLine(string.Format(CommonLocalizableStrings.ProjectReferenceCouldNotBeFound, reference)); + Reporter.Output.WriteLine(string.Format( + CommonLocalizableStrings.ProjectReferenceCouldNotBeFound, + reference)); } return numberOfRemovedRefs; @@ -312,11 +292,11 @@ namespace Microsoft.DotNet.Tools // There is ProjectRootElement.TryOpen but it does not work as expected // I.e. it returns null for some valid projects - private static ProjectRootElement TryOpenProject(ProjectCollection collection, string filename) + private static ProjectRootElement TryOpenProject(ProjectCollection projects, string filename) { try { - return ProjectRootElement.Open(filename, collection, preserveFormatting: true); + return ProjectRootElement.Open(filename, projects, preserveFormatting: true); } catch (InvalidProjectFileException) { diff --git a/src/dotnet/ProjectExtensions.cs b/src/dotnet/ProjectExtensions.cs new file mode 100644 index 000000000..1ed154477 --- /dev/null +++ b/src/dotnet/ProjectExtensions.cs @@ -0,0 +1,36 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Build.Construction; +using Microsoft.Build.Evaluation; +using Microsoft.DotNet.ProjectJsonMigration; +using NuGet.Frameworks; +using System.Collections.Generic; +using System.Linq; + +namespace Microsoft.DotNet.Tools.ProjectExtensions +{ + internal static class ProjectExtensions + { + public static IEnumerable GetTargetFrameworks(this Project project) + { + var targetFramewoksStrings = project + .GetPropertyCommaSeparatedValues("TargetFramework") + .Union(project.GetPropertyCommaSeparatedValues("TargetFrameworks")) + .Select((value) => value.ToLower()); + + var uniqueTargetFrameworkStrings = new HashSet(targetFramewoksStrings); + + return uniqueTargetFrameworkStrings + .Select((frameworkString) => NuGetFramework.Parse(frameworkString)); + } + + public static IEnumerable GetPropertyCommaSeparatedValues(this Project project, string propertyName) + { + return project.GetPropertyValue(propertyName) + .Split(';') + .Select((value) => value.Trim()) + .Where((value) => !string.IsNullOrEmpty(value)); + } + } +} diff --git a/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs b/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs index 24a5b3085..04dbe7b25 100644 --- a/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs +++ b/src/dotnet/commands/dotnet-add/dotnet-add-p2p/Program.cs @@ -7,6 +7,7 @@ using Microsoft.DotNet.Cli.Utils; using NuGet.Frameworks; using System.Collections.Generic; using System.Linq; +using System.Text; namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference { @@ -47,8 +48,8 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference throw new GracefulException(CommonLocalizableStrings.RequiredArgumentNotPassed, $"<{LocalizableStrings.ProjectException}>"); } - ProjectCollection collection = new ProjectCollection(); - var msbuildProj = MsbuildProject.FromFileOrDirectory(collection, projectArgument.Value); + var projects = new ProjectCollection(); + var msbuildProj = MsbuildProject.FromFileOrDirectory(projects, projectArgument.Value); if (app.RemainingArguments.Count == 0) { @@ -60,17 +61,20 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference if (!forceOption.HasValue()) { MsbuildProject.EnsureAllReferencesExist(references); - IEnumerable refs = references.Select((r) => MsbuildProject.FromFile(collection, r)); + IEnumerable refs = references.Select((r) => MsbuildProject.FromFile(projects, r)); if (frameworkString == null) { foreach (var tfm in msbuildProj.GetTargetFrameworks()) { - foreach (var r in refs) + foreach (var @ref in refs) { - if (!r.CanWorkOnFramework(tfm)) + if (!@ref.CanWorkOnFramework(tfm)) { - throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectNotCompatibleWithFramework, r.ProjectRoot.FullPath, GetFrameworkDisplayString(tfm))); + Reporter.Error.Write(GetProjectNotCompatibleWithFrameworksDisplayString( + @ref, + msbuildProj.GetTargetFrameworks().Select((fx) => fx.GetShortFolderName()))); + return 1; } } } @@ -78,16 +82,20 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference else { var framework = NuGetFramework.Parse(frameworkString); - if (!msbuildProj.TargetsFramework(framework)) + if (!msbuildProj.IsTargettingFramework(framework)) { - throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectDoesNotTargetFramework, msbuildProj.ProjectRoot.FullPath, GetFrameworkDisplayString(framework))); + Reporter.Error.WriteLine(string.Format(CommonLocalizableStrings.ProjectDoesNotTargetFramework, msbuildProj.ProjectRootElement.FullPath, frameworkString)); + return 1; } - foreach (var r in refs) + foreach (var @ref in refs) { - if (!r.CanWorkOnFramework(framework)) + if (!@ref.CanWorkOnFramework(framework)) { - throw new GracefulException(string.Format(CommonLocalizableStrings.ProjectNotCompatibleWithFramework, r.ProjectRoot.FullPath, GetFrameworkDisplayString(framework))); + Reporter.Error.Write(GetProjectNotCompatibleWithFrameworksDisplayString( + @ref, + new string[] { frameworkString })); + return 1; } } } @@ -101,7 +109,7 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference if (numberOfAddedReferences != 0) { - msbuildProj.ProjectRoot.Save(); + msbuildProj.ProjectRootElement.Save(); } return 0; @@ -119,9 +127,16 @@ namespace Microsoft.DotNet.Tools.Add.ProjectToProjectReference } } - private static string GetFrameworkDisplayString(NuGetFramework framework) + private static string GetProjectNotCompatibleWithFrameworksDisplayString(MsbuildProject project, IEnumerable frameworksDisplayStrings) { - return framework.GetShortFolderName(); + var sb = new StringBuilder(); + sb.AppendLine(string.Format(CommonLocalizableStrings.ProjectNotCompatibleWithFrameworks, project.ProjectRootElement.FullPath)); + foreach (var tfm in frameworksDisplayStrings) + { + sb.AppendLine($" - {tfm}"); + } + + return sb.ToString(); } } } diff --git a/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs b/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs index ec646b6d2..44f90871a 100644 --- a/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs +++ b/src/dotnet/commands/dotnet-remove/dotnet-remove-p2p/Program.cs @@ -55,7 +55,7 @@ namespace Microsoft.DotNet.Tools.Remove.ProjectToProjectReference if (numberOfRemovedReferences != 0) { - msbuildProj.ProjectRoot.Save(); + msbuildProj.ProjectRootElement.Save(); } return 0; diff --git a/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs b/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs index a22706726..d16ab4f8a 100644 --- a/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs +++ b/test/dotnet-add-p2p.Tests/GivenDotnetAddP2P.cs @@ -18,8 +18,10 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests const string ConditionFrameworkNet451 = "== 'net451'"; const string FrameworkNetCoreApp10Arg = "-f netcoreapp1.0"; const string ConditionFrameworkNetCoreApp10 = "== 'netcoreapp1.0'"; + const string ProjectNotCompatibleErrorMessageRegEx = "Project `[^`]*` cannot be added due to incompatible targeted frameworks between the two projects\\. Please review the project you are trying to add and verify that is compatible with the following targets\\:"; + const string ProjectDoesNotTargetFrameworkErrorMessageRegEx = "Project `[^`]*` does not target framework `[^`]*`."; static readonly string[] DefaultFrameworks = new string[] { "netcoreapp1.0", "net451" }; - + private TestSetup Setup([System.Runtime.CompilerServices.CallerMemberName] string callingMethod = nameof(Setup), string identifier = "") { return new TestSetup( @@ -644,7 +646,7 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests } [Fact] - public void ItCanAddRefWithCondOnCompatibleFramework() + public void ItCanAddReferenceWithConditionOnCompatibleFramework() { var setup = Setup(); var lib = new ProjDir(setup.LibDir); @@ -695,7 +697,8 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests .WithProject(lib.CsProjPath) .Execute($"-f {framework} \"{net45lib.CsProjPath}\""); cmd.Should().Fail(); - cmd.StdErr.Should().Contain("does not target"); + cmd.StdErr.Should().MatchRegex(ProjectDoesNotTargetFrameworkErrorMessageRegEx); + cmd.StdErr.Should().Contain($"`{framework}`"); lib.CsProjContent().Should().BeEquivalentTo(csProjContent); } @@ -713,7 +716,8 @@ namespace Microsoft.DotNet.Cli.Add.P2P.Tests .WithProject(net45lib.CsProjPath) .Execute($"{frameworkArg} \"{lib.CsProjPath}\""); cmd.Should().Fail(); - cmd.StdErr.Should().Contain("is not compatible with"); + cmd.StdErr.Should().MatchRegex(ProjectNotCompatibleErrorMessageRegEx); + cmd.StdErr.Should().MatchRegex(" - net45(\n|\r)"); net45lib.CsProjContent().Should().BeEquivalentTo(csProjContent); } }