-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Refactor FileUtilities.cs and add methods for absolute paths. #13079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d6f86d4 to
6362a96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors file utility functions by moving several methods from Shared/FileUtilities.cs to a new Framework/FileUtilities.cs file in the FrameworkFileUtilities class. This enables support for AbsolutePath operations as referenced in PRs #12914 and #12868, since shared files cannot reference Framework types.
Changes:
- Created new
FrameworkFileUtilitiesclass insrc/Framework/FileUtilities.cswith methods for slash handling, path fixing, and trailing slash operations - Added new overloads for
AbsolutePathoperations includingEnsureTrailingSlash,EnsureNoTrailingSlash,NormalizePath,RemoveRelativeSegments, andFixFilePath - Updated 64 files across the codebase to call
FrameworkFileUtilitiesmethods instead ofFileUtilitiesmethods for the moved functions - Added
using Microsoft.Build.Framework;imports where needed and removed conditional compilation guards that are no longer necessary
Reviewed changes
Copilot reviewed 63 out of 63 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/FileUtilities.cs | New file containing FrameworkFileUtilities class with moved utility methods and new AbsolutePath overloads |
| src/Framework/ErrorUtilities.cs | Changed FrameworkErrorUtilities from non-static to static class |
| src/Shared/FileUtilities.cs | Removed methods moved to FrameworkFileUtilities; updated remaining methods to call FrameworkFileUtilities |
| src/Shared/FileMatcher.cs | Updated to use FrameworkFileUtilities for directory separator characters |
| src/Shared/Modifiers.cs | Updated to use FrameworkFileUtilities methods for slash operations |
| src/Shared/FrameworkLocationHelper.cs | Added Framework namespace import and updated method calls |
| src/Shared/TempFileUtilities.cs | Added Framework namespace import and updated EnsureTrailingSlash call |
| src/MSBuildTaskHost/MSBuildTaskHost.csproj | Included both Framework and Shared FileUtilities files with appropriate Link names |
| src/MSBuild/XMake.cs | Updated FixFilePath calls to use FrameworkFileUtilities |
| src/MSBuild/CommandLine/CommandLineParser.cs | Updated method calls to FrameworkFileUtilities |
| src/Utilities/*.cs (7 files) | Updated method calls from FileUtilities to FrameworkFileUtilities |
| src/Tasks/*.cs (17 files) | Updated method calls from FileUtilities to FrameworkFileUtilities with Framework namespace imports |
| src/Build/*.cs (13 files) | Updated method calls to FrameworkFileUtilities; removed conditional #if NET guard in Toolset.cs |
| Test files (14 files) | Updated test assertions and helpers to use FrameworkFileUtilities |
Comments suppressed due to low confidence (2)
src/Tasks/GetSDKReferenceFiles.cs:649
- This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
src/Shared/FileUtilities.cs:797 - These 'if' statements can be combined.
if (NativeMethodsShared.IsWindows && !FrameworkFileUtilities.EndsWithSlash(fullPath))
{
if (FileUtilitiesRegex.IsDrivePattern(fileSpec) ||
FileUtilitiesRegex.IsUncPattern(fullPath))
{
// append trailing slash if Path.GetFullPath failed to (this happens with drive-specs and UNC shares)
fullPath += Path.DirectorySeparatorChar;
}
}
JanProvaznik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annoying that there are now both utilities but it's needed and a positive step in the broad refactor shared utilities goal
…#13079) ### Context PRs dotnet#12914 and dotnet#12868 showed that we need to be able to fix and normalize `AbsolutePath`. Corresponding methods for strings live in `FileUtilities.cs` that is a Shared file and cannot reference Framework classes like `AbsolutePath`. ### Changes Made - Moved some related functions to FrameworkFileUtilities class. - Added the needed functions for `AbsolutePath` ### Testing unit tests
Context
PRs #12914 and #12868 showed that we need to be able to fix and normalize
AbsolutePath. Corresponding methods for strings live inFileUtilities.csthat is a Shared file and cannot reference Framework classes likeAbsolutePath.Changes Made
AbsolutePathTesting
unit tests