Skip to content

Commit 3f25a27

Browse files
xtqqczzejkotas
andauthored
Refactor known folder GUIDs to use static properties (#118165)
* Refactor known folder GUIDs to use static ReadOnlySpan<byte> properties instead of constant strings * Refactor GetFolderPath methods to eliminate redundant checks Co-authored-by: Jan Kotas <[email protected]>
1 parent 1ae288b commit 3f25a27

File tree

6 files changed

+182
-65
lines changed

6 files changed

+182
-65
lines changed

src/libraries/Common/src/Interop/Windows/Shell32/Interop.SHGetKnownFolderPath.cs

Lines changed: 150 additions & 50 deletions
Large diffs are not rendered by default.

src/libraries/System.Private.CoreLib/src/System/Environment.Android.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ public static partial class Environment
1515

1616
private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOption _ /*option*/)
1717
{
18+
// No need to validate if 'folder' is defined; GetSpecialFolder handles this check.
19+
1820
if (s_specialFolders == null)
1921
{
2022
Interlocked.CompareExchange(ref s_specialFolders, new Dictionary<SpecialFolder, string>(), null);
@@ -92,7 +94,9 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
9294
return "/usr/share";
9395

9496
default:
95-
return string.Empty;
97+
if (!Enum.IsDefined(folder))
98+
throw new ArgumentOutOfRangeException(nameof(folder), folder, SR.Format(SR.Arg_EnumIllegalVal, folder));
99+
return null;
96100
}
97101
}
98102
}

src/libraries/System.Private.CoreLib/src/System/Environment.GetFolderPathCore.Unix.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ public static partial class Environment
1919
{
2020
private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOption option)
2121
{
22-
// Get the path for the SpecialFolder
23-
string path = GetFolderPathCoreWithoutValidation(folder) ?? string.Empty;
24-
Debug.Assert(path != null);
22+
// No need to validate if 'folder' is defined; GetSpecialFolder handles this check.
23+
24+
string path = GetSpecialFolder(folder) ?? string.Empty;
2525

2626
// If we didn't get one, or if we got one but we're not supposed to verify it,
2727
// or if we're supposed to verify it and it passes verification, return the path.
@@ -46,7 +46,7 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
4646
return path;
4747
}
4848

49-
private static string? GetFolderPathCoreWithoutValidation(SpecialFolder folder)
49+
private static string? GetSpecialFolder(SpecialFolder folder)
5050
{
5151
// First handle any paths that involve only static paths, avoiding the overheads of getting user-local paths.
5252
// https://www.freedesktop.org/software/systemd/man/file-hierarchy.html
@@ -143,8 +143,11 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
143143
#endif
144144
}
145145

146+
if (!Enum.IsDefined(folder))
147+
throw new ArgumentOutOfRangeException(nameof(folder), folder, SR.Format(SR.Arg_EnumIllegalVal, folder));
148+
146149
// No known path for the SpecialFolder
147-
return string.Empty;
150+
return null;
148151
}
149152

150153
private static string GetXdgConfig(string home)

src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,16 +216,14 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
216216
//
217217
// The only SpecialFolderOption defines we have are equivalent to KnownFolderFlags.
218218

219-
string folderGuid;
219+
ReadOnlySpan<byte> folderGuid;
220220
string? fallbackEnv = null;
221221
switch (folder)
222222
{
223223
// Special-cased values to not use SHGetFolderPath when we have a more direct option available.
224224
case SpecialFolder.System:
225225
// This assumes the system directory always exists and thus we don't need to do anything special for any SpecialFolderOption.
226226
return SystemDirectory;
227-
default:
228-
return string.Empty;
229227

230228
// Map the SpecialFolder to the appropriate Guid
231229
case SpecialFolder.ApplicationData:
@@ -374,6 +372,9 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
374372
folderGuid = Interop.Shell32.KnownFolders.Windows;
375373
fallbackEnv = "windir";
376374
break;
375+
default:
376+
Debug.Assert(!Enum.IsDefined(folder), $"Unexpected SpecialFolder value: {folder}. Please ensure all SpecialFolder enum values are handled in the switch statement.");
377+
throw new ArgumentOutOfRangeException(nameof(folder), folder, SR.Format(SR.Arg_EnumIllegalVal, folder));
377378
}
378379

379380
Guid folderId = new Guid(folderGuid);

src/libraries/System.Private.CoreLib/src/System/Environment.cs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,20 @@ public static string ExpandEnvironmentVariables(string name)
148148
return ExpandEnvironmentVariablesCore(name);
149149
}
150150

151-
public static string GetFolderPath(SpecialFolder folder) => GetFolderPath(folder, SpecialFolderOption.None);
151+
public static string GetFolderPath(SpecialFolder folder) => GetFolderPathCore(folder, SpecialFolderOption.None);
152152

153153
public static string GetFolderPath(SpecialFolder folder, SpecialFolderOption option)
154154
{
155-
if (!Enum.IsDefined(folder))
156-
throw new ArgumentOutOfRangeException(nameof(folder), folder, SR.Format(SR.Arg_EnumIllegalVal, folder));
155+
// No need to validate if 'folder' is defined; GetFolderPathCore handles this check.
157156

158-
if (option != SpecialFolderOption.None && !Enum.IsDefined(option))
159-
throw new ArgumentOutOfRangeException(nameof(option), option, SR.Format(SR.Arg_EnumIllegalVal, option));
157+
if (option is not SpecialFolderOption.None and not SpecialFolderOption.Create and not SpecialFolderOption.DoNotVerify)
158+
{
159+
// Use a throw helper so that if 'option' is a constant,
160+
// the JIT can inline this method and remove the validation check entirely.
161+
Throw(option);
162+
static void Throw(SpecialFolderOption option) =>
163+
throw new ArgumentOutOfRangeException(nameof(option), option, SR.Format(SR.Arg_EnumIllegalVal, option));
164+
}
160165

161166
return GetFolderPathCore(folder, option);
162167
}

src/libraries/System.Private.CoreLib/src/System/Environment.iOS.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ public static partial class Environment
2020

2121
private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOption _ /*option*/)
2222
{
23+
// No need to validate if 'folder' is defined; GetSpecialFolder handles this check.
24+
2325
if (s_specialFolders == null)
2426
{
2527
Interlocked.CompareExchange(ref s_specialFolders, new Dictionary<SpecialFolder, string>(), null);
@@ -91,7 +93,9 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
9193
return "/usr/share";
9294

9395
default:
94-
return string.Empty;
96+
if (!Enum.IsDefined(folder))
97+
throw new ArgumentOutOfRangeException(nameof(folder), folder, SR.Format(SR.Arg_EnumIllegalVal, folder));
98+
return null;
9599
}
96100

97101
static string CombineSearchPath(NSSearchPathDirectory searchPath, string subdirectory)

0 commit comments

Comments
 (0)