Skip to content

Commit 7cf9eb5

Browse files
michaelnebelmichaelnebel
committed
C#: Use ArgumentList instead of Arguments for ProcessStartInfo when invoking dotnet in the dependency fetcher.
1 parent 9bddf51 commit 7cf9eb5

10 files changed

Lines changed: 78 additions & 85 deletions

File tree

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ private void Info()
4949
// Allow up to four attempts (with up to three retries) to run `dotnet --info`, to mitigate transient issues
5050
for (int attempt = 0; attempt < 4; attempt++)
5151
{
52-
var exitCode = dotnetCliInvoker.RunCommandExitCode("--info", silent: false);
52+
var exitCode = dotnetCliInvoker.RunCommandExitCode(["--info"], silent: false);
5353
switch (exitCode)
5454
{
5555
case 0:
@@ -63,9 +63,9 @@ private void Info()
6363
}
6464
}
6565

66-
private string GetRestoreArgs(RestoreSettings restoreSettings)
66+
private List<string> GetRestoreArgs(RestoreSettings restoreSettings)
6767
{
68-
var args = $"restore --no-dependencies \"{restoreSettings.File}\" --packages \"{restoreSettings.PackageDirectory}\" /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal";
68+
List<string> args = ["restore", "--no-dependencies", restoreSettings.File, "--packages", restoreSettings.PackageDirectory, "/p:DisableImplicitNuGetFallbackFolder=true", "--verbosity", "normal"];
6969

7070
if (restoreSettings.ForceDotnetRefAssemblyFetching)
7171
{
@@ -77,23 +77,20 @@ private string GetRestoreArgs(RestoreSettings restoreSettings)
7777
Directory.CreateDirectory(path);
7878
}
7979

80-
args += $" /p:TargetFrameworkRootPath=\"{path}\" /p:NetCoreTargetingPackRoot=\"{path}\" /p:AllowMissingPrunePackageData=true";
80+
args.AddRange([$"/p:TargetFrameworkRootPath={path}", $"/p:NetCoreTargetingPackRoot={path}", "/p:AllowMissingPrunePackageData=true"]);
8181
}
8282

8383
if (restoreSettings.ForceReevaluation)
8484
{
85-
args += " --force";
85+
args.Add("--force");
8686
}
8787

8888
if (restoreSettings.TargetWindows)
8989
{
90-
args += " /p:EnableWindowsTargeting=true";
90+
args.Add("/p:EnableWindowsTargeting=true");
9191
}
9292

93-
if (restoreSettings.NugetSources is not null)
94-
{
95-
args += $" {restoreSettings.NugetSources}";
96-
}
93+
args.AddRange(restoreSettings.NugetSources);
9794

9895
return args;
9996
}
@@ -107,48 +104,48 @@ public RestoreResult Restore(RestoreSettings restoreSettings)
107104

108105
public bool New(string folder)
109106
{
110-
var args = $"new console --no-restore --output \"{folder}\"";
107+
List<string> args = ["new", "console", "--no-restore", "--output", folder];
111108
return dotnetCliInvoker.RunCommand(args);
112109
}
113110

114111
public bool AddPackage(string folder, string package)
115112
{
116-
var args = $"add \"{folder}\" package \"{package}\" --no-restore";
113+
List<string> args = ["add", folder, "package", package, "--no-restore"];
117114
return dotnetCliInvoker.RunCommand(args);
118115
}
119116

120-
public IList<string> GetListedRuntimes() => GetResultList("--list-runtimes");
117+
public IList<string> GetListedRuntimes() => GetResultList(["--list-runtimes"]);
121118

122-
public IList<string> GetListedSdks() => GetResultList("--list-sdks");
119+
public IList<string> GetListedSdks() => GetResultList(["--list-sdks"]);
123120

124-
private IList<string> GetResultList(string args, string? workingDirectory = null, bool silent = true)
121+
private IList<string> GetResultList(List<string> args, string? workingDirectory = null, bool silent = true)
125122
{
126123
if (dotnetCliInvoker.RunCommand(args, workingDirectory, out var results, silent))
127124
{
128125
return results;
129126
}
130-
logger.LogWarning($"Running 'dotnet {args}' failed.");
131-
return [];
127+
logger.LogWarning($"Running 'dotnet {string.Join(" ", args)}' failed.");
128+
return new List<string>();
132129
}
133130

134-
public bool Exec(string execArgs)
131+
public bool Exec(List<string> execArgs)
135132
{
136-
var args = $"exec {execArgs}";
133+
List<string> args = ["exec", .. execArgs];
137134
return dotnetCliInvoker.RunCommand(args);
138135
}
139136

140-
private const string nugetListSourceCommand = "nuget list source --format Short";
137+
private static readonly IReadOnlyList<string> nugetListSourceCommandArgs = ["nuget", "list", "source", "--format", "Short"];
141138

142139
public IList<string> GetNugetFeeds(string nugetConfig)
143140
{
144141
logger.LogInfo($"Getting NuGet feeds from '{nugetConfig}'...");
145-
return GetResultList($"{nugetListSourceCommand} --configfile \"{nugetConfig}\"");
142+
return GetResultList([.. nugetListSourceCommandArgs, "--configfile", nugetConfig]);
146143
}
147144

148145
public IList<string> GetNugetFeedsFromFolder(string folderPath)
149146
{
150147
logger.LogInfo($"Getting NuGet feeds in folder '{folderPath}'...");
151-
return GetResultList(nugetListSourceCommand, folderPath);
148+
return GetResultList(nugetListSourceCommandArgs.ToList(), folderPath);
152149
}
153150

154151
// The version number should be kept in sync with the version .NET version used for building the application.

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetCliInvoker.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Collections.ObjectModel;
43
using System.Diagnostics;
54
using Semmle.Util;
65
using Semmle.Util.Logging;
@@ -25,14 +24,15 @@ public DotNetCliInvoker(ILogger logger, string exec, DependabotProxy? dependabot
2524
logger.LogInfo($"Using .NET CLI executable: '{Exec}'");
2625
}
2726

28-
private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirectory)
27+
private ProcessStartInfo MakeDotnetStartInfo(List<string> args, string? workingDirectory)
2928
{
3029
var startInfo = new ProcessStartInfo(Exec, args)
3130
{
3231
UseShellExecute = false,
3332
RedirectStandardOutput = true,
3433
RedirectStandardError = true
3534
};
35+
3636
if (!string.IsNullOrWhiteSpace(workingDirectory))
3737
{
3838
startInfo.WorkingDirectory = workingDirectory;
@@ -57,39 +57,39 @@ private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirecto
5757
return startInfo;
5858
}
5959

60-
private int RunCommandExitCodeAux(string args, string? workingDirectory, out IList<string> output, out string dirLog, bool silent)
60+
private int RunCommandExitCodeAux(List<string> args, string? workingDirectory, out IList<string> output, out string dirLog, bool silent)
6161
{
6262
dirLog = string.IsNullOrWhiteSpace(workingDirectory) ? "" : $" in {workingDirectory}";
6363
var pi = MakeDotnetStartInfo(args, workingDirectory);
6464
var threadId = Environment.CurrentManagedThreadId;
6565
void onOut(string s) => logger.Log(silent ? Severity.Debug : Severity.Info, s, threadId);
6666
void onError(string s) => logger.LogError(s, threadId);
67-
logger.LogInfo($"Running '{Exec} {args}'{dirLog}");
67+
logger.LogInfo($"Running '{Exec} {string.Join(" ", args)}'{dirLog}");
6868
var exitCode = pi.ReadOutput(out output, onOut, onError);
6969
return exitCode;
7070
}
7171

72-
private bool RunCommandAux(string args, string? workingDirectory, out IList<string> output, bool silent)
72+
private bool RunCommandAux(List<string> args, string? workingDirectory, out IList<string> output, bool silent)
7373
{
7474
var exitCode = RunCommandExitCodeAux(args, workingDirectory, out output, out var dirLog, silent);
7575
if (exitCode != 0)
7676
{
77-
logger.LogError($"Command '{Exec} {args}'{dirLog} failed with exit code {exitCode}");
77+
logger.LogError($"Command '{Exec} {string.Join(" ", args)}'{dirLog} failed with exit code {exitCode}");
7878
return false;
7979
}
8080
return true;
8181
}
8282

83-
public bool RunCommand(string args, bool silent = true) =>
83+
public bool RunCommand(List<string> args, bool silent = true) =>
8484
RunCommandAux(args, null, out _, silent);
8585

86-
public int RunCommandExitCode(string args, bool silent = true) =>
86+
public int RunCommandExitCode(List<string> args, bool silent = true) =>
8787
RunCommandExitCodeAux(args, null, out _, out _, silent);
8888

89-
public bool RunCommand(string args, out IList<string> output, bool silent = true) =>
89+
public bool RunCommand(List<string> args, out IList<string> output, bool silent = true) =>
9090
RunCommandAux(args, null, out output, silent);
9191

92-
public bool RunCommand(string args, string? workingDirectory, out IList<string> output, bool silent = true) =>
92+
public bool RunCommand(List<string> args, string? workingDirectory, out IList<string> output, bool silent = true) =>
9393
RunCommandAux(args, workingDirectory, out output, silent);
9494
}
9595
}

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -158,24 +158,18 @@ private IEnumerable<string> GetFeedsFromNugetConfig(string nugetConfigPath) =>
158158
/// </summary>
159159
/// <param name="feeds">The list of feeds to use for the restore command.</param>
160160
/// <param name="sourceArgumentPrefix">The prefix to use for each source argument (e.g., "-s").</param>
161-
/// <returns>The constructed NuGet sources argument for the restore command.</returns>
162-
public string FeedsToRestoreArgument(IEnumerable<string> feeds, string sourceArgumentPrefix)
161+
/// <returns>The list of NuGet sources arguments for the restore command.</returns>
162+
public List<string> FeedsToRestoreArgument(IEnumerable<string> feeds, string sourceArgumentPrefix)
163163
{
164164
// If there are no feeds, we want to override any default feeds that `restore` would use by passing a dummy source argument.
165165
if (!feeds.Any())
166166
{
167-
return $" {sourceArgumentPrefix} \"{emptyPackageDirectory.DirInfo.FullName}\"";
167+
return [sourceArgumentPrefix, emptyPackageDirectory.DirInfo.FullName];
168168
}
169169

170170
// Add package sources. If any are present, they override all sources specified in
171171
// the configuration file(s).
172-
var feedArgs = new StringBuilder();
173-
foreach (var feed in feeds)
174-
{
175-
feedArgs.Append($" {sourceArgumentPrefix} \"{feed}\"");
176-
}
177-
178-
return feedArgs.ToString();
172+
return feeds.SelectMany<string, string>(feed => [sourceArgumentPrefix, feed]).ToList();
179173
}
180174

