Skip to content

Commit e4f68ae

Browse files
committed
C#: Address review comments
1 parent 836daaf commit e4f68ae

File tree

8 files changed

+23
-45
lines changed

8 files changed

+23
-45
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,6 @@ string IBuildActions.PathCombine(params string[] parts)
135135

136136
string IBuildActions.GetFullPath(string path) => path;
137137

138-
string IBuildActions.GetDirectoryName(string path) => System.IO.Path.GetDirectoryName(path);
139-
140138
void IBuildActions.WriteAllText(string filename, string contents)
141139
{
142140
}

csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,10 @@ public Autobuilder(IBuildActions actions, AutobuildOptions options)
128128

129129
projectsOrSolutionsToBuildLazy = new Lazy<IList<IProjectOrSolution>>(() =>
130130
{
131+
List<IProjectOrSolution> ret;
131132
if (options.Solution.Any())
132133
{
133-
var ret = new List<IProjectOrSolution>();
134+
ret = new List<IProjectOrSolution>();
134135
foreach (var solution in options.Solution)
135136
{
136137
if (actions.FileExists(solution))
@@ -141,53 +142,43 @@ public Autobuilder(IBuildActions actions, AutobuildOptions options)
141142
return ret;
142143
}
143144

144-
bool FindFiles(string extension, Func<string, ProjectOrSolution> create, out IEnumerable<IProjectOrSolution> files)
145+
IEnumerable<IProjectOrSolution> FindFiles(string extension, Func<string, ProjectOrSolution> create)
145146
{
146-
var allFiles = GetExtensions(extension).
147+
var matchingFiles = GetExtensions(extension).
147148
Select(p => (ProjectOrSolution: create(p.Item1), DistanceFromRoot: p.Item2)).
148149
Where(p => p.ProjectOrSolution.HasLanguage(this.Options.Language)).
149150
ToArray();
150151

151-
if (allFiles.Length == 0)
152-
{
153-
files = null;
154-
return false;
155-
}
152+
if (matchingFiles.Length == 0)
153+
return null;
156154

157155
if (options.AllSolutions)
158-
{
159-
files = allFiles.Select(p => p.ProjectOrSolution);
160-
return true;
161-
}
156+
return matchingFiles.Select(p => p.ProjectOrSolution);
162157

163-
var firstIsClosest = allFiles.Length > 1 && allFiles[0].DistanceFromRoot < allFiles[1].DistanceFromRoot;
164-
if (allFiles.Length == 1 || firstIsClosest)
165-
{
166-
files = allFiles.Select(p => p.ProjectOrSolution).Take(1);
167-
return true;
168-
}
158+
var firstIsClosest = matchingFiles.Length > 1 && matchingFiles[0].DistanceFromRoot < matchingFiles[1].DistanceFromRoot;
159+
if (matchingFiles.Length == 1 || firstIsClosest)
160+
return matchingFiles.Select(p => p.ProjectOrSolution).Take(1);
169161

170-
var candidates = allFiles.
171-
Where(f => f.DistanceFromRoot == allFiles[0].DistanceFromRoot).
162+
var candidates = matchingFiles.
163+
Where(f => f.DistanceFromRoot == matchingFiles[0].DistanceFromRoot).
172164
Select(f => f.ProjectOrSolution);
173165
Log(Severity.Info, $"Found multiple '{extension}' files, giving up: {string.Join(", ", candidates)}.");
174-
files = new IProjectOrSolution[0];
175-
return true;
166+
return new IProjectOrSolution[0];
176167
}
177168

178169
// First look for `.proj` files
179-
if (FindFiles(".proj", f => new Project(this, f), out var ret1))
180-
return ret1.ToList();
170+
ret = FindFiles(".proj", f => new Project(this, f))?.ToList();
171+
if (ret != null)
172+
return ret;
181173

182174
// Then look for `.sln` files
183-
if (FindFiles(".sln", f => new Solution(this, f), out var ret2))
184-
return ret2.ToList();
175+
ret = FindFiles(".sln", f => new Solution(this, f))?.ToList();
176+
if (ret != null)
177+
return ret;
185178

186179
// Finally look for language specific project files, e.g. `.csproj` files
187-
if (FindFiles(this.Options.Language.ProjectExtension, f => new Project(this, f), out var ret3))
188-
return ret3.ToList();
189-
190-
return new List<IProjectOrSolution>();
180+
ret = FindFiles(this.Options.Language.ProjectExtension, f => new Project(this, f))?.ToList();
181+
return ret ?? new List<IProjectOrSolution>();
191182
});
192183

193184
SemmleDist = Actions.GetEnvironmentVariable("SEMMLE_DIST");

csharp/autobuilder/Semmle.Autobuild/BuildActions.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,6 @@ public interface IBuildActions
102102
/// </summary>
103103
string GetFullPath(string path);
104104

105-
/// <summary>
106-
/// Gets the directory of <paramref name="path"/>, Path.GetDirectoryName().
107-
/// </summary>
108-
string GetDirectoryName(string path);
109-
110105
/// <summary>
111106
/// Writes contents to file, File.WriteAllText().
112107
/// </summary>
@@ -192,8 +187,6 @@ XmlDocument IBuildActions.LoadXml(string filename)
192187

193188
string IBuildActions.GetFullPath(string path) => Path.GetFullPath(path);
194189

195-
string IBuildActions.GetDirectoryName(string path) => Path.GetDirectoryName(path);
196-
197190
public static readonly IBuildActions Instance = new SystemBuildActions();
198191
}
199192
}

csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public BuildScript Analyse(Autobuilder builder)
4040
chmod.RunCommand("/bin/chmod", $"u+x {scriptPath}");
4141
var chmodScript = builder.Actions.IsWindows() ? BuildScript.Success : BuildScript.Try(chmod.Script);
4242

43-
var dir = builder.Actions.GetDirectoryName(scriptPath);
43+
var dir = Path.GetDirectoryName(scriptPath);
4444

4545
// A specific .NET Core version may be required
4646
return chmodScript & DotNetRule.WithDotNet(builder, dotNet =>

csharp/autobuilder/Semmle.Autobuild/BuildScript.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Diagnostics;
43
using System.IO;
5-
using Semmle.Util;
64

75
namespace Semmle.Autobuild
86
{

csharp/autobuilder/Semmle.Autobuild/MsBuildRule.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using Semmle.Util.Logging;
2-
using System.IO;
32
using System.Linq;
43

54
namespace Semmle.Autobuild

csharp/autobuilder/Semmle.Autobuild/Project.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public Project(Autobuilder builder, string path) : base(builder, path)
7878
foreach (var include in projectFileIncludes.Concat(projectFilesIncludes))
7979
{
8080
var includePath = builder.Actions.IsWindows() ? include.Value : include.Value.Replace("\\", "/");
81-
includedProjects.Add(new Project(builder, builder.Actions.PathCombine(builder.Actions.GetDirectoryName(this.FullPath), includePath)));
81+
includedProjects.Add(new Project(builder, builder.Actions.PathCombine(Path.GetDirectoryName(this.FullPath), includePath)));
8282
}
8383
}
8484
}

csharp/autobuilder/Semmle.Autobuild/ProjectOrSolution.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System.Collections.Generic;
2-
using System.IO;
32
using System.Linq;
43

54
namespace Semmle.Autobuild

0 commit comments

Comments
 (0)