C#: Improve log messages in buildless mode + some cleanup/refactoring #15424
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
Fixed
Show fixed
Hide fixed
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
Fixed
Show fixed
Hide fixed
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
Fixed
Show fixed
Hide fixed
tamasvajk
force-pushed
the
standalone/logging
branch
from
a00f860 to
30095e3
Compare
| succeededProjects + failedProjects, | ||
| failedProjects, | ||
| DateTime.Now - startTime); | ||
| const int align = 6; |
Check warning
Code scanning / CodeQL
Useless assignment to local variable
There was a problem hiding this comment.
There is already an issue for this - I will find it.
There was a problem hiding this comment.
| @@ -162,9 +174,9 @@ private void RestoreNugetPackages(List<FileInfo> allNonBinaryFiles, IEnumerable< | |||
| nugetPackageDllPaths.ExceptWith(excludedPaths); | |||
| dllPaths.UnionWith(nugetPackageDllPaths); | |||
| } | |||
| catch (FileNotFoundException) | |||
| catch (Exception) | |||
There was a problem hiding this comment.
Have you observed other exception types or is just to be on the safe side?
There was a problem hiding this comment.
It's only for safety. There's a lot going on in this try-catch, for example we're enumerating files to find packages.config files, and that logic could potentially throw many exceptions.
| { | ||
| var sln = new SolutionFile(solutionFile); | ||
| progressMonitor.LogInfo($"Analyzing {solutionFile}..."); | ||
| foreach (var proj in sln.Projects.Select(p => new FileInfo(p))) |
There was a problem hiding this comment.
It looks like the filter .Where(p => p.Exists) was removed as a part of the re-factor. Is that intentional?
There was a problem hiding this comment.
AnalyseProject checks and checked for the existence of the files. Now if they don't exist, we'll log a message, previously that part of the code was just never reached.
This pull request mainly focuses on improving the log messages written during buildless extraction. Additionally code is refactored to remove the
ProgressMonitorclass and instead use the logger directly. Furthermore, thedotnet restorecalls are also refactored to reduce the number of (out) parameter of the corresponding methods.Commit-by-commit review is suggested.