181175
private IEnumerable<string> FeedsToUseAux(HashSet<string> feedsToConsider)
@@ -212,8 +206,8 @@ public IEnumerable<string> FeedsToUse(string path)
212206
/// Constructs the NuGet sources argument for the `dotnet restore` command based on the given feeds.
213207
/// </summary>
214208
/// <param name="feeds">The list of NuGet feeds to use for the restore command.</param>
215-
/// <returns>A string representing the NuGet sources argument for the `dotnet restore` command.</returns>
216-
public string FeedsToDotnetRestoreArgument(IEnumerable<string> feeds)
209+
/// <returns>A list representing the NuGet sources arguments for the `dotnet restore` command.</returns>
210+
public List<string> FeedsToDotnetRestoreArgument(IEnumerable<string> feeds)
217211
{
218212
return FeedsToRestoreArgument(feeds, "-s");
219213
}
@@ -225,12 +219,12 @@ public string FeedsToDotnetRestoreArgument(IEnumerable<string> feeds)
225219
/// </summary>
226220
/// <param name="path">Path to project/solution</param>
227221
/// <returns>A string representing the NuGet sources argument for the `dotnet restore` command.</returns>
228-
public string? MakeDotnetRestoreSourcesArgument(string path)
222+
public List<string> MakeDotnetRestoreSourcesArguments(string path)
229223
{
230224
// Do not construct a set of explicit NuGet sources to use for restore.
231225
if (!CheckNugetFeedResponsiveness && !HasPrivateRegistryFeeds)
232226
{
233-
return null;
227+
return [];
234228
}
235229

236230
var feedsToUse = FeedsToUse(path);

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ public interface IDotNet
1212
bool AddPackage(string folder, string package);
1313
IList<string> GetListedRuntimes();
1414
IList<string> GetListedSdks();
15-
bool Exec(string execArgs);
15+
bool Exec(List<string> execArgs);
1616
IList<string> GetNugetFeeds(string nugetConfig);
1717
IList<string> GetNugetFeedsFromFolder(string folderPath);
1818
}
1919

20-
public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? NugetSources = null, bool ForceReevaluation = false, bool TargetWindows = false);
20+
public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, List<string> NugetSources, bool ForceReevaluation = false, bool TargetWindows = false);
2121

