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); } }