From e8e1dc03901193df543c6e1f69353c5cdc995674 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 18 Mar 2024 14:23:40 +0100 Subject: [PATCH 1/5] C#: Add integration test with extraction and compilation messages --- .../all-platforms/standalone/CompilerMessage.expected | 10 ++++++++++ .../all-platforms/standalone/CompilerMessage.ql | 6 ++++++ .../standalone/ExtractortMessages.expected | 8 ++++++++ .../all-platforms/standalone/ExtractortMessages.ql | 6 ++++++ .../all-platforms/standalone/Program.cs | 3 ++- 5 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected create mode 100644 csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql create mode 100644 csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected create mode 100644 csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql diff --git a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected new file mode 100644 index 000000000000..94606c655478 --- /dev/null +++ b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected @@ -0,0 +1,10 @@ +| Program.cs:2:9:2:9 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | +| Program.cs:2:15:2:15 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | +| Program.cs:2:21:2:21 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | +| test-db/working/implicitUsings/GlobalUsings.g.cs:3:1:3:28 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | +| test-db/working/implicitUsings/GlobalUsings.g.cs:4:1:4:48 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | +| test-db/working/implicitUsings/GlobalUsings.g.cs:5:1:5:31 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | +| test-db/working/implicitUsings/GlobalUsings.g.cs:6:1:6:33 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | +| test-db/working/implicitUsings/GlobalUsings.g.cs:7:1:7:37 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | +| test-db/working/implicitUsings/GlobalUsings.g.cs:8:1:8:38 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | +| test-db/working/implicitUsings/GlobalUsings.g.cs:9:1:9:44 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql new file mode 100644 index 000000000000..495226cf99a5 --- /dev/null +++ b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql @@ -0,0 +1,6 @@ +import csharp +import semmle.code.csharp.commons.Diagnostics + +from Diagnostic diagnostic +select diagnostic, + diagnostic.getSeverityText() + " " + diagnostic.getTag() + " " + diagnostic.getFullMessage() diff --git a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected new file mode 100644 index 000000000000..b8a1ea220792 --- /dev/null +++ b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected @@ -0,0 +1,8 @@ +| Program.cs:2:9:2:11 | Failed to determine type | 5 | M() | +| Program.cs:2:9:2:11 | Unable to resolve target for call. (Compilation error?) | 5 | M() | +| Program.cs:2:9:2:17 | Failed to determine type | 5 | M() + M() | +| Program.cs:2:9:2:23 | Failed to determine type | 5 | M() + M() + M() | +| Program.cs:2:15:2:17 | Failed to determine type | 5 | M() | +| Program.cs:2:15:2:17 | Unable to resolve target for call. (Compilation error?) | 5 | M() | +| Program.cs:2:21:2:23 | Failed to determine type | 5 | M() | +| Program.cs:2:21:2:23 | Unable to resolve target for call. (Compilation error?) | 5 | M() | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql new file mode 100644 index 000000000000..10b9822fc2e6 --- /dev/null +++ b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql @@ -0,0 +1,6 @@ +import csharp +import semmle.code.csharp.commons.Diagnostics + +query predicate extractorMessages(ExtractorMessage msg, int severity, string elementText) { + msg.getSeverity() = severity and msg.getElementText() = elementText +} diff --git a/csharp/ql/integration-tests/all-platforms/standalone/Program.cs b/csharp/ql/integration-tests/all-platforms/standalone/Program.cs index 47eee48cc791..371ed98c7247 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone/Program.cs +++ b/csharp/ql/integration-tests/all-platforms/standalone/Program.cs @@ -1 +1,2 @@ -var dummy = "dummy"; \ No newline at end of file +var dummy = "dummy"; +dummy = M() + M() + M(); \ No newline at end of file From d749335f54bf5714ead1a2a39376ab1c6c24693e Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 18 Mar 2024 14:24:34 +0100 Subject: [PATCH 2/5] C#: Limit extracted compilation and extraction messages --- .../Entities/Compilations/Compilation.cs | 3 +- .../Compilations/CompilerDiagnostic.cs | 45 +++++++++++++++++++ .../Entities/Compilations/Diagnostic.cs | 21 --------- .../Semmle.Extraction.CSharp/Tuples.cs | 4 +- .../Entities/ExtractionError.cs | 21 --------- .../Entities/ExtractionMessage.cs | 37 +++++++++++++++ .../Semmle.Util/EnvironmentVariables.cs | 12 +++++ .../standalone/CompilerMessage.expected | 6 --- .../standalone/ExtractortMessages.expected | 3 -- .../all-platforms/standalone/test.py | 3 ++ 10 files changed, 100 insertions(+), 55 deletions(-) create mode 100644 csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs delete mode 100644 csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Diagnostic.cs delete mode 100644 csharp/extractor/Semmle.Extraction/Entities/ExtractionError.cs create mode 100644 csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs index a194323c9a61..e9706b3b5795 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs @@ -80,8 +80,7 @@ public override void Populate(TextWriter trapFile) // Diagnostics Context.Compilation .GetDiagnostics() - .Select(d => new Diagnostic(Context, d)) - .ForEach((diag, index) => trapFile.diagnostic_for(diag, this, 0, index)); + .ForEach((diag, index) => new CompilerDiagnostic(Context, diag, this, index)); } public void PopulatePerformance(PerformanceMetrics p) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs new file mode 100644 index 000000000000..8de1442f26ed --- /dev/null +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs @@ -0,0 +1,45 @@ +using System.Collections.Concurrent; +using System.IO; +using Semmle.Util; + +namespace Semmle.Extraction.CSharp.Entities +{ + internal class CompilerDiagnostic : FreshEntity + { + private static readonly int limit = EnvironmentVariables.TryGetExtractorOption("COMPILER_DIAGNOSTIC_LIMIT") ?? 1000; + private static readonly ConcurrentDictionary messageCounts = new(); + + private readonly Microsoft.CodeAnalysis.Diagnostic diagnostic; + private readonly Compilation compilation; + private readonly int index; + + public CompilerDiagnostic(Context cx, Microsoft.CodeAnalysis.Diagnostic diag, Compilation compilation, int index) : base(cx) + { + diagnostic = diag; + this.compilation = compilation; + this.index = index; + TryPopulate(); + } + + protected override void Populate(TextWriter trapFile) + { + // The below doesn't limit the extractor messages to the exact limit, but it's good enough. + var key = diagnostic.Id; + var messageCount = messageCounts.AddOrUpdate(key, 1, (_, c) => c + 1); + if (messageCount > limit) + { + if (messageCount == limit + 1) + { + Context.Extractor.Logger.LogWarning($"Stopped logging {key} compiler diagnostics after reaching {limit}"); + } + + return; + } + + trapFile.diagnostics(this, (int)diagnostic.Severity, key, diagnostic.Descriptor.Title.ToString(), + diagnostic.GetMessage(), Context.CreateLocation(diagnostic.Location)); + + trapFile.diagnostic_for(this, compilation, 0, index); + } + } +} diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Diagnostic.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Diagnostic.cs deleted file mode 100644 index a53ee5797f27..000000000000 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Diagnostic.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.IO; - -namespace Semmle.Extraction.CSharp.Entities -{ - internal class Diagnostic : FreshEntity - { - private readonly Microsoft.CodeAnalysis.Diagnostic diagnostic; - - public Diagnostic(Context cx, Microsoft.CodeAnalysis.Diagnostic diag) : base(cx) - { - diagnostic = diag; - TryPopulate(); - } - - protected override void Populate(TextWriter trapFile) - { - trapFile.diagnostics(this, (int)diagnostic.Severity, diagnostic.Id, diagnostic.Descriptor.Title.ToString(), - diagnostic.GetMessage(), Context.CreateLocation(diagnostic.Location)); - } - } -} diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs b/csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs index 71ed85cb201c..9d4f913ff9c3 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs @@ -122,10 +122,10 @@ internal static void destructor_location(this TextWriter trapFile, Destructor de internal static void destructors(this TextWriter trapFile, Destructor destructor, string name, Type containingType, Destructor original) => trapFile.WriteTuple("destructors", destructor, name, containingType, original); - internal static void diagnostic_for(this TextWriter trapFile, Diagnostic diag, Compilation comp, int fileNo, int index) => + internal static void diagnostic_for(this TextWriter trapFile, CompilerDiagnostic diag, Compilation comp, int fileNo, int index) => trapFile.WriteTuple("diagnostic_for", diag, comp, fileNo, index); - internal static void diagnostics(this TextWriter trapFile, Diagnostic diag, int severity, string errorTag, string errorMessage, string fullErrorMessage, Location location) => + internal static void diagnostics(this TextWriter trapFile, CompilerDiagnostic diag, int severity, string errorTag, string errorMessage, string fullErrorMessage, Location location) => trapFile.WriteTuple("diagnostics", diag, severity, errorTag, errorMessage, fullErrorMessage, location); internal static void dynamic_member_name(this TextWriter trapFile, Expression e, string name) => diff --git a/csharp/extractor/Semmle.Extraction/Entities/ExtractionError.cs b/csharp/extractor/Semmle.Extraction/Entities/ExtractionError.cs deleted file mode 100644 index 99f175377909..000000000000 --- a/csharp/extractor/Semmle.Extraction/Entities/ExtractionError.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.IO; - -namespace Semmle.Extraction.Entities -{ - internal class ExtractionMessage : FreshEntity - { - private readonly Message msg; - - public ExtractionMessage(Context cx, Message msg) : base(cx) - { - this.msg = msg; - TryPopulate(); - } - - protected override void Populate(TextWriter trapFile) - { - trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText ?? string.Empty, - msg.Location ?? Context.CreateLocation(), msg.StackTrace ?? string.Empty); - } - } -} diff --git a/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs b/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs new file mode 100644 index 000000000000..35df66a6b0a9 --- /dev/null +++ b/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs @@ -0,0 +1,37 @@ +using System.IO; +using System.Threading; +using Semmle.Util; + +namespace Semmle.Extraction.Entities +{ + internal class ExtractionMessage : FreshEntity + { + private static readonly int limit = EnvironmentVariables.TryGetExtractorOption("MESSAGE_LIMIT") ?? 10000; + private static int messageCount = 0; + + private readonly Message msg; + + public ExtractionMessage(Context cx, Message msg) : base(cx) + { + this.msg = msg; + TryPopulate(); + } + + protected override void Populate(TextWriter trapFile) + { + // The below doesn't limit the extractor messages to the exact limit, but it's good enough. + Interlocked.Increment(ref messageCount); + if (messageCount > limit) + { + if (messageCount == limit + 1) + { + Context.Extractor.Logger.LogWarning($"Stopped logging extractor messages after reaching {limit}"); + } + return; + } + + trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText ?? string.Empty, + msg.Location ?? Context.CreateLocation(), msg.StackTrace ?? string.Empty); + } + } +} diff --git a/csharp/extractor/Semmle.Util/EnvironmentVariables.cs b/csharp/extractor/Semmle.Util/EnvironmentVariables.cs index 9dcccf6d8785..d7bb81440ecd 100644 --- a/csharp/extractor/Semmle.Util/EnvironmentVariables.cs +++ b/csharp/extractor/Semmle.Util/EnvironmentVariables.cs @@ -1,4 +1,6 @@ using System; +using System.Globalization; +using System.Numerics; namespace Semmle.Util { @@ -7,6 +9,16 @@ public class EnvironmentVariables public static string? GetExtractorOption(string name) => Environment.GetEnvironmentVariable($"CODEQL_EXTRACTOR_CSHARP_OPTION_{name.ToUpper()}"); + public static T? TryGetExtractorOption(string name) where T : struct, INumberBase + { + var value = GetExtractorOption(name); + if (T.TryParse(value, NumberStyles.Number, CultureInfo.InvariantCulture, out var result)) + { + return result; + } + return null; + } + public static int GetDefaultNumberOfThreads() { if (!int.TryParse(Environment.GetEnvironmentVariable("CODEQL_THREADS"), out var threads) || threads == -1) diff --git a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected index 94606c655478..cd10fe6d1f09 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected @@ -1,10 +1,4 @@ | Program.cs:2:9:2:9 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | | Program.cs:2:15:2:15 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | -| Program.cs:2:21:2:21 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | -| test-db/working/implicitUsings/GlobalUsings.g.cs:3:1:3:28 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | -| test-db/working/implicitUsings/GlobalUsings.g.cs:4:1:4:48 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | -| test-db/working/implicitUsings/GlobalUsings.g.cs:5:1:5:31 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | -| test-db/working/implicitUsings/GlobalUsings.g.cs:6:1:6:33 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | | test-db/working/implicitUsings/GlobalUsings.g.cs:7:1:7:37 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | | test-db/working/implicitUsings/GlobalUsings.g.cs:8:1:8:38 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | -| test-db/working/implicitUsings/GlobalUsings.g.cs:9:1:9:44 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected index b8a1ea220792..02b76fec42a7 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected @@ -1,8 +1,5 @@ | Program.cs:2:9:2:11 | Failed to determine type | 5 | M() | -| Program.cs:2:9:2:11 | Unable to resolve target for call. (Compilation error?) | 5 | M() | | Program.cs:2:9:2:17 | Failed to determine type | 5 | M() + M() | | Program.cs:2:9:2:23 | Failed to determine type | 5 | M() + M() + M() | -| Program.cs:2:15:2:17 | Failed to determine type | 5 | M() | -| Program.cs:2:15:2:17 | Unable to resolve target for call. (Compilation error?) | 5 | M() | | Program.cs:2:21:2:23 | Failed to determine type | 5 | M() | | Program.cs:2:21:2:23 | Unable to resolve target for call. (Compilation error?) | 5 | M() | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/test.py b/csharp/ql/integration-tests/all-platforms/standalone/test.py index a17966e148a9..cd9d65c57410 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone/test.py +++ b/csharp/ql/integration-tests/all-platforms/standalone/test.py @@ -1,3 +1,6 @@ +import os from create_database_utils import * +os.environ['CODEQL_EXTRACTOR_CSHARP_OPTION_COMPILER_DIAGNOSTIC_LIMIT'] = '2' +os.environ['CODEQL_EXTRACTOR_CSHARP_OPTION_MESSAGE_LIMIT'] = '5' run_codeql_database_create([], lang="csharp", extra_args=["--build-mode=none"]) From 322fb6c507aec483de3bd06bba26f6d9304bf406 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 18 Mar 2024 14:53:49 +0100 Subject: [PATCH 3/5] Change integration test to return stable results --- .../all-platforms/standalone/CompilerMessage.expected | 4 ---- .../all-platforms/standalone/CompilerMessage.ql | 6 ------ .../all-platforms/standalone/Diag.expected | 4 ++++ .../ql/integration-tests/all-platforms/standalone/Diag.ql | 6 ++++++ .../all-platforms/standalone/ExtractortMessages.expected | 5 ----- .../all-platforms/standalone/ExtractortMessages.ql | 6 ------ 6 files changed, 10 insertions(+), 21 deletions(-) delete mode 100644 csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected delete mode 100644 csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql create mode 100644 csharp/ql/integration-tests/all-platforms/standalone/Diag.expected create mode 100644 csharp/ql/integration-tests/all-platforms/standalone/Diag.ql delete mode 100644 csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected delete mode 100644 csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql diff --git a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected deleted file mode 100644 index cd10fe6d1f09..000000000000 --- a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.expected +++ /dev/null @@ -1,4 +0,0 @@ -| Program.cs:2:9:2:9 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | -| Program.cs:2:15:2:15 | CS0103: The name 'M' does not exist in the current context | Error CS0103 The name 'M' does not exist in the current context | -| test-db/working/implicitUsings/GlobalUsings.g.cs:7:1:7:37 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | -| test-db/working/implicitUsings/GlobalUsings.g.cs:8:1:8:38 | CS8019: Unnecessary using directive. | Hidden CS8019 Unnecessary using directive. | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql b/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql deleted file mode 100644 index 495226cf99a5..000000000000 --- a/csharp/ql/integration-tests/all-platforms/standalone/CompilerMessage.ql +++ /dev/null @@ -1,6 +0,0 @@ -import csharp -import semmle.code.csharp.commons.Diagnostics - -from Diagnostic diagnostic -select diagnostic, - diagnostic.getSeverityText() + " " + diagnostic.getTag() + " " + diagnostic.getFullMessage() diff --git a/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected b/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected new file mode 100644 index 000000000000..1fcc47687caf --- /dev/null +++ b/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected @@ -0,0 +1,4 @@ +extractorMessages +| 5 | +compilerDiagnostics +| 4 | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql b/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql new file mode 100644 index 000000000000..bbd142d3af3e --- /dev/null +++ b/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql @@ -0,0 +1,6 @@ +import csharp +import semmle.code.csharp.commons.Diagnostics + +query predicate extractorMessages(int c) { c = count(ExtractorMessage msg) } + +query predicate compilerDiagnostics(int c) { c = count(Diagnostic diag) } diff --git a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected deleted file mode 100644 index 02b76fec42a7..000000000000 --- a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.expected +++ /dev/null @@ -1,5 +0,0 @@ -| Program.cs:2:9:2:11 | Failed to determine type | 5 | M() | -| Program.cs:2:9:2:17 | Failed to determine type | 5 | M() + M() | -| Program.cs:2:9:2:23 | Failed to determine type | 5 | M() + M() + M() | -| Program.cs:2:21:2:23 | Failed to determine type | 5 | M() | -| Program.cs:2:21:2:23 | Unable to resolve target for call. (Compilation error?) | 5 | M() | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql b/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql deleted file mode 100644 index 10b9822fc2e6..000000000000 --- a/csharp/ql/integration-tests/all-platforms/standalone/ExtractortMessages.ql +++ /dev/null @@ -1,6 +0,0 @@ -import csharp -import semmle.code.csharp.commons.Diagnostics - -query predicate extractorMessages(ExtractorMessage msg, int severity, string elementText) { - msg.getSeverity() = severity and msg.getElementText() = elementText -} From fa7f437e7142207c74435b7b68158a3245b474ae Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 22 Mar 2024 08:16:11 +0100 Subject: [PATCH 4/5] Code quality improvement --- .../Entities/Compilations/CompilerDiagnostic.cs | 2 +- .../extractor/Semmle.Extraction/Entities/ExtractionMessage.cs | 2 +- csharp/extractor/Semmle.Util/EnvironmentVariables.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs index 8de1442f26ed..193869e6e804 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs @@ -6,7 +6,7 @@ namespace Semmle.Extraction.CSharp.Entities { internal class CompilerDiagnostic : FreshEntity { - private static readonly int limit = EnvironmentVariables.TryGetExtractorOption("COMPILER_DIAGNOSTIC_LIMIT") ?? 1000; + private static readonly int limit = EnvironmentVariables.TryGetExtractorNumberOption("COMPILER_DIAGNOSTIC_LIMIT") ?? 1000; private static readonly ConcurrentDictionary messageCounts = new(); private readonly Microsoft.CodeAnalysis.Diagnostic diagnostic; diff --git a/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs b/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs index 35df66a6b0a9..bc6ea5aa27dc 100644 --- a/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs +++ b/csharp/extractor/Semmle.Extraction/Entities/ExtractionMessage.cs @@ -6,7 +6,7 @@ namespace Semmle.Extraction.Entities { internal class ExtractionMessage : FreshEntity { - private static readonly int limit = EnvironmentVariables.TryGetExtractorOption("MESSAGE_LIMIT") ?? 10000; + private static readonly int limit = EnvironmentVariables.TryGetExtractorNumberOption("MESSAGE_LIMIT") ?? 10000; private static int messageCount = 0; private readonly Message msg; diff --git a/csharp/extractor/Semmle.Util/EnvironmentVariables.cs b/csharp/extractor/Semmle.Util/EnvironmentVariables.cs index d7bb81440ecd..c96aa16357c3 100644 --- a/csharp/extractor/Semmle.Util/EnvironmentVariables.cs +++ b/csharp/extractor/Semmle.Util/EnvironmentVariables.cs @@ -9,7 +9,7 @@ public class EnvironmentVariables public static string? GetExtractorOption(string name) => Environment.GetEnvironmentVariable($"CODEQL_EXTRACTOR_CSHARP_OPTION_{name.ToUpper()}"); - public static T? TryGetExtractorOption(string name) where T : struct, INumberBase + public static T? TryGetExtractorNumberOption(string name) where T : struct, INumberBase { var value = GetExtractorOption(name); if (T.TryParse(value, NumberStyles.Number, CultureInfo.InvariantCulture, out var result)) From 205d6a3bc5e55e9a2e4cfa9e522cae1219c22461 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 22 Mar 2024 08:53:45 +0100 Subject: [PATCH 5/5] Extract total number of diagnostic per ID and compilation --- .../Entities/Compilations/Compilation.cs | 11 ++++++++--- .../Entities/Compilations/CompilerDiagnostic.cs | 6 ++---- .../all-platforms/standalone/Diag.expected | 3 +++ .../all-platforms/standalone/Diag.ql | 8 ++++++++ .../CompilationInfo.ql | 1 + csharp/ql/src/Telemetry/ExtractorInformation.ql | 1 + 6 files changed, 23 insertions(+), 7 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs index e9706b3b5795..0b575df2b696 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/Compilation.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.IO; using System.Linq; using Microsoft.CodeAnalysis; @@ -8,6 +9,8 @@ namespace Semmle.Extraction.CSharp.Entities { internal class Compilation : CachedEntity { + internal readonly ConcurrentDictionary messageCounts = new(); + private static (string Cwd, string[] Args) settings; private static int hashCode; @@ -78,9 +81,11 @@ public override void Populate(TextWriter trapFile) .ForEach((file, index) => trapFile.compilation_referencing_files(this, index, file)); // Diagnostics - Context.Compilation - .GetDiagnostics() - .ForEach((diag, index) => new CompilerDiagnostic(Context, diag, this, index)); + var diags = Context.Compilation.GetDiagnostics(); + diags.ForEach((diag, index) => new CompilerDiagnostic(Context, diag, this, index)); + + var diagCounts = diags.GroupBy(diag => diag.Id).ToDictionary(group => group.Key, group => group.Count()); + diagCounts.ForEach(pair => trapFile.compilation_info(this, $"Compiler diagnostic count for {pair.Key}", pair.Value.ToString())); } public void PopulatePerformance(PerformanceMetrics p) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs index 193869e6e804..c1227f2ffc0b 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Compilations/CompilerDiagnostic.cs @@ -1,4 +1,3 @@ -using System.Collections.Concurrent; using System.IO; using Semmle.Util; @@ -7,7 +6,6 @@ namespace Semmle.Extraction.CSharp.Entities internal class CompilerDiagnostic : FreshEntity { private static readonly int limit = EnvironmentVariables.TryGetExtractorNumberOption("COMPILER_DIAGNOSTIC_LIMIT") ?? 1000; - private static readonly ConcurrentDictionary messageCounts = new(); private readonly Microsoft.CodeAnalysis.Diagnostic diagnostic; private readonly Compilation compilation; @@ -25,12 +23,12 @@ protected override void Populate(TextWriter trapFile) { // The below doesn't limit the extractor messages to the exact limit, but it's good enough. var key = diagnostic.Id; - var messageCount = messageCounts.AddOrUpdate(key, 1, (_, c) => c + 1); + var messageCount = compilation.messageCounts.AddOrUpdate(key, 1, (_, c) => c + 1); if (messageCount > limit) { if (messageCount == limit + 1) { - Context.Extractor.Logger.LogWarning($"Stopped logging {key} compiler diagnostics after reaching {limit}"); + Context.Extractor.Logger.LogWarning($"Stopped logging {key} compiler diagnostics for the current compilation after reaching {limit}"); } return; diff --git a/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected b/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected index 1fcc47687caf..b48630869ee8 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone/Diag.expected @@ -2,3 +2,6 @@ extractorMessages | 5 | compilerDiagnostics | 4 | +compilationInfo +| Compiler diagnostic count for CS0103 | 3.0 | +| Compiler diagnostic count for CS8019 | 7.0 | diff --git a/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql b/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql index bbd142d3af3e..e391b345b20b 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql +++ b/csharp/ql/integration-tests/all-platforms/standalone/Diag.ql @@ -4,3 +4,11 @@ import semmle.code.csharp.commons.Diagnostics query predicate extractorMessages(int c) { c = count(ExtractorMessage msg) } query predicate compilerDiagnostics(int c) { c = count(Diagnostic diag) } + +query predicate compilationInfo(string key, float value) { + exists(Compilation c, string infoValue | + infoValue = c.getInfo(key) and key.matches("Compiler diagnostic count for%") + | + value = infoValue.toFloat() + ) +} diff --git a/csharp/ql/integration-tests/posix-only/standalone_dependencies_nuget_config_error/CompilationInfo.ql b/csharp/ql/integration-tests/posix-only/standalone_dependencies_nuget_config_error/CompilationInfo.ql index 87a9e20f0273..073ffe3b224d 100644 --- a/csharp/ql/integration-tests/posix-only/standalone_dependencies_nuget_config_error/CompilationInfo.ql +++ b/csharp/ql/integration-tests/posix-only/standalone_dependencies_nuget_config_error/CompilationInfo.ql @@ -3,6 +3,7 @@ import semmle.code.csharp.commons.Diagnostics query predicate compilationInfo(string key, float value) { key != "Resolved references" and + not key.matches("Compiler diagnostic count for%") and exists(Compilation c, string infoKey, string infoValue | infoValue = c.getInfo(infoKey) | key = infoKey and value = infoValue.toFloat() diff --git a/csharp/ql/src/Telemetry/ExtractorInformation.ql b/csharp/ql/src/Telemetry/ExtractorInformation.ql index d09e1c9d5d3f..08efbd7b6ec8 100644 --- a/csharp/ql/src/Telemetry/ExtractorInformation.ql +++ b/csharp/ql/src/Telemetry/ExtractorInformation.ql @@ -10,6 +10,7 @@ import csharp import semmle.code.csharp.commons.Diagnostics predicate compilationInfo(string key, float value) { + not key.matches("Compiler diagnostic count for%") and exists(Compilation c, string infoKey, string infoValue | infoValue = c.getInfo(infoKey) | key = infoKey and value = infoValue.toFloat()