apply review feedback
This commit is contained in:
parent
9a8d158e09
commit
524fe3cb6b
8 changed files with 126 additions and 91 deletions
|
@ -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}";
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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<NuGetFramework> _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<string> 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<ProjectItemElement> GetProjectToProjectReferences()
|
||||
{
|
||||
return ProjectRoot.GetAllItemsWithElementType(ProjectItemElementType);
|
||||
return ProjectRootElement.GetAllItemsWithElementType(ProjectItemElementType);
|
||||
}
|
||||
|
||||
public void ConvertPathsToRelative(ref List<string> 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<string>();
|
||||
|
||||
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)
|
||||
{
|
||||
|
|
36
src/dotnet/ProjectExtensions.cs
Normal file
36
src/dotnet/ProjectExtensions.cs
Normal file
|
@ -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<NuGetFramework> GetTargetFrameworks(this Project project)
|
||||
{
|
||||
var targetFramewoksStrings = project
|
||||
.GetPropertyCommaSeparatedValues("TargetFramework")
|
||||
.Union(project.GetPropertyCommaSeparatedValues("TargetFrameworks"))
|
||||
.Select((value) => value.ToLower());
|
||||
|
||||
var uniqueTargetFrameworkStrings = new HashSet<string>(targetFramewoksStrings);
|
||||
|
||||
return uniqueTargetFrameworkStrings
|
||||
.Select((frameworkString) => NuGetFramework.Parse(frameworkString));
|
||||
}
|
||||
|
||||
public static IEnumerable<string> GetPropertyCommaSeparatedValues(this Project project, string propertyName)
|
||||
{
|
||||
return project.GetPropertyValue(propertyName)
|
||||
.Split(';')
|
||||
.Select((value) => value.Trim())
|
||||
.Where((value) => !string.IsNullOrEmpty(value));
|
||||
}
|
||||
}
|
||||
}
|
|
@ -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<MsbuildProject> refs = references.Select((r) => MsbuildProject.FromFile(collection, r));
|
||||
IEnumerable<MsbuildProject> 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<string> 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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -55,7 +55,7 @@ namespace Microsoft.DotNet.Tools.Remove.ProjectToProjectReference
|
|||
|
||||
if (numberOfRemovedReferences != 0)
|
||||
{
|
||||
msbuildProj.ProjectRoot.Save();
|
||||
msbuildProj.ProjectRootElement.Save();
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
|
|
@ -18,6 +18,8 @@ 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 = "")
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue