From 3b98df1692760a48563c93322fa6f5fb8bb1064e Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 3 Mar 2021 07:07:40 -0800 Subject: [PATCH 1/4] EditorConfig comments cannot follow a property value Related to editorconfig/specification#23 Fixes #44596 --- .../Analyzers/AnalyzerConfigTests.cs | 13 ++++++++----- .../Core/Portable/CommandLine/AnalyzerConfig.cs | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs index 1624752b3330e..7ad3a2d889460 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs @@ -177,15 +177,18 @@ public void SpacesInProperties() properties); } - [Fact] - public void EndOfLineComments() + [Theory] + [WorkItem(44596, "https://github.com/dotnet/roslyn/issues/44596")] + [InlineData(";")] + [InlineData("#")] + public void EndOfLineComments(string commentStartCharacter) { - var config = ParseConfigFile(@" -my_prop2 = my val2 # Comment"); + var config = ParseConfigFile($@" +my_prop2 = my val2 {commentStartCharacter} Not A Comment"); var properties = config.GlobalSection.Properties; AssertEx.SetEqual( - new[] { KeyValuePair.Create("my_prop2", "my val2") }, + new[] { KeyValuePair.Create("my_prop2", $"my val2 {commentStartCharacter} Not A Comment") }, properties); } diff --git a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs index a1c4f920e136e..a4d67b9d708a1 100644 --- a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs +++ b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs @@ -21,7 +21,7 @@ public sealed partial class AnalyzerConfig // Matches EditorConfig section header such as "[*.{js,py}]", see https://editorconfig.org for details private static readonly Regex s_sectionMatcher = new Regex(@"^\s*\[(([^#;]|\\#|\\;)+)\]\s*([#;].*)?$", RegexOptions.Compiled); // Matches EditorConfig property such as "indent_style = space", see https://editorconfig.org for details - private static readonly Regex s_propertyMatcher = new Regex(@"^\s*([\w\.\-_]+)\s*[=:]\s*(.*?)\s*([#;].*)?$", RegexOptions.Compiled); + private static readonly Regex s_propertyMatcher = new Regex(@"^\s*([\w\.\-_]+)\s*[=:]\s*(.*?)\s*$", RegexOptions.Compiled); /// /// Key that indicates if this config is a global config From 1b6c125cc8783de44be40dce85917edb43dc8f42 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 3 Mar 2021 08:24:24 -0800 Subject: [PATCH 2/4] Update parsing of dotnet_diagnostic lines to ignore "trailing comments" --- .../Portable/CommandLine/AnalyzerConfigSet.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs index b6ac8fe1ab9e2..cc9df629c5257 100644 --- a/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs +++ b/src/Compilers/Core/Portable/CommandLine/AnalyzerConfigSet.cs @@ -337,33 +337,40 @@ static void freeKey(List
sectionKey, ObjectPool> pool) internal static bool TryParseSeverity(string value, out ReportDiagnostic severity) { + ReadOnlySpan commentStartCharacters = stackalloc char[] { ';', '#' }; + var trimmed = value.AsSpan(); + var commentStartIndex = trimmed.IndexOfAny(commentStartCharacters); + if (commentStartIndex >= 0) + trimmed = trimmed[0..commentStartIndex].TrimEnd(); + var comparer = StringComparer.OrdinalIgnoreCase; - if (comparer.Equals(value, "default")) + if (trimmed.Equals("default".AsSpan(), StringComparison.OrdinalIgnoreCase)) { severity = ReportDiagnostic.Default; return true; } - else if (comparer.Equals(value, "error")) + else if (trimmed.Equals("error".AsSpan(), StringComparison.OrdinalIgnoreCase)) { severity = ReportDiagnostic.Error; return true; } - else if (comparer.Equals(value, "warning")) + else if (trimmed.Equals("warning".AsSpan(), StringComparison.OrdinalIgnoreCase)) { severity = ReportDiagnostic.Warn; return true; } - else if (comparer.Equals(value, "suggestion")) + else if (trimmed.Equals("suggestion".AsSpan(), StringComparison.OrdinalIgnoreCase)) { severity = ReportDiagnostic.Info; return true; } - else if (comparer.Equals(value, "silent") || comparer.Equals(value, "refactoring")) + else if (trimmed.Equals("silent".AsSpan(), StringComparison.OrdinalIgnoreCase) + || trimmed.Equals("refactoring".AsSpan(), StringComparison.OrdinalIgnoreCase)) { severity = ReportDiagnostic.Hidden; return true; } - else if (comparer.Equals(value, "none")) + else if (trimmed.Equals("none".AsSpan(), StringComparison.OrdinalIgnoreCase)) { severity = ReportDiagnostic.Suppress; return true; From d797c324228aafc4ab12fc5588625d50bb229aa8 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 5 Jan 2024 14:58:40 -0600 Subject: [PATCH 3/4] Update tests and breaking change documentation --- .../Compiler Breaking Changes - DotNet 9.md | 24 ++++++ .../Analyzers/AnalyzerConfigTests.cs | 77 +++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md diff --git a/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md b/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md new file mode 100644 index 0000000000000..b55219afc9584 --- /dev/null +++ b/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md @@ -0,0 +1,24 @@ +# This document lists known breaking changes in Roslyn after .NET 8 all the way to .NET 9. + +## .editorconfig values no longer support trailing comments + +***Introduced in Visual Studio 2022 version 17.[TBD]*** + +The compiler is updated based on the EditorConfig specification clarification in +[editorconfig/specification#31](https://github.com/editorconfig/specification/pull/31). Following this change, comments +in **.editorconfig** and **.globalconfig** files must now appear on their own line. Comments which appear at the end of +a property value are now treated as part of the property value itself. This changes the way values are passed to +analyzers for lines with the following form: + +```ini +[*.cs] +key = value # text +key2 = value2 ; text2 +``` + +The following table shows how this change affects values passed to analyzers: + +| EditorConfig line | New compiler interpretation | Old interpretation | +| ----------------------- | --------------------------- | ------------------ | +| `key = value # text` | `value # text` | `value` | +| `key2 = value2 ; text2` | `value2 ; text2` | `value2` | diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs index 866714014092f..368e8df9310b4 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs @@ -10,6 +10,7 @@ using System.Linq; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Test.Utilities; +using Roslyn.Utilities; using Xunit; using static Microsoft.CodeAnalysis.AnalyzerConfig; using static Roslyn.Test.Utilities.TestHelpers; @@ -935,6 +936,82 @@ public void EditorConfigToDiagnostics() }, options.Select(o => o.TreeOptions).ToArray()); } + [Theory] + [InlineData("default")] + [InlineData("none")] + [InlineData("silent")] + [InlineData("refactoring")] + [InlineData("suggestion")] + [InlineData("warning")] + [InlineData("error")] + public void EditorConfigToDiagnosticsValidSeverity(string severity) + { + var expected = severity switch + { + "default" => ReportDiagnostic.Default, + "none" => ReportDiagnostic.Suppress, + "silent" => ReportDiagnostic.Hidden, + "refactoring" => ReportDiagnostic.Hidden, + "suggestion" => ReportDiagnostic.Info, + "warning" => ReportDiagnostic.Warn, + "error" => ReportDiagnostic.Error, + _ => throw ExceptionUtilities.UnexpectedValue(severity), + }; + + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse($@" +[*.cs] +dotnet_diagnostic.cs000.severity = {severity} +", "/.editorconfig")); + + var options = GetAnalyzerConfigOptions(["/test.cs"], configs); + configs.Free(); + + Assert.Equal([CreateImmutableDictionary(("cs000", expected))], Array.ConvertAll(options, o => o.TreeOptions)); + } + + [Theory] + [InlineData("unset")] + [InlineData("warn")] + [InlineData("")] + public void EditorConfigToDiagnosticsInvalidSeverity(string severity) + { + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse($@" +[*.cs] +dotnet_diagnostic.cs000.severity = {severity} +", "/.editorconfig")); + + var options = GetAnalyzerConfigOptions(["/test.cs"], configs); + configs.Free(); + + Assert.Equal([ImmutableDictionary.Empty], Array.ConvertAll(options, o => o.TreeOptions)); + } + + /// + /// Verifies that the AnalyzerConfig parser ignores comment-like trailing text when parsing severity values. + /// This ensures the change from https://github.com/dotnet/roslyn/pull/51625 does not affect the parsing of + /// diagnostic severities. + /// + [Theory] + [InlineData("#")] + [InlineData(";")] + [WorkItem("https://github.com/dotnet/roslyn/pull/51625")] + public void EditorConfigToDiagnosticsIgnoresCommentLikeText(string delimiter) + { + var configs = ArrayBuilder.GetInstance(); + configs.Add(Parse($@" +[*.cs] +dotnet_diagnostic.cs000.severity = error {delimiter} ignored text +dotnet_diagnostic.cs001.severity = warning {delimiter} ignored text +", "/.editorconfig")); + + var options = GetAnalyzerConfigOptions(["/test.cs"], configs); + configs.Free(); + + Assert.Equal([CreateImmutableDictionary(("cs000", ReportDiagnostic.Error), ("cs001", ReportDiagnostic.Warn))], Array.ConvertAll(options, o => o.TreeOptions)); + } + [Fact] public void LaterSectionOverrides() { From bf03a75479837f2949192f783bbbe7ab39331da8 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 8 Jan 2024 08:19:21 -0600 Subject: [PATCH 4/4] Update CodeStyleOption parsing to ignore trailing comments --- .../EditorConfigCodeStyleParserTests.cs | 10 ++++++++++ .../Compiler/Core/CodeStyle/CodeStyleHelpers.cs | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/Workspaces/CoreTest/CodeStyle/EditorConfigCodeStyleParserTests.cs b/src/Workspaces/CoreTest/CodeStyle/EditorConfigCodeStyleParserTests.cs index 67e1c3e9b779d..419af9816599b 100644 --- a/src/Workspaces/CoreTest/CodeStyle/EditorConfigCodeStyleParserTests.cs +++ b/src/Workspaces/CoreTest/CodeStyle/EditorConfigCodeStyleParserTests.cs @@ -42,6 +42,16 @@ public class EditorConfigCodeStyleParserTests [InlineData("false : warning", false, ReportDiagnostic.Warn)] [InlineData("true : error", true, ReportDiagnostic.Error)] [InlineData("false : error", false, ReportDiagnostic.Error)] + + [WorkItem("https://github.com/dotnet/roslyn/issues/44596")] + [InlineData("true:warning # comment", true, ReportDiagnostic.Warn)] + [InlineData("false:warning # comment", false, ReportDiagnostic.Warn)] + [InlineData("true:error # comment", true, ReportDiagnostic.Error)] + [InlineData("false:error # comment", false, ReportDiagnostic.Error)] + [InlineData("true:warning ; comment", true, ReportDiagnostic.Warn)] + [InlineData("false:warning ; comment", false, ReportDiagnostic.Warn)] + [InlineData("true:error ; comment", true, ReportDiagnostic.Error)] + [InlineData("false:error ; comment", false, ReportDiagnostic.Error)] public void TestParseEditorConfigCodeStyleOption(string args, bool isEnabled, ReportDiagnostic severity) { CodeStyleHelpers.TryParseBoolEditorConfigCodeStyleOption(args, defaultValue: CodeStyleOption2.Default, out var result); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CodeStyle/CodeStyleHelpers.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CodeStyle/CodeStyleHelpers.cs index 5a163a9ed36a5..7d0d245eb0b58 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CodeStyle/CodeStyleHelpers.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CodeStyle/CodeStyleHelpers.cs @@ -13,6 +13,13 @@ namespace Microsoft.CodeAnalysis.CodeStyle { internal static class CodeStyleHelpers { + /// + /// Delimiters which the compiler previously treated as embedded comment delimiters for parsing EditorConfig + /// files. For code style options maintained by Roslyn, we continue to detect and remove these "comments" for + /// backwards compatibility. + /// + private static readonly char[] s_commentDelimiters = ['#', ';']; + public static bool TryParseStringEditorConfigCodeStyleOption(string arg, CodeStyleOption2 defaultValue, [NotNullWhen(true)] out CodeStyleOption2? option) { if (TryGetCodeStyleValueAndOptionalNotification( @@ -62,6 +69,16 @@ public static bool TryGetCodeStyleValue( public static bool TryGetCodeStyleValueAndOptionalNotification( string arg, NotificationOption2 defaultNotification, [NotNullWhen(true)] out string? value, [NotNullWhen(true)] out NotificationOption2 notification) { + var embeddedCommentTextIndex = arg.IndexOfAny(s_commentDelimiters); + if (embeddedCommentTextIndex >= 0) + { + // For backwards compatibility, remove the embedded comment before continuing. In following the + // EditorConfig specification, the compiler no longer treats text after a '#' or ';' as an embedded + // comment. + // https://github.com/dotnet/roslyn/issues/44596 + arg = arg[..embeddedCommentTextIndex]; + } + var firstColonIndex = arg.IndexOf(':'); // We allow a single value to be provided without an explicit notification.