From af2a78ea4d3a7bcd3a1d064c51f77a3af779fe26 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Wed, 12 Jun 2024 11:12:57 +0200 Subject: [PATCH] Reduce references to `Extract` class --- .../Extractor.cs | 2 +- .../Entities/NonGeneratedSourceLocation.cs | 22 +-------- .../LineOrSpanDirective.cs | 2 +- .../PragmaChecksumDirective.cs | 2 +- .../Extractor/Analyser.cs | 35 ++++++++++++-- .../Extractor/Context.cs | 21 +++++++++ .../Extractor/Extractor.cs | 15 +++--- .../Extractor/StandaloneAnalyser.cs | 7 +-- .../Extractor/TracingAnalyser.cs | 46 +++++++------------ csharp/extractor/Semmle.Extraction/Context.cs | 2 +- .../Semmle.Extraction/Entities/Base/Entity.cs | 4 +- .../Entities/Base/IEntity.cs | 2 +- .../Semmle.Extraction/Extractor/Extractor.cs | 16 ------- 13 files changed, 87 insertions(+), 89 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs index e578dd4aa31..a72d04ac8ba 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs @@ -81,7 +81,7 @@ namespace Semmle.Extraction.CSharp.Standalone var canonicalPathCache = CanonicalPathCache.Create(logger, 1000); var pathTransformer = new PathTransformer(canonicalPathCache); - using var analyser = new StandaloneAnalyser(pm, logger, false, pathTransformer); + using var analyser = new StandaloneAnalyser(pm, logger, pathTransformer, canonicalPathCache, false); try { AnalyseStandalone(analyser, extractionInput, options, pm, stopwatch); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/NonGeneratedSourceLocation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/NonGeneratedSourceLocation.cs index a3b7877af4e..df41abd524d 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/NonGeneratedSourceLocation.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/NonGeneratedSourceLocation.cs @@ -27,7 +27,7 @@ namespace Semmle.Extraction.CSharp.Entities var mapped = Symbol.GetMappedLineSpan(); if (mapped.HasMappedPath && mapped.IsValid) { - var path = TryAdjustRelativeMappedFilePath(mapped.Path, Position.Path, Context.Extractor.Logger); + var path = Context.TryAdjustRelativeMappedFilePath(mapped.Path, Position.Path); var mappedLoc = Create(Context, Location.Create(path, default, mapped.Span)); trapFile.locations_mapped(this, mappedLoc); @@ -64,25 +64,5 @@ namespace Semmle.Extraction.CSharp.Entities public override NonGeneratedSourceLocation Create(Context cx, Location init) => new NonGeneratedSourceLocation(cx, init); } - - public static string TryAdjustRelativeMappedFilePath(string mappedToPath, string mappedFromPath, ILogger logger) - { - if (!Path.IsPathRooted(mappedToPath)) - { - try - { - var fullPath = Path.GetFullPath(Path.Combine(Path.GetDirectoryName(mappedFromPath)!, mappedToPath)); - logger.LogDebug($"Found relative path in line mapping: '{mappedToPath}', interpreting it as '{fullPath}'"); - - mappedToPath = fullPath; - } - catch (Exception e) - { - logger.LogDebug($"Failed to compute absolute path for relative path in line mapping: '{mappedToPath}': {e}"); - } - } - - return mappedToPath; - } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/LineOrSpanDirective.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/LineOrSpanDirective.cs index 6d236220331..4c4806110f0 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/LineOrSpanDirective.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/LineOrSpanDirective.cs @@ -28,7 +28,7 @@ namespace Semmle.Extraction.CSharp.Entities var path = Symbol.File.ValueText; if (!string.IsNullOrWhiteSpace(path)) { - path = NonGeneratedSourceLocation.TryAdjustRelativeMappedFilePath(path, Symbol.SyntaxTree.FilePath, Context.Extractor.Logger); + path = Context.TryAdjustRelativeMappedFilePath(path, Symbol.SyntaxTree.FilePath); var file = File.Create(Context, path); trapFile.directive_line_file(this, file); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/PragmaChecksumDirective.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/PragmaChecksumDirective.cs index 3e0c468d85b..66eab50331f 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/PragmaChecksumDirective.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/PreprocessorDirectives/PragmaChecksumDirective.cs @@ -12,7 +12,7 @@ namespace Semmle.Extraction.CSharp.Entities protected override void PopulatePreprocessor(TextWriter trapFile) { - var path = NonGeneratedSourceLocation.TryAdjustRelativeMappedFilePath(Symbol.File.ValueText, Symbol.SyntaxTree.FilePath, Context.Extractor.Logger); + var path = Context.TryAdjustRelativeMappedFilePath(Symbol.File.ValueText, Symbol.SyntaxTree.FilePath); var file = File.Create(Context, path); trapFile.pragma_checksums(this, file, Symbol.Guid.ToString(), Symbol.Bytes.ToString()); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs index 281ce9e9349..a21b71ed196 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis.CSharp; using Semmle.Util; using Semmle.Util.Logging; using Semmle.Extraction.CSharp.Populators; +using System.Reflection; namespace Semmle.Extraction.CSharp { @@ -38,14 +39,23 @@ namespace Semmle.Extraction.CSharp public PathTransformer PathTransformer { get; } - protected Analyser(IProgressMonitor pm, ILogger logger, bool addAssemblyTrapPrefix, PathTransformer pathTransformer) + public IPathCache PathCache { get; } + + protected Analyser( + IProgressMonitor pm, + ILogger logger, + PathTransformer pathTransformer, + IPathCache pathCache, + bool addAssemblyTrapPrefix) { Logger = logger; + PathTransformer = pathTransformer; + PathCache = pathCache; this.addAssemblyTrapPrefix = addAssemblyTrapPrefix; + this.progressMonitor = pm; + Logger.Log(Severity.Info, "EXTRACTION STARTING at {0}", DateTime.Now); stopWatch.Start(); - progressMonitor = pm; - PathTransformer = pathTransformer; } /// @@ -333,11 +343,26 @@ namespace Semmle.Extraction.CSharp /// /// Logs information about the extractor. /// - public void LogExtractorInfo(string extractorVersion) + public void LogExtractorInfo() { Logger.Log(Severity.Info, " Extractor: {0}", Environment.GetCommandLineArgs().First()); - Logger.Log(Severity.Info, " Extractor version: {0}", extractorVersion); + Logger.Log(Severity.Info, " Extractor version: {0}", Version); Logger.Log(Severity.Info, " Current working directory: {0}", Directory.GetCurrentDirectory()); } + + private static string Version + { + get + { + // the attribute for the git information are always attached to the entry assembly by our build system + var assembly = Assembly.GetEntryAssembly(); + var versionString = assembly?.GetCustomAttribute(); + if (versionString == null) + { + return "unknown (not built from internal bazel workspace)"; + } + return versionString.InformationalVersion; + } + } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs index e799eb4a387..338b8490070 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.IO; using Microsoft.CodeAnalysis; using Semmle.Extraction.Entities; @@ -178,5 +179,25 @@ namespace Semmle.Extraction.CSharp extractedGenerics.Add(entity.Label); return true; } + + public string TryAdjustRelativeMappedFilePath(string mappedToPath, string mappedFromPath) + { + if (!Path.IsPathRooted(mappedToPath)) + { + try + { + var fullPath = Path.GetFullPath(Path.Combine(Path.GetDirectoryName(mappedFromPath)!, mappedToPath)); + Extractor.Logger.LogDebug($"Found relative path in line mapping: '{mappedToPath}', interpreting it as '{fullPath}'"); + + mappedToPath = fullPath; + } + catch (Exception e) + { + Extractor.Logger.LogDebug($"Failed to compute absolute path for relative path in line mapping: '{mappedToPath}': {e}"); + } + } + + return mappedToPath; + } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs index f6913103ad8..cdeb915de58 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs @@ -105,7 +105,7 @@ namespace Semmle.Extraction.CSharp var canonicalPathCache = CanonicalPathCache.Create(logger, 1000); var pathTransformer = new PathTransformer(canonicalPathCache); - using var analyser = new TracingAnalyser(new LogProgressMonitor(logger), logger, options.AssemblySensitiveTrap, pathTransformer); + using var analyser = new TracingAnalyser(new LogProgressMonitor(logger), logger, pathTransformer, canonicalPathCache, options.AssemblySensitiveTrap); try { @@ -144,7 +144,7 @@ namespace Semmle.Extraction.CSharp return ExitCode.Ok; } - return AnalyseTracing(workingDirectory, compilerArgs, analyser, compilerArguments, options, canonicalPathCache, stopwatch); + return AnalyseTracing(workingDirectory, compilerArgs, analyser, compilerArguments, options, stopwatch); } catch (Exception ex) // lgtm[cs/catch-of-all-exceptions] { @@ -226,7 +226,7 @@ namespace Semmle.Extraction.CSharp /// The resolved references will be added (thread-safely) to the supplied /// list . /// - private static IEnumerable ResolveReferences(Microsoft.CodeAnalysis.CommandLineArguments args, Analyser analyser, CanonicalPathCache canonicalPathCache, BlockingCollection ret) + private static IEnumerable ResolveReferences(Microsoft.CodeAnalysis.CommandLineArguments args, Analyser analyser, BlockingCollection ret) { var referencePaths = new Lazy(() => FixedReferencePaths(args).ToArray()); return args.MetadataReferences.Select(clref => () => @@ -235,7 +235,7 @@ namespace Semmle.Extraction.CSharp { if (File.Exists(clref.Reference)) { - var reference = MakeReference(clref, canonicalPathCache.GetCanonicalPath(clref.Reference)); + var reference = MakeReference(clref, analyser.PathCache.GetCanonicalPath(clref.Reference)); ret.Add(reference); } else @@ -252,7 +252,7 @@ namespace Semmle.Extraction.CSharp var composed = referencePaths.Value .Select(path => Path.Combine(path, clref.Reference)) .Where(path => File.Exists(path)) - .Select(path => canonicalPathCache.GetCanonicalPath(path)) + .Select(path => analyser.PathCache.GetCanonicalPath(path)) .FirstOrDefault(); if (composed is not null) @@ -382,11 +382,10 @@ namespace Semmle.Extraction.CSharp TracingAnalyser analyser, CSharpCommandLineArguments compilerArguments, Options options, - CanonicalPathCache canonicalPathCache, Stopwatch stopwatch) { return Analyse(stopwatch, analyser, options, - references => ResolveReferences(compilerArguments, analyser, canonicalPathCache, references), + references => ResolveReferences(compilerArguments, analyser, references), (analyser, syntaxTrees) => { var paths = compilerArguments.SourceFiles @@ -399,7 +398,7 @@ namespace Semmle.Extraction.CSharp } return ReadSyntaxTrees( - paths.Select(canonicalPathCache.GetCanonicalPath), + paths.Select(analyser.PathCache.GetCanonicalPath), analyser, compilerArguments.ParseOptions, compilerArguments.Encoding, diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/StandaloneAnalyser.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/StandaloneAnalyser.cs index 82ea40c2d94..192fe7f34fd 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/StandaloneAnalyser.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/StandaloneAnalyser.cs @@ -2,14 +2,15 @@ using System; using System.Collections.Generic; using System.IO; using Microsoft.CodeAnalysis.CSharp; +using Semmle.Util; using Semmle.Util.Logging; namespace Semmle.Extraction.CSharp { public class StandaloneAnalyser : Analyser { - public StandaloneAnalyser(IProgressMonitor pm, ILogger logger, bool addAssemblyTrapPrefix, PathTransformer pathTransformer) - : base(pm, logger, addAssemblyTrapPrefix, pathTransformer) + public StandaloneAnalyser(IProgressMonitor pm, ILogger logger, PathTransformer pathTransformer, IPathCache pathCache, bool addAssemblyTrapPrefix) + : base(pm, logger, pathTransformer, pathCache, addAssemblyTrapPrefix) { } @@ -18,7 +19,7 @@ namespace Semmle.Extraction.CSharp compilation = compilationIn; extractor = new Extraction.Extractor(Directory.GetCurrentDirectory(), [], outputPath, compilationInfos, Logger, PathTransformer, ExtractorMode.Standalone, options.QlTest); this.options = options; - LogExtractorInfo(Extraction.Extractor.Version); + LogExtractorInfo(); SetReferencePaths(); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/TracingAnalyser.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/TracingAnalyser.cs index 43c5562736c..5418ebed07f 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/TracingAnalyser.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/TracingAnalyser.cs @@ -13,8 +13,8 @@ namespace Semmle.Extraction.CSharp { private bool init; - public TracingAnalyser(IProgressMonitor pm, ILogger logger, bool addAssemblyTrapPrefix, PathTransformer pathTransformer) - : base(pm, logger, addAssemblyTrapPrefix, pathTransformer) + public TracingAnalyser(IProgressMonitor pm, ILogger logger, PathTransformer pathTransformer, IPathCache pathCache, bool addAssemblyTrapPrefix) + : base(pm, logger, pathTransformer, pathCache, addAssemblyTrapPrefix) { } @@ -25,7 +25,8 @@ namespace Semmle.Extraction.CSharp /// A Boolean indicating whether to proceed with extraction. public bool BeginInitialize(IEnumerable roslynArgs) { - return init = LogRoslynArgs(roslynArgs, Extraction.Extractor.Version); + LogExtractorInfo(); + return init = LogRoslynArgs(roslynArgs); } /// @@ -47,11 +48,11 @@ namespace Semmle.Extraction.CSharp this.options = options; this.compilation = compilation; this.extractor = new Extraction.Extractor(cwd, args, GetOutputName(compilation, commandLineArguments), [], Logger, PathTransformer, ExtractorMode.None, options.QlTest); - LogDiagnostics(); + var errorCount = LogDiagnostics(compilation); SetReferencePaths(); - CompilationErrors += FilteredDiagnostics.Count(); + CompilationErrors += errorCount; } /// @@ -59,9 +60,8 @@ namespace Semmle.Extraction.CSharp /// /// The arguments passed to Roslyn. /// A Boolean indicating whether the same arguments have been logged previously. - private bool LogRoslynArgs(IEnumerable roslynArgs, string extractorVersion) + private bool LogRoslynArgs(IEnumerable roslynArgs) { - LogExtractorInfo(extractorVersion); Logger.Log(Severity.Info, $" Arguments to Roslyn: {string.Join(' ', roslynArgs)}"); var tempFile = Extractor.GetCSharpArgsLogPath(Path.GetRandomFileName()); @@ -137,27 +137,27 @@ namespace Semmle.Extraction.CSharp return Path.Combine(commandLineArguments.OutputDirectory, commandLineArguments.OutputFileName); } -#nullable disable warnings - - /// - /// Logs detailed information about this invocation, - /// in the event that errors were detected. - /// - /// A Boolean indicating whether to proceed with extraction. - private void LogDiagnostics() + private int LogDiagnostics(CSharpCompilation compilation) { - foreach (var error in FilteredDiagnostics) + var filteredDiagnostics = compilation + .GetDiagnostics() + .Where(e => e.Severity >= DiagnosticSeverity.Error && !errorsToIgnore.Contains(e.Id)) + .ToList(); + + foreach (var error in filteredDiagnostics) { Logger.Log(Severity.Error, " Compilation error: {0}", error); } - if (FilteredDiagnostics.Any()) + if (filteredDiagnostics.Count != 0) { foreach (var reference in compilation.References) { Logger.Log(Severity.Info, " Resolved reference {0}", reference.Display); } } + + return filteredDiagnostics.Count; } private static readonly HashSet errorsToIgnore = new HashSet @@ -166,17 +166,5 @@ namespace Semmle.Extraction.CSharp "CS1589", // XML referencing not supported "CS1569" // Error writing XML documentation }; - - private IEnumerable FilteredDiagnostics - { - get - { - return extractor is null || extractor.Mode.HasFlag(ExtractorMode.Standalone) || compilation is null ? Enumerable.Empty() : - compilation. - GetDiagnostics(). - Where(e => e.Severity >= DiagnosticSeverity.Error && !errorsToIgnore.Contains(e.Id)); - } - } -#nullable restore warnings } } diff --git a/csharp/extractor/Semmle.Extraction/Context.cs b/csharp/extractor/Semmle.Extraction/Context.cs index 6697bfb06fd..622d0a21ccd 100644 --- a/csharp/extractor/Semmle.Extraction/Context.cs +++ b/csharp/extractor/Semmle.Extraction/Context.cs @@ -51,7 +51,7 @@ namespace Semmle.Extraction try { writingLabel = true; - entity.DefineLabel(TrapWriter.Writer, Extractor); + entity.DefineLabel(TrapWriter.Writer); } finally { diff --git a/csharp/extractor/Semmle.Extraction/Entities/Base/Entity.cs b/csharp/extractor/Semmle.Extraction/Entities/Base/Entity.cs index 49186582b0d..e28b89ad0eb 100644 --- a/csharp/extractor/Semmle.Extraction/Entities/Base/Entity.cs +++ b/csharp/extractor/Semmle.Extraction/Entities/Base/Entity.cs @@ -28,7 +28,7 @@ namespace Semmle.Extraction public abstract TrapStackBehaviour TrapStackBehaviour { get; } - public void DefineLabel(TextWriter trapFile, Extractor extractor) + public void DefineLabel(TextWriter trapFile) { trapFile.WriteLabel(this); trapFile.Write("="); @@ -40,7 +40,7 @@ namespace Semmle.Extraction catch (Exception ex) // lgtm[cs/catch-of-all-exceptions] { trapFile.WriteLine("\""); - extractor.Message(new Message($"Unhandled exception generating id: {ex.Message}", ToString() ?? "", null, ex.StackTrace)); + Context.Extractor.Message(new Message($"Unhandled exception generating id: {ex.Message}", ToString() ?? "", null, ex.StackTrace)); } trapFile.WriteLine(); } diff --git a/csharp/extractor/Semmle.Extraction/Entities/Base/IEntity.cs b/csharp/extractor/Semmle.Extraction/Entities/Base/IEntity.cs index acb5d79d597..3700e61b22e 100644 --- a/csharp/extractor/Semmle.Extraction/Entities/Base/IEntity.cs +++ b/csharp/extractor/Semmle.Extraction/Entities/Base/IEntity.cs @@ -48,6 +48,6 @@ namespace Semmle.Extraction /// TrapStackBehaviour TrapStackBehaviour { get; } - void DefineLabel(TextWriter trapFile, Extractor extractor); + void DefineLabel(TextWriter trapFile); } } diff --git a/csharp/extractor/Semmle.Extraction/Extractor/Extractor.cs b/csharp/extractor/Semmle.Extraction/Extractor/Extractor.cs index c8da1265559..a57da8feabd 100644 --- a/csharp/extractor/Semmle.Extraction/Extractor/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction/Extractor/Extractor.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Reflection; using Semmle.Util.Logging; using CompilationInfo = (string key, string value); @@ -110,21 +109,6 @@ namespace Semmle.Extraction public ILogger Logger { get; private set; } - public static string Version - { - get - { - // the attribute for the git information are always attached to the entry assembly by our build system - var assembly = Assembly.GetEntryAssembly(); - var versionString = assembly?.GetCustomAttribute(); - if (versionString == null) - { - return "unknown (not built from internal bazel workspace)"; - } - return versionString.InformationalVersion; - } - } - public PathTransformer PathTransformer { get; } } }