Make improvements based on PR feedback

This commit is contained in:
Michael B. Gale 2023-02-22 12:29:10 +00:00
Родитель 62cd8ca26f
Коммит 9d19752c2e
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: FF5E2765BD00628F
10 изменённых файлов: 60 добавлений и 69 удалений

Просмотреть файл

@ -1,5 +1,6 @@
using Xunit;
using Semmle.Autobuild.Shared;
using Semmle.Util;
using System.Collections.Generic;
using System;
using System.Linq;
@ -79,10 +80,7 @@ namespace Semmle.Autobuild.Cpp.Tests
{
var ret = (this as IBuildActions).RunProcess(cmd, args, workingDirectory, env, out var stdout);
foreach (var line in stdout)
{
onOutput(line);
}
stdout.ForEach(line => onOutput(line));
return ret;
}

Просмотреть файл

@ -21,12 +21,10 @@ namespace Semmle.Autobuild.Cpp
public class CppAutobuilder : Autobuilder<CppAutobuildOptions>
{
private DiagnosticClassifier classifier;
private readonly DiagnosticClassifier classifier;
public CppAutobuilder(IBuildActions actions, CppAutobuildOptions options) : base(actions, options)
{
public CppAutobuilder(IBuildActions actions, CppAutobuildOptions options) : base(actions, options) =>
classifier = new DiagnosticClassifier();
}
protected override DiagnosticClassifier DiagnosticClassifier => classifier;

Просмотреть файл

@ -1,5 +1,6 @@
using Xunit;
using Semmle.Autobuild.Shared;
using Semmle.Util;
using System.Collections.Generic;
using System;
using System.Linq;
@ -89,10 +90,7 @@ namespace Semmle.Autobuild.CSharp.Tests
{
var ret = (this as IBuildActions).RunProcess(cmd, args, workingDirectory, env, out var stdout);
foreach (var line in stdout)
{
onOutput(line);
}
stdout.ForEach(line => onOutput(line));
return ret;
}

Просмотреть файл

@ -41,10 +41,8 @@ namespace Semmle.Autobuild.CSharp
protected override DiagnosticClassifier DiagnosticClassifier => diagnosticClassifier;
public CSharpAutobuilder(IBuildActions actions, CSharpAutobuildOptions options) : base(actions, options)
{
public CSharpAutobuilder(IBuildActions actions, CSharpAutobuildOptions options) : base(actions, options) =>
diagnosticClassifier = new CSharpDiagnosticClassifier();
}
public override BuildScript GetBuildScript()
{
@ -164,7 +162,7 @@ namespace Semmle.Autobuild.CSharp
}
message.Severity = DiagnosticMessage.TspSeverity.Error;
Diagnostic(message);
AddDiagnostic(message);
}
// both dotnet and msbuild builds require project or solution files; if we haven't found any
@ -177,7 +175,7 @@ namespace Semmle.Autobuild.CSharp
"You can manually specify a suitable build command for your project.";
message.Severity = DiagnosticMessage.TspSeverity.Error;
Diagnostic(message);
AddDiagnostic(message);
}
else if (dotNetRule is not null && dotNetRule.NotDotNetProjects.Any())
{
@ -187,7 +185,7 @@ namespace Semmle.Autobuild.CSharp
string.Join('\n', dotNetRule.NotDotNetProjects.Select(p => $"- `{p.FullPath}`"));
message.Severity = DiagnosticMessage.TspSeverity.Warning;
Diagnostic(message);
AddDiagnostic(message);
}
// report any projects that failed to build with .NET Core
@ -201,7 +199,7 @@ namespace Semmle.Autobuild.CSharp
"or to ensure that they can be built successfully.";
message.Severity = DiagnosticMessage.TspSeverity.Error;
Diagnostic(message);
AddDiagnostic(message);
}
// report any projects that failed to build with MSBuild
@ -215,7 +213,7 @@ namespace Semmle.Autobuild.CSharp
"or to ensure that they can be built successfully.";;
message.Severity = DiagnosticMessage.TspSeverity.Error;
Diagnostic(message);
AddDiagnostic(message);
}
}

Просмотреть файл

