Skip to content

Commit df3da23

Browse files
belavbelav
authored
Improving the logic around editor config parsing so that we only look for them on demand instead of finding them all (#1481)
closes #1228
1 parent 8551fa1 commit df3da23

File tree

11 files changed

+202
-237
lines changed

11 files changed

+202
-237
lines changed

‎Src/CSharpier.Cli/CommandLineFormatter.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ CancellationToken cancellationToken
4747
commandLineOptions.ConfigPath,
4848
fileSystem,
4949
logger,
50-
cancellationToken,
51-
limitConfigSearch: true
50+
cancellationToken
5251
);
5352

5453
if (

Src/CSharpier.Cli/EditorConfig/ConfigFile.cs renamed to Src/CSharpier.Cli/EditorConfig/EditorConfigFile.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
namespace CSharpier.Cli.EditorConfig;
22

3-
internal class ConfigFile
3+
internal class EditorConfigFile
44
{
55
public required IReadOnlyCollection<Section> Sections { get; init; }
66
public bool IsRoot { get; init; }

Src/CSharpier.Cli/EditorConfig/ConfigFileParser.cs renamed to Src/CSharpier.Cli/EditorConfig/EditorConfigFileParser.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace CSharpier.Cli.EditorConfig;
88

9-
internal static class ConfigFileParser
9+
internal static class EditorConfigFileParser
1010
{
1111
// According to https://spec.editorconfig.org/#file-format
1212
// "Comment: starts with a ; or a #."
@@ -22,8 +22,9 @@ internal static class ConfigFileParser
2222
ThrowExceptionsOnError = false,
2323
};
2424

25-
public static ConfigFile Parse(string filePath, IFileSystem fileSystem)
25+
public static EditorConfigFile Parse(string filePath, IFileSystem fileSystem)
2626
{
27+
DebugLogger.Log("Reading file at " + filePath);
2728
var directory = fileSystem.Path.GetDirectoryName(filePath);
2829

2930
ArgumentNullException.ThrowIfNull(directory);
@@ -40,7 +41,7 @@ public static ConfigFile Parse(string filePath, IFileSystem fileSystem)
4041
sections.AddRange(configData.Sections.Select(s => new Section(s, directory)));
4142
}
4243

43-
return new ConfigFile
44+
return new EditorConfigFile
4445
{
4546
IsRoot = configData?.Global["root"] == "true",
4647
Sections = sections,

Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs renamed to Src/CSharpier.Cli/EditorConfig/EditorConfigLocator.cs

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2,68 +2,42 @@
22

33
namespace CSharpier.Cli.EditorConfig;
44

5-
// TODO for a given directory find the config in it and going up til root
6-
// keep track of directories + their corresponding configs
7-
// if searching a new directory and we go up to a parent that contains the first config, use that
8-
// how many directories would we run into? enough to matter?
9-
internal static class EditorConfigParser
5+
internal static class EditorConfigLocator
106
{
11-
/// <summary>Finds all configs above the given directory as well as within the subtree of this directory</summary>
12-
public static List<EditorConfigSections> FindForDirectoryName(
7+
public static EditorConfigSections? FindForDirectoryName(
138
string directoryName,
149
IFileSystem fileSystem,
15-
// not the best name, but I plan on rewriting this to not find all of the configs up front
16-
// which will remove this parameter
17-
bool limitEditorConfigSearch,
1810
IgnoreFile ignoreFile
1911
)
2012
{
2113
if (directoryName is "")
2214
{
23-
return new List<EditorConfigSections>();
15+
return null;
2416
}
2517

2618
var directoryInfo = fileSystem.DirectoryInfo.New(directoryName);
27-
var editorConfigFiles = directoryInfo
28-
.EnumerateFiles(
29-
".editorconfig",
30-
limitEditorConfigSearch
31-
? SearchOption.TopDirectoryOnly
32-
: SearchOption.AllDirectories
33-
)
34-
.Where(x => !ignoreFile.IsIgnored(x.FullName))
35-
.ToList();
36-
37-
// already found any in this directory above
38-
directoryInfo = directoryInfo.Parent;
3919

4020
while (directoryInfo is not null)
4121
{
4222
var file = fileSystem.FileInfo.New(
4323
fileSystem.Path.Combine(directoryInfo.FullName, ".editorconfig")
4424
);
45-
if (file.Exists)
46-
{
47-
editorConfigFiles.Add(file);
48-
}
49-
50-
directoryInfo = directoryInfo.Parent;
51-
}
52-
53-
return editorConfigFiles
54-
.Select(o =>
25+
if (file.Exists && !ignoreFile.IsIgnored(file.FullName))
5526
{
56-
var dirName = fileSystem.Path.GetDirectoryName(o.FullName);
27+
var dirName = fileSystem.Path.GetDirectoryName(file.FullName);
5728
ArgumentNullException.ThrowIfNull(dirName);
5829

5930
return new EditorConfigSections
6031
{
6132
DirectoryName = dirName,
62-
SectionsIncludingParentFiles = FindSections(o.FullName, fileSystem),
33+
SectionsIncludingParentFiles = FindSections(file.FullName, fileSystem),
6334
};
64-
})
65-
.OrderByDescending(o => o.DirectoryName.Length)
66-
.ToList();
35+
}
36+
37+
directoryInfo = directoryInfo.Parent;
38+
}
39+
40+
return null;
6741
}
6842

6943
private static List<Section> FindSections(string filePath, IFileSystem fileSystem)
@@ -74,7 +48,7 @@ private static List<Section> FindSections(string filePath, IFileSystem fileSyste
7448
.ToList();
7549
}
7650

77-
private static IEnumerable<ConfigFile> ParseConfigFiles(
51+
private static IEnumerable<EditorConfigFile> ParseConfigFiles(
7852
string directoryPath,
7953
IFileSystem fileSystem
8054
)
@@ -85,7 +59,7 @@ IFileSystem fileSystem
8559
var potentialPath = fileSystem.Path.Combine(directory.FullName, ".editorconfig");
8660
if (fileSystem.File.Exists(potentialPath))
8761
{
88-
var configFile = ConfigFileParser.Parse(potentialPath, fileSystem);
62+
var configFile = EditorConfigFileParser.Parse(potentialPath, fileSystem);
8963

9064
yield return configFile;
9165
if (configFile.IsRoot)

‎Src/CSharpier.Cli/HasMismatchedCliAndMsBuildVersions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public static bool Check(string directoryName, IFileSystem fileSystem, ILogger l
1919

2020
string? GetPackagesVersion(string pathToCsProj)
2121
{
22-
var directory = new DirectoryInfo(Path.GetDirectoryName(pathToCsProj)!);
22+
var directory = fileSystem.DirectoryInfo.New(Path.GetDirectoryName(pathToCsProj)!);
2323
while (directory != null)
2424
{
2525
var filePath = Path.Combine(directory.FullName, "Directory.Packages.props");

‎Src/CSharpier.Cli/IgnoreFile.cs‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,32 @@ protected IgnoreFile(Ignore.Ignore ignore, string ignoreBaseDirectoryPath)
1616

1717
public bool IsIgnored(string filePath)
1818
{
19-
var normalizedFilePath = filePath.Replace('\\', '/');
20-
if (!normalizedFilePath.StartsWith(this.IgnoreBaseDirectoryPath, StringComparison.Ordinal))
19+
var pathRelativeToIgnoreFile = filePath.Replace('\\', '/');
20+
if (
21+
!pathRelativeToIgnoreFile.StartsWith(
22+
this.IgnoreBaseDirectoryPath,
23+
StringComparison.Ordinal
24+
)
25+
)
2126
{
22-
throw new Exception(
23-
"The filePath of "
24-
+ filePath
25-
+ " does not start with the ignoreBaseDirectoryPath of "
26-
+ this.IgnoreBaseDirectoryPath
27-
);
27+
// TODO #631
28+
// the ignore file was created for /test/subfolder
29+
// and we are checking if /test/.editorconfig is ignored, which it CAN'T be
30+
// in main, we did not check if parent folder editorconfigs were ignored, which is probably a bug
31+
// we need to figure out a better way to deal with ignorefiles, which will come in the PR for 631
32+
33+
return false;
34+
// throw new Exception(
35+
// $"The filePath of {filePath} does not start with the ignoreBaseDirectoryPath of {this.IgnoreBaseDirectoryPath}"
36+
// );
2837
}
2938

30-
normalizedFilePath =
31-
normalizedFilePath.Length > this.IgnoreBaseDirectoryPath.Length
32-
? normalizedFilePath[(this.IgnoreBaseDirectoryPath.Length + 1)..]
39+
pathRelativeToIgnoreFile =
40+
pathRelativeToIgnoreFile.Length > this.IgnoreBaseDirectoryPath.Length
41+
? pathRelativeToIgnoreFile[(this.IgnoreBaseDirectoryPath.Length + 1)..]
3342
: string.Empty;
3443

35-
return this.Ignore.IsIgnored(normalizedFilePath);
44+
return this.Ignore.IsIgnored(pathRelativeToIgnoreFile);
3645
}
3746

3847
public static async Task<IgnoreFile> Create(

Src/CSharpier.Cli/Options/ConfigFileParser.cs renamed to Src/CSharpier.Cli/Options/CSharpierConfigParser.cs

Lines changed: 7 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,54 +6,22 @@
66

77
namespace CSharpier.Cli.Options;
88

9-
internal static class ConfigFileParser
9+
internal static class CSharpierConfigParser
1010
{
1111
private static readonly string[] validExtensions = [".csharpierrc", ".json", ".yml", ".yaml"];
1212
private static readonly JsonSerializerOptions CaseInsensitiveJson = new()
1313
{
1414
PropertyNameCaseInsensitive = true,
1515
};
1616

17-
/// <summary>Finds all configs above the given directory as well as within the subtree of this directory</summary>
18-
internal static List<CSharpierConfigData> FindForDirectoryName(
17+
internal static CSharpierConfigData? FindForDirectoryName(
1918
string directoryName,
2019
IFileSystem fileSystem,
21-
ILogger logger,
22-
bool limitEditorConfigSearch
20+
ILogger logger
2321
)
2422
{
25-
var results = new List<CSharpierConfigData>();
2623
var directoryInfo = fileSystem.DirectoryInfo.New(directoryName);
2724

28-
var filesByDirectory = directoryInfo
29-
.EnumerateFiles(
30-
".csharpierrc*",
31-
limitEditorConfigSearch
32-
? SearchOption.TopDirectoryOnly
33-
: SearchOption.AllDirectories
34-
)
35-
.GroupBy(o => o.DirectoryName);
36-
37-
foreach (var group in filesByDirectory)
38-
{
39-
var firstFile = group
40-
.Where(o => validExtensions.Contains(o.Extension, StringComparer.OrdinalIgnoreCase))
41-
.MinBy(o => o.Extension);
42-
43-
if (firstFile != null)
44-
{
45-
results.Add(
46-
new CSharpierConfigData(
47-
firstFile.DirectoryName!,
48-
Create(firstFile.FullName, fileSystem, logger)
49-
)
50-
);
51-
}
52-
}
53-
54-
// already found any in this directory above
55-
directoryInfo = directoryInfo.Parent;
56-
5725
while (directoryInfo is not null)
5826
{
5927
var file = directoryInfo
@@ -63,18 +31,16 @@ bool limitEditorConfigSearch
6331

6432
if (file != null)
6533
{
66-
results.Add(
67-
new CSharpierConfigData(
68-
file.DirectoryName!,
69-
Create(file.FullName, fileSystem, logger)
70-
)
34+
return new CSharpierConfigData(
35+
file.DirectoryName!,
36+
Create(file.FullName, fileSystem, logger)
7137
);
7238
}
7339

7440
directoryInfo = directoryInfo.Parent;
7541
}
7642

77-
return results.OrderByDescending(o => o.DirectoryName.Length).ToList();
43+
return null;
7844
}
7945

8046
internal static ConfigurationFileOptions Create(

0 commit comments

Comments
 (0)