2222
public partial record class RestoreResult(bool Success, IList<string> Output)
2323
{

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNetCliInvoker.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,26 @@ internal interface IDotNetCliInvoker
3030
/// Execute `dotnet <paramref name="args"/>` and return true if the command succeeded, otherwise false.
3131
/// If `silent` is true the output of the command is logged as `debug` otherwise as `info`.
3232
/// </summary>
33-
bool RunCommand(string args, bool silent = true);
33+
bool RunCommand(List<string> args, bool silent = true);
3434

3535
/// <summary>
3636
/// Execute `dotnet <paramref name="args"/>` and return the exit code.
3737
/// If `silent` is true the output of the command is logged as `debug` otherwise as `info`.
3838
/// </summary>
39-
int RunCommandExitCode(string args, bool silent = true);
39+
int RunCommandExitCode(List<string> args, bool silent = true);
4040

4141
/// <summary>
4242
/// Execute `dotnet <paramref name="args"/>` and return true if the command succeeded, otherwise false.
4343
/// The output of the command is returned in `output`.
4444
/// If `silent` is true the output of the command is logged as `debug` otherwise as `info`.
4545
/// </summary>
46-
bool RunCommand(string args, out IList<string> output, bool silent = true);
46+
bool RunCommand(List<string> args, out IList<string> output, bool silent = true);
4747

4848
/// <summary>
4949
/// Execute `dotnet <paramref name="args"/>` in `<paramref name="workingDirectory"/>` and return true if the command succeeded, otherwise false.
5050
/// The output of the command is returned in `output`.
5151
/// If `silent` is true the output of the command is logged as `debug` otherwise as `info`.
5252
/// </summary>
53-
bool RunCommand(string args, string? workingDirectory, out IList<string> output, bool silent = true);
53+
bool RunCommand(List<string> args, string? workingDirectory, out IList<string> output, bool silent = true);
5454
}
5555
}

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ private IEnumerable<string> RestoreSolutions(out DependencyContainer dependencie
216216
var projects = fileProvider.Solutions.SelectMany(solution =>
217217
{
218218
logger.LogInfo($"Restoring solution {solution}...");
219-
var nugetSources = feedManager.MakeDotnetRestoreSourcesArgument(solution);
219+
var nugetSources = feedManager.MakeDotnetRestoreSourcesArguments(solution);
220220
var res = dotnet.Restore(new(solution, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, NugetSources: nugetSources, TargetWindows: isWindows));
221221
if (res.Success)
222222
{
@@ -264,7 +264,7 @@ private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<Dep
264264
foreach (var project in projectGroup)
265265
{
266266
logger.LogInfo($"Restoring project {project}...");
267-
var nugetSources = feedManager.MakeDotnetRestoreSourcesArgument(project);
267+
var nugetSources = feedManager.MakeDotnetRestoreSourcesArguments(project);
268268
var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, NugetSources: nugetSources, TargetWindows: isWindows));
269269
assets.AddDependenciesRange(res.AssetsFilePaths);
270270
lock (sync)
@@ -432,7 +432,7 @@ private static IEnumerable<string> GetRestoredPackageDirectoryNames(DirectoryInf
432432
.Select(d => Path.GetFileName(d).ToLowerInvariant());
433433
}
434434

435-
private bool TryRestorePackageManually(string package, string? nugetSources, PackageReferenceSource packageReferenceSource = PackageReferenceSource.SdkCsProj, bool tryPrereleaseVersion = true)
435+
private bool TryRestorePackageManually(string package, List<string> nugetSources, PackageReferenceSource packageReferenceSource = PackageReferenceSource.SdkCsProj, bool tryPrereleaseVersion = true)
436436
{
437437
logger.LogInfo($"Restoring package {package}...");
438438
using var tempDir = new TemporaryDirectory(
@@ -464,7 +464,7 @@ private bool TryRestorePackageManually(string package, string? nugetSources, Pac
464464
{
465465
logger.LogDebug($"Trying to restore '{package}' without explicitly providing NuGet sources.");
466466
// Restore could not be completed because the listed source is unavailable. Try without an explicit restore source argument.
467-
res = TryRestorePackageManually(package, nugetSources: null, tempDir, tryPrereleaseVersion);
467+
res = TryRestorePackageManually(package, [], tempDir, tryPrereleaseVersion);
468468
if (res.Success)
469469
{
470470
return true;
@@ -475,16 +475,16 @@ private bool TryRestorePackageManually(string package, string? nugetSources, Pac
475475
return false;
476476
}
477477

478-
private RestoreResult TryRestorePackageManually(string package, string? nugetSources, TemporaryDirectory tempDir, bool tryPrereleaseVersion)
478+
private RestoreResult TryRestorePackageManually(string package, List<string> nugetSources, TemporaryDirectory tempDir, bool tryPrereleaseVersion)
479479
{
480-
var res = dotnet.Restore(new(tempDir.DirInfo.FullName, missingPackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: false, NugetSources: nugetSources, ForceReevaluation: true));
480+
var res = dotnet.Restore(new(tempDir.DirInfo.FullName, missingPackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: false, nugetSources, ForceReevaluation: true));
481481

482482
if (!res.Success && tryPrereleaseVersion && res.HasNugetNoStablePackageVersionError)
483483
{
484484
logger.LogDebug($"Failed to restore nuget package {package} because no stable version was found.");
485485
TryChangePackageVersion(tempDir.DirInfo, "*-*");
486486

487-
res = dotnet.Restore(new(tempDir.DirInfo.FullName, missingPackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: false, NugetSources: nugetSources, ForceReevaluation: true));
487+
res = dotnet.Restore(new(tempDir.DirInfo.FullName, missingPackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: false, nugetSources, ForceReevaluation: true));
488488
if (!res.Success)
489489
{
490490
TryChangePackageVersion(tempDir.DirInfo, "*");

0 commit comments

Comments
 (0)