@ -15,22 +15,14 @@ namespace Semmle.Autobuild.CSharp
/// </summary>
internal class DotNetRule : IBuildRule<CSharpAutobuildOptions>
{
private IEnumerable<Project<CSharpAutobuildOptions>> notDotNetProjects;
public readonly List<IProjectOrSolution> FailedProjectsOrSolutions = new();
/// <summary>
/// A list of projects which are incompatible with DotNet.
/// </summary>
public IEnumerable<Project<CSharpAutobuildOptions>> NotDotNetProjects
{
get { return this.notDotNetProjects; }
}
public IEnumerable<Project<CSharpAutobuildOptions>> NotDotNetProjects { get; private set; }
public DotNetRule()
{
this.notDotNetProjects = new List<Project<CSharpAutobuildOptions>>();
}
public DotNetRule() => NotDotNetProjects = new List<Project<CSharpAutobuildOptions>>();
public BuildScript Analyse(IAutobuilder<CSharpAutobuildOptions> builder, bool auto)
{
@ -39,11 +31,11 @@ namespace Semmle.Autobuild.CSharp
if (auto)
{
notDotNetProjects = builder.ProjectsOrSolutionsToBuild
NotDotNetProjects = builder.ProjectsOrSolutionsToBuild
.SelectMany(p => Enumerators.Singleton(p).Concat(p.IncludedProjects))
.OfType<Project<CSharpAutobuildOptions>>()
.Where(p => !p.DotNetProject);
var notDotNetProject = notDotNetProjects.FirstOrDefault();
var notDotNetProject = NotDotNetProjects.FirstOrDefault();
if (notDotNetProject is not null)
{

Просмотреть файл

@ -284,7 +284,7 @@ namespace Semmle.Autobuild.Shared
/// Write <paramref name="diagnostic"/> to the diagnostics file.
/// </summary>
/// <param name="diagnostic">The diagnostics entry to write.</param>
public void Diagnostic(DiagnosticMessage diagnostic)
public void AddDiagnostic(DiagnosticMessage diagnostic)
{
diagnostics.AddEntry(diagnostic);
}
@ -320,13 +320,11 @@ namespace Semmle.Autobuild.Shared
// if the build succeeded, all diagnostics we captured from the build output should be warnings;
// otherwise they should all be errors
var diagSeverity = buildResult == 0 ? DiagnosticMessage.TspSeverity.Warning : DiagnosticMessage.TspSeverity.Error;
foreach (var diagResult in this.DiagnosticClassifier.Results)
this.DiagnosticClassifier.Results.Select(result => result.ToDiagnosticMessage(this)).ForEach(result =>
{
var diag = diagResult.ToDiagnosticMessage(this);
diag.Severity = diagSeverity;
Diagnostic(diag);
}
result.Severity = diagSeverity;
AddDiagnostic(result);
});
return buildResult;
}
@ -345,8 +343,11 @@ namespace Semmle.Autobuild.Shared
/// <returns>The resulting <see cref="DiagnosticMessage" />.</returns>
public DiagnosticMessage MakeDiagnostic(string id, string name)
{
DiagnosticMessage diag = new(new($"{this.Options.Language.UpperCaseName.ToLower()}/autobuilder/{id}", name));
diag.Source.ExtractorName = Options.Language.UpperCaseName.ToLower();
DiagnosticMessage diag = new(new(
$"{this.Options.Language.UpperCaseName.ToLower()}/autobuilder/{id}",
name,
Options.Language.UpperCaseName.ToLower()
));
diag.Visibility.StatusPage = true;
return diag;
@ -367,7 +368,7 @@ namespace Semmle.Autobuild.Shared
"You can manually specify a suitable build command for your project.";
message.Severity = DiagnosticMessage.TspSeverity.Error;
Diagnostic(message);
AddDiagnostic(message);
}
/// <summary>

Просмотреть файл

@ -10,29 +10,21 @@ namespace Semmle.Autobuild.Shared
public class BuildCommandAutoRule : IBuildRule<AutobuildOptionsShared>
{
private readonly WithDotNet<AutobuildOptionsShared> withDotNet;
private IEnumerable<string> candidatePaths;
private string? scriptPath;
/// <summary>
/// A list of paths to files in the project directory which we classified as scripts.
/// </summary>
public IEnumerable<string> CandidatePaths
{
get { return this.candidatePaths; }
}
public IEnumerable<string> CandidatePaths { get; private set; }
/// <summary>
/// The path of the script we decided to run, if any.
/// </summary>
public string? ScriptPath
{
get { return this.scriptPath; }
}
public string? ScriptPath { get; private set; }
public BuildCommandAutoRule(WithDotNet<AutobuildOptionsShared> withDotNet)
{
this.withDotNet = withDotNet;
this.candidatePaths = new List<string>();
this.CandidatePaths = new List<string>();
}
/// <summary>
@ -70,18 +62,18 @@ namespace Semmle.Autobuild.Shared
var scripts = buildScripts.SelectMany(s => extensions.Select(e => s + e));
// search through the files in the project directory for paths which end in one of
// the names given by `scripts`, then order them by their distance from the root
this.candidatePaths = builder.Paths.Where(p => scripts.Any(p.Item1.ToLower().EndsWith)).OrderBy(p => p.Item2).Select(p => p.Item1);
this.CandidatePaths = builder.Paths.Where(p => scripts.Any(p.Item1.ToLower().EndsWith)).OrderBy(p => p.Item2).Select(p => p.Item1);
// pick the first matching path, if there is one
this.scriptPath = candidatePaths.FirstOrDefault();
this.ScriptPath = this.CandidatePaths.FirstOrDefault();
if (scriptPath is null)
if (this.ScriptPath is null)
return BuildScript.Failure;
var chmod = new CommandBuilder(builder.Actions);
chmod.RunCommand("/bin/chmod", $"u+x {scriptPath}");
chmod.RunCommand("/bin/chmod", $"u+x {this.ScriptPath}");
var chmodScript = builder.Actions.IsWindows() ? BuildScript.Success : BuildScript.Try(chmod.Script);
var dir = builder.Actions.GetDirectoryName(scriptPath);
var dir = builder.Actions.GetDirectoryName(this.ScriptPath);
// A specific .NET Core version may be required
return chmodScript & withDotNet(builder, environment =>
@ -93,7 +85,7 @@ namespace Semmle.Autobuild.Shared
if (vsTools is not null)
command.CallBatFile(vsTools.Path);
command.RunCommand(scriptPath);
command.RunCommand(this.ScriptPath);
return command.Script;
});
}

Просмотреть файл

@ -80,15 +80,14 @@ namespace Semmle.Autobuild.Shared
/// <param name="line">The line to which the rules should be applied to.</param>
public void ClassifyLine(string line)
{
foreach (var rule in this.rules)
this.rules.ForEach(rule =>
{
var match = rule.Pattern.Match(line);
if (match.Success)
{
rule.Fire(this, match);
}
}
});
}
}
}

