From 2c72d7ea1ea7837d2cdd0eaa64b94bae31359f00 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 25 Jan 2024 15:18:55 +0100 Subject: [PATCH 1/6] C#: Improve code quality --- .../extractor/Semmle.Extraction.CIL.Driver/Program.cs | 2 +- csharp/extractor/Semmle.Util/Logger.cs | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs b/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs index cb70bfe9c38f..8fcc6fefcb35 100644 --- a/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs +++ b/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs @@ -38,7 +38,7 @@ public static void Main(string[] args) } var options = new ExtractorOptions(args); - using var logger = new ConsoleLogger(options.Verbosity, logThreadId: false); + using ILogger logger = new ConsoleLogger(options.Verbosity, logThreadId: false); var actions = options.AssembliesToExtract .Select(asm => asm.Filename) diff --git a/csharp/extractor/Semmle.Util/Logger.cs b/csharp/extractor/Semmle.Util/Logger.cs index 5e9aa3e219f8..229f6745a3b2 100644 --- a/csharp/extractor/Semmle.Util/Logger.cs +++ b/csharp/extractor/Semmle.Util/Logger.cs @@ -46,17 +46,8 @@ public interface ILogger : IDisposable void LogInfo(string text, int? threadId = null) => Log(Severity.Info, text, threadId); void LogDebug(string text, int? threadId = null) => Log(Severity.Debug, text, threadId); - } - public static class LoggerExtensions - { - /// - /// Log the given text with the given severity. - /// - public static void Log(this ILogger logger, Severity s, string text, params object?[] args) - { - logger.Log(s, string.Format(text, args)); - } + void Log(Severity s, string text, params object?[] args) => Log(s, string.Format(text, args)); } /// From 9b4cdd0d4f0a880cf827d4b83f1ead04c957cc1a Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 25 Jan 2024 15:38:16 +0100 Subject: [PATCH 2/6] Separate logging classes into separate files --- csharp/extractor/Semmle.Util/Logger.cs | 208 ------------------ .../Semmle.Util/Logging/CombinedLogger.cs | 29 +++ .../Semmle.Util/Logging/ConsoleLogger.cs | 55 +++++ .../Semmle.Util/Logging/FileLogger.cs | 58 +++++ .../extractor/Semmle.Util/Logging/ILogger.cs | 25 +++ .../PidStreamWriter.cs} | 2 +- .../extractor/Semmle.Util/Logging/Severity.cs | 14 ++ .../Semmle.Util/Logging/Verbosity.cs | 16 ++ .../Logging/VerbosityExtensions.cs | 30 +++ 9 files changed, 228 insertions(+), 209 deletions(-) delete mode 100644 csharp/extractor/Semmle.Util/Logger.cs create mode 100644 csharp/extractor/Semmle.Util/Logging/CombinedLogger.cs create mode 100644 csharp/extractor/Semmle.Util/Logging/ConsoleLogger.cs create mode 100644 csharp/extractor/Semmle.Util/Logging/FileLogger.cs create mode 100644 csharp/extractor/Semmle.Util/Logging/ILogger.cs rename csharp/extractor/Semmle.Util/{LoggerUtils.cs => Logging/PidStreamWriter.cs} (97%) create mode 100644 csharp/extractor/Semmle.Util/Logging/Severity.cs create mode 100644 csharp/extractor/Semmle.Util/Logging/Verbosity.cs create mode 100644 csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs diff --git a/csharp/extractor/Semmle.Util/Logger.cs b/csharp/extractor/Semmle.Util/Logger.cs deleted file mode 100644 index 229f6745a3b2..000000000000 --- a/csharp/extractor/Semmle.Util/Logger.cs +++ /dev/null @@ -1,208 +0,0 @@ -using System; -using System.IO; - -namespace Semmle.Util.Logging -{ - /// - /// The severity of a log message. - /// - public enum Severity - { - Trace = 1, - Debug = 2, - Info = 3, - Warning = 4, - Error = 5 - } - - /// - /// Log verbosity level. - /// - public enum Verbosity - { - Off = 0, - Error = 1, - Warning = 2, - Info = 3, - Debug = 4, - Trace = 5, - All = 6 - } - - /// - /// A logger. - /// - public interface ILogger : IDisposable - { - /// - /// Log the given text with the given severity. - /// - void Log(Severity s, string text, int? threadId = null); - - void LogError(string text, int? threadId = null) => Log(Severity.Error, text, threadId); - - void LogWarning(string text, int? threadId = null) => Log(Severity.Warning, text, threadId); - - void LogInfo(string text, int? threadId = null) => Log(Severity.Info, text, threadId); - - void LogDebug(string text, int? threadId = null) => Log(Severity.Debug, text, threadId); - - void Log(Severity s, string text, params object?[] args) => Log(s, string.Format(text, args)); - } - - /// - /// A logger that outputs to a csharp.log - /// file. - /// - public sealed class FileLogger : ILogger - { - private readonly StreamWriter writer; - private readonly Verbosity verbosity; - private readonly bool logThreadId; - - public FileLogger(Verbosity verbosity, string outputFile, bool logThreadId) - { - this.verbosity = verbosity; - this.logThreadId = logThreadId; - - try - { - var dir = Path.GetDirectoryName(outputFile); - if (!string.IsNullOrEmpty(dir) && !System.IO.Directory.Exists(dir)) - Directory.CreateDirectory(dir); - writer = new PidStreamWriter( - new FileStream(outputFile, FileMode.Append, FileAccess.Write, FileShare.ReadWrite, 8192)); - } - catch (Exception ex) // lgtm[cs/catch-of-all-exceptions] - { - Console.Error.WriteLine("CodeQL: Couldn't initialise C# extractor output: " + ex.Message + "\n" + ex.StackTrace); - Console.Error.Flush(); - throw; - } - } - - public void Dispose() - { - writer.Dispose(); - } - - private static string GetSeverityPrefix(Severity s) - { - return "[" + s.ToString().ToUpper() + "] "; - } - - public void Log(Severity s, string text, int? threadId = null) - { - if (verbosity.Includes(s)) - { - threadId ??= Environment.CurrentManagedThreadId; - - var prefix = this.logThreadId ? $"[{threadId:D3}] " : ""; - writer.WriteLine(prefix + GetSeverityPrefix(s) + text); - } - } - } - - /// - /// A logger that outputs to stdout/stderr. - /// - public sealed class ConsoleLogger : ILogger - { - private readonly Verbosity verbosity; - private readonly bool logThreadId; - - public ConsoleLogger(Verbosity verbosity, bool logThreadId) - { - this.verbosity = verbosity; - this.logThreadId = logThreadId; - } - - public void Dispose() { } - - private static TextWriter GetConsole(Severity s) - { - return s == Severity.Error ? Console.Error : Console.Out; - } - - private static string GetSeverityPrefix(Severity s) - { - switch (s) - { - case Severity.Trace: - case Severity.Debug: - case Severity.Info: - return ""; - case Severity.Warning: - return "Warning: "; - case Severity.Error: - return "Error: "; - default: - throw new ArgumentOutOfRangeException(nameof(s)); - } - } - - public void Log(Severity s, string text, int? threadId = null) - { - if (verbosity.Includes(s)) - { - threadId ??= Environment.CurrentManagedThreadId; - - var prefix = this.logThreadId ? $"[{threadId:D3}] " : ""; - GetConsole(s).WriteLine(prefix + GetSeverityPrefix(s) + text); - } - } - } - - /// - /// A combined logger. - /// - public sealed class CombinedLogger : ILogger - { - private readonly ILogger logger1; - private readonly ILogger logger2; - - public CombinedLogger(ILogger logger1, ILogger logger2) - { - this.logger1 = logger1; - this.logger2 = logger2; - } - - public void Dispose() - { - logger1.Dispose(); - logger2.Dispose(); - } - - public void Log(Severity s, string text, int? threadId = null) - { - logger1.Log(s, text, threadId); - logger2.Log(s, text, threadId); - } - } - - internal static class VerbosityExtensions - { - /// - /// Whether a message with the given severity must be included - /// for this verbosity level. - /// - public static bool Includes(this Verbosity v, Severity s) - { - switch (s) - { - case Severity.Trace: - return v >= Verbosity.Trace; - case Severity.Debug: - return v >= Verbosity.Debug; - case Severity.Info: - return v >= Verbosity.Info; - case Severity.Warning: - return v >= Verbosity.Warning; - case Severity.Error: - return v >= Verbosity.Error; - default: - throw new ArgumentOutOfRangeException(nameof(s)); - } - } - } -} diff --git a/csharp/extractor/Semmle.Util/Logging/CombinedLogger.cs b/csharp/extractor/Semmle.Util/Logging/CombinedLogger.cs new file mode 100644 index 000000000000..15878e97a197 --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/CombinedLogger.cs @@ -0,0 +1,29 @@ +namespace Semmle.Util.Logging +{ + /// + /// A combined logger. + /// + public sealed class CombinedLogger : ILogger + { + private readonly ILogger logger1; + private readonly ILogger logger2; + + public CombinedLogger(ILogger logger1, ILogger logger2) + { + this.logger1 = logger1; + this.logger2 = logger2; + } + + public void Dispose() + { + logger1.Dispose(); + logger2.Dispose(); + } + + public void Log(Severity s, string text, int? threadId = null) + { + logger1.Log(s, text, threadId); + logger2.Log(s, text, threadId); + } + } +} diff --git a/csharp/extractor/Semmle.Util/Logging/ConsoleLogger.cs b/csharp/extractor/Semmle.Util/Logging/ConsoleLogger.cs new file mode 100644 index 000000000000..59fd7b40aa55 --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/ConsoleLogger.cs @@ -0,0 +1,55 @@ +using System; +using System.IO; + +namespace Semmle.Util.Logging +{ + /// + /// A logger that outputs to stdout/stderr. + /// + public sealed class ConsoleLogger : ILogger + { + private readonly Verbosity verbosity; + private readonly bool logThreadId; + + public ConsoleLogger(Verbosity verbosity, bool logThreadId) + { + this.verbosity = verbosity; + this.logThreadId = logThreadId; + } + + public void Dispose() { } + + private static TextWriter GetConsole(Severity s) + { + return s == Severity.Error ? Console.Error : Console.Out; + } + + private static string GetSeverityPrefix(Severity s) + { + switch (s) + { + case Severity.Trace: + case Severity.Debug: + case Severity.Info: + return ""; + case Severity.Warning: + return "Warning: "; + case Severity.Error: + return "Error: "; + default: + throw new ArgumentOutOfRangeException(nameof(s)); + } + } + + public void Log(Severity s, string text, int? threadId = null) + { + if (verbosity.Includes(s)) + { + threadId ??= Environment.CurrentManagedThreadId; + + var prefix = this.logThreadId ? $"[{threadId:D3}] " : ""; + GetConsole(s).WriteLine(prefix + GetSeverityPrefix(s) + text); + } + } + } +} diff --git a/csharp/extractor/Semmle.Util/Logging/FileLogger.cs b/csharp/extractor/Semmle.Util/Logging/FileLogger.cs new file mode 100644 index 000000000000..49632458a8ef --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/FileLogger.cs @@ -0,0 +1,58 @@ +using System; +using System.IO; + +namespace Semmle.Util.Logging +{ + /// + /// A logger that outputs to a csharp.log + /// file. + /// + public sealed class FileLogger : ILogger + { + private readonly StreamWriter writer; + private readonly Verbosity verbosity; + private readonly bool logThreadId; + + public FileLogger(Verbosity verbosity, string outputFile, bool logThreadId) + { + this.verbosity = verbosity; + this.logThreadId = logThreadId; + + try + { + var dir = Path.GetDirectoryName(outputFile); + if (!string.IsNullOrEmpty(dir) && !System.IO.Directory.Exists(dir)) + Directory.CreateDirectory(dir); + writer = new PidStreamWriter( + new FileStream(outputFile, FileMode.Append, FileAccess.Write, FileShare.ReadWrite, 8192)); + } + catch (Exception ex) // lgtm[cs/catch-of-all-exceptions] + { + Console.Error.WriteLine("CodeQL: Couldn't initialise C# extractor output: " + ex.Message + "\n" + ex.StackTrace); + Console.Error.Flush(); + throw; + } + } + + public void Dispose() + { + writer.Dispose(); + } + + private static string GetSeverityPrefix(Severity s) + { + return "[" + s.ToString().ToUpper() + "] "; + } + + public void Log(Severity s, string text, int? threadId = null) + { + if (verbosity.Includes(s)) + { + threadId ??= Environment.CurrentManagedThreadId; + + var prefix = this.logThreadId ? $"[{threadId:D3}] " : ""; + writer.WriteLine(prefix + GetSeverityPrefix(s) + text); + } + } + } +} diff --git a/csharp/extractor/Semmle.Util/Logging/ILogger.cs b/csharp/extractor/Semmle.Util/Logging/ILogger.cs new file mode 100644 index 000000000000..f9d1153110e6 --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/ILogger.cs @@ -0,0 +1,25 @@ +using System; + +namespace Semmle.Util.Logging +{ + /// + /// A logger. + /// + public interface ILogger : IDisposable + { + /// + /// Log the given text with the given severity. + /// + void Log(Severity s, string text, int? threadId = null); + + void LogError(string text, int? threadId = null) => Log(Severity.Error, text, threadId); + + void LogWarning(string text, int? threadId = null) => Log(Severity.Warning, text, threadId); + + void LogInfo(string text, int? threadId = null) => Log(Severity.Info, text, threadId); + + void LogDebug(string text, int? threadId = null) => Log(Severity.Debug, text, threadId); + + void Log(Severity s, string text, params object?[] args) => Log(s, string.Format(text, args)); + } +} diff --git a/csharp/extractor/Semmle.Util/LoggerUtils.cs b/csharp/extractor/Semmle.Util/Logging/PidStreamWriter.cs similarity index 97% rename from csharp/extractor/Semmle.Util/LoggerUtils.cs rename to csharp/extractor/Semmle.Util/Logging/PidStreamWriter.cs index 79c2b0118b9a..26b0944a429b 100644 --- a/csharp/extractor/Semmle.Util/LoggerUtils.cs +++ b/csharp/extractor/Semmle.Util/Logging/PidStreamWriter.cs @@ -1,7 +1,7 @@ using System.IO; using System.Diagnostics; -namespace Semmle.Util +namespace Semmle.Util.Logging { /// /// Utility stream writer that prefixes the current PID to some writes. diff --git a/csharp/extractor/Semmle.Util/Logging/Severity.cs b/csharp/extractor/Semmle.Util/Logging/Severity.cs new file mode 100644 index 000000000000..675d09e9071a --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/Severity.cs @@ -0,0 +1,14 @@ +namespace Semmle.Util.Logging +{ + /// + /// The severity of a log message. + /// + public enum Severity + { + Trace = 1, + Debug = 2, + Info = 3, + Warning = 4, + Error = 5 + } +} diff --git a/csharp/extractor/Semmle.Util/Logging/Verbosity.cs b/csharp/extractor/Semmle.Util/Logging/Verbosity.cs new file mode 100644 index 000000000000..d768ab72f242 --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/Verbosity.cs @@ -0,0 +1,16 @@ +namespace Semmle.Util.Logging +{ + /// + /// Log verbosity level. + /// + public enum Verbosity + { + Off = 0, + Error = 1, + Warning = 2, + Info = 3, + Debug = 4, + Trace = 5, + All = 6 + } +} diff --git a/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs b/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs new file mode 100644 index 000000000000..66a76b8a0300 --- /dev/null +++ b/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs @@ -0,0 +1,30 @@ +using System; + +namespace Semmle.Util.Logging +{ + internal static class VerbosityExtensions + { + /// + /// Whether a message with the given severity must be included + /// for this verbosity level. + /// + public static bool Includes(this Verbosity v, Severity s) + { + switch (s) + { + case Severity.Trace: + return v >= Verbosity.Trace; + case Severity.Debug: + return v >= Verbosity.Debug; + case Severity.Info: + return v >= Verbosity.Info; + case Severity.Warning: + return v >= Verbosity.Warning; + case Severity.Error: + return v >= Verbosity.Error; + default: + throw new ArgumentOutOfRangeException(nameof(s)); + } + } + } +} From 0e5e57dc564b1cd55ad5ab5a879ad7d9421c777a Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 25 Jan 2024 16:00:44 +0100 Subject: [PATCH 3/6] Adjust 'silent' flag parsing --- .../extractor/Semmle.Extraction.CSharp.Standalone/Options.cs | 3 --- csharp/extractor/Semmle.Extraction/Options.cs | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Options.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Options.cs index aafea778cf01..f6e21b32bbd0 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Options.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Options.cs @@ -15,9 +15,6 @@ public override bool HandleFlag(string key, bool value) { switch (key) { - case "silent": - Verbosity = value ? Verbosity.Off : Verbosity.Info; - return true; case "help": Help = true; return true; diff --git a/csharp/extractor/Semmle.Extraction/Options.cs b/csharp/extractor/Semmle.Extraction/Options.cs index 337292467dc1..044dd6c95962 100644 --- a/csharp/extractor/Semmle.Extraction/Options.cs +++ b/csharp/extractor/Semmle.Extraction/Options.cs @@ -87,8 +87,7 @@ public virtual bool HandleFlag(string flag, bool value) switch (flag) { case "silent": - if (value) - Verbosity = Verbosity.Off; + Verbosity = value ? Verbosity.Off : Verbosity.Info; return true; case "verbose": Verbosity = value ? Verbosity.Debug : Verbosity.Error; From bb4327294d1620f59f708a329415e2aa99340ff9 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 25 Jan 2024 16:03:23 +0100 Subject: [PATCH 4/6] Rename verbosity property to legacy --- .../Semmle.Extraction.CIL.Driver/Program.cs | 2 +- .../Extractor.cs | 4 ++-- .../Extractor/Extractor.cs | 2 +- .../Semmle.Extraction.Tests/Options.cs | 18 +++++++++--------- csharp/extractor/Semmle.Extraction/Options.cs | 10 +++++----- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs b/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs index 8fcc6fefcb35..b5420d8bb67f 100644 --- a/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs +++ b/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs @@ -38,7 +38,7 @@ public static void Main(string[] args) } var options = new ExtractorOptions(args); - using ILogger logger = new ConsoleLogger(options.Verbosity, logThreadId: false); + using ILogger logger = new ConsoleLogger(options.LegacyVerbosity, logThreadId: false); var actions = options.AssembliesToExtract .Select(asm => asm.Filename) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs index e9dbf3525eee..f9874344b378 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs @@ -136,7 +136,7 @@ public static ExitCode Run(Options options) var stopwatch = new Stopwatch(); stopwatch.Start(); - using var logger = new ConsoleLogger(options.Verbosity, logThreadId: true); + using var logger = new ConsoleLogger(options.LegacyVerbosity, logThreadId: true); logger.Log(Severity.Info, "Running C# standalone extractor"); using var a = new Analysis(logger, options); var sourceFileCount = a.Extraction.Sources.Count; @@ -147,7 +147,7 @@ public static ExitCode Run(Options options) return ExitCode.Errors; } - using var fileLogger = CSharp.Extractor.MakeLogger(options.Verbosity, false); + using var fileLogger = CSharp.Extractor.MakeLogger(options.LegacyVerbosity, false); logger.Log(Severity.Info, ""); logger.Log(Severity.Info, "Extracting..."); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs index 5d5bc5860f42..0b051fdc849e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs @@ -97,7 +97,7 @@ public static ExitCode Run(string[] args) var options = Options.CreateWithEnvironment(args); Entities.Compilation.Settings = (Directory.GetCurrentDirectory(), options.CompilerArguments.ToArray()); - using var logger = MakeLogger(options.Verbosity, options.Console); + using var logger = MakeLogger(options.LegacyVerbosity, options.Console); var canonicalPathCache = CanonicalPathCache.Create(logger, 1000); var pathTransformer = new PathTransformer(canonicalPathCache); diff --git a/csharp/extractor/Semmle.Extraction.Tests/Options.cs b/csharp/extractor/Semmle.Extraction.Tests/Options.cs index 235a504e8e5a..08d54431e79e 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/Options.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/Options.cs @@ -27,7 +27,7 @@ public void DefaultOptions() Assert.Null(options.CompilerName); Assert.Empty(options.CompilerArguments); Assert.True(options.Threads >= 1); - Assert.Equal(Verbosity.Info, options.Verbosity); + Assert.Equal(Verbosity.Info, options.LegacyVerbosity); Assert.False(options.Console); Assert.False(options.PDB); Assert.False(options.Fast); @@ -80,25 +80,25 @@ public void CompilerArguments() public void VerbosityTests() { options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbose" }); - Assert.Equal(Verbosity.Debug, options.Verbosity); + Assert.Equal(Verbosity.Debug, options.LegacyVerbosity); options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "0" }); - Assert.Equal(Verbosity.Off, options.Verbosity); + Assert.Equal(Verbosity.Off, options.LegacyVerbosity); options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "1" }); - Assert.Equal(Verbosity.Error, options.Verbosity); + Assert.Equal(Verbosity.Error, options.LegacyVerbosity); options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "2" }); - Assert.Equal(Verbosity.Warning, options.Verbosity); + Assert.Equal(Verbosity.Warning, options.LegacyVerbosity); options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "3" }); - Assert.Equal(Verbosity.Info, options.Verbosity); + Assert.Equal(Verbosity.Info, options.LegacyVerbosity); options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "4" }); - Assert.Equal(Verbosity.Debug, options.Verbosity); + Assert.Equal(Verbosity.Debug, options.LegacyVerbosity); options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "5" }); - Assert.Equal(Verbosity.Trace, options.Verbosity); + Assert.Equal(Verbosity.Trace, options.LegacyVerbosity); Assert.Throws(() => CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "X" })); } @@ -142,7 +142,7 @@ public void StandaloneDefaults() public void StandaloneOptions() { standaloneOptions = CSharp.Standalone.Options.Create(new string[] { "--silent" }); - Assert.Equal(Verbosity.Off, standaloneOptions.Verbosity); + Assert.Equal(Verbosity.Off, standaloneOptions.LegacyVerbosity); Assert.False(standaloneOptions.Errors); Assert.False(standaloneOptions.Help); } diff --git a/csharp/extractor/Semmle.Extraction/Options.cs b/csharp/extractor/Semmle.Extraction/Options.cs index 044dd6c95962..dae79e26f100 100644 --- a/csharp/extractor/Semmle.Extraction/Options.cs +++ b/csharp/extractor/Semmle.Extraction/Options.cs @@ -16,9 +16,9 @@ public abstract class CommonOptions : ICommandLineOptions public int Threads { get; private set; } = EnvironmentVariables.GetDefaultNumberOfThreads(); /// - /// The verbosity used in output and logging. + /// The verbosity used specified by the '--silent' or '--verbose' flags or the '--verbosity' option. /// - public Verbosity Verbosity { get; protected set; } = Verbosity.Info; + public Verbosity LegacyVerbosity { get; protected set; } = Verbosity.Info; /// /// Whether to output to the console. @@ -63,7 +63,7 @@ public virtual bool HandleOption(string key, string value) Threads = int.Parse(value); return true; case "verbosity": - Verbosity = (Verbosity)int.Parse(value); + LegacyVerbosity = (Verbosity)int.Parse(value); return true; case "trap_compression": if (Enum.TryParse(value, true, out var mode)) @@ -87,10 +87,10 @@ public virtual bool HandleFlag(string flag, bool value) switch (flag) { case "silent": - Verbosity = value ? Verbosity.Off : Verbosity.Info; + LegacyVerbosity = value ? Verbosity.Off : Verbosity.Info; return true; case "verbose": - Verbosity = value ? Verbosity.Debug : Verbosity.Error; + LegacyVerbosity = value ? Verbosity.Debug : Verbosity.Error; return true; case "console": Console = value; From b8c8f52efc8fd6226c1fcc7fcea0ee745ff8fe6d Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 25 Jan 2024 17:20:47 +0100 Subject: [PATCH 5/6] C#: Introduce extractor option for logging verbosity --- .../Semmle.Autobuild.Shared/Autobuilder.cs | 6 +++- csharp/codeql-extractor.yml | 18 +++++++++++ .../Semmle.Extraction.CIL.Driver/Program.cs | 2 +- .../Extractor.cs | 4 +-- .../Extractor/Extractor.cs | 2 +- .../Semmle.Extraction.Tests/Options.cs | 31 +++++++++++++++++++ csharp/extractor/Semmle.Extraction/Options.cs | 30 ++++++++++++++++++ .../Logging/VerbosityExtensions.cs | 31 ++++++++++++++++++- 8 files changed, 118 insertions(+), 6 deletions(-) diff --git a/csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs b/csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs index c6cf3e32b7ea..d6757418875d 100644 --- a/csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs +++ b/csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs @@ -267,7 +267,11 @@ protected string RequireEnvironmentVariable(string name) protected DiagnosticClassifier DiagnosticClassifier { get; } - private readonly ILogger logger = new ConsoleLogger(Verbosity.Info, logThreadId: false); + private readonly ILogger logger = new ConsoleLogger( + VerbosityExtensions.ParseVerbosity( + Environment.GetEnvironmentVariable("CODEQL_VERBOSITY"), + logThreadId: false) ?? Verbosity.Info, + logThreadId: false); private readonly IDiagnosticsWriter diagnostics; diff --git a/csharp/codeql-extractor.yml b/csharp/codeql-extractor.yml index 0a3e22f1ff9d..1990b2080e7c 100644 --- a/csharp/codeql-extractor.yml +++ b/csharp/codeql-extractor.yml @@ -48,3 +48,21 @@ options: The default is 'true'. type: string pattern: "^(false|true)$" + logging: + title: Options pertaining to logging. + type: object + properties: + verbosity: + title: Extractor logging verbosity level. + description: > + Controls the level of verbosity of the extractor. + The supported levels are (in order of increasing verbosity): + - off + - errors + - warnings + - info or progress + - debug or progress+ + - trace or progress++ + - progress+++ + type: string + pattern: "^(off|errors|warnings|(info|progress)|(debug|progress\\+)|(trace|progress\\+\\+)|progress\\+\\+\\+)$" diff --git a/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs b/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs index b5420d8bb67f..8fcc6fefcb35 100644 --- a/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs +++ b/csharp/extractor/Semmle.Extraction.CIL.Driver/Program.cs @@ -38,7 +38,7 @@ public static void Main(string[] args) } var options = new ExtractorOptions(args); - using ILogger logger = new ConsoleLogger(options.LegacyVerbosity, logThreadId: false); + using ILogger logger = new ConsoleLogger(options.Verbosity, logThreadId: false); var actions = options.AssembliesToExtract .Select(asm => asm.Filename) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs index f9874344b378..e9dbf3525eee 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs @@ -136,7 +136,7 @@ public static ExitCode Run(Options options) var stopwatch = new Stopwatch(); stopwatch.Start(); - using var logger = new ConsoleLogger(options.LegacyVerbosity, logThreadId: true); + using var logger = new ConsoleLogger(options.Verbosity, logThreadId: true); logger.Log(Severity.Info, "Running C# standalone extractor"); using var a = new Analysis(logger, options); var sourceFileCount = a.Extraction.Sources.Count; @@ -147,7 +147,7 @@ public static ExitCode Run(Options options) return ExitCode.Errors; } - using var fileLogger = CSharp.Extractor.MakeLogger(options.LegacyVerbosity, false); + using var fileLogger = CSharp.Extractor.MakeLogger(options.Verbosity, false); logger.Log(Severity.Info, ""); logger.Log(Severity.Info, "Extracting..."); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs index 0b051fdc849e..5d5bc5860f42 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs @@ -97,7 +97,7 @@ public static ExitCode Run(string[] args) var options = Options.CreateWithEnvironment(args); Entities.Compilation.Settings = (Directory.GetCurrentDirectory(), options.CompilerArguments.ToArray()); - using var logger = MakeLogger(options.LegacyVerbosity, options.Console); + using var logger = MakeLogger(options.Verbosity, options.Console); var canonicalPathCache = CanonicalPathCache.Create(logger, 1000); var pathTransformer = new PathTransformer(canonicalPathCache); diff --git a/csharp/extractor/Semmle.Extraction.Tests/Options.cs b/csharp/extractor/Semmle.Extraction.Tests/Options.cs index 08d54431e79e..81b750368ca4 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/Options.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/Options.cs @@ -103,6 +103,37 @@ public void VerbosityTests() Assert.Throws(() => CSharp.Options.CreateWithEnvironment(new string[] { "--verbosity", "X" })); } + + private const string extractorVariableName = "CODEQL_EXTRACTOR_CSHARP_OPTION_LOGGING_VERBOSITY"; + private const string cliVariableName = "CODEQL_VERBOSITY"; + + private void CheckVerbosity(string? extractor, string? cli, Verbosity expected) + { + var currentExtractorVerbosity = Environment.GetEnvironmentVariable(extractorVariableName); + var currentCliVerbosity = Environment.GetEnvironmentVariable(cliVariableName); + try + { + Environment.SetEnvironmentVariable(extractorVariableName, extractor); + Environment.SetEnvironmentVariable(cliVariableName, cli); + + options = CSharp.Options.CreateWithEnvironment(new string[] { "--verbose" }); + Assert.Equal(expected, options.Verbosity); + } + finally + { + Environment.SetEnvironmentVariable(extractorVariableName, currentExtractorVerbosity); + Environment.SetEnvironmentVariable(cliVariableName, currentCliVerbosity); + } + } + + [Fact] + public void VerbosityTests_WithExtractorOption() + { + CheckVerbosity("progress+++", "progress++", Verbosity.All); + CheckVerbosity(null, "progress++", Verbosity.Trace); + CheckVerbosity(null, null, Verbosity.Debug); + } + [Fact] public void Console() { diff --git a/csharp/extractor/Semmle.Extraction/Options.cs b/csharp/extractor/Semmle.Extraction/Options.cs index dae79e26f100..576d7a4762e5 100644 --- a/csharp/extractor/Semmle.Extraction/Options.cs +++ b/csharp/extractor/Semmle.Extraction/Options.cs @@ -20,6 +20,36 @@ public abstract class CommonOptions : ICommandLineOptions /// public Verbosity LegacyVerbosity { get; protected set; } = Verbosity.Info; + private Verbosity? verbosity = null; + public Verbosity Verbosity + { + get + { + if (verbosity != null) + { + return verbosity.Value; + } + + var envVarValue = EnvironmentVariables.GetExtractorOption("LOGGING_VERBOSITY"); + verbosity = VerbosityExtensions.ParseVerbosity(envVarValue, logThreadId: true); + if (verbosity != null) + { + return verbosity.Value; + } + + envVarValue = Environment.GetEnvironmentVariable("CODEQL_VERBOSITY"); + verbosity = VerbosityExtensions.ParseVerbosity(envVarValue, logThreadId: true); + if (verbosity != null) + { + return verbosity.Value; + } + + // This only works, because we already parsed the provided options, so `LegacyVerbosity` is already set (or it still has the default value). + verbosity = LegacyVerbosity; + return verbosity.Value; + } + } + /// /// Whether to output to the console. /// diff --git a/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs b/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs index 66a76b8a0300..96cc9abe9d87 100644 --- a/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs +++ b/csharp/extractor/Semmle.Util/Logging/VerbosityExtensions.cs @@ -2,7 +2,7 @@ namespace Semmle.Util.Logging { - internal static class VerbosityExtensions + public static class VerbosityExtensions { /// /// Whether a message with the given severity must be included @@ -26,5 +26,34 @@ public static bool Includes(this Verbosity v, Severity s) throw new ArgumentOutOfRangeException(nameof(s)); } } + + public static Verbosity? ParseVerbosity(string? str, bool logThreadId) + { + if (str == null) + { + return null; + } + + Verbosity? verbosity = str.ToLowerInvariant() switch + { + "off" => Verbosity.Off, + "errors" => Verbosity.Error, + "warnings" => Verbosity.Warning, + "info" or "progress" => Verbosity.Info, + "debug" or "progress+" => Verbosity.Debug, + "trace" or "progress++" => Verbosity.Trace, + "progress+++" => Verbosity.All, + _ => null + }; + + if (verbosity == null && str != null) + { + // We don't have a logger when this setting is parsed, so writing it to the console: + var prefix = logThreadId ? $"[{Environment.CurrentManagedThreadId:D3}] " : ""; + Console.WriteLine($"{prefix}Error: Invalid verbosity level: '{str}'"); + } + + return verbosity; + } } } From c4849f9a173d457ac7601fbd53d3e9828e1fcfb5 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 25 Jan 2024 17:33:06 +0100 Subject: [PATCH 6/6] Add change note --- .../lib/change-notes/2024-01-25-extractor-option-logging.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 csharp/ql/lib/change-notes/2024-01-25-extractor-option-logging.md diff --git a/csharp/ql/lib/change-notes/2024-01-25-extractor-option-logging.md b/csharp/ql/lib/change-notes/2024-01-25-extractor-option-logging.md new file mode 100644 index 000000000000..71cb32026756 --- /dev/null +++ b/csharp/ql/lib/change-notes/2024-01-25-extractor-option-logging.md @@ -0,0 +1,6 @@ +--- +category: minorAnalysis +--- +* The C# extractor now accepts an extractor option `logging.verbosity` that specifies the verbosity of the logs. The +option is added via `codeql database create --language=csharp -Ologging.verbosity=debug ...` or by setting the +corresponding environment variable `CODEQL_EXTRACTOR_CSHARP_OPTION_LOGGING_VERBOSITY`. \ No newline at end of file