Skip to content

Commit 0f32099

Browse files
mbgmbg
committed
Make improvements based on PR feedback
1 parent 8e83fd0 commit 0f32099

File tree

10 files changed

+60
-69
lines changed

10 files changed

+60
-69
lines changed

‎cpp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Xunit;
22
using Semmle.Autobuild.Shared;
3+
using Semmle.Util;
34
using System.Collections.Generic;
45
using System;
56
using System.Linq;
@@ -79,10 +80,7 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory,
7980
{
8081
var ret = (this as IBuildActions).RunProcess(cmd, args, workingDirectory, env, out var stdout);
8182

82-
foreach (var line in stdout)
83-
{
84-
onOutput(line);
85-
}
83+
stdout.ForEach(line => onOutput(line));
8684

8785
return ret;
8886
}

‎cpp/autobuilder/Semmle.Autobuild.Cpp/CppAutobuilder.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,10 @@ public CppAutobuildOptions(IBuildActions actions) : base(actions)
2121

2222
public class CppAutobuilder : Autobuilder<CppAutobuildOptions>
2323
{
24-
private DiagnosticClassifier classifier;
24+
private readonly DiagnosticClassifier classifier;
2525

26-
public CppAutobuilder(IBuildActions actions, CppAutobuildOptions options) : base(actions, options)
27-
{
26+
public CppAutobuilder(IBuildActions actions, CppAutobuildOptions options) : base(actions, options) =>
2827
classifier = new DiagnosticClassifier();
29-
}
3028

3129
protected override DiagnosticClassifier DiagnosticClassifier => classifier;
3230

‎csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Xunit;
22
using Semmle.Autobuild.Shared;
3+
using Semmle.Util;
34
using System.Collections.Generic;
45
using System;
56
using System.Linq;
@@ -89,10 +90,7 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory,
8990
{
9091
var ret = (this as IBuildActions).RunProcess(cmd, args, workingDirectory, env, out var stdout);
9192

92-
foreach (var line in stdout)
93-
{
94-
onOutput(line);
95-
}
93+
stdout.ForEach(line => onOutput(line));
9694

9795
return ret;
9896
}

‎csharp/autobuilder/Semmle.Autobuild.CSharp/CSharpAutobuilder.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,8 @@ public class CSharpAutobuilder : Autobuilder<CSharpAutobuildOptions>
4141

4242
protected override DiagnosticClassifier DiagnosticClassifier => diagnosticClassifier;
4343

44-
public CSharpAutobuilder(IBuildActions actions, CSharpAutobuildOptions options) : base(actions, options)
45-
{
44+
public CSharpAutobuilder(IBuildActions actions, CSharpAutobuildOptions options) : base(actions, options) =>
4645
diagnosticClassifier = new CSharpDiagnosticClassifier();
47-
}
4846

4947
public override BuildScript GetBuildScript()
5048
{
@@ -164,7 +162,7 @@ protected override void AutobuildFailureDiagnostic()
164162
}
165163

166164
message.Severity = DiagnosticMessage.TspSeverity.Error;
167-
Diagnostic(message);
165+
AddDiagnostic(message);
168166
}
169167

170168
// both dotnet and msbuild builds require project or solution files; if we haven't found any
@@ -177,7 +175,7 @@ protected override void AutobuildFailureDiagnostic()
177175
"You can manually specify a suitable build command for your project.";
178176
message.Severity = DiagnosticMessage.TspSeverity.Error;
179177

180-
Diagnostic(message);
178+
AddDiagnostic(message);
181179
}
182180
else if (dotNetRule is not null && dotNetRule.NotDotNetProjects.Any())
183181
{
@@ -187,7 +185,7 @@ protected override void AutobuildFailureDiagnostic()
187185
string.Join('\n', dotNetRule.NotDotNetProjects.Select(p => $"- `{p.FullPath}`"));
188186
message.Severity = DiagnosticMessage.TspSeverity.Warning;
189187

190-
Diagnostic(message);
188+
AddDiagnostic(message);
191189
}
192190

193191
// report any projects that failed to build with .NET Core
@@ -201,7 +199,7 @@ protected override void AutobuildFailureDiagnostic()
201199
"or to ensure that they can be built successfully.";
202200
message.Severity = DiagnosticMessage.TspSeverity.Error;
203201

204-
Diagnostic(message);
202+
AddDiagnostic(message);
205203
}
206204

207205
// report any projects that failed to build with MSBuild
@@ -215,7 +213,7 @@ protected override void AutobuildFailureDiagnostic()
215213
"or to ensure that they can be built successfully.";;
216214
message.Severity = DiagnosticMessage.TspSeverity.Error;
217215

218-
Diagnostic(message);
216+
AddDiagnostic(message);
219217
}
220218
}
221219

‎csharp/autobuilder/Semmle.Autobuild.CSharp/DotNetRule.cs‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,14 @@ namespace Semmle.Autobuild.CSharp
1515
/// </summary>
1616
internal class DotNetRule : IBuildRule<CSharpAutobuildOptions>
1717
{
18-
private IEnumerable<Project<CSharpAutobuildOptions>> notDotNetProjects;
19-
2018
public readonly List<IProjectOrSolution> FailedProjectsOrSolutions = new();
2119

2220
/// <summary>
2321
/// A list of projects which are incompatible with DotNet.
2422
/// </summary>
25-
public IEnumerable<Project<CSharpAutobuildOptions>> NotDotNetProjects
26-
{
27-
get { return this.notDotNetProjects; }
28-
}
23+
public IEnumerable<Project<CSharpAutobuildOptions>> NotDotNetProjects { get; private set; }
2924

30-
public DotNetRule()
31-
{
32-
this.notDotNetProjects = new List<Project<CSharpAutobuildOptions>>();
33-
}
25+
public DotNetRule() => NotDotNetProjects = new List<Project<CSharpAutobuildOptions>>();
3426

3527
public BuildScript Analyse(IAutobuilder<CSharpAutobuildOptions> builder, bool auto)
3628
{
@@ -39,11 +31,11 @@ public BuildScript Analyse(IAutobuilder<CSharpAutobuildOptions> builder, bool au
3931

4032
if (auto)
4133
{
42-
notDotNetProjects = builder.ProjectsOrSolutionsToBuild
34+
NotDotNetProjects = builder.ProjectsOrSolutionsToBuild
4335
.SelectMany(p => Enumerators.Singleton(p).Concat(p.IncludedProjects))
4436
.OfType<Project<CSharpAutobuildOptions>>()
4537
.Where(p => !p.DotNetProject);
46-
var notDotNetProject = notDotNetProjects.FirstOrDefault();
38+
var notDotNetProject = NotDotNetProjects.FirstOrDefault();
4739

4840
if (notDotNetProject is not null)
4941
{

‎csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ public void Log(Severity severity, string format, params object[] args)
284284
/// Write <paramref name="diagnostic"/> to the diagnostics file.
285285
/// </summary>
286286
/// <param name="diagnostic">The diagnostics entry to write.</param>
287-
public void Diagnostic(DiagnosticMessage diagnostic)
287+
public void AddDiagnostic(DiagnosticMessage diagnostic)
288288
{
289289
diagnostics.AddEntry(diagnostic);
290290
}
@@ -320,13 +320,11 @@ void exitCallback(int ret, string msg, bool silent)
320320
// if the build succeeded, all diagnostics we captured from the build output should be warnings;
321321
// otherwise they should all be errors
322322
var diagSeverity = buildResult == 0 ? DiagnosticMessage.TspSeverity.Warning : DiagnosticMessage.TspSeverity.Error;
323-
foreach (var diagResult in this.DiagnosticClassifier.Results)
323+
this.DiagnosticClassifier.Results.Select(result => result.ToDiagnosticMessage(this)).ForEach(result =>
324324
{
325-
var diag = diagResult.ToDiagnosticMessage(this);
326-
diag.Severity = diagSeverity;
327-
328-
Diagnostic(diag);
329-
}
325+
result.Severity = diagSeverity;
326+
AddDiagnostic(result);
327+
});
330328

331329
return buildResult;
332330
}
@@ -345,8 +343,11 @@ void exitCallback(int ret, string msg, bool silent)
345343
/// <returns>The resulting <see cref="DiagnosticMessage" />.</returns>
346344
public DiagnosticMessage MakeDiagnostic(string id, string name)
347345
{
348-
DiagnosticMessage diag = new(new($"{this.Options.Language.UpperCaseName.ToLower()}/autobuilder/{id}", name));
349-
diag.Source.ExtractorName = Options.Language.UpperCaseName.ToLower();
346+
DiagnosticMessage diag = new(new(
347+
$"{this.Options.Language.UpperCaseName.ToLower()}/autobuilder/{id}",
348+
name,
349+
Options.Language.UpperCaseName.ToLower()
350+
));
350351
diag.Visibility.StatusPage = true;
351352

352353
return diag;
@@ -367,7 +368,7 @@ protected virtual void AutobuildFailureDiagnostic()
367368
"You can manually specify a suitable build command for your project.";
368369
message.Severity = DiagnosticMessage.TspSeverity.Error;
369370

370-
Diagnostic(message);
371+
AddDiagnostic(message);
371372
}
372373

373374
/// <summary>

‎csharp/autobuilder/Semmle.Autobuild.Shared/BuildCommandAutoRule.cs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,21 @@ namespace Semmle.Autobuild.Shared
1010
public class BuildCommandAutoRule : IBuildRule<AutobuildOptionsShared>
1111
{
1212
private readonly WithDotNet<AutobuildOptionsShared> withDotNet;
13-
private IEnumerable<string> candidatePaths;
14-
private string? scriptPath;
1513

1614
/// <summary>
1715
/// A list of paths to files in the project directory which we classified as scripts.
1816
/// </summary>
19-
public IEnumerable<string> CandidatePaths
20-
{
21-
get { return this.candidatePaths; }
22-
}
17+
public IEnumerable<string> CandidatePaths { get; private set; }
2318

2419
/// <summary>
2520
/// The path of the script we decided to run, if any.
2621
/// </summary>
27-
public string? ScriptPath
28-
{
29-
get { return this.scriptPath; }
30-
}
22+
public string? ScriptPath { get; private set; }
3123

3224
public BuildCommandAutoRule(WithDotNet<AutobuildOptionsShared> withDotNet)
3325
{
3426
this.withDotNet = withDotNet;
35-
this.candidatePaths = new List<string>();
27+
this.CandidatePaths = new List<string>();
3628
}
3729

3830
/// <summary>
@@ -70,18 +62,18 @@ public BuildScript Analyse(IAutobuilder<AutobuildOptionsShared> builder, bool au
7062
var scripts = buildScripts.SelectMany(s => extensions.Select(e => s + e));
7163
// search through the files in the project directory for paths which end in one of
7264
// the names given by `scripts`, then order them by their distance from the root
73-
this.candidatePaths = builder.Paths.Where(p => scripts.Any(p.Item1.ToLower().EndsWith)).OrderBy(p => p.Item2).Select(p => p.Item1);
65+
this.CandidatePaths = builder.Paths.Where(p => scripts.Any(p.Item1.ToLower().EndsWith)).OrderBy(p => p.Item2).Select(p => p.Item1);
7466
// pick the first matching path, if there is one
75-
this.scriptPath = candidatePaths.FirstOrDefault();
67+
this.ScriptPath = this.CandidatePaths.FirstOrDefault();
7668

77-
if (scriptPath is null)
69+
if (this.ScriptPath is null)
7870
return BuildScript.Failure;
7971

8072
var chmod = new CommandBuilder(builder.Actions);
81-
chmod.RunCommand("/bin/chmod", $"u+x {scriptPath}");
73+
chmod.RunCommand("/bin/chmod", $"u+x {this.ScriptPath}");
8274
var chmodScript = builder.Actions.IsWindows() ? BuildScript.Success : BuildScript.Try(chmod.Script);
8375

84-
var dir = builder.Actions.GetDirectoryName(scriptPath);
76+
var dir = builder.Actions.GetDirectoryName(this.ScriptPath);
8577

8678
// A specific .NET Core version may be required
8779
return chmodScript & withDotNet(builder, environment =>
@@ -93,7 +85,7 @@ public BuildScript Analyse(IAutobuilder<AutobuildOptionsShared> builder, bool au
9385
if (vsTools is not null)
9486
command.CallBatFile(vsTools.Path);
9587

96-
command.RunCommand(scriptPath);
88+
command.RunCommand(this.ScriptPath);
9789
return command.Script;
9890
});
9991
}

‎csharp/autobuilder/Semmle.Autobuild.Shared/DiagnosticClassifier.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,14 @@ protected void AddRule(DiagnosticRule rule)
8080
/// <param name="line">The line to which the rules should be applied to.</param>
8181
public void ClassifyLine(string line)
8282
{
83-
foreach (var rule in this.rules)
83+
this.rules.ForEach(rule =>
8484
{
8585
var match = rule.Pattern.Match(line);
8686
if (match.Success)
8787
{
8888
rule.Fire(this, match);
8989
}
90-
}
91-
90+
});
9291
}
9392
}
9493
}

‎csharp/extractor/Semmle.Util/Semmle.Util.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
<ItemGroup>
1717
<PackageReference Include="Mono.Posix.NETStandard" Version="1.0.0" />
18-
<PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
18+
<PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
1919
</ItemGroup>
2020

2121
</Project>

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,13 @@ public class TspSource
3131
/// <summary>
3232
/// Name of the CodeQL extractor. This is used to identify which tool component the reporting descriptor object should be nested under in SARIF.
3333
/// </summary>
34-
public string? ExtractorName { get; set; }
34+
public string? ExtractorName { get; }
3535

36-
public TspSource(string id, string name)
36+
public TspSource(string id, string name, string? extractorName = null)
3737
{
3838
Id = id;
3939
Name = name;
40+
ExtractorName = extractorName;
4041
}
4142
}
4243

@@ -66,6 +67,13 @@ public class TspVisibility
6667
/// True if the message should be sent to telemetry (defaults to false).
6768
/// </summary>
6869
public bool? Telemetry { get; set; }
70+
71+
public TspVisibility(bool? statusPage = null, bool? cliSummaryTable = null, bool? telemetry = null)
72+
{
73+
this.StatusPage = statusPage;
74+
this.CLISummaryTable = cliSummaryTable;
75+
this.Telemetry = telemetry;
76+
}
6977
}
7078

7179
public class TspLocation
@@ -78,6 +86,15 @@ public class TspLocation
7886
public int? StartColumn { get; set; }
7987
public int? EndLine { get; set; }
8088
public int? EndColumn { get; set; }
89+
90+
public TspLocation(string? file = null, int? startLine = null, int? startColumn = null, int? endLine = null, int? endColumn = null)
91+
{
92+
this.File = file;
93+
this.StartLine = startLine;
94+
this.StartColumn = startColumn;
95+
this.EndLine = endLine;
96+
this.EndColumn = endColumn;
97+
}
8198
}
8299

83100
/// <summary>
@@ -109,9 +126,6 @@ public class TspLocation
109126
/// If true, then this message won't be presented to users.
110127
/// </summary>
111128
public bool Internal { get; set; }
112-
/// <summary>
113-
///
114-
/// </summary>
115129
public TspVisibility Visibility { get; }
116130
public TspLocation Location { get; }
117131
/// <summary>
@@ -162,7 +176,8 @@ public DiagnosticsStream(StreamWriter streamWriter)
162176

163177
serializer = new JsonSerializer
164178
{
165-
ContractResolver = contractResolver
179+
ContractResolver = contractResolver,
180+
NullValueHandling = NullValueHandling.Ignore
166181
};
167182

168183
writer = streamWriter;

0 commit comments

Comments
 (0)