Skip to content

Commit

Permalink
Merge pull request #59301 from jasonmalinowski/fix-global-config-perf…
Browse files Browse the repository at this point in the history
…-issue

Cache the unescaped form of globalconfig section names
  • Loading branch information
jasonmalinowski authored Feb 15, 2022
2 parents 05ddb2a + 761400f commit 73eeb59
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2536,6 +2536,29 @@ public void EquivalentSourcePathNames(string sourcePath, bool shouldMatch)
}
}


[Fact]
public void CorrectlyMergeGlobalConfigWithEscapedPaths()
{
var configs = ArrayBuilder<AnalyzerConfig>.GetInstance();
configs.Add(Parse(@"
is_global = true
[/Test.cs]
a = a
[/\Test.cs]
b = b
", "/.editorconfig"));

var configSet = AnalyzerConfigSet.Create(configs, out var diagnostics);
configs.Free();

var options = configSet.GetOptionsForSourcePath("/Test.cs");

Assert.Equal(2, options.AnalyzerOptions.Count);
Assert.Equal("a", options.AnalyzerOptions["a"]);
Assert.Equal("b", options.AnalyzerOptions["b"]);
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public bool IsMatch(string s)
numberRangePairs.ToImmutableAndFree());
}

internal static bool TryUnescapeSectionName(string sectionName, out string? escapedSectionName)
internal static string UnescapeSectionName(string sectionName)
{
var sb = new StringBuilder();
SectionNameLexer lexer = new SectionNameLexer(sectionName);
Expand All @@ -125,9 +125,14 @@ internal static bool TryUnescapeSectionName(string sectionName, out string? esca
{
sb.Append(lexer.EatCurrentCharacter());
}
else
{
// We only call this on strings that were already passed through IsAbsoluteEditorConfigPath, so
// we shouldn't have any other token kinds here.
throw ExceptionUtilities.UnexpectedValue(tokenKind);
}
}
escapedSectionName = sb.ToString();
return true;
return sb.ToString();
}

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,8 @@ public Section(string name, ImmutableDictionary<string, string> properties)
}

/// <summary>
/// The name as present directly in the section specification of the editorconfig file.
/// For regular files, the name as present directly in the section specification of the editorconfig file. For sections in
/// global configs, this is the unescaped full file path.
/// </summary>
public string Name { get; }

Expand Down
13 changes: 9 additions & 4 deletions src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public sealed class AnalyzerConfigSet
{
/// <summary>
/// The list of <see cref="AnalyzerConfig" />s in this set. This list has been sorted per <see cref="AnalyzerConfig.DirectoryLengthComparer"/>.
/// This does not include any of the global configs that were merged into <see cref="_globalConfig"/>.
/// </summary>
private readonly ImmutableArray<AnalyzerConfig> _analyzerConfigs;

Expand Down Expand Up @@ -197,13 +198,14 @@ public AnalyzerConfigOptionsResult GetOptionsForSourcePath(string sourcePath)
var normalizedPath = PathUtilities.NormalizeWithForwardSlash(sourcePath);
normalizedPath = PathUtilities.ExpandAbsolutePathWithRelativeParts(normalizedPath);

// If we have a global config, add any sections that match the full path
// If we have a global config, add any sections that match the full path. We can have at most one section since
// we would have merged them earlier.
foreach (var section in _globalConfig.NamedSections)
{
var escapedSectionName = TryUnescapeSectionName(section.Name, out var sectionName);
if (escapedSectionName && normalizedPath.Equals(sectionName, Section.NameComparer))
if (normalizedPath.Equals(section.Name, Section.NameComparer))
{
sectionKey.Add(section);
break;
}
}
int globalConfigOptionsCount = sectionKey.Count;
Expand Down Expand Up @@ -502,7 +504,10 @@ internal void MergeIntoGlobalConfig(AnalyzerConfig config, DiagnosticBag diagnos
{
if (IsAbsoluteEditorConfigPath(section.Name))
{
MergeSection(config.PathToFile, section, config.GlobalLevel, isGlobalSection: false);
// Let's recreate the section with the name unescaped, since we can then properly merge and match it later
var unescapedSection = new Section(UnescapeSectionName(section.Name), section.Properties);

MergeSection(config.PathToFile, unescapedSection, config.GlobalLevel, isGlobalSection: false);
}
else
{
Expand Down

0 comments on commit 73eeb59

Please sign in to comment.