diff --git a/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs b/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs index 1f6d35ac9001004b7dd1e9b3f20d00cdb6b9b482..02d4e68c97d51f109583d04cfa29bb8756b7f7cd 100644 --- a/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs +++ b/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs @@ -13016,11 +13016,13 @@ class C var analyzerConfigFile = dir.CreateFile(".globalconfig"); var analyzerConfig = analyzerConfigFile.WriteAllText(@" is_global = true +global_level = 100 option1 = abc"); var analyzerConfigFile2 = dir.CreateFile(".globalconfig2"); var analyzerConfig2 = analyzerConfigFile2.WriteAllText(@" is_global = true +global_level = 100 option1 = def"); var output = VerifyOutput(dir, src, additionalFlags: new[] { "/analyzerconfig:" + analyzerConfig.Path + "," + analyzerConfig2.Path }, expectedWarningCount: 1, includeCurrentAssemblyAsAnalyzerReference: false); @@ -13033,11 +13035,13 @@ class C analyzerConfig = analyzerConfigFile.WriteAllText(@" is_global = true +global_level = 100 [/file.cs] option1 = abc"); analyzerConfig2 = analyzerConfigFile2.WriteAllText(@" is_global = true +global_level = 100 [/file.cs] option1 = def"); diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs index d3e47d7a43aac7c012082c043792d3411c8e02fb..1624752b3330ea0e122e6c7dacf560caa2e6f727 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs @@ -2175,6 +2175,258 @@ public void GlobalConfigIssuesWarningWithInvalidSectionNames(string sectionName, public void GlobalConfigIssuesWarningWithInvalidSectionNames_PlatformSpecific(string sectionName, bool isValidWindows, bool isValidOther) => GlobalConfigIssuesWarningWithInvalidSectionNames(sectionName, ExecutionConditionUtil.IsWindows ? isValidWindows : isValidOther); + [Theory] + [InlineData("/.globalconfig", true)] + [InlineData("/.GLOBALCONFIG", true)] + [InlineData("/.glObalConfiG", true)] + [InlineData("/path/to/.globalconfig", true)] + [InlineData("/my.globalconfig", false)] + [InlineData("/globalconfig", false)] + [InlineData("/path/to/globalconfig", false)] + [InlineData("/path/to/my.globalconfig", false)] + [InlineData("/.editorconfig", false)] + [InlineData("/.globalconfİg", false)] + public void FileNameCausesConfigToBeReportedAsGlobal(string fileName, bool shouldBeTreatedAsGlobal) + { + var config = Parse("", fileName); + Assert.Equal(shouldBeTreatedAsGlobal, config.IsGlobal); + } + + [Fact] + public void GlobalLevelCanBeReadFromAnyConfig() + { + var config = Parse("global_level = 5", "/.editorconfig"); + Assert.Equal(5, config.GlobalLevel); + } + + [Fact] + public void GlobalLevelDefaultsTo100ForUserGlobalConfigs() + { + var config = Parse("", "/" + AnalyzerConfig.UserGlobalConfigName); + + Assert.True(config.IsGlobal); + Assert.Equal(100, config.GlobalLevel); + } + + [Fact] + public void GlobalLevelCanBeOverriddenForUserGlobalConfigs() + { + var config = Parse("global_level = 5", "/" + AnalyzerConfig.UserGlobalConfigName); + + Assert.True(config.IsGlobal); + Assert.Equal(5, config.GlobalLevel); + } + + [Fact] + public void GlobalLevelDefaultsToZeroForNonUserGlobalConfigs() + { + var config = Parse("is_global = true", "/.nugetconfig"); + + Assert.True(config.IsGlobal); + Assert.Equal(0, config.GlobalLevel); + } + + [Fact] + public void GlobalLevelIsNotPresentInConfigSet() + { + var config = Parse("global_level = 123", "/.globalconfig"); + + var set = AnalyzerConfigSet.Create(ImmutableArray.Create(config)); + var globalOptions = set.GlobalConfigOptions; + + Assert.Empty(globalOptions.AnalyzerOptions); + Assert.Empty(globalOptions.TreeOptions); + Assert.Empty(globalOptions.Diagnostics); + } + + [Fact] + public void GlobalLevelInSectionIsPresentInConfigSet() + { + var config = Parse(@" +[/path] +global_level = 123", "/.globalconfig"); + + var set = AnalyzerConfigSet.Create(ImmutableArray.Create(config)); + var globalOptions = set.GlobalConfigOptions; + + Assert.Empty(globalOptions.AnalyzerOptions); + Assert.Empty(globalOptions.TreeOptions); + Assert.Empty(globalOptions.Diagnostics); + + var sectionOptions = set.GetOptionsForSourcePath("/path"); + + Assert.Single(sectionOptions.AnalyzerOptions); + Assert.Equal("123", sectionOptions.AnalyzerOptions["global_level"]); + Assert.Empty(sectionOptions.TreeOptions); + Assert.Empty(sectionOptions.Diagnostics); + } + + [Theory] + [InlineData(1, 2)] + [InlineData(2, 1)] + [InlineData(-2, -1)] + [InlineData(2, -1)] + public void GlobalLevelAllowsOverrideOfGlobalKeys(int level1, int level2) + { + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse($@" +is_global = true +global_level = {level1} +option1 = value1 +", "/.globalconfig1")); + + configs.Add(Parse($@" +is_global = true +global_level = {level2} +option1 = value2", +"/.globalconfig2")); + + var globalConfig = AnalyzerConfigSet.MergeGlobalConfigs(configs, out var diagnostics); + diagnostics.Verify(); + + Assert.Single(globalConfig.GlobalSection.Properties.Keys, "option1"); + + string expectedValue = level1 > level2 ? "value1" : "value2"; + Assert.Single(globalConfig.GlobalSection.Properties.Values, expectedValue); + + configs.Free(); + } + + [Fact] + public void GlobalLevelAllowsOverrideOfSectionKeys() + { + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse(@" +is_global = true +global_level = 1 + +[/path] +option1 = value1 +", "/.globalconfig1")); + + configs.Add(Parse(@" +is_global = true +global_level = 2 + +[/path] +option1 = value2", +"/.globalconfig2")); + + var globalConfig = AnalyzerConfigSet.MergeGlobalConfigs(configs, out var diagnostics); + diagnostics.Verify(); + + Assert.Single(globalConfig.NamedSections); + Assert.Equal("/path", globalConfig.NamedSections[0].Name); + Assert.Single(globalConfig.NamedSections[0].Properties.Keys, "option1"); + Assert.Single(globalConfig.NamedSections[0].Properties.Values, "value2"); + + configs.Free(); + } + + [Theory] + [InlineData(1, 2, 3, "value3")] + [InlineData(2, 1, 3, "value3")] + [InlineData(3, 2, 1, "value1")] + [InlineData(1, 2, 1, "value2")] + [InlineData(1, 1, 2, "value3")] + [InlineData(2, 1, 1, "value1")] + public void GlobalLevelAllowsOverrideOfDuplicateGlobalKeys(int level1, int level2, int level3, string expectedValue) + { + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse($@" +is_global = true +global_level = {level1} +option1 = value1 +", "/.globalconfig1")); + + configs.Add(Parse($@" +is_global = true +global_level = {level2} +option1 = value2", +"/.globalconfig2")); + + configs.Add(Parse($@" +is_global = true +global_level = {level3} +option1 = value3", +"/.globalconfig3")); + + var globalConfig = AnalyzerConfigSet.MergeGlobalConfigs(configs, out var diagnostics); + diagnostics.Verify(); + + Assert.Single(globalConfig.GlobalSection.Properties.Keys, "option1"); + Assert.Single(globalConfig.GlobalSection.Properties.Values, expectedValue); + + configs.Free(); + } + + [Fact] + public void GlobalLevelReportsConflictsOnlyAtTheHighestLevel() + { + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse($@" +is_global = true +global_level = 1 +option1 = value1 +", "/.globalconfig1")); + + configs.Add(Parse($@" +is_global = true +global_level = 1 +option1 = value2", +"/.globalconfig2")); + + configs.Add(Parse($@" +is_global = true +global_level = 3 +option1 = value3", +"/.globalconfig3")); + + configs.Add(Parse($@" +is_global = true +global_level = 3 +option1 = value4", +"/.globalconfig4")); + + configs.Add(Parse($@" +is_global = true +global_level = 2 +option1 = value5", +"/.globalconfig5")); + + configs.Add(Parse($@" +is_global = true +global_level = 2 +option1 = value6", +"/.globalconfig6")); + + var globalConfig = AnalyzerConfigSet.MergeGlobalConfigs(configs, out var diagnostics); + + // we don't report config1, 2, 5, or 6, because they didn't conflict: 3 + 4 overrode them, but then themselves were conflicting + diagnostics.Verify( + Diagnostic("MultipleGlobalAnalyzerKeys").WithArguments("option1", "Global Section", "/.globalconfig3, /.globalconfig4").WithLocation(1, 1) + ); + + configs.Free(); + } + + [Fact] + public void InvalidGlobalLevelIsIgnored() + { + var userGlobalConfig = Parse($@" +is_global = true +global_level = abc +", "/.globalconfig"); + + var nonUserGlobalConfig = Parse($@" +is_global = true +global_level = abc +", "/.editorconfig"); + + Assert.Equal(100, userGlobalConfig.GlobalLevel); + Assert.Equal(0, nonUserGlobalConfig.GlobalLevel); + } + #endregion } } diff --git a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs index 895e515ea2a49fcde831344aa164190d8ddde071..a1c4f920e136e28ea115ef42ac3f696fddfa6697 100644 --- a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs +++ b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs @@ -28,6 +28,16 @@ public sealed partial class AnalyzerConfig /// internal const string GlobalKey = "is_global"; + /// + /// Key that indicates the precedence of this config when is true + /// + internal const string GlobalLevelKey = "global_level"; + + /// + /// Filename that indicates this file is a user provided global config + /// + internal const string UserGlobalConfigName = ".globalconfig"; + /// /// A set of keys that are reserved for special interpretation for the editorconfig specification. /// All values corresponding to reserved keys in a (key,value) property pair are always lowercased @@ -86,7 +96,45 @@ public sealed partial class AnalyzerConfig /// /// Gets whether this editorconfig is a global editorconfig. /// - internal bool IsGlobal => GlobalSection.Properties.ContainsKey(GlobalKey); + internal bool IsGlobal => _hasGlobalFileName || GlobalSection.Properties.ContainsKey(GlobalKey); + + /// + /// Get the global level of this config, used to resolve conflicting keys + /// + /// + /// A user can explicitly set the global level via the . + /// When no global level is explicitly set, we use a heuristic: + /// + /// + /// Any file matching the is determined to be a user supplied global config and gets a level of 100 + /// + /// + /// Any other file gets a default level of 0 + /// + /// + /// + /// This value is unused when is false. + /// + internal int GlobalLevel + { + get + { + if (GlobalSection.Properties.TryGetValue(GlobalLevelKey, out string? val) && int.TryParse(val, out int level)) + { + return level; + } + else if (_hasGlobalFileName) + { + return 100; + } + else + { + return 0; + } + } + } + + private readonly bool _hasGlobalFileName; private AnalyzerConfig( Section globalSection, @@ -96,6 +144,7 @@ public sealed partial class AnalyzerConfig GlobalSection = globalSection; NamedSections = namedSections; PathToFile = pathToFile; + _hasGlobalFileName = Path.GetFileName(pathToFile).Equals(UserGlobalConfigName, StringComparison.OrdinalIgnoreCase); // Find the containing directory and normalize the path separators string directory = Path.GetDirectoryName(pathToFile) ?? pathToFile; diff --git a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs index e8b4ab4940166667c0452ae8d7bc4f2e32cee8ee..b6ac8fe1ab9e2e61b3d37ac43740603e4204f599 100644 --- a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs +++ b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs @@ -481,8 +481,8 @@ internal static GlobalAnalyzerConfig MergeGlobalConfigs(ArrayBuilder internal struct GlobalAnalyzerConfigBuilder { - private ImmutableDictionary.Builder>.Builder? _values; - private ImmutableDictionary>.Builder>.Builder? _duplicates; + private ImmutableDictionary.Builder>.Builder? _values; + private ImmutableDictionary configPaths)>.Builder>.Builder? _duplicates; internal const string GlobalConfigPath = ""; internal const string GlobalSectionName = "Global Section"; @@ -491,16 +491,16 @@ internal void MergeIntoGlobalConfig(AnalyzerConfig config, DiagnosticBag diagnos { if (_values is null) { - _values = ImmutableDictionary.CreateBuilder.Builder>(Section.NameEqualityComparer); - _duplicates = ImmutableDictionary.CreateBuilder>.Builder>(Section.NameEqualityComparer); + _values = ImmutableDictionary.CreateBuilder.Builder>(Section.NameEqualityComparer); + _duplicates = ImmutableDictionary.CreateBuilder)>.Builder>(Section.NameEqualityComparer); } - MergeSection(config.PathToFile, config.GlobalSection, isGlobalSection: true); + MergeSection(config.PathToFile, config.GlobalSection, config.GlobalLevel, isGlobalSection: true); foreach (var section in config.NamedSections) { if (IsAbsoluteEditorConfigPath(section.Name)) { - MergeSection(config.PathToFile, section, isGlobalSection: false); + MergeSection(config.PathToFile, section, config.GlobalLevel, isGlobalSection: false); } else { @@ -525,7 +525,7 @@ internal GlobalAnalyzerConfig Build(DiagnosticBag diagnostics) { bool isGlobalSection = string.IsNullOrWhiteSpace(section); string sectionName = isGlobalSection ? GlobalSectionName : section; - foreach ((var keyName, var configPaths) in keys) + foreach ((var keyName, (_, var configPaths)) in keys) { diagnostics.Add(Diagnostic.Create( MultipleGlobalAnalyzerKeysDescriptor, @@ -562,52 +562,71 @@ private Section GetSection(string sectionName) return new Section(sectionName, result); } - private void MergeSection(string configPath, Section section, bool isGlobalSection) + private void MergeSection(string configPath, Section section, int globalLevel, bool isGlobalSection) { Debug.Assert(_values is object); Debug.Assert(_duplicates is object); if (!_values.TryGetValue(section.Name, out var sectionDict)) { - sectionDict = ImmutableDictionary.CreateBuilder(Section.PropertiesKeyComparer); + sectionDict = ImmutableDictionary.CreateBuilder(Section.PropertiesKeyComparer); _values.Add(section.Name, sectionDict); } _duplicates.TryGetValue(section.Name, out var duplicateDict); foreach ((var key, var value) in section.Properties) { - if (isGlobalSection && Section.PropertiesKeyComparer.Equals(key, GlobalKey)) + if (isGlobalSection && (Section.PropertiesKeyComparer.Equals(key, GlobalKey) || Section.PropertiesKeyComparer.Equals(key, GlobalLevelKey))) { continue; } - bool keyInSection = sectionDict.ContainsKey(key); - bool keyDuplicated = duplicateDict?.ContainsKey(key) ?? false; + bool keyInSection = sectionDict.TryGetValue(key, out var sectionValue); - // if this key is neither already present, or already duplicate, we can add it + (int globalLevel, ArrayBuilder configPaths) duplicateValue = default; + bool keyDuplicated = !keyInSection && duplicateDict?.TryGetValue(key, out duplicateValue) == true; + + // if this key is neither already present, or already duplicate, we can add it if (!keyInSection && !keyDuplicated) { - sectionDict.Add(key, (value, configPath)); + sectionDict.Add(key, (value, configPath, globalLevel)); } else { - if (duplicateDict is null) + int currentGlobalLevel = keyInSection ? sectionValue.globalLevel : duplicateValue.globalLevel; + + // if this key overrides one we knew about previously, replace it + if (currentGlobalLevel < globalLevel) { - duplicateDict = ImmutableDictionary.CreateBuilder>(Section.PropertiesKeyComparer); - _duplicates.Add(section.Name, duplicateDict); + sectionDict[key] = (value, configPath, globalLevel); + if (keyDuplicated) + { + duplicateDict!.Remove(key); + } } + // this key conflicts with a previous one + else if (currentGlobalLevel == globalLevel) + { + if (duplicateDict is null) + { + duplicateDict = ImmutableDictionary.CreateBuilder)>(Section.PropertiesKeyComparer); + _duplicates.Add(section.Name, duplicateDict); + } - // record that this key is now a duplicate - ArrayBuilder configList = keyDuplicated ? duplicateDict[key] : ArrayBuilder.GetInstance(); - configList.Add(configPath); - duplicateDict[key] = configList; + // record that this key is now a duplicate + ArrayBuilder configList = duplicateValue.configPaths ?? ArrayBuilder.GetInstance(); + configList.Add(configPath); + duplicateDict[key] = (globalLevel, configList); - // if we'd previously added this key, remove it and remember the extra duplicate location - if (keyInSection) - { - var originalConfigPath = sectionDict[key].configPath; - sectionDict.Remove(key); - duplicateDict[key].Insert(0, originalConfigPath); + // if we'd previously added this key, remove it and remember the extra duplicate location + if (keyInSection) + { + var originalEntry = sectionValue; + Debug.Assert(originalEntry.globalLevel == globalLevel); + + sectionDict.Remove(key); + configList.Insert(0, originalEntry.configPath); + } } } }