Skip to content

Commit 04c4e73

Browse files
hvitvedhvitved
committed
Address review comments
1 parent 2343e5e commit 04c4e73

File tree

4 files changed

+38
-58
lines changed

4 files changed

+38
-58
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyStubGenerator/Program.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
using Semmle.Extraction.CSharp.StubGenerator;
33
using Semmle.Util.Logging;
44

5-
var logger = new ConsoleLogger(Verbosity.Info);
5+
var logger = new ConsoleLogger(Verbosity.Info, logThreadId: false);
66
using var dependencyManager = new DependencyManager(".", DependencyOptions.Default, logger);
77
StubGenerator.GenerateStubs(logger, dependencyManager.ReferenceFiles, "codeql_csharp_stubs");
88

‎csharp/extractor/Semmle.Extraction.CSharp.StubGenerator/StubGenerator.cs

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,12 @@ public static void GenerateStubs(ILogger logger, IEnumerable<string> referencesP
2727
var threads = EnvironmentVariables.GetDefaultNumberOfThreads();
2828

2929
using var references = new BlockingCollection<(MetadataReference Reference, string Path)>();
30-
var referenceResolveTasks = GetResolvedReferenceTasks(referencesPaths, references);
3130

32-
Parallel.Invoke(
33-
new ParallelOptions { MaxDegreeOfParallelism = threads },
34-
referenceResolveTasks.ToArray());
31+
Parallel.ForEach(referencesPaths, new ParallelOptions { MaxDegreeOfParallelism = threads }, path =>
32+
{
33+
var reference = MetadataReference.CreateFromFile(path);
34+
references.Add((reference, path));
35+
});
3536

3637
logger.Log(Severity.Info, $"Generating stubs for {references.Count} assemblies.");
3738

@@ -41,43 +42,33 @@ public static void GenerateStubs(ILogger logger, IEnumerable<string> referencesP
4142
references.Select(tuple => tuple.Item1),
4243
new CSharpCompilationOptions(OutputKind.ConsoleApplication, allowUnsafe: true));
4344

44-
var referenceStubTasks = references.Select(@ref => (Action)(() => StubReference(compilation, outputPath, @ref.Reference, @ref.Path)));
45-
Parallel.Invoke(
46-
new ParallelOptions { MaxDegreeOfParallelism = threads },
47-
referenceStubTasks.ToArray());
45+
Parallel.ForEach(references, new ParallelOptions { MaxDegreeOfParallelism = threads }, @ref =>
46+
{
47+
StubReference(logger, compilation, outputPath, @ref.Reference, @ref.Path);
48+
});
4849

4950
stopWatch.Stop();
5051
logger.Log(Severity.Info, $"Stub generation took {stopWatch.Elapsed}.");
5152
}
5253

53-
private static IEnumerable<Action> GetResolvedReferenceTasks(IEnumerable<string> referencePaths, BlockingCollection<(MetadataReference, string)> references)
54+
private static void StubReference(ILogger logger, CSharpCompilation compilation, string outputPath, MetadataReference reference, string path)
5455
{
55-
return referencePaths.Select<string, Action>(path => () =>
56-
{
57-
var reference = MetadataReference.CreateFromFile(path);
58-
references.Add((reference, path));
59-
});
60-
}
56+
if (compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol assembly)
57+
return;
6158

62-
private static void StubReference(CSharpCompilation compilation, string outputPath, MetadataReference reference, string path)
63-
{
64-
if (compilation.GetAssemblyOrModuleSymbol(reference) is IAssemblySymbol assembly)
65-
{
66-
var logger = new ConsoleLogger(Verbosity.Info);
67-
using var fileStream = new FileStream(FileUtils.NestPaths(logger, outputPath, path.Replace(".dll", ".cs")), FileMode.Create, FileAccess.Write);
68-
using var writer = new StreamWriter(fileStream, new UTF8Encoding(false));
59+
using var fileStream = new FileStream(FileUtils.NestPaths(logger, outputPath, path.Replace(".dll", ".cs")), FileMode.Create, FileAccess.Write);
60+
using var writer = new StreamWriter(fileStream, new UTF8Encoding(false));
6961

70-
writer.WriteLine("// This file contains auto-generated code.");
71-
writer.WriteLine($"// Generated from `{assembly.Identity}`.");
62+
writer.WriteLine("// This file contains auto-generated code.");
63+
writer.WriteLine($"// Generated from `{assembly.Identity}`.");
7264

73-
var visitor = new StubVisitor(assembly, writer);
65+
var visitor = new StubVisitor(assembly, writer);
7466

75-
visitor.StubAttributes(assembly.GetAttributes(), "assembly: ");
67+
visitor.StubAttributes(assembly.GetAttributes(), "assembly: ");
7668

77-
foreach (var module in assembly.Modules)
78-
{
79-
module.GlobalNamespace.Accept(new StubVisitor(assembly, writer));
80-
}
69+
foreach (var module in assembly.Modules)
70+
{
71+
module.GlobalNamespace.Accept(visitor);
8172
}
8273
}
8374
}

‎csharp/extractor/Semmle.Extraction.CSharp.StubGenerator/StubVisitor.cs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ private bool IsRelevantNamedType(INamedTypeSymbol symbol) =>
3636
IsRelevantBaseType(symbol) &&
3737
SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, assembly);
3838

39-
private bool IsRelevantNamespace(INamespaceSymbol symbol) => isRelevantNamespace[symbol];
39+
private bool IsRelevantNamespace(INamespaceSymbol symbol) => isRelevantNamespace.Invoke(symbol);
4040

4141
private void StubExplicitInterface(ISymbol symbol, ISymbol? explicitInterfaceSymbol, bool writeName = true)
4242
{
@@ -109,7 +109,7 @@ private void StubAccessibility(Accessibility accessibility)
109109
case Accessibility.Internal:
110110
stubWriter.Write("internal ");
111111
break;
112-
case Accessibility.ProtectedAndInternal or Accessibility.ProtectedOrInternal:
112+
case Accessibility.ProtectedAndInternal:
113113
stubWriter.Write("protected internal ");
114114
break;
115115
default:
@@ -156,7 +156,7 @@ private void StubModifiers(ISymbol symbol, bool skipAccessibility = false)
156156
stubWriter.Write("extern ");
157157
}
158158

159-
public void StubTypedConstant(TypedConstant c)
159+
private void StubTypedConstant(TypedConstant c)
160160
{
161161
switch (c.Kind)
162162
{
@@ -221,7 +221,9 @@ private void StubAttribute(AttributeData a, string prefix)
221221
if (!attributeAllowList.Contains(qualifiedName))
222222
return;
223223

224-
stubWriter.Write($"[{prefix}{qualifiedName.AsSpan(0, @class.GetQualifiedName().Length - 9)}");
224+
if (qualifiedName.EndsWith("Attribute"))
225+
qualifiedName = qualifiedName[..^9];
226+
stubWriter.Write($"[{prefix}{qualifiedName}");
225227
if (a.ConstructorArguments.Any())
226228
{
227229
stubWriter.Write("(");
@@ -295,12 +297,8 @@ private static bool IsUnsafe(ITypeSymbol symbol) =>
295297
"volatile", "while"
296298
};
297299

298-
private static string EscapeIdentifier(string identifier)
299-
{
300-
if (keywords.Contains(identifier))
301-
return "@" + identifier;
302-
return identifier;
303-
}
300+
private static string EscapeIdentifier(string identifier) =>
301+
keywords.Contains(identifier) ? "@" + identifier : identifier;
304302

305303
public override void VisitField(IFieldSymbol symbol)
306304
{
@@ -739,7 +737,7 @@ public override void VisitNamedType(INamedTypeSymbol symbol)
739737
else
740738
{
741739
var seenCtor = false;
742-
foreach (var childSymbol in symbol.GetMembers())
740+
foreach (var childSymbol in symbol.GetMembers().OrderBy(m => m.GetName()))
743741
{
744742
seenCtor |= childSymbol is IMethodSymbol method && method.MethodKind == MethodKind.Constructor;
745743
childSymbol.Accept(this);
@@ -768,7 +766,7 @@ public override void VisitNamespace(INamespaceSymbol symbol)
768766
if (!isGlobal)
769767
stubWriter.WriteLine($"namespace {symbol.Name} {{");
770768

771-
foreach (var childSymbol in symbol.GetMembers())
769+
foreach (var childSymbol in symbol.GetMembers().OrderBy(m => m.GetName()))
772770
{
773771
childSymbol.Accept(this);
774772
}

‎csharp/extractor/Semmle.Util/MemoizedFunc.cs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,14 @@ public MemoizedFunc(Func<T1, T2> f)
1414
this.f = f;
1515
}
1616

17-
public T2 this[T1 s]
17+
public T2 Invoke(T1 s)
1818
{
19-
get
19+
if (!cache.TryGetValue(s, out var t))
2020
{
21-
if (!cache.TryGetValue(s, out var t))
22-
{
23-
t = f(s);
24-
cache[s] = t;
25-
}
26-
return t;
21+
t = f(s);
22+
cache[s] = t;
2723
}
24+
return t;
2825
}
2926
}
3027

@@ -38,11 +35,5 @@ public ConcurrentMemoizedFunc(Func<T1, T2> f)
3835
this.f = f;
3936
}
4037

41-
public T2 this[T1 s]
42-
{
43-
get
44-
{
45-
return cache.GetOrAdd(s, f);
46-
}
47-
}
38+
public T2 Invoke(T1 s) => cache.GetOrAdd(s, f);
4839
}

0 commit comments

Comments
 (0)