sln-add: Refactor logic for autogenerating solution folders #48726
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -15,12 +15,11 @@ namespace Microsoft.DotNet.Cli.Commands.Solution.Add; | ||||||||
|
|
||||||||
| internal class SolutionAddCommand : CommandBase | ||||||||
| { | ||||||||
| private readonly string _fileOrDirectory; | ||||||||
| private readonly bool _inRoot; | ||||||||
| private readonly IReadOnlyCollection<string> _projects; | ||||||||
| private readonly string? _solutionFolderPath; | ||||||||
| private string _solutionFileFullPath = string.Empty; | ||||||||
|
|
||||||||
| private static string GetSolutionFolderPathWithForwardSlashes(string path) | ||||||||
| { | ||||||||
| @@ -29,13 +28,21 @@ private static string GetSolutionFolderPathWithForwardSlashes(string path) | ||||||||
| return "/" + string.Join("/", PathUtility.GetPathWithDirectorySeparator(path).Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries)) + "/"; | ||||||||
| } | ||||||||
|
|
||||||||
| private static bool IsSolutionFolderPathInDirectoryScope(string relativePath) | ||||||||
| { | ||||||||
| return !string.IsNullOrWhiteSpace(relativePath) | ||||||||
| && !Path.IsPathRooted(relativePath) // This means path is in a different volume | ||||||||
| && !relativePath.StartsWith(".."); // This means path is outside the solution directory | ||||||||
| } | ||||||||
|
|
||||||||
| public SolutionAddCommand(ParseResult parseResult) : base(parseResult) | ||||||||
| { | ||||||||
| _fileOrDirectory = parseResult.GetValue(SolutionCommandParser.SlnArgument); | ||||||||
| _projects = (IReadOnlyCollection<string>)(parseResult.GetValue(SolutionAddCommandParser.ProjectPathArgument) ?? []); | ||||||||
| _inRoot = parseResult.GetValue(SolutionAddCommandParser.InRootOption); | ||||||||
| _solutionFolderPath = parseResult.GetValue(SolutionAddCommandParser.SolutionFolderOption); | ||||||||
| SolutionArgumentValidator.ParseAndValidateArguments(_fileOrDirectory, _projects, SolutionArgumentValidator.CommandType.Add, _inRoot, _solutionFolderPath); | ||||||||
| _solutionFileFullPath = SlnFileFactory.GetSolutionFileFullPath(_fileOrDirectory); | ||||||||
| } | ||||||||
|
|
||||||||
| public override int Execute() | ||||||||
| @@ -44,115 +51,135 @@ public override int Execute() | ||||||||
| { | ||||||||
| throw new GracefulException(CliStrings.SpecifyAtLeastOneProjectToAdd); | ||||||||
| } | ||||||||
|
|
||||||||
| // Get project paths from the command line arguments | ||||||||
| PathUtility.EnsureAllPathsExist(_projects, CliStrings.CouldNotFindProjectOrDirectory, true); | ||||||||
|
|
||||||||
| IEnumerable<string> fullProjectPaths = _projects.Select(project => | ||||||||
| { | ||||||||
| var fullPath = Path.GetFullPath(project); | ||||||||
| return Directory.Exists(fullPath) ? MsbuildProject.GetProjectFileFromDirectory(fullPath).FullName : fullPath; | ||||||||
| }); | ||||||||
|
|
||||||||
| // Add projects to the solution | ||||||||
| AddProjectsToSolutionAsync(fullProjectPaths, CancellationToken.None).GetAwaiter().GetResult(); | ||||||||
| return 0; | ||||||||
| } | ||||||||
|
|
||||||||
| private SolutionFolderModel? GenerateIntermediateSolutionFoldersForProjectPath(SolutionModel solution, string relativeProjectPath) | ||||||||
| { | ||||||||
| if (_inRoot) | ||||||||
| { | ||||||||
| return null; | ||||||||
| } | ||||||||
|
|
||||||||
| string relativeSolutionFolderPath = string.Empty; | ||||||||
|
|
||||||||
| if (string.IsNullOrEmpty(_solutionFolderPath)) | ||||||||
| { | ||||||||
| // Generate the solution folder path based on the project path | ||||||||
| relativeSolutionFolderPath = Path.GetDirectoryName(relativeProjectPath); | ||||||||
|
|
||||||||
| // If the project is in a folder with the same name as the project, we need to go up one level | ||||||||
| if (relativeSolutionFolderPath.Split(Path.DirectorySeparatorChar).LastOrDefault() == Path.GetFileNameWithoutExtension(relativeProjectPath)) | ||||||||
| { | ||||||||
| relativeSolutionFolderPath = Path.Combine([.. relativeSolutionFolderPath.Split(Path.DirectorySeparatorChar).SkipLast(1)]); | ||||||||
|
There was a problem hiding this comment. [nitpick] Using a collection expression directly within Path.Combine may reduce code clarity. Consider extracting the computed array into a separate variable before passing it to Path.Combine.
Suggested change
Copilot uses AI. Check for mistakes. |
||||||||
| } | ||||||||
|
|
||||||||
| // If the generated path is outside the solution directory, we need to set it to empty | ||||||||
| if (!IsSolutionFolderPathInDirectoryScope(relativeSolutionFolderPath)) | ||||||||
| { | ||||||||
| relativeSolutionFolderPath = string.Empty; | ||||||||
| } | ||||||||
| } | ||||||||
| else | ||||||||
| { | ||||||||
| // Use the provided solution folder path | ||||||||
| relativeSolutionFolderPath = _solutionFolderPath; | ||||||||
| } | ||||||||
|
|
||||||||
| return string.IsNullOrEmpty(relativeSolutionFolderPath) | ||||||||
| ? null | ||||||||
| : solution.AddFolder(GetSolutionFolderPathWithForwardSlashes(relativeSolutionFolderPath)); | ||||||||
| } | ||||||||
|
|
||||||||
| private async Task AddProjectsToSolutionAsync(IEnumerable<string> projectPaths, CancellationToken cancellationToken) | ||||||||
| { | ||||||||
| SolutionModel solution = SlnFileFactory.CreateFromFileOrDirectory(_solutionFileFullPath); | ||||||||
| ISolutionSerializer serializer = solution.SerializerExtension.Serializer; | ||||||||
|
|
||||||||
| // set UTF8 BOM encoding for .sln | ||||||||
| if (serializer is ISolutionSerializer<SlnV12SerializerSettings> v12Serializer) | ||||||||
| { | ||||||||
| solution.SerializerExtension = v12Serializer.CreateModelExtension(new() | ||||||||
| { | ||||||||
| Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: true) | ||||||||
| }); | ||||||||
|
|
||||||||
| // Set default configurations and platforms for sln file | ||||||||
| foreach (var platform in SlnFileFactory.DefaultPlatforms) | ||||||||
| { | ||||||||
| solution.AddPlatform(platform); | ||||||||
| } | ||||||||
|
|
||||||||
| foreach (var buildType in SlnFileFactory.DefaultBuildTypes) | ||||||||
| { | ||||||||
| solution.AddBuildType(buildType); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| foreach (var projectPath in projectPaths) | ||||||||
| { | ||||||||
| AddProject(solution, projectPath, serializer); | ||||||||
| } | ||||||||
|
|
||||||||
| await serializer.SaveAsync(_solutionFileFullPath, solution, cancellationToken); | ||||||||
| } | ||||||||
|
|
||||||||
| private void AddProject(SolutionModel solution, string fullProjectPath, ISolutionSerializer serializer = null) | ||||||||
| { | ||||||||
| string solutionRelativeProjectPath = Path.GetRelativePath(Path.GetDirectoryName(_solutionFileFullPath), fullProjectPath); | ||||||||
|
There was a problem hiding this comment. [nitpick] The variable name 'solutionRelativeProjectPath' is used in multiple scopes with similar contexts. Renaming one occurrence (or scoping it more tightly) may improve code clarity and reduce potential confusion.
Suggested change
Copilot uses AI. Check for mistakes. |
||||||||
|
|
||||||||
| // Open project instance to see if it is a valid project | ||||||||
| ProjectRootElement projectRootElement; | ||||||||
| try | ||||||||
| { | ||||||||
| projectRootElement = ProjectRootElement.Open(fullProjectPath); | ||||||||
| } | ||||||||
| catch (InvalidProjectFileException ex) | ||||||||
| { | ||||||||
| Reporter.Error.WriteLine(string.Format(CliStrings.InvalidProjectWithExceptionMessage, fullProjectPath, ex.Message)); | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| ProjectInstance projectInstance = new ProjectInstance(projectRootElement); | ||||||||
|
|
||||||||
| string projectTypeGuid = solution.ProjectTypes.FirstOrDefault(t => t.Extension == Path.GetExtension(fullProjectPath))?.ProjectTypeId.ToString() | ||||||||
| ?? projectRootElement.GetProjectTypeGuid() ?? projectInstance.GetDefaultProjectTypeGuid(); | ||||||||
|
|
||||||||
| // Generate the solution folder path based on the project path | ||||||||
| SolutionFolderModel? solutionFolder = GenerateIntermediateSolutionFoldersForProjectPath(solution, solutionRelativeProjectPath); | ||||||||
|
|
||||||||
| SolutionProjectModel project; | ||||||||
|
|
||||||||
| try | ||||||||
| { | ||||||||
| project = solution.AddProject(solutionRelativeProjectPath, projectTypeGuid, solutionFolder); | ||||||||
| } | ||||||||
| catch (SolutionArgumentException ex) when (ex.Type == SolutionErrorType.InvalidProjectTypeReference) | ||||||||
|
There was a problem hiding this comment. == is seldom best except for primitives |
||||||||
| { | ||||||||
| Reporter.Error.WriteLine(CliStrings.UnsupportedProjectType, fullProjectPath); | ||||||||
| return; | ||||||||
| } | ||||||||
| catch (SolutionArgumentException ex) when (ex.Type == SolutionErrorType.DuplicateProjectName || solution.FindProject(solutionRelativeProjectPath) is not null) | ||||||||
| { | ||||||||
| Reporter.Output.WriteLine(CliStrings.SolutionAlreadyContainsProject, _solutionFileFullPath, solutionRelativeProjectPath); | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| // Add settings based on existing project instance | ||||||||
| string projectInstanceId = projectInstance.GetProjectId(); | ||||||||
|
|
||||||||
| if (!string.IsNullOrEmpty(projectInstanceId) && serializer is ISolutionSerializer<SlnV12SerializerSettings>) | ||||||||
| { | ||||||||
| project.Id = new Guid(projectInstanceId); | ||||||||
| @@ -164,7 +191,7 @@ private static void AddProject(SolutionModel solution, string solutionRelativePr | ||||||||
| foreach (var solutionPlatform in solution.Platforms) | ||||||||
| { | ||||||||
| var projectPlatform = projectInstancePlatforms.FirstOrDefault( | ||||||||
| platform => platform.Replace(" ", string.Empty) == solutionPlatform.Replace(" ", string.Empty), projectInstancePlatforms.FirstOrDefault()); | ||||||||
| project.AddProjectConfigurationRule(new ConfigurationRule(BuildDimension.Platform, "*", solutionPlatform, projectPlatform)); | ||||||||
| } | ||||||||
|
|
||||||||
| @@ -174,6 +201,7 @@ private static void AddProject(SolutionModel solution, string solutionRelativePr | ||||||||
| buildType => buildType.Replace(" ", string.Empty) == solutionBuildType.Replace(" ", string.Empty), projectInstanceBuildTypes.FirstOrDefault()); | ||||||||
| project.AddProjectConfigurationRule(new ConfigurationRule(BuildDimension.BuildType, solutionBuildType, "*", projectBuildType)); | ||||||||
| } | ||||||||
|
|
||||||||
| Reporter.Output.WriteLine(CliStrings.ProjectAddedToTheSolution, solutionRelativeProjectPath); | ||||||||
| } | ||||||||
| } | ||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Text.Json; | ||
| using Microsoft.DotNet.Cli.Utils; | ||
| using Microsoft.VisualStudio.SolutionPersistence; | ||
| @@ -11,6 +12,9 @@ namespace Microsoft.DotNet.Cli; | ||
|
|
||
| public static class SlnFileFactory | ||
| { | ||
| public static readonly string[] DefaultPlatforms = new[] { "Any CPU", "x64", "x86" }; | ||
joeloff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public static readonly string[] DefaultBuildTypes = new[] { "Debug", "Release" }; | ||
|
|
||
| public static string GetSolutionFileFullPath(string slnFileOrDirectory, bool includeSolutionFilterFiles = false, bool includeSolutionXmlFiles = true) | ||
| { | ||
| // Throw error if slnFileOrDirectory is an invalid path | ||
| @@ -63,11 +67,17 @@ public static SolutionModel CreateFromFileOrDirectory(string fileOrDirectory, bo | ||
| { | ||
| return CreateFromFilteredSolutionFile(solutionPath); | ||
| } | ||
| try | ||
| { | ||
| ISolutionSerializer serializer = SolutionSerializers.GetSerializerByMoniker(solutionPath)!; | ||
| return serializer.OpenAsync(solutionPath, CancellationToken.None).Result; | ||
| } | ||
| catch (Exception ex) | ||
|
There was a problem hiding this comment. The catch block in CreateFromFileOrDirectory rethrows a new GracefulException using only ex.Message, which can lose the original stack trace. Consider including the exception object itself (e.g. passing 'ex' instead of 'ex.Message') to aid in debugging. Copilot uses AI. Check for mistakes. |
||
| { | ||
| throw new GracefulException( | ||
| CliStrings.InvalidSolutionFormatString, | ||
| solutionPath, ex.Message); | ||
| } | ||
| } | ||
|
|
||
| public static SolutionModel CreateFromFilteredSolutionFile(string filteredSolutionPath) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.