Skip to content

C#: Fix the cs/path-combine code quality issues in the extractor. #21996

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.

Sign up for GitHub

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

Draft
michaelnebel wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

C#: Fix the cs/path-combine code quality issues in the extractor. #21996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3364c20
C#: Fix the cs/path-combine code quality issues in the extractor.
michaelnebel Jun 17, 2026
b2addbf
C#: Rename PathCombine to PathJoin.
michaelnebel Jun 18, 2026
27c1900
C#: Handle the places where we could risk that Path.Combine would hav…
michaelnebel Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ IEnumerable<string> IBuildActions.EnumerateFiles(string dir)
if (!EnumerateFiles.TryGetValue(dir, out var str))
throw new ArgumentException("Missing EnumerateFiles " + dir);

return str.Split("\n").Select(p => PathCombine(dir, p));
return str.Split("\n").Select(p => PathJoin(dir, p));
}

public IDictionary<string, string> EnumerateDirectories { get; } = new Dictionary<string, string>();
Expand All @@ -147,7 +147,7 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)

return string.IsNullOrEmpty(str)
? Enumerable.Empty<string>()
: str.Split("\n").Select(p => PathCombine(dir, p));
: str.Split("\n").Select(p => PathJoin(dir, p));
}

public bool IsWindows { get; set; }
Expand All @@ -170,7 +170,7 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)

bool IBuildActions.IsMonoInstalled() => IsMonoInstalled;

public string PathCombine(params string[] parts)
public string PathJoin(params string[] parts)
{
return string.Join(IsWindows ? '\\' : '/', parts.Where(p => !string.IsNullOrWhiteSpace(p)));
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild.CSharp/DotNetRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public static BuildScript WithDotNet(IAutobuilder<AutobuildOptionsShared> builde
=> WithDotNet(builder, ensureDotNetAvailable: false, (_, env) => f(env));

private static string DotNetCommand(IBuildActions actions, string? dotNetPath) =>
dotNetPath is not null ? actions.PathCombine(dotNetPath, "dotnet") : "dotnet";
dotNetPath is not null ? actions.PathJoin(dotNetPath, "dotnet") : "dotnet";

private static CommandBuilder GetCleanCommand(IBuildActions actions, string? dotNetPath, IDictionary<string, string>? environment)
{
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)

bool IBuildActions.IsMonoInstalled() => IsMonoInstalled;

string IBuildActions.PathCombine(params string[] parts)
string IBuildActions.PathJoin(params string[] parts)
{
return string.Join(IsWindows ? '\\' : '/', parts.Where(p => !string.IsNullOrWhiteSpace(p)));
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public abstract class Autobuilder<TAutobuildOptions> : IDisposable, IAutobuilder
/// </summary>
/// <param name="path">The relative path.</param>
/// <returns>True iff the path was found.</returns>
public bool HasRelativePath(string path) => HasPath(Actions.PathCombine(RootDirectory, path));
public bool HasRelativePath(string path) => HasPath(Actions.PathJoin(RootDirectory, path));

/// <summary>
/// List of project/solution files to build.
Expand Down
18 changes: 9 additions & 9 deletions csharp/autobuilder/Semmle.Autobuild.Shared/BuildTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static IEnumerable<VcVarsBatFile> GetCandidateVcVarsFiles(IBuildActions a
yield break;

// Attempt to use vswhere to find installations of Visual Studio
var vswhere = actions.PathCombine(programFilesx86, "Microsoft Visual Studio", "Installer", "vswhere.exe");
var vswhere = actions.PathJoin(programFilesx86, "Microsoft Visual Studio", "Installer", "vswhere.exe");

if (actions.FileExists(vswhere))
{
Expand All @@ -51,14 +51,14 @@ public static IEnumerable<VcVarsBatFile> GetCandidateVcVarsFiles(IBuildActions a
if (majorVersion < 15)
{
// Visual Studio 2015 and below
yield return new VcVarsBatFile(actions.PathCombine(vsInstallation.InstallationPath, @"VC\vcvarsall.bat"), majorVersion);
yield return new VcVarsBatFile(actions.PathJoin(vsInstallation.InstallationPath, @"VC\vcvarsall.bat"), majorVersion);
}
else
{
// Visual Studio 2017 and above
yield return new VcVarsBatFile(actions.PathCombine(vsInstallation.InstallationPath, @"VC\Auxiliary\Build\vcvars32.bat"), majorVersion);
yield return new VcVarsBatFile(actions.PathCombine(vsInstallation.InstallationPath, @"VC\Auxiliary\Build\vcvars64.bat"), majorVersion);
yield return new VcVarsBatFile(actions.PathCombine(vsInstallation.InstallationPath, @"Common7\Tools\VsDevCmd.bat"), majorVersion);
yield return new VcVarsBatFile(actions.PathJoin(vsInstallation.InstallationPath, @"VC\Auxiliary\Build\vcvars32.bat"), majorVersion);
yield return new VcVarsBatFile(actions.PathJoin(vsInstallation.InstallationPath, @"VC\Auxiliary\Build\vcvars64.bat"), majorVersion);
yield return new VcVarsBatFile(actions.PathJoin(vsInstallation.InstallationPath, @"Common7\Tools\VsDevCmd.bat"), majorVersion);
}
}
// else: Skip installation without a version
Expand All @@ -68,10 +68,10 @@ public static IEnumerable<VcVarsBatFile> GetCandidateVcVarsFiles(IBuildActions a
}

// vswhere not installed or didn't run correctly - return legacy Visual Studio versions
yield return new VcVarsBatFile(actions.PathCombine(programFilesx86, @"Microsoft Visual Studio 14.0\VC\vcvarsall.bat"), 14);
yield return new VcVarsBatFile(actions.PathCombine(programFilesx86, @"Microsoft Visual Studio 12.0\VC\vcvarsall.bat"), 12);
yield return new VcVarsBatFile(actions.PathCombine(programFilesx86, @"Microsoft Visual Studio 11.0\VC\vcvarsall.bat"), 11);
yield return new VcVarsBatFile(actions.PathCombine(programFilesx86, @"Microsoft Visual Studio 10.0\VC\vcvarsall.bat"), 10);
yield return new VcVarsBatFile(actions.PathJoin(programFilesx86, @"Microsoft Visual Studio 14.0\VC\vcvarsall.bat"), 14);
yield return new VcVarsBatFile(actions.PathJoin(programFilesx86, @"Microsoft Visual Studio 12.0\VC\vcvarsall.bat"), 12);
yield return new VcVarsBatFile(actions.PathJoin(programFilesx86, @"Microsoft Visual Studio 11.0\VC\vcvarsall.bat"), 11);
yield return new VcVarsBatFile(actions.PathJoin(programFilesx86, @"Microsoft Visual Studio 10.0\VC\vcvarsall.bat"), 10);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public BuildScript Analyse(IAutobuilder<AutobuildOptionsShared> builder, bool au
// Use `nuget.exe` from source code repo, if present, otherwise first attempt with global
// `nuget` command, and if that fails, attempt to download `nuget.exe` from nuget.org
var nuget = builder.GetFilename("nuget.exe").Select(t => t.Item1).FirstOrDefault() ?? "nuget";
var nugetDownloadPath = builder.Actions.PathCombine(FileUtils.GetTemporaryWorkingDirectory(builder.Actions.GetEnvironmentVariable, builder.Options.Language.UpperCaseName, out _), ".nuget", "nuget.exe");
var nugetDownloadPath = builder.Actions.PathJoin(FileUtils.GetTemporaryWorkingDirectory(builder.Actions.GetEnvironmentVariable, builder.Options.Language.UpperCaseName, out _), ".nuget", "nuget.exe");
var nugetDownloaded = false;

var ret = BuildScript.Success;
Expand Down
5 changes: 3 additions & 2 deletions csharp/autobuilder/Semmle.Autobuild.Shared/Project.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ public Project(Autobuilder<TAutobuildOptions> builder, string path) : base(build
continue;
}

var includePath = builder.Actions.PathCombine(include.Value.Split('\\', StringSplitOptions.RemoveEmptyEntries));
ret.Add(new Project<TAutobuildOptions>(builder, builder.Actions.PathCombine(DirectoryName, includePath)));
var includePath = builder.Actions.PathJoin(include.Value.Split('\\', StringSplitOptions.RemoveEmptyEntries));
var path = Path.IsPathRooted(includePath) ? includePath : builder.Actions.PathJoin(DirectoryName, includePath);
ret.Add(new Project<TAutobuildOptions>(builder, path));
}
return ret;
});
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild.Shared/Solution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public Solution(Autobuilder<TAutobuildOptions> builder, string path, bool allowP

includedProjects = solution.ProjectsInOrder
.Where(p => p.ProjectType == SolutionProjectType.KnownToBeMSBuildFormat)
.Select(p => builder.Actions.PathCombine(DirectoryName, builder.Actions.PathCombine(p.RelativePath.Split('\\', StringSplitOptions.RemoveEmptyEntries))))
.Select(p => builder.Actions.PathJoin(DirectoryName, builder.Actions.PathJoin(p.RelativePath.Split('\\', StringSplitOptions.RemoveEmptyEntries))))
.Select(p => new Project<TAutobuildOptions>(builder, p))
.ToArray();
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyContainer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void Add(string package, string dependency)
return;
}

var path = Path.Combine(p, ParseFilePath(d));
var path = Path.Join(p, ParseFilePath(d));
Paths.Add(path);
Packages.Add(GetPackageName(p));
}
Expand Down
4 changes: 2 additions & 2 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public DependencyManager(string srcDir, ILogger logger)
}
}

this.diagnosticsWriter = new DiagnosticsStream(Path.Combine(
this.diagnosticsWriter = new DiagnosticsStream(Path.Join(
diagDirEnv ?? "",
$"dependency-manager-{DateTime.UtcNow:yyyyMMddHHmm}-{Environment.ProcessId}.jsonc"));
this.sourceDir = new DirectoryInfo(srcDir);
Expand Down Expand Up @@ -327,7 +327,7 @@ private void AddNetFrameworkDlls(ISet<AssemblyLookupLocation> dllLocations, ISet
private void RemoveNugetPackageReference(string packagePrefix, ISet<AssemblyLookupLocation> dllLocations)
{
var packageFolder = nugetPackageRestorer.PackageDirectory.DirInfo.FullName.ToLowerInvariant();
var packagePathPrefix = Path.Combine(packageFolder, packagePrefix.ToLowerInvariant());
var packagePathPrefix = Path.Join(packageFolder, packagePrefix.ToLowerInvariant());
var toRemove = dllLocations.Where(s => s.Path.StartsWith(packagePathPrefix, StringComparison.InvariantCultureIgnoreCase));
foreach (var path in toRemove)
{
Expand Down
10 changes: 5 additions & 5 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ private DotNet(IDotNetCliInvoker dotnetCliInvoker, ILogger logger, bool runDotne
}
}

private DotNet(ILogger logger, string? dotNetPath, TemporaryDirectory tempWorkingDirectory, DependabotProxy? dependabotProxy) : this(new DotNetCliInvoker(logger, Path.Combine(dotNetPath ?? string.Empty, "dotnet"), dependabotProxy), logger, dotNetPath is null, tempWorkingDirectory) { }
private DotNet(ILogger logger, string? dotNetPath, TemporaryDirectory tempWorkingDirectory, DependabotProxy? dependabotProxy) : this(new DotNetCliInvoker(logger, Path.Join(dotNetPath ?? string.Empty, "dotnet"), dependabotProxy), logger, dotNetPath is null, tempWorkingDirectory) { }

internal static IDotNet Make(IDotNetCliInvoker dotnetCliInvoker, ILogger logger, bool runDotnetInfo) => new DotNet(dotnetCliInvoker, logger, runDotnetInfo);

Expand Down Expand Up @@ -73,7 +73,7 @@ private string GetRestoreArgs(RestoreSettings restoreSettings)
var path = ".empty";
if (tempWorkingDirectory != null)
{
path = Path.Combine(tempWorkingDirectory.ToString(), "emptyFakeDotnetRoot");
path = Path.Join(tempWorkingDirectory.ToString(), "emptyFakeDotnetRoot");
Directory.CreateDirectory(path);
}

Expand Down Expand Up @@ -303,7 +303,7 @@ BuildScript GetInstall(string pwsh) =>
}
else
{
var dotnetInstallPath = actions.PathCombine(tempWorkingDirectory, ".dotnet", "dotnet-install.sh");
var dotnetInstallPath = actions.PathJoin(tempWorkingDirectory, ".dotnet", "dotnet-install.sh");
var downloadDotNetInstallSh = BuildScript.DownloadFile(
"https://dot.net/v1/dotnet-install.sh",
dotnetInstallPath,
Expand Down Expand Up @@ -339,7 +339,7 @@ BuildScript GetInstall(string pwsh) =>
};
}

var dotnetInfo = InfoScript(actions, actions.PathCombine(path, "dotnet"), MinimalEnvironment.ToDictionary(), logger);
var dotnetInfo = InfoScript(actions, actions.PathJoin(path, "dotnet"), MinimalEnvironment.ToDictionary(), logger);

Func<string, BuildScript> getInstallAndVerify = version =>
// run `dotnet --info` after install, to check that it executes successfully
Expand Down Expand Up @@ -384,7 +384,7 @@ private static BuildScript GetInstalledSdksScript(IBuildActions actions)
/// </summary>
public static BuildScript WithDotNet(IBuildActions actions, ILogger logger, IEnumerable<string> files, string tempWorkingDirectory, bool shouldCleanUp, bool ensureDotNetAvailable, string? version, Func<string?, BuildScript> f)
{
var installDir = actions.PathCombine(tempWorkingDirectory, ".dotnet");
var installDir = actions.PathJoin(tempWorkingDirectory, ".dotnet");
var installScript = DownloadDotNet(actions, logger, files, tempWorkingDirectory, shouldCleanUp, installDir, version, ensureDotNetAvailable);
return BuildScript.Bind(installScript, installed =>
{
Expand Down
4 changes: 2 additions & 2 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetVersion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal record DotNetVersion : IComparable<DotNetVersion>
private string FullVersion =>
version.ToString();

public string FullPath => Path.Combine(dir, FullVersion);
public string FullPath => Path.Join(dir, FullVersion);

/**
* The full path to the reference assemblies for this runtime.
Expand All @@ -33,7 +33,7 @@ public string? FullPathReferenceAssemblies
{
directories[^2] = "packs";
directories[^1] = $"{directories[^1]}.Ref";
return Path.Combine(string.Join(Path.DirectorySeparatorChar, directories), FullVersion, "ref");
return Path.Join(string.Join(Path.DirectorySeparatorChar, directories), FullVersion, "ref");
}
return null;
}
Expand Down
10 changes: 5 additions & 5 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public NugetExeWrapper(FileProvider fileProvider, DependencyDirectory packageDir
if (HasNoPackageSource() && useDefaultFeed())
{
// We only modify or add a top level nuget.config file
nugetConfigPath = Path.Combine(fileProvider.SourceDir.FullName, "nuget.config");
nugetConfigPath = Path.Join(fileProvider.SourceDir.FullName, "nuget.config");
try
{
if (File.Exists(nugetConfigPath))
Expand All @@ -55,7 +55,7 @@ public NugetExeWrapper(FileProvider fileProvider, DependencyDirectory packageDir

do
{
backupNugetConfig = Path.Combine(tempFolderPath, Path.GetRandomFileName());
backupNugetConfig = Path.Join(tempFolderPath, Path.GetRandomFileName());
}
while (File.Exists(backupNugetConfig));
File.Copy(nugetConfigPath, backupNugetConfig, true);
Expand Down Expand Up @@ -123,7 +123,7 @@ private string ResolveNugetExe()
var nugetPath = FileUtils.FindProgramOnPath(executableName);
if (nugetPath is not null)
{
nugetPath = Path.Combine(nugetPath, executableName);
nugetPath = Path.Join(nugetPath, executableName);
logger.LogInfo($"Using nuget.exe from PATH: {nugetPath}");
return nugetPath;
}
Expand All @@ -133,8 +133,8 @@ private string ResolveNugetExe()

private string DownloadNugetExe(string sourceDir)
{
var directory = Path.Combine(sourceDir, ".nuget");
var nuget = Path.Combine(directory, "nuget.exe");
var directory = Path.Join(sourceDir, ".nuget");
var nuget = Path.Join(directory, "nuget.exe");

// Nuget.exe already exists in the .nuget directory.
if (File.Exists(nuget))
Expand Down
8 changes: 4 additions & 4 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public HashSet<AssemblyLookupLocation> Restore()

var paths = dependencies
.Paths
.Select(d => Path.Combine(PackageDirectory.DirInfo.FullName, d))
.Select(d => Path.Join(PackageDirectory.DirInfo.FullName, d))
.ToList();
assemblyLookupLocations.UnionWith(paths.Select(p => new AssemblyLookupLocation(p)));

Expand Down Expand Up @@ -527,7 +527,7 @@ private void RestoreProjects(IEnumerable<string> projects, HashSet<string> reach
var sb = new StringBuilder();
fallbackNugetFeeds.ForEach((feed, index) => sb.AppendLine($"<add key=\"feed{index}\" value=\"{feed}\" />"));

var nugetConfigPath = Path.Combine(folderPath, "nuget.config");
var nugetConfigPath = Path.Join(folderPath, "nuget.config");
logger.LogInfo($"Creating fallback nuget.config file {nugetConfigPath}.");
File.WriteAllText(nugetConfigPath,
$"""
Expand Down Expand Up @@ -1052,15 +1052,15 @@ public void Dispose()
/// </summary>
private static string ComputeTempDirectoryPath(string subfolderName)
{
return Path.Combine(FileUtils.GetTemporaryWorkingDirectory(out _), subfolderName);
return Path.Join(FileUtils.GetTemporaryWorkingDirectory(out _), subfolderName);
}

/// <summary>
/// Computes a unique temporary directory path based on the source directory and the subfolder name.
/// </summary>
private static string ComputeTempDirectoryPath(string srcDir, string subfolderName)
{
return Path.Combine(FileUtils.GetTemporaryWorkingDirectory(out _), FileUtils.ComputeHash(srcDir), subfolderName);
return Path.Join(FileUtils.GetTemporaryWorkingDirectory(out _), FileUtils.ComputeHash(srcDir), subfolderName);
}
}
}
2 changes: 1 addition & 1 deletion csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private IEnumerable<string> DesktopRuntimes

var monoPath = FileUtils.FindProgramOnPath(Win32.IsWindows() ? "mono.exe" : "mono");
string[] monoDirs = monoPath is not null
? [Path.GetFullPath(Path.Combine(monoPath, "..", "lib", "mono")), monoPath]
? [Path.GetFullPath(Path.Join(monoPath, "..", "lib", "mono")), monoPath]
: ["/usr/lib/mono", "/usr/local/mono", "/usr/local/bin/mono", @"C:\Program Files\Mono\lib\mono"];

var monoDir = monoDirs.FirstOrDefault(Directory.Exists);
Expand Down
2 changes: 1 addition & 1 deletion csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Sdk.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private static HashSet<DotNetVersion> ParseSdks(IList<string> listed)
return null;
}

var path = Path.Combine(version.FullPath, "Roslyn", "bincore", "csc.dll");
var path = Path.Join(version.FullPath, "Roslyn", "bincore", "csc.dll");
logger.LogDebug($"Source generator CSC: '{path}'");
if (!File.Exists(path))
{
Expand Down
Loading
Loading