Просмотреть файл

@ -15,7 +15,7 @@
<ItemGroup>
<PackageReference Include="Mono.Posix.NETStandard" Version="1.0.0" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
</ItemGroup>
</Project>

Просмотреть файл

@ -31,12 +31,13 @@ namespace Semmle.Util
/// <summary>
/// Name of the CodeQL extractor. This is used to identify which tool component the reporting descriptor object should be nested under in SARIF.
/// </summary>
public string? ExtractorName { get; set; }
public string? ExtractorName { get; }
public TspSource(string id, string name)
public TspSource(string id, string name, string? extractorName = null)
{
Id = id;
Name = name;
ExtractorName = extractorName;
}
}
@ -66,6 +67,13 @@ namespace Semmle.Util
/// True if the message should be sent to telemetry (defaults to false).
/// </summary>
public bool? Telemetry { get; set; }
public TspVisibility(bool? statusPage = null, bool? cliSummaryTable = null, bool? telemetry = null)
{
this.StatusPage = statusPage;
this.CLISummaryTable = cliSummaryTable;
this.Telemetry = telemetry;
}
}
public class TspLocation
@ -78,6 +86,15 @@ namespace Semmle.Util
public int? StartColumn { get; set; }
public int? EndLine { get; set; }
public int? EndColumn { get; set; }
public TspLocation(string? file = null, int? startLine = null, int? startColumn = null, int? endLine = null, int? endColumn = null)
{
this.File = file;
this.StartLine = startLine;
this.StartColumn = startColumn;
this.EndLine = endLine;
this.EndColumn = endColumn;
}
}
/// <summary>
@ -109,9 +126,6 @@ namespace Semmle.Util
/// If true, then this message won't be presented to users.
/// </summary>
public bool Internal { get; set; }
/// <summary>
///
/// </summary>
public TspVisibility Visibility { get; }
public TspLocation Location { get; }
/// <summary>
@ -162,7 +176,8 @@ namespace Semmle.Util
serializer = new JsonSerializer
{
ContractResolver = contractResolver
ContractResolver = contractResolver,
NullValueHandling = NullValueHandling.Ignore
};
writer = streamWriter;