From db655befbbc4f6287adddd98f9180c9a5110d1c2 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 30 Oct 2026 09:49:15 +0100 Subject: [PATCH 1/7] C#: Use XmlReader instead of just inspecting files as text to decide properties about the project being extracted. --- .../FileContent.cs | 176 +++++++----------- 1 file changed, 70 insertions(+), 106 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs index 7a1a4502ce5e..a64740bb6504 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs @@ -1,20 +1,17 @@ using System; using System.Collections.Generic; using System.IO; -using System.Text.RegularExpressions; +using System.Xml; using Semmle.Util; using Semmle.Util.Logging; namespace Semmle.Extraction.CSharp.DependencyFetching { - // - // This class is used to read a set of files and decide different properties about the - // content (by reading the content of the files only once). - // The implementation is lazy, so the properties are only calculated when - // the first property is accessed. - // - internal partial class FileContent + /// + /// Lazily reads XML files once to detect dependency and project settings without evaluating MSBuild. + /// + internal class FileContent { private readonly ILogger logger; private readonly IUnsafeFileReader unsafeFileReader; @@ -135,46 +132,26 @@ internal FileContent(ILogger logger, public FileContent(ILogger logger, IEnumerable files) : this(logger, files, new UnsafeFileReader()) { } - private static string GetGroup(ReadOnlySpan input, ValueMatch valueMatch, string groupPrefix) + private static string? GetAttribute(XmlReader reader, string name) { - var match = input.Slice(valueMatch.Index, valueMatch.Length); - var includeIndex = match.IndexOf(groupPrefix, StringComparison.OrdinalIgnoreCase); - if (includeIndex == -1) + while (reader.MoveToNextAttribute()) { - return string.Empty; - } - - match = match.Slice(includeIndex + groupPrefix.Length + 1); - - var quoteIndex1 = match.IndexOf("\""); - var quoteIndex2 = match.Slice(quoteIndex1 + 1).IndexOf("\""); - - return match.Slice(quoteIndex1 + 1, quoteIndex2).ToString(); - } - - private static bool IsGroupMatch(ReadOnlySpan line, Regex regex, string groupPrefix, string value) - { - foreach (var valueMatch in regex.EnumerateMatches(line)) - { - // We can't get the group from the ValueMatch, so doing it manually: - if (string.Equals(GetGroup(line, valueMatch, groupPrefix), value, StringComparison.OrdinalIgnoreCase)) + if (string.Equals(reader.LocalName, name, StringComparison.OrdinalIgnoreCase)) { - return true; + var value = reader.Value; + reader.MoveToElement(); + return value; } } - return false; + reader.MoveToElement(); + return null; } - private void AddPackageReference(ReadOnlySpan line, string groupName, Func regex, PackageReferenceSource source) + private void AddPackageReference(string? packageName, PackageReferenceSource source) { - foreach (var valueMatch in regex().EnumerateMatches(line)) + if (!string.IsNullOrEmpty(packageName)) { - // We can't get the group from the ValueMatch, so doing it manually: - var packageName = GetGroup(line, valueMatch, groupName).ToLowerInvariant(); - if (!string.IsNullOrEmpty(packageName)) - { - allPackages.Add(new PackageReference(packageName, source)); - } + allPackages.Add(new PackageReference(packageName.ToLowerInvariant(), source)); } } @@ -185,56 +162,68 @@ private void DoInitialize() try { var isPackagesConfig = string.Equals(FileUtils.SafeGetFileName(file, logger), "packages.config", StringComparison.OrdinalIgnoreCase); + using var stream = unsafeFileReader.OpenRead(file); + using var reader = XmlReader.Create(stream, new XmlReaderSettings + { + DtdProcessing = DtdProcessing.Prohibit, + XmlResolver = null, + IgnoreComments = true, + IgnoreProcessingInstructions = true + }); - foreach (ReadOnlySpan line in unsafeFileReader.ReadLines(file)) + while (!reader.EOF) { - // Find all the packages. - if (isPackagesConfig) - { - AddPackageReference(line, "id", LegacyPackageReference, PackageReferenceSource.PackagesConfig); - } - else + if (reader.NodeType != XmlNodeType.Element) { - AddPackageReference(line, "Include", PackageReference, PackageReferenceSource.SdkCsProj); + reader.Read(); + continue; } - // Determine if ASP.NET is used. - useAspNetCoreDlls = useAspNetCoreDlls - || IsGroupMatch(line, ProjectSdk(), "Sdk", "Microsoft.NET.Sdk.Web") - || IsGroupMatch(line, FrameworkReference(), "Include", "Microsoft.AspNetCore.App"); - - // Determine if implicit usings are used. - useImplicitUsings = useImplicitUsings - || line.Contains("enable".AsSpan(), StringComparison.OrdinalIgnoreCase) - || line.Contains("true".AsSpan(), StringComparison.OrdinalIgnoreCase); - - // Determine if WPF is used. - useWpf = useWpf - || line.Contains("true".AsSpan(), StringComparison.OrdinalIgnoreCase); - - // Determine if Windows Forms is used. - useWindowsForms = useWindowsForms - || line.Contains("true".AsSpan(), StringComparison.OrdinalIgnoreCase); - - // Find all custom implicit usings. - foreach (var valueMatch in CustomImplicitUsingDeclarations().EnumerateMatches(line)) + switch (reader.LocalName.ToLowerInvariant()) { - var ns = GetGroup(line, valueMatch, "Include"); - if (!string.IsNullOrEmpty(ns)) - { - implicitUsingNamespaces.Add(ns); - } + case "package" when isPackagesConfig: + AddPackageReference(GetAttribute(reader, "id"), PackageReferenceSource.PackagesConfig); + break; + case "packagereference" when !isPackagesConfig: + AddPackageReference(GetAttribute(reader, "Include"), PackageReferenceSource.SdkCsProj); + break; + case "project": + var sdk = GetAttribute(reader, "Sdk"); + isSdkProjectStructureUsed |= sdk != null; + useAspNetCoreDlls |= string.Equals(sdk, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase); + break; + case "frameworkreference": + var framework = GetAttribute(reader, "Include"); + isSdkProjectStructureUsed |= framework != null; + useAspNetCoreDlls |= string.Equals(framework, "Microsoft.AspNetCore.App", StringComparison.OrdinalIgnoreCase); + break; + case "implicitusings": + var implicitUsings = reader.ReadElementContentAsString().Trim(); + useImplicitUsings |= string.Equals(implicitUsings, "enable", StringComparison.OrdinalIgnoreCase) + || string.Equals(implicitUsings, "true", StringComparison.OrdinalIgnoreCase); + continue; + case "usewpf": + useWpf |= string.Equals(reader.ReadElementContentAsString().Trim(), "true", StringComparison.OrdinalIgnoreCase); + continue; + case "usewindowsforms": + useWindowsForms |= string.Equals(reader.ReadElementContentAsString().Trim(), "true", StringComparison.OrdinalIgnoreCase); + continue; + case "using": + var ns = GetAttribute(reader, "Include"); + if (!string.IsNullOrEmpty(ns)) + { + implicitUsingNamespaces.Add(ns); + } + break; + case "import": + isLegacyProjectStructureUsed |= GetAttribute(reader, "Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; + break; } - // Determine project structure: - isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || MicrosoftCSharpTargets().IsMatch(line); - isSdkProjectStructureUsed = isSdkProjectStructureUsed - || ProjectSdk().IsMatch(line) - || FrameworkReference().IsMatch(line); - // TODO: we could also check `` + reader.Read(); } } - catch (Exception ex) + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or XmlException) { logger.LogInfo($"Failed to read file {file}"); logger.LogDebug($"Failed to read file {file}, exception: {ex}"); @@ -250,41 +239,16 @@ private void DoInitialize() logger.LogInfo($"Implicit usings enabled: {useImplicitUsings}"); } - [GeneratedRegex("(?", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] - private static partial Regex PackageReference(); - - [GeneratedRegex("(?", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] - private static partial Regex LegacyPackageReference(); - - [GeneratedRegex("(?", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] - private static partial Regex FrameworkReference(); - - [GeneratedRegex("(?", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] - private static partial Regex ProjectSdk(); - - [GeneratedRegex("(?", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] - private static partial Regex CustomImplicitUsingDeclarations(); - - [GeneratedRegex("(?", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] - private static partial Regex MicrosoftCSharpTargets(); } internal interface IUnsafeFileReader { - IEnumerable ReadLines(string file); + Stream OpenRead(string file); } internal class UnsafeFileReader : IUnsafeFileReader { - public IEnumerable ReadLines(string file) - { - using var sr = new StreamReader(file); - string? line; - while ((line = sr.ReadLine()) != null) - { - yield return line; - } - } + public Stream OpenRead(string file) => File.OpenRead(file); } public enum PackageReferenceSource From fbab1395da6b37248b693f5af87e29578a721a38 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 30 Oct 2026 09:58:46 +0100 Subject: [PATCH 2/7] C#: Improve the generated implementation. --- .../FileContent.cs | 210 ++++++++++++------ 1 file changed, 140 insertions(+), 70 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs index a64740bb6504..67091041d498 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs @@ -20,6 +20,9 @@ internal class FileContent private readonly HashSet implicitUsingNamespaces = new HashSet(); private readonly Initializer initialize; + /// + /// All detected package references in the project. + /// public HashSet AllPackages { get @@ -34,7 +37,7 @@ public HashSet AllPackages /// /// True if any file in the source directory indicates that ASP.NET Core is used. /// The following heuristic is used to decide, if ASP.NET Core is used: - /// If any file in the source directory contains something like (this will most like be a .csproj file) + /// If any XML file in the source directory contains something like (this will most like be a .csproj file) /// <Project Sdk="Microsoft.NET.Sdk.Web"> /// <FrameworkReference Include="Microsoft.AspNetCore.App"/> /// @@ -47,6 +50,9 @@ public bool UseAspNetCoreDlls } } + /// + /// True if there is an indication that ASP.NET Core is used. + /// public bool IsAspNetCoreDetected { get @@ -57,6 +63,9 @@ public bool IsAspNetCoreDetected private bool useImplicitUsings = false; + /// + /// True if there is an indication that implicit usings are used. + /// public bool UseImplicitUsings { get @@ -68,6 +77,9 @@ public bool UseImplicitUsings private bool useWpf = false; + /// + /// True if there is an indication that WPF is used. + /// public bool UseWpf { get @@ -79,6 +91,9 @@ public bool UseWpf private bool useWindowsForms = false; + /// + /// True if there is an indication that Windows Forms is used. + /// public bool UseWindowsForms { get @@ -90,6 +105,9 @@ public bool UseWindowsForms private bool isLegacyProjectStructureUsed = false; + /// + /// True if there is an indication that the legacy project structure is used (e.g., non-SDK-style .csproj files). + /// public bool IsLegacyProjectStructureUsed { get @@ -100,6 +118,10 @@ public bool IsLegacyProjectStructureUsed } private bool isSdkProjectStructureUsed = false; + + /// + /// True if there is an indication that the new project structure is used (e.g., SDK-style .csproj files). + /// public bool IsSdkProjectStructureUsed { get @@ -132,98 +154,146 @@ internal FileContent(ILogger logger, public FileContent(ILogger logger, IEnumerable files) : this(logger, files, new UnsafeFileReader()) { } - private static string? GetAttribute(XmlReader reader, string name) + private void AddPackageReference(string? packageName, PackageReferenceSource source) { - while (reader.MoveToNextAttribute()) + if (!string.IsNullOrEmpty(packageName)) { - if (string.Equals(reader.LocalName, name, StringComparison.OrdinalIgnoreCase)) - { - var value = reader.Value; - reader.MoveToElement(); - return value; - } + allPackages.Add(new PackageReference(packageName.ToLowerInvariant(), source)); } - reader.MoveToElement(); - return null; } - private void AddPackageReference(string? packageName, PackageReferenceSource source) + private void ProcessFile(string file, Action processElement) { - if (!string.IsNullOrEmpty(packageName)) + try { - allPackages.Add(new PackageReference(packageName.ToLowerInvariant(), source)); + using var stream = unsafeFileReader.OpenRead(file); + using var reader = XmlReader.Create(stream, new XmlReaderSettings + { + DtdProcessing = DtdProcessing.Prohibit, + XmlResolver = null, + IgnoreComments = true, + IgnoreProcessingInstructions = true + }); + + while (reader.Read()) + { + if (reader.NodeType != XmlNodeType.Element) + { + continue; + } + + processElement(reader); + } + } + catch (XmlException) + { + logger.LogDebug($"Failed to parse {file} as an XML file."); } } - private void DoInitialize() + /// + /// Parses a packages.config XML file to extract package references. + /// + /// + private void ParsePackagesConfigFile(string file) { - foreach (var file in files) + ProcessFile(file, r => { - try + if (r.LocalName == "package") { - var isPackagesConfig = string.Equals(FileUtils.SafeGetFileName(file, logger), "packages.config", StringComparison.OrdinalIgnoreCase); - using var stream = unsafeFileReader.OpenRead(file); - using var reader = XmlReader.Create(stream, new XmlReaderSettings - { - DtdProcessing = DtdProcessing.Prohibit, - XmlResolver = null, - IgnoreComments = true, - IgnoreProcessingInstructions = true - }); + AddPackageReference(r.GetAttribute("id"), PackageReferenceSource.PackagesConfig); + } + }); + } - while (!reader.EOF) + /// + /// Checks if the content of the current XML element equals any of the specified values, ignoring case. + /// A new reader is created to avoid modifying the state of the original reader. + /// + /// The XML reader positioned at the element to read. + /// True if the element content equals any of the specified values, ignoring case; otherwise, false. + private bool ElementContentEquals(XmlReader reader, params string[] values) + { + using (XmlReader subReader = reader.ReadSubtree()) + { + if (subReader.Read()) + { + var content = subReader.ReadElementContentAsString().Trim(); + foreach (var value in values) { - if (reader.NodeType != XmlNodeType.Element) + if (string.Equals(content, value, StringComparison.OrdinalIgnoreCase)) { - reader.Read(); - continue; + return true; } + } + } + } + return false; + } - switch (reader.LocalName.ToLowerInvariant()) + /// + /// Attempt to parse the specified file as an XML file for project dependencies and settings. + /// + /// + private void Parse(string file) + { + ProcessFile(file, reader => + { + switch (reader.LocalName) + { + case "PackageReference": + AddPackageReference(reader.GetAttribute("Include"), PackageReferenceSource.SdkCsProj); + break; + case "Project": + var sdk = reader.GetAttribute("Sdk"); + isSdkProjectStructureUsed |= sdk != null; + useAspNetCoreDlls |= string.Equals(sdk, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase); + break; + case "FrameworkReference": + var framework = reader.GetAttribute("Include"); + isSdkProjectStructureUsed |= framework != null; + useAspNetCoreDlls |= string.Equals(framework, "Microsoft.AspNetCore.App", StringComparison.OrdinalIgnoreCase); + break; + case "ImplicitUsings": + useImplicitUsings = useImplicitUsings || ElementContentEquals(reader, "enable", "true"); + break; + case "UseWPF": + useWpf = useWpf || ElementContentEquals(reader, "true"); + break; + case "UseWindowsForms": + useWindowsForms = useWindowsForms || ElementContentEquals(reader, "true"); + break; + case "Using": + var ns = reader.GetAttribute("Include"); + if (!string.IsNullOrEmpty(ns)) { - case "package" when isPackagesConfig: - AddPackageReference(GetAttribute(reader, "id"), PackageReferenceSource.PackagesConfig); - break; - case "packagereference" when !isPackagesConfig: - AddPackageReference(GetAttribute(reader, "Include"), PackageReferenceSource.SdkCsProj); - break; - case "project": - var sdk = GetAttribute(reader, "Sdk"); - isSdkProjectStructureUsed |= sdk != null; - useAspNetCoreDlls |= string.Equals(sdk, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase); - break; - case "frameworkreference": - var framework = GetAttribute(reader, "Include"); - isSdkProjectStructureUsed |= framework != null; - useAspNetCoreDlls |= string.Equals(framework, "Microsoft.AspNetCore.App", StringComparison.OrdinalIgnoreCase); - break; - case "implicitusings": - var implicitUsings = reader.ReadElementContentAsString().Trim(); - useImplicitUsings |= string.Equals(implicitUsings, "enable", StringComparison.OrdinalIgnoreCase) - || string.Equals(implicitUsings, "true", StringComparison.OrdinalIgnoreCase); - continue; - case "usewpf": - useWpf |= string.Equals(reader.ReadElementContentAsString().Trim(), "true", StringComparison.OrdinalIgnoreCase); - continue; - case "usewindowsforms": - useWindowsForms |= string.Equals(reader.ReadElementContentAsString().Trim(), "true", StringComparison.OrdinalIgnoreCase); - continue; - case "using": - var ns = GetAttribute(reader, "Include"); - if (!string.IsNullOrEmpty(ns)) - { - implicitUsingNamespaces.Add(ns); - } - break; - case "import": - isLegacyProjectStructureUsed |= GetAttribute(reader, "Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; - break; + implicitUsingNamespaces.Add(ns); } + break; + case "Import": + isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || reader.GetAttribute("Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; + break; + } + }); + } - reader.Read(); + private void DoInitialize() + { + foreach (var file in files) + { + try + { + var isPackagesConfig = string.Equals(FileUtils.SafeGetFileName(file, logger), "packages.config", StringComparison.OrdinalIgnoreCase); + if (isPackagesConfig) + { + ParsePackagesConfigFile(file); + } + else + { + Parse(file); } } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or XmlException) + catch (Exception ex) { logger.LogInfo($"Failed to read file {file}"); logger.LogDebug($"Failed to read file {file}, exception: {ex}"); From c69039fd7350286491d563afa0f22b53d31917f7 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 30 Oct 2026 13:09:37 +0100 Subject: [PATCH 3/7] C#: Adjust the existing tests and add some more tests. --- .../Semmle.Extraction.Tests/FileContent.cs | 252 ++++++++++++++---- 1 file changed, 202 insertions(+), 50 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs index 8d042fcc33b2..a6858a49b6b7 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs @@ -1,33 +1,37 @@ using Xunit; using System; using System.Collections.Generic; +using System.IO; +using System.Text; using Semmle.Extraction.CSharp.DependencyFetching; +using Semmle.Util.Logging; namespace Semmle.Extraction.Tests { internal class UnsafeFileReaderStub : IUnsafeFileReader { - private readonly IEnumerable lines; + private readonly IReadOnlyDictionary contents; + public int OpenCount { get; private set; } + public MemoryStream? LastStream { get; private set; } - public UnsafeFileReaderStub(IEnumerable lines) + public UnsafeFileReaderStub(IReadOnlyDictionary contents) { - this.lines = lines; + this.contents = contents; } - public IEnumerable ReadLines(string file) + public Stream OpenRead(string file) { - foreach (var line in lines) - { - yield return line; - } + OpenCount++; + LastStream = new MemoryStream(Encoding.UTF8.GetBytes(contents[file])); + return LastStream; } } internal class TestFileContent : FileContent { - public TestFileContent(IEnumerable lines) : base(new LoggerStub(), - new List() { "test1.cs" }, - new UnsafeFileReaderStub(lines)) + public TestFileContent(string content, string file = "test1.csproj") : base(new LoggerStub(), + new[] { file }, + new UnsafeFileReaderStub(new Dictionary { [file] = content })) { } } @@ -37,15 +41,16 @@ public class FileContentTests public void TestFileContent1() { // Setup - var lines = new List() - { - "", - "", - "", - "", - "", - "" - }; + var lines = + """ + + + + + + + + """; var fileContent = new TestFileContent(lines); // Execute @@ -64,13 +69,14 @@ public void TestFileContent1() public void TestFileContent2() { // Setup - var lines = new List() - { - "", - "", - "", - "" - }; + var lines = + """ + + + + + + """; var fileContent = new TestFileContent(lines); // Execute @@ -84,14 +90,10 @@ public void TestFileContent2() Assert.Contains(new PackageReference("StyleCop.Analyzers".ToLowerInvariant(), PackageReferenceSource.SdkCsProj), allPackages); } - private static void CsProjSettingsTest(string line, bool expected, Func func) + private static void CsProjSettingsTest(string content, bool expected, Func func) { // Setup - var lines = new List() - { - line - }; - var fileContent = new TestFileContent(lines); + var fileContent = new TestFileContent(content); // Execute var actual = func(fileContent); @@ -146,13 +148,15 @@ public void TestFileContent_UseWindowsForms1() public void TestFileContent_ImplicitUsingsAdditional() { // Setup - var lines = new List() - { - "", - "", - "", - }; - var fileContent = new TestFileContent(lines); + var content = + """ + + + + + + """; + var fileContent = new TestFileContent(content); // Execute var customImplicitUsings = fileContent.CustomImplicitUsings; @@ -173,21 +177,21 @@ public void TestFileContent_LegacyProjectStructure() + """; - var lines = input.Split(Environment.NewLine); - var fileContent = new TestFileContent(lines); + var fileContent = new TestFileContent(input); // Execute var isLegacy = fileContent.IsLegacyProjectStructureUsed; - var isNew = fileContent.IsSdkProjectStructureUsed; + var isSdk = fileContent.IsSdkProjectStructureUsed; // Verify Assert.True(isLegacy); - Assert.False(isNew); + Assert.False(isSdk); } [Fact] - public void TestFileContent_NewProjectStructure() + public void TestFileContent_SdkProjectStructure() { // Setup var input = @@ -198,17 +202,165 @@ public void TestFileContent_NewProjectStructure() """; - var lines = input.Split(Environment.NewLine); - - var fileContent = new TestFileContent(lines); + var fileContent = new TestFileContent(input); // Execute var isLegacy = fileContent.IsLegacyProjectStructureUsed; - var isNew = fileContent.IsSdkProjectStructureUsed; + var isSdk = fileContent.IsSdkProjectStructureUsed; // Verify - Assert.True(isNew); + Assert.True(isSdk); Assert.False(isLegacy); } + + [Fact] + public void TestFileContent_XmlSyntax() + { + // Setup + var input = + """ + + + + 2.0 + + + + + + + + ENABLE + true + + + + """; + var fileContent = new TestFileContent(input); + + // Execute + var allPackages = fileContent.AllPackages; + var isAspNetCoreDetected = fileContent.IsAspNetCoreDetected; + var useImplicitUsings = fileContent.UseImplicitUsings; + var useWpf = fileContent.UseWpf; + var useWindowsForms = fileContent.UseWindowsForms; + var customImplicitUsings = fileContent.CustomImplicitUsings; + + // Verify + Assert.Equal(2, allPackages.Count); + Assert.Contains(new PackageReference("my.package", PackageReferenceSource.SdkCsProj), allPackages); + Assert.Contains(new PackageReference("other.package", PackageReferenceSource.SdkCsProj), allPackages); + Assert.True(isAspNetCoreDetected); + Assert.True(useImplicitUsings); + Assert.True(useWpf); + Assert.True(useWindowsForms); + Assert.Contains("My.Namespace", customImplicitUsings); + } + + [Fact] + public void TestFileContent_CommentsAreIgnored() + { + // Setup + var input = + """ + + + + """; + var fileContent = new TestFileContent(input); + + // Execute + var allPackages = fileContent.AllPackages; + var isAspNetCoreDetected = fileContent.IsAspNetCoreDetected; + var useImplicitUsings = fileContent.UseImplicitUsings; + var useWpf = fileContent.UseWpf; + var useWindowsForms = fileContent.UseWindowsForms; + var customImplicitUsings = fileContent.CustomImplicitUsings; + var isLegacyProjectStructureUsed = fileContent.IsLegacyProjectStructureUsed; + var isSdkProjectStructureUsed = fileContent.IsSdkProjectStructureUsed; + + // Verify + Assert.Empty(allPackages); + Assert.Empty(customImplicitUsings); + Assert.False(isAspNetCoreDetected); + Assert.False(useImplicitUsings); + Assert.False(useWpf); + Assert.False(useWindowsForms); + Assert.False(isLegacyProjectStructureUsed); + Assert.False(isSdkProjectStructureUsed); + } + + [Fact] + public void TestFileContent_PackagesConfig() + { + // Setup + var input = + """ + + + + + + + + + + """; + var fileContent = new TestFileContent(input, "PACKAGES.CONFIG"); + + // Execute + var allPackages = fileContent.AllPackages; + + // Verify + Assert.Equal(2, allPackages.Count); + Assert.Contains(new PackageReference("my.package", PackageReferenceSource.PackagesConfig), allPackages); + Assert.Contains(new PackageReference("other.package", PackageReferenceSource.PackagesConfig), allPackages); + } + + private sealed class RecordingLogger : ILogger + { + public List Messages { get; } = new List(); + public void Log(Severity severity, string message, int? threadId = null) => Messages.Add(message); + public void Dispose() { } + } + + [Theory] + [InlineData("This is not XML.")] + [InlineData("]>")] + public void TestFileContent_InvalidXmlDoesNotPreventReadingOtherFiles(string invalidXml) + { + // Setup + var logger = new RecordingLogger(); + var stub = new UnsafeFileReaderStub(new Dictionary + { + ["invalid.csproj"] = invalidXml, + ["valid.csproj"] = "" + }); + var fileContent = new FileContent(logger, ["invalid.csproj", "valid.csproj"], stub); + + // Execute + var allPackages = fileContent.AllPackages; + var isAspNetCoreDetected = fileContent.IsAspNetCoreDetected; + + // Verify + Assert.True(isAspNetCoreDetected); + Assert.Contains(new PackageReference("my.package", PackageReferenceSource.SdkCsProj), allPackages); + Assert.Contains("Failed to parse invalid.csproj as an XML file.", logger.Messages); + Assert.Equal(2, stub.OpenCount); + } } } From 9e8ff9fb1a0d8b2b6a3b228c4689a67a3957a7f2 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 30 Oct 2026 13:26:48 +0100 Subject: [PATCH 4/7] C#: Further improve the tests and include some comments. --- .../Semmle.Extraction.Tests/FileContent.cs | 108 +++++++++++------- 1 file changed, 68 insertions(+), 40 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs index a6858a49b6b7..5b740798f823 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs @@ -37,8 +37,11 @@ public TestFileContent(string content, string file = "test1.csproj") : base(new public class FileContentTests { + /// + /// Verifies that `FileContent` correctly identifies SDK project structure, package references, and no ASP.NET Core usage. + /// [Fact] - public void TestFileContent1() + public void TestFileContent_ProjectSdkAndNoAspNet() { // Setup var lines = @@ -55,16 +58,21 @@ public void TestFileContent1() // Execute var allPackages = fileContent.AllPackages; + var isSdkProject = fileContent.IsSdkProjectStructureUsed; var useAspNetDlls = fileContent.UseAspNetCoreDlls; // Verify Assert.False(useAspNetDlls); + Assert.True(isSdkProject); Assert.Equal(3, allPackages.Count); Assert.Contains(new PackageReference("DotNetAnalyzers.DocumentationAnalyzers".ToLowerInvariant(), PackageReferenceSource.SdkCsProj), allPackages); Assert.Contains(new PackageReference("Microsoft.CodeAnalysis.NetAnalyzers".ToLowerInvariant(), PackageReferenceSource.SdkCsProj), allPackages); Assert.Contains(new PackageReference("StyleCop.Analyzers".ToLowerInvariant(), PackageReferenceSource.SdkCsProj), allPackages); } + /// + /// Verifies that `FileContent` correctly identifies package references and ASP.NET Core usage. + /// [Fact] public void TestFileContent2() { @@ -90,60 +98,64 @@ public void TestFileContent2() Assert.Contains(new PackageReference("StyleCop.Analyzers".ToLowerInvariant(), PackageReferenceSource.SdkCsProj), allPackages); } - private static void CsProjSettingsTest(string content, bool expected, Func func) + /// + /// Verifies that `FileContent` correctly identifies the use of implicit usings. + /// + [Theory] + [InlineData("false", false)] + [InlineData("true", true)] + [InlineData("enable", true)] + public void TestFileContent_ImplicitUsings(string content, bool expectedImplicitUsings) { // Setup var fileContent = new TestFileContent(content); // Execute - var actual = func(fileContent); + var actual = fileContent.UseImplicitUsings; // Verify - Assert.Equal(expected, actual); - } - - [Fact] - public void TestFileContent_ImplicitUsings0() - { - CsProjSettingsTest("false", false, fc => fc.UseImplicitUsings); + Assert.Equal(expectedImplicitUsings, actual); } - [Fact] - public void TestFileContent_ImplicitUsings1() + /// + /// Verifies that `FileContent` correctly identifies the use of WPF. + /// + [Theory] + [InlineData("false", false)] + [InlineData("true", true)] + public void TestFileContent_UseWpf(string content, bool expectedUseWpf) { - CsProjSettingsTest("true", true, fc => fc.UseImplicitUsings); - } + // Setup + var fileContent = new TestFileContent(content); - [Fact] - public void TestFileContent_ImplicitUsings2() - { - CsProjSettingsTest("enable", true, fc => fc.UseImplicitUsings); - } + // Execute + var actual = fileContent.UseWpf; - [Fact] - public void TestFileContent_UseWpf0() - { - CsProjSettingsTest("false", false, fc => fc.UseWpf); + // Verify + Assert.Equal(expectedUseWpf, actual); } - [Fact] - public void TestFileContent_UseWpf1() + /// + /// Verifies that `FileContent` correctly identifies the use of Windows Forms. + /// + [Theory] + [InlineData("false", false)] + [InlineData("true", true)] + public void TestFileContent_UseWindowsForms(string content, bool expectedUseWindowsForms) { - CsProjSettingsTest("true", true, fc => fc.UseWpf); - } + // Setup + var fileContent = new TestFileContent(content); - [Fact] - public void TestFileContent_UseWindowsForms0() - { - CsProjSettingsTest("false", false, fc => fc.UseWindowsForms); - } + // Execute + var actual = fileContent.UseWindowsForms; - [Fact] - public void TestFileContent_UseWindowsForms1() - { - CsProjSettingsTest("true", true, fc => fc.UseWindowsForms); + // Verify + Assert.Equal(expectedUseWindowsForms, actual); } + /// + /// Verifies that `FileContent` correctly identifies custom implicit usings. + /// [Fact] public void TestFileContent_ImplicitUsingsAdditional() { @@ -162,11 +174,12 @@ public void TestFileContent_ImplicitUsingsAdditional() var customImplicitUsings = fileContent.CustomImplicitUsings; // Verify - Assert.Equal(2, customImplicitUsings.Count); - Assert.Contains("Ns0.Ns1", customImplicitUsings); - Assert.Contains("Ns2", customImplicitUsings); + Assert.Equal(["Ns0.Ns1", "Ns2"], customImplicitUsings); } + /// + /// Verifies that `FileContent` correctly identifies legacy project structure. + /// [Fact] public void TestFileContent_LegacyProjectStructure() { @@ -190,6 +203,9 @@ public void TestFileContent_LegacyProjectStructure() Assert.False(isSdk); } + /// + /// Verifies that `FileContent` correctly identifies SDK project structure. + /// [Fact] public void TestFileContent_SdkProjectStructure() { @@ -213,6 +229,9 @@ public void TestFileContent_SdkProjectStructure() Assert.False(isLegacy); } + /// + /// Verifies that `FileContent` correctly handles XML syntax with namespaces and various elements. + /// [Fact] public void TestFileContent_XmlSyntax() { @@ -257,9 +276,12 @@ public void TestFileContent_XmlSyntax() Assert.True(useImplicitUsings); Assert.True(useWpf); Assert.True(useWindowsForms); - Assert.Contains("My.Namespace", customImplicitUsings); + Assert.Equal(["My.Namespace"], customImplicitUsings); } + /// + /// Verify that `FileContent` correctly ignores commented-out XML elements. + /// [Fact] public void TestFileContent_CommentsAreIgnored() { @@ -302,6 +324,9 @@ public void TestFileContent_CommentsAreIgnored() Assert.False(isSdkProjectStructureUsed); } + /// + /// Verify that `FileContent` correctly reads package references from a `packages.config` file. + /// [Fact] public void TestFileContent_PackagesConfig() { @@ -337,6 +362,9 @@ private sealed class RecordingLogger : ILogger public void Dispose() { } } + /// + /// Verify that `FileContent` correctly handles invalid XML files without preventing reading other valid files. + /// [Theory] [InlineData("This is not XML.")] [InlineData(" Date: Fri, 30 Oct 2026 14:00:58 +0100 Subject: [PATCH 5/7] C#: Address review comments. --- .../FileContent.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs index 67091041d498..6f96d4eae406 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs @@ -9,7 +9,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { /// - /// Lazily reads XML files once to detect dependency and project settings without evaluating MSBuild. + /// Lazily reads XML files to detect dependency and project settings without evaluating MSBuild. /// internal class FileContent { @@ -239,38 +239,38 @@ private void Parse(string file) { ProcessFile(file, reader => { - switch (reader.LocalName) + switch (reader.LocalName.ToLowerInvariant()) { - case "PackageReference": + case "packagereference": AddPackageReference(reader.GetAttribute("Include"), PackageReferenceSource.SdkCsProj); break; - case "Project": + case "project": var sdk = reader.GetAttribute("Sdk"); isSdkProjectStructureUsed |= sdk != null; useAspNetCoreDlls |= string.Equals(sdk, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase); break; - case "FrameworkReference": + case "frameworkreference": var framework = reader.GetAttribute("Include"); isSdkProjectStructureUsed |= framework != null; useAspNetCoreDlls |= string.Equals(framework, "Microsoft.AspNetCore.App", StringComparison.OrdinalIgnoreCase); break; - case "ImplicitUsings": + case "implicitusings": useImplicitUsings = useImplicitUsings || ElementContentEquals(reader, "enable", "true"); break; - case "UseWPF": + case "usewpf": useWpf = useWpf || ElementContentEquals(reader, "true"); break; - case "UseWindowsForms": + case "usewindowsforms": useWindowsForms = useWindowsForms || ElementContentEquals(reader, "true"); break; - case "Using": + case "using": var ns = reader.GetAttribute("Include"); if (!string.IsNullOrEmpty(ns)) { implicitUsingNamespaces.Add(ns); } break; - case "Import": + case "import": isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || reader.GetAttribute("Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; break; } From fea9c7914a60c59a049e6e9701e729073cd22581 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 2 Nov 2026 11:08:44 +0100 Subject: [PATCH 6/7] C#: Detect SDK-style project imports Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../FileContent.cs | 5 ++++- .../Semmle.Extraction.Tests/FileContent.cs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs index 6f96d4eae406..27389b0499ec 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs @@ -271,8 +271,11 @@ private void Parse(string file) } break; case "import": - isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || reader.GetAttribute("Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; + var project = reader.GetAttribute("Project"); + isSdkProjectStructureUsed = isSdkProjectStructureUsed || project is not null && reader.GetAttribute("Sdk") is not null; + isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || project?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; break; + // TODO: we could also check `` } }); } diff --git a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs index 5b740798f823..4f2ddef89167 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs @@ -229,6 +229,24 @@ public void TestFileContent_SdkProjectStructure() Assert.False(isLegacy); } + /// + /// Verifies that an SDK attribute on an import indicates SDK project structure. + /// + [Fact] + public void TestFileContent_SdkProjectImport() + { + // Setup + var fileContent = new TestFileContent(""); + + // Execute + var isLegacy = fileContent.IsLegacyProjectStructureUsed; + var isSdk = fileContent.IsSdkProjectStructureUsed; + + // Verify + Assert.True(isSdk); + Assert.False(isLegacy); + } + /// /// Verifies that `FileContent` correctly handles XML syntax with namespaces and various elements. /// From fd2ad1f307ee6b51dea2b859c9937d76b2451feb Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 2 Nov 2026 14:13:35 +0100 Subject: [PATCH 7/7] C#: Detect SDK elements in project files Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../FileContent.cs | 6 ++- .../Semmle.Extraction.Tests/FileContent.cs | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs index 27389b0499ec..0c6723f72214 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileContent.cs @@ -275,7 +275,11 @@ private void Parse(string file) isSdkProjectStructureUsed = isSdkProjectStructureUsed || project is not null && reader.GetAttribute("Sdk") is not null; isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || project?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true; break; - // TODO: we could also check `` + case "sdk": + var sdkName = reader.GetAttribute("Name"); + isSdkProjectStructureUsed |= sdkName != null; + useAspNetCoreDlls |= string.Equals(sdkName, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase); + break; } }); } diff --git a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs index 4f2ddef89167..68b092f31fb3 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/FileContent.cs @@ -247,6 +247,44 @@ public void TestFileContent_SdkProjectImport() Assert.False(isLegacy); } + /// + /// Verifies that an SDK element indicates SDK project structure. + /// + [Fact] + public void TestFileContent_SdkElement() + { + // Setup + var fileContent = new TestFileContent(""); + + // Execute + var isSdk = fileContent.IsSdkProjectStructureUsed; + var isLegacy = fileContent.IsLegacyProjectStructureUsed; + + // Verify + Assert.True(isSdk); + Assert.False(isLegacy); + } + + /// + /// Verifies that an ASP.NET SDK element indicates ASP.NET Core usage. + /// + [Fact] + public void TestFileContent_AspNetCoreSdkElement() + { + // Setup + var fileContent = new TestFileContent(""); + + // Execute + var isSdk = fileContent.IsSdkProjectStructureUsed; + var useAspNetCoreDlls = fileContent.UseAspNetCoreDlls; + var isAspNetCoreDetected = fileContent.IsAspNetCoreDetected; + + // Verify + Assert.True(isSdk); + Assert.True(useAspNetCoreDlls); + Assert.True(isAspNetCoreDetected); + } + /// /// Verifies that `FileContent` correctly handles XML syntax with namespaces and various elements. ///