hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

C#: Address review comments.

Browse Source
This commit is contained in:
Michael Nebel
2023-11-06 09:52:39 +01:00
parent df4f2a367b
commit 4bcf9e50a0
4 changed files with 48 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Assets.cs
View File

@@ -3,6 +3,7 @@ using System.Collections.Generic;
using System.IO;
using System.Linq;
using Newtonsoft.Json.Linq;
using Semmle.Util;
namespace Semmle.Extraction.CSharp.DependencyFetching
{
@@ -29,21 +30,15 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
/// <summary>
/// Class needed for deserializing parts of an assets file.
/// It holds information about a reference.
///
/// Type carries the type of the reference.
/// We are only interested in package references.
///
/// Compile holds information about the files needed for compilation.
/// However, if it is a .NET framework reference we assume that all files in the
/// package are needed for compilation.
/// </summary>
private class ReferenceInfo
{
/// <summary>
/// This carries the type of the reference.
/// We are only interested in package references.
/// </summary>
public string Type { get; set; } = "";
/// <summary>
/// If not a .NET framework reference we assume that only the files mentioned
/// in the compile section are needed for compilation.
/// </summary>
public Dictionary<string, object> Compile { get; set; } = new();
}
private record class ReferenceInfo(string? Type, Dictionary<string, object>? Compile);
/// <summary>
/// Add the package dependencies from the assets file to dependencies.
@@ -74,7 +69,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
/// }
///
/// Returns dependencies
/// Required = {
/// RequiredPaths = {
/// "castle.core/4.4.1/lib/netstandard1.5/Castle.Core.dll",
/// "json.net/1.0.33/lib/netstandard2.0/Json.Net.dll"
/// }
@@ -83,7 +78,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
/// "json.net"
/// }
/// </summary>
private Dependencies AddPackageDependencies(JObject json, Dependencies dependencies)
private DependencyContainer AddPackageDependencies(JObject json, DependencyContainer dependencies)
{
// If there are more than one framework we need to pick just one.
// To ensure stability we pick one based on the lexicographic order of
@@ -103,23 +98,29 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
// Find all the compile dependencies for each reference and
// create the relative path to the dependency.
return references
.Aggregate(dependencies, (deps, r) =>
references
.ForEach(r =>
{
var info = r.Value;
var name = r.Key.ToLowerInvariant();
if (info.Type != "package")
{
return deps;
return;
}
// If this is a .NET framework reference then include everything.
return netFrameworks.Any(framework => name.StartsWith(framework))
? deps.Add(name, "")
: info
.Compile
.Aggregate(deps, (d, p) => d.Add(name, p.Key));
if (netFrameworks.Any(framework => name.StartsWith(framework)))
{
dependencies.Add(name);
}
else
{
info.Compile?
.ForEach(r => dependencies.Add(name, r.Key));
}
});
return dependencies;
}
/// <summary>
@@ -127,7 +128,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
/// (together with used package information) required for compilation.
/// </summary>
/// <returns>True if parsing succeeds, otherwise false.</returns>
public bool TryParse(string json, Dependencies dependencies)
public bool TryParse(string json, DependencyContainer dependencies)
{
try
{
@@ -142,15 +143,16 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
}
}
public static Dependencies GetCompilationDependencies(ProgressMonitor progressMonitor, IEnumerable<string> assets)
public static DependencyContainer GetCompilationDependencies(ProgressMonitor progressMonitor, IEnumerable<string> assets)
{
var parser = new Assets(progressMonitor);
return assets.Aggregate(new Dependencies(), (dependencies, asset) =>
var dependencies = new DependencyContainer();
assets.ForEach(asset =>
{
var json = File.ReadAllText(asset);
parser.TryParse(json, dependencies);
return dependencies;
});
return dependencies;
}
}

20
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Dependencies.cs → csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyContainer.cs
View File

@@ -7,9 +7,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
/// <summary>
/// Container class for dependencies found in the assets file.
/// </summary>
internal class Dependencies
internal class DependencyContainer
{
private readonly List<string> required = new();
private readonly List<string> requiredPaths = new();
private readonly HashSet<string> usedPackages = new();
/// <summary>
@@ -33,9 +33,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
.First();
/// <summary>
/// Dependencies required for Compilation.
/// Paths to dependencies required for compilation.
/// </summary>
public IEnumerable<string> Required => required;
public IEnumerable<string> RequiredPaths => requiredPaths;
/// <summary>
/// Packages that are used as a part of the required dependencies.
@@ -45,29 +45,25 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
/// <summary>
/// Add a dependency inside a package.
/// </summary>
public Dependencies Add(string package, string dependency)
public void Add(string package, string dependency)
{
var p = package.Replace('/', Path.DirectorySeparatorChar);
var d = dependency.Replace('/', Path.DirectorySeparatorChar);
var path = Path.Combine(p, ParseFilePath(d));
required.Add(path);
requiredPaths.Add(path);
usedPackages.Add(GetPackageName(p));
return this;
}
/// <summary>
/// Add a dependency to an entire package
/// </summary>
public Dependencies Add(string package)
public void Add(string package)
{
var p = package.Replace('/', Path.DirectorySeparatorChar);
required.Add(p);
requiredPaths.Add(p);
usedPackages.Add(GetPackageName(p));
return this;
}
}
}

4
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
View File

@@ -104,7 +104,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
var dependencies = Assets.GetCompilationDependencies(progressMonitor, assets1.Union(assets2));
var paths = dependencies
.Required
.RequiredPaths
.Select(d => Path.Combine(packageDirectory.DirInfo.FullName, d))
.ToList();
dllPaths.AddRange(paths);
@@ -303,7 +303,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
.Select(d => d.FullName);
}
private void LogAllUnusedPackages(Dependencies dependencies) =>
private void LogAllUnusedPackages(DependencyContainer dependencies) =>
GetAllPackageDirectories()
.Where(package => !dependencies.UsedPackages.Contains(package))
.ForEach(package => progressMonitor.LogInfo($"Unused package: {package}"));

16
csharp/extractor/Semmle.Extraction.Tests/Assets.cs
View File

@@ -14,17 +14,17 @@ namespace Semmle.Extraction.Tests
// Setup
var assets = new Assets(new ProgressMonitor(new LoggerStub()));
var json = assetsJson1;
var dependencies = new Dependencies();
var dependencies = new DependencyContainer();
// Execute
var success = assets.TryParse(json, dependencies);
// Verify
Assert.True(success);
Assert.Equal(5, dependencies.Required.Count());
Assert.Equal(5, dependencies.RequiredPaths.Count());
Assert.Equal(4, dependencies.UsedPackages.Count());
var normalizedPaths = dependencies.Required.Select(FixExpectedPathOnWindows);
var normalizedPaths = dependencies.RequiredPaths.Select(FixExpectedPathOnWindows);
// Required references
Assert.Contains("castle.core/4.4.1/lib/netstandard1.5/Castle.Core.dll", normalizedPaths);
Assert.Contains("castle.core/4.4.1/lib/netstandard1.5/Castle.Core2.dll", normalizedPaths);
@@ -44,16 +44,16 @@ namespace Semmle.Extraction.Tests
// Setup
var assets = new Assets(new ProgressMonitor(new LoggerStub()));
var json = assetsJson2;
var dependencies = new Dependencies();
var dependencies = new DependencyContainer();
// Execute
var success = assets.TryParse(json, dependencies);
// Verify
Assert.True(success);
Assert.Equal(2, dependencies.Required.Count());
Assert.Equal(2, dependencies.RequiredPaths.Count());
var normalizedPaths = dependencies.Required.Select(FixExpectedPathOnWindows);
var normalizedPaths = dependencies.RequiredPaths.Select(FixExpectedPathOnWindows);
// Required references
Assert.Contains("microsoft.netframework.referenceassemblies/1.0.3", normalizedPaths);
Assert.Contains("microsoft.netframework.referenceassemblies.net48/1.0.3", normalizedPaths);
@@ -68,14 +68,14 @@ namespace Semmle.Extraction.Tests
// Setup
var assets = new Assets(new ProgressMonitor(new LoggerStub()));
var json = "garbage data";
var dependencies = new Dependencies();
var dependencies = new DependencyContainer();
// Execute
var success = assets.TryParse(json, dependencies);
// Verify
Assert.False(success);
Assert.Empty(dependencies.Required);
Assert.Empty(dependencies.RequiredPaths);
}
private readonly string assetsJson1 = """