Skip to content

Commit 291ac2e

Browse files
michaelnebelmichaelnebel
committed
C#: Improve the converted implementation.
1 parent 53af6d4 commit 291ac2e

2 files changed

Lines changed: 143 additions & 85 deletions

File tree

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

Lines changed: 140 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ internal class FileContent
2020
private readonly HashSet<string> implicitUsingNamespaces = new HashSet<string>();
2121
private readonly Initializer initialize;
2222

23+
/// <summary>
24+
/// All detected package references in the project.
25+
/// </summary>
2326
public HashSet<PackageReference> AllPackages
2427
{
2528
get
@@ -34,7 +37,7 @@ public HashSet<PackageReference> AllPackages
3437
/// <summary>
3538
/// True if any file in the source directory indicates that ASP.NET Core is used.
3639
/// The following heuristic is used to decide, if ASP.NET Core is used:
37-
/// If any file in the source directory contains something like (this will most like be a .csproj file)
40+
/// If any XML file in the source directory contains something like (this will most like be a .csproj file)
3841
/// &lt;Project Sdk="Microsoft.NET.Sdk.Web"&gt;
3942
/// &lt;FrameworkReference Include="Microsoft.AspNetCore.App"/&gt;
4043
/// </summary>
@@ -47,6 +50,9 @@ public bool UseAspNetCoreDlls
4750
}
4851
}
4952

53+
/// <summary>
54+
/// True if there is an indication that ASP.NET Core is used.
55+
/// </summary>
5056
public bool IsAspNetCoreDetected
5157
{
5258
get
@@ -57,6 +63,9 @@ public bool IsAspNetCoreDetected
5763

5864
private bool useImplicitUsings = false;
5965

66+
/// <summary>
67+
/// True if there is an indication that implicit usings are used.
68+
/// </summary>
6069
public bool UseImplicitUsings
6170
{
6271
get
@@ -68,6 +77,9 @@ public bool UseImplicitUsings
6877

6978
private bool useWpf = false;
7079

80+
/// <summary>
81+
/// True if there is an indication that WPF is used.
82+
/// </summary>
7183
public bool UseWpf
7284
{
7385
get
@@ -79,6 +91,9 @@ public bool UseWpf
7991

8092
private bool useWindowsForms = false;
8193

94+
/// <summary>
95+
/// True if there is an indication that Windows Forms is used.
96+
/// </summary>
8297
public bool UseWindowsForms
8398
{
8499
get
@@ -90,6 +105,9 @@ public bool UseWindowsForms
90105

91106
private bool isLegacyProjectStructureUsed = false;
92107

108+
/// <summary>
109+
/// True if there is an indication that the legacy project structure is used (e.g., non-SDK-style .csproj files).
110+
/// </summary>
93111
public bool IsLegacyProjectStructureUsed
94112
{
95113
get
@@ -100,6 +118,10 @@ public bool IsLegacyProjectStructureUsed
100118
}
101119

102120
private bool isNewProjectStructureUsed = false;
121+
122+
/// <summary>
123+
/// True if there is an indication that the new project structure is used (e.g., SDK-style .csproj files).
124+
/// </summary>
103125
public bool IsNewProjectStructureUsed
104126
{
105127
get
@@ -132,98 +154,146 @@ internal FileContent(ILogger logger,
132154
public FileContent(ILogger logger, IEnumerable<string> files) : this(logger, files, new UnsafeFileReader())
133155
{ }
134156

135-
private static string? GetAttribute(XmlReader reader, string name)
157+
private void AddPackageReference(string? packageName, PackageReferenceSource source)
136158
{
137-
while (reader.MoveToNextAttribute())
159+
if (!string.IsNullOrEmpty(packageName))
138160
{
139-
if (string.Equals(reader.LocalName, name, StringComparison.OrdinalIgnoreCase))
140-
{
141-
var value = reader.Value;
142-
reader.MoveToElement();
143-
return value;
144-
}
161+
allPackages.Add(new PackageReference(packageName.ToLowerInvariant(), source));
145162
}
146-
reader.MoveToElement();
147-
return null;
148163
}
149164

150-
private void AddPackageReference(string? packageName, PackageReferenceSource source)
165+
private void ProcessFile(string file, Action<XmlReader> processElement)
151166
{
152-
if (!string.IsNullOrEmpty(packageName))
167+
try
153168
{
154-
allPackages.Add(new PackageReference(packageName.ToLowerInvariant(), source));
169+
using var stream = unsafeFileReader.OpenRead(file);
170+
using var reader = XmlReader.Create(stream, new XmlReaderSettings
171+
{
172+
DtdProcessing = DtdProcessing.Prohibit,
173+
XmlResolver = null,
174+
IgnoreComments = true,
175+
IgnoreProcessingInstructions = true
176+
});
177+
178+
while (reader.Read())
179+
{
180+
if (reader.NodeType != XmlNodeType.Element)
181+
{
182+
continue;
183+
}
184+
185+
processElement(reader);
186+
}
187+
}
188+
catch (XmlException)
189+
{
190+
logger.LogDebug($"Failed to parse {file} as an XML file.");
155191
}
156192
}
157193

158-
private void DoInitialize()
194+
/// <summary>
195+
/// Parses a packages.config XML file to extract package references.
196+
/// </summary>
197+
/// <param name="file"></param>
198+
private void ParsePackagesConfigFile(string file)
159199
{
160-
foreach (var file in files)
200+
ProcessFile(file, r =>
161201
{
162-
try
202+
if (r.LocalName == "package")
163203
{
164-
var isPackagesConfig = string.Equals(FileUtils.SafeGetFileName(file, logger), "packages.config", StringComparison.OrdinalIgnoreCase);
165-
using var stream = unsafeFileReader.OpenRead(file);
166-
using var reader = XmlReader.Create(stream, new XmlReaderSettings
167-
{
168-
DtdProcessing = DtdProcessing.Prohibit,
169-
XmlResolver = null,
170-
IgnoreComments = true,
171-
IgnoreProcessingInstructions = true
172-
});
204+
AddPackageReference(r.GetAttribute("id"), PackageReferenceSource.PackagesConfig);
205+
}
206+
});
207+
}
173208

174-
while (!reader.EOF)
209+
/// <summary>
210+
/// Checks if the content of the current XML element equals any of the specified values, ignoring case.
211+
/// A new reader is created to avoid modifying the state of the original reader.
212+
/// </summary>
213+
/// <param name="reader">The XML reader positioned at the element to read.</param>
214+
/// <returns>True if the element content equals any of the specified values, ignoring case; otherwise, false.</returns>
215+
private bool ElementContentEquals(XmlReader reader, params string[] values)
216+
{
217+
using (XmlReader subReader = reader.ReadSubtree())
218+
{
219+
if (subReader.Read())
220+
{
221+
var content = subReader.ReadElementContentAsString().Trim();
222+
foreach (var value in values)
175223
{
176-
if (reader.NodeType != XmlNodeType.Element)
224+
if (string.Equals(content, value, StringComparison.OrdinalIgnoreCase))
177225
{
178-
reader.Read();
179-
continue;
226+
return true;
180227
}
228+
}
229+
}
230+
}
231+
return false;
232+
}
181233

182-
switch (reader.LocalName.ToLowerInvariant())
234+
/// <summary>
235+
/// Attempt to parse the specified file as an XML file for project dependencies and settings.
236+
/// </summary>
237+
/// <param name="file"></param>
238+
private void Parse(string file)
239+
{
240+
ProcessFile(file, reader =>
241+
{
242+
switch (reader.LocalName)
243+
{
244+
case "PackageReference":
245+
AddPackageReference(reader.GetAttribute("Include"), PackageReferenceSource.SdkCsProj);
246+
break;
247+
case "Project":
248+
var sdk = reader.GetAttribute("Sdk");
249+
isNewProjectStructureUsed |= sdk != null;
250+
useAspNetCoreDlls |= string.Equals(sdk, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase);
251+
break;
252+
case "FrameworkReference":
253+
var framework = reader.GetAttribute("Include");
254+
isNewProjectStructureUsed |= framework != null;
255+
useAspNetCoreDlls |= string.Equals(framework, "Microsoft.AspNetCore.App", StringComparison.OrdinalIgnoreCase);
256+
break;
257+
case "ImplicitUsings":
258+
useImplicitUsings = useImplicitUsings || ElementContentEquals(reader, "enable", "true");
259+
break;
260+
case "UseWPF":
261+
useWpf = useWpf || ElementContentEquals(reader, "true");
262+
break;
263+
case "UseWindowsForms":
264+
useWindowsForms = useWindowsForms || ElementContentEquals(reader, "true");
265+
break;
266+
case "Using":
267+
var ns = reader.GetAttribute("Include");
268+
if (!string.IsNullOrEmpty(ns))
183269
{
184-
case "package" when isPackagesConfig:
185-
AddPackageReference(GetAttribute(reader, "id"), PackageReferenceSource.PackagesConfig);
186-
break;
187-
case "packagereference" when !isPackagesConfig:
188-
AddPackageReference(GetAttribute(reader, "Include"), PackageReferenceSource.SdkCsProj);
189-
break;
190-
case "project":
191-
var sdk = GetAttribute(reader, "Sdk");
192-
isNewProjectStructureUsed |= sdk != null;
193-
useAspNetCoreDlls |= string.Equals(sdk, "Microsoft.NET.Sdk.Web", StringComparison.OrdinalIgnoreCase);
194-
break;
195-
case "frameworkreference":
196-
var framework = GetAttribute(reader, "Include");
197-
isNewProjectStructureUsed |= framework != null;
198-
useAspNetCoreDlls |= string.Equals(framework, "Microsoft.AspNetCore.App", StringComparison.OrdinalIgnoreCase);
199-
break;
200-
case "implicitusings":
201-
var implicitUsings = reader.ReadElementContentAsString().Trim();
202-
useImplicitUsings |= string.Equals(implicitUsings, "enable", StringComparison.OrdinalIgnoreCase)
203-
|| string.Equals(implicitUsings, "true", StringComparison.OrdinalIgnoreCase);
204-
continue;
205-
case "usewpf":
206-
useWpf |= string.Equals(reader.ReadElementContentAsString().Trim(), "true", StringComparison.OrdinalIgnoreCase);
207-
continue;
208-
case "usewindowsforms":
209-
useWindowsForms |= string.Equals(reader.ReadElementContentAsString().Trim(), "true", StringComparison.OrdinalIgnoreCase);
210-
continue;
211-
case "using":
212-
var ns = GetAttribute(reader, "Include");
213-
if (!string.IsNullOrEmpty(ns))
214-
{
215-
implicitUsingNamespaces.Add(ns);
216-
}
217-
break;
218-
case "import":
219-
isLegacyProjectStructureUsed |= GetAttribute(reader, "Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true;
220-
break;
270+
implicitUsingNamespaces.Add(ns);
221271
}
272+
break;
273+
case "Import":
274+
isLegacyProjectStructureUsed = isLegacyProjectStructureUsed || reader.GetAttribute("Project")?.EndsWith("Microsoft.CSharp.targets", StringComparison.OrdinalIgnoreCase) == true;
275+
break;
276+
}
277+
});
278+
}
222279

223-
reader.Read();
280+
private void DoInitialize()
281+
{
282+
foreach (var file in files)
283+
{
284+
try
285+
{
286+
var isPackagesConfig = string.Equals(FileUtils.SafeGetFileName(file, logger), "packages.config", StringComparison.OrdinalIgnoreCase);
287+
if (isPackagesConfig)
288+
{
289+
ParsePackagesConfigFile(file);
290+
}
291+
else
292+
{
293+
Parse(file);
224294
}
225295
}
226-
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or XmlException)
296+
catch (Exception ex)
227297
{
228298
logger.LogInfo($"Failed to read file {file}");
229299
logger.LogDebug($"Failed to read file {file}, exception: {ex}");

‎csharp/extractor/Semmle.Extraction.Tests/FileContent.cs‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ public void TestFileContent_XmlSyntax()
240240
<msb:PropertyGroup>
241241
<msb:ImplicitUsings>
242242
ENABLE
243-
</msb:ImplicitUsings><msb:UseWPF> true </msb:UseWPF><msb:UseWindowsForms><![CDATA[true]]></msb:UseWindowsForms>
243+
</msb:ImplicitUsings><msb:UseWPF> true </msb:UseWPF>
244+
<msb:UseWindowsForms><![CDATA[true]]></msb:UseWindowsForms>
244245
</msb:PropertyGroup>
245246
</msb:Project>
246247
""";
@@ -309,19 +310,6 @@ public void TestFileContent_PackagesConfig()
309310
Assert.Contains(new PackageReference("other.package", PackageReferenceSource.PackagesConfig), fileContent.AllPackages);
310311
}
311312

312-
[Fact]
313-
public void TestFileContent_CaseInsensitive()
314-
{
315-
var fileContent = new TestFileContent(new[]
316-
{
317-
"<project sdk='microsoft.net.sdk.web'><packagereference include='My.Package' /><usewpf>TRUE</usewpf></project>"
318-
});
319-
320-
Assert.True(fileContent.IsAspNetCoreDetected);
321-
Assert.True(fileContent.UseWpf);
322-
Assert.Contains(new PackageReference("my.package", PackageReferenceSource.SdkCsProj), fileContent.AllPackages);
323-
}
324-
325313
[Fact]
326314
public void TestFileContent_ReadsOnceAndDisposesStream()
327315
{
@@ -363,7 +351,7 @@ public void TestFileContent_InvalidXmlDoesNotPreventReadingOtherFiles(string inv
363351

364352
Assert.True(fileContent.IsAspNetCoreDetected);
365353
Assert.Contains(new PackageReference("my.package", PackageReferenceSource.SdkCsProj), fileContent.AllPackages);
366-
Assert.Contains("Failed to read file invalid.csproj", logger.Messages);
354+
Assert.Contains("Failed to parse invalid.csproj as an XML file.", logger.Messages);
367355
Assert.Equal(2, stub.OpenCount);
368356
}
369357
}

0 commit comments

Comments
 (0)