From 9ea02da59d5598a43496c02c632c439fdc9dac67 Mon Sep 17 00:00:00 2001 From: Matt Chaulklin Date: Thu, 22 Aug 2024 14:46:44 -0400 Subject: [PATCH 1/2] Updated WithArgument to validate placeholders --- .../DiagnosticResult.cs | 17 ++++ .../CompilerErrorTests.cs | 87 +++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs index c07e2599f4..052da234b0 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs @@ -224,6 +224,17 @@ public DiagnosticResult WithOptions(DiagnosticOptions options) public DiagnosticResult WithArguments(params object[] arguments) { + if (MessageFormat != null) + { + int placeholderCount = CountPlaceholders(MessageFormat.ToString()); + + if (arguments.Length != placeholderCount) + { + var message = $"Incorrect number of arguments provided. The message expects {placeholderCount} argument(s), but received {arguments.Length}."; + throw new ArgumentException(message); + } + } + return new DiagnosticResult( spans: _spans, suppressMessage: _suppressMessage, @@ -479,5 +490,11 @@ public override string ToString() return builder.ToString(); } + + private int CountPlaceholders(string messageFormat) + { + var regex = new System.Text.RegularExpressions.Regex(@"\{[0-9]+(:[^}]*)?\}"); + return regex.Matches(messageFormat).Count; + } } } diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs index 79e67a24a6..e92e5a6a10 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs +++ b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs @@ -550,5 +550,92 @@ public async Task TestTopLevelStatements() }.RunAsync(); } #endif + + [Fact] + [WorkItem(516, "https://github.com/dotnet/roslyn-sdk/issues/516")] + public void WithArguments_TooManyArguments_ShouldThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var exception = Assert.Throws(() => result.WithArguments("arg1", "arg2", "arg3")); + Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 3.", exception.Message); + } + + [Fact] + public void WithArguments_TooFewArguments_ShouldThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var exception = Assert.Throws(() => result.WithArguments("arg1")); + Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 1.", exception.Message); + } + + [Fact] + public void WithArguments_NoPlaceholdersButWithArguments_ShouldThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "This message has no placeholders.", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var exception = Assert.Throws(() => result.WithArguments("arg1")); + Assert.Equal("Incorrect number of arguments provided. The message expects 0 argument(s), but received 1.", exception.Message); + } + + [Fact] + public void WithArguments_NoArgumentsAndNoPlaceholders_ShouldNotThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "This message has no placeholders.", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var formattedResult = result.WithArguments(); + + Assert.Empty(formattedResult.MessageArguments!); + Assert.Equal("This message has no placeholders.", formattedResult.Message); + } + + [Fact] + public void WithArguments_PlaceholdersWithoutArguments_ShouldThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var exception = Assert.Throws(() => result.WithArguments()); + Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 0.", exception.Message); + } + + [Fact] + public void WithArguments_ComplexPlaceholders_ShouldFormatCorrectly() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0:MMMM dd, yyyy}' and '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var formattedResult = result.WithArguments(DateTime.Now, "arg2"); + + Assert.Contains("arg2", formattedResult.Message); + } + + [Fact] + public void WithArguments_DifferentDataTypes_ShouldFormatCorrectly() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' and '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var formattedResult = result.WithArguments(123, DateTime.Now); + + Assert.Contains("123", formattedResult.Message); + } + + [Fact] + public void WithArguments_CorrectNumberOfArguments_ShouldNotThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor); + + var formattedResult = result.WithArguments("arg1", "arg2"); + + Assert.Equal(new object[] { "arg1", "arg2" }, formattedResult.MessageArguments); + Assert.Equal("'arg1' calls 'arg2'", formattedResult.Message); + } } } From 60c7b266518215143339a1ff42e87cf680086178 Mon Sep 17 00:00:00 2001 From: Matt Chaulklin Date: Tue, 27 Aug 2024 12:44:30 -0400 Subject: [PATCH 2/2] Moved validation to Message property. Revised tests --- .../DiagnosticResult.cs | 35 ++++++------- .../CompilerErrorTests.cs | 52 ++++++++++--------- .../DiagnosticResultTests.cs | 14 +++-- 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs index 052da234b0..418d48bb2e 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticResult.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Immutable; using System.Text; +using System.Text.RegularExpressions; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.Testing @@ -122,13 +123,26 @@ public string? Message if (MessageFormat != null) { + var messageFormatString = MessageFormat.ToString(); + + // Ensure placeholders in the MessageFormat match the provided arguments + int placeholderCount = Regex.Matches(messageFormatString, @"\{[0-9]+(:[^}]*)?\}").Count; + + // Initialize MessageArguments if null + var arguments = MessageArguments ?? EmptyArguments; + + if (arguments.Length != placeholderCount) + { + throw new ArgumentException($"Incorrect number of arguments provided. The message expects {placeholderCount} argument(s), but received {arguments.Length}."); + } + try { - return string.Format(MessageFormat.ToString(), MessageArguments ?? EmptyArguments); + return string.Format(messageFormatString, arguments); } catch (FormatException) { - return MessageFormat.ToString(); + return messageFormatString; } } @@ -224,17 +238,6 @@ public DiagnosticResult WithOptions(DiagnosticOptions options) public DiagnosticResult WithArguments(params object[] arguments) { - if (MessageFormat != null) - { - int placeholderCount = CountPlaceholders(MessageFormat.ToString()); - - if (arguments.Length != placeholderCount) - { - var message = $"Incorrect number of arguments provided. The message expects {placeholderCount} argument(s), but received {arguments.Length}."; - throw new ArgumentException(message); - } - } - return new DiagnosticResult( spans: _spans, suppressMessage: _suppressMessage, @@ -490,11 +493,5 @@ public override string ToString() return builder.ToString(); } - - private int CountPlaceholders(string messageFormat) - { - var regex = new System.Text.RegularExpressions.Regex(@"\{[0-9]+(:[^}]*)?\}"); - return regex.Matches(messageFormat).Count; - } } } diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs index e92e5a6a10..ad93cbcb75 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs +++ b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/CompilerErrorTests.cs @@ -556,9 +556,9 @@ public async Task TestTopLevelStatements() public void WithArguments_TooManyArguments_ShouldThrowException() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); + var result = new DiagnosticResult(descriptor).WithArguments("arg1", "arg2", "arg3"); - var exception = Assert.Throws(() => result.WithArguments("arg1", "arg2", "arg3")); + var exception = Assert.Throws(() => result.Message); Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 3.", exception.Message); } @@ -566,9 +566,9 @@ public void WithArguments_TooManyArguments_ShouldThrowException() public void WithArguments_TooFewArguments_ShouldThrowException() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); + var result = new DiagnosticResult(descriptor).WithArguments("arg1"); - var exception = Assert.Throws(() => result.WithArguments("arg1")); + var exception = Assert.Throws(() => result.Message); Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 1.", exception.Message); } @@ -576,9 +576,9 @@ public void WithArguments_TooFewArguments_ShouldThrowException() public void WithArguments_NoPlaceholdersButWithArguments_ShouldThrowException() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "This message has no placeholders.", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); + var result = new DiagnosticResult(descriptor).WithArguments("arg1"); - var exception = Assert.Throws(() => result.WithArguments("arg1")); + var exception = Assert.Throws(() => result.Message); Assert.Equal("Incorrect number of arguments provided. The message expects 0 argument(s), but received 1.", exception.Message); } @@ -586,21 +586,29 @@ public void WithArguments_NoPlaceholdersButWithArguments_ShouldThrowException() public void WithArguments_NoArgumentsAndNoPlaceholders_ShouldNotThrowException() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "This message has no placeholders.", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); - - var formattedResult = result.WithArguments(); + var result = new DiagnosticResult(descriptor).WithArguments(); - Assert.Empty(formattedResult.MessageArguments!); - Assert.Equal("This message has no placeholders.", formattedResult.Message); + Assert.Empty(result.MessageArguments!); + Assert.Equal("This message has no placeholders.", result.Message); } [Fact] public void WithArguments_PlaceholdersWithoutArguments_ShouldThrowException() + { + var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); + var result = new DiagnosticResult(descriptor).WithArguments(); + + var exception = Assert.Throws(() => result.Message); + Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 0.", exception.Message); + } + + [Fact] + public void WithArguments_PlaceholdersAndWithArgumentsNotCalled_ShouldThrowException() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); var result = new DiagnosticResult(descriptor); - var exception = Assert.Throws(() => result.WithArguments()); + var exception = Assert.Throws(() => result.Message); Assert.Equal("Incorrect number of arguments provided. The message expects 2 argument(s), but received 0.", exception.Message); } @@ -608,34 +616,28 @@ public void WithArguments_PlaceholdersWithoutArguments_ShouldThrowException() public void WithArguments_ComplexPlaceholders_ShouldFormatCorrectly() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0:MMMM dd, yyyy}' and '{1}'", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); - - var formattedResult = result.WithArguments(DateTime.Now, "arg2"); + var result = new DiagnosticResult(descriptor).WithArguments(DateTime.Now, "arg2"); - Assert.Contains("arg2", formattedResult.Message); + Assert.Contains("arg2", result.Message); } [Fact] public void WithArguments_DifferentDataTypes_ShouldFormatCorrectly() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' and '{1}'", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); - - var formattedResult = result.WithArguments(123, DateTime.Now); + var result = new DiagnosticResult(descriptor).WithArguments(123, DateTime.Now); - Assert.Contains("123", formattedResult.Message); + Assert.Contains("123", result.Message); } [Fact] public void WithArguments_CorrectNumberOfArguments_ShouldNotThrowException() { var descriptor = new DiagnosticDescriptor("TestId", "TestTitle", "'{0}' calls '{1}'", "Category", DiagnosticSeverity.Warning, true); - var result = new DiagnosticResult(descriptor); - - var formattedResult = result.WithArguments("arg1", "arg2"); + var result = new DiagnosticResult(descriptor).WithArguments("arg1", "arg2"); - Assert.Equal(new object[] { "arg1", "arg2" }, formattedResult.MessageArguments); - Assert.Equal("'arg1' calls 'arg2'", formattedResult.Message); + Assert.Equal(new object[] { "arg1", "arg2" }, result.MessageArguments); + Assert.Equal("'arg1' calls 'arg2'", result.Message); } } } diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticResultTests.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticResultTests.cs index 3ea74abc0e..62d04dec7e 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticResultTests.cs +++ b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticResultTests.cs @@ -100,12 +100,18 @@ public void TestToStringWithoutLocationOrMessage() [Fact] public void TestToStringWithMessageFormat() { - Assert.Equal( - "error CS1002: {0} expected", - DiagnosticResult.CompilerError("CS1002").WithMessageFormat("{0} expected").ToString()); + var exception = Assert.Throws(() => + DiagnosticResult.CompilerError("CS1002") + .WithMessageFormat("{0} expected") + .ToString()); + Assert.Equal("Incorrect number of arguments provided. The message expects 1 argument(s), but received 0.", exception.Message); + Assert.Equal( "error CS1002: ; expected", - DiagnosticResult.CompilerError("CS1002").WithMessageFormat("{0} expected").WithArguments(";").ToString()); + DiagnosticResult.CompilerError("CS1002") + .WithMessageFormat("{0} expected") + .WithArguments(";") + .ToString()); } [Fact]