Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,12 @@ private static bool ExtractTemplates(string? message, Dictionary<string, string>
break;
}

templateMap[templateName] = templateName;
templateList.Add(templateName);
// Only add the template name to the list if it hasn't been seen before
if (!templateMap.ContainsKey(templateName))
{
templateMap[templateName] = templateName;
templateList.Add(templateName);
}

scanIndex = closeBraceIndex + 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ internal sealed class LogValuesFormatter
{
private const string NullValue = "(null)";
private readonly List<string> _valueNames = new List<string>();

#if NET
private readonly CompositeFormat _format;
#else
Expand All @@ -33,6 +34,7 @@ public LogValuesFormatter(string format)

OriginalFormat = format;

Dictionary<string, int>? valueNameIndices = null;
var vsb = new ValueStringBuilder(stackalloc char[256]);
int scanIndex = 0;
int endIndex = format.Length;
Expand Down Expand Up @@ -66,8 +68,40 @@ public LogValuesFormatter(string format)
formatDelimiterIndex = formatDelimiterIndex < 0 ? closeBraceIndex : formatDelimiterIndex + openBraceIndex;

vsb.Append(format.AsSpan(scanIndex, openBraceIndex - scanIndex + 1));
vsb.Append(_valueNames.Count.ToString(CultureInfo.InvariantCulture));
_valueNames.Add(format.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1));
string valueName = format.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1);

int valueIndex;
if (valueNameIndices != null)
{
// Use dictionary for lookup when we have many placeholders
if (!valueNameIndices.TryGetValue(valueName, out valueIndex))
{
valueIndex = _valueNames.Count;
_valueNames.Add(valueName);
valueNameIndices[valueName] = valueIndex;
}
}
else
{
// For small number of placeholders, use linear search to avoid dictionary allocation and hashing overhead
valueIndex = FindValueName(valueName);
if (valueIndex < 0)
{
valueIndex = _valueNames.Count;
_valueNames.Add(valueName);

// Switch to dictionary when we have many unique placeholders
if (_valueNames.Count > 4)
{
valueNameIndices = new Dictionary<string, int>(_valueNames.Count, StringComparer.OrdinalIgnoreCase);
for (int i = 0; i < _valueNames.Count; i++)
{
valueNameIndices[_valueNames[i]] = i;
}
}
}
}
vsb.Append(valueIndex.ToString(CultureInfo.InvariantCulture));
vsb.Append(format.AsSpan(formatDelimiterIndex, closeBraceIndex - formatDelimiterIndex + 1));

scanIndex = closeBraceIndex + 1;
Expand All @@ -85,6 +119,19 @@ public LogValuesFormatter(string format)
public string OriginalFormat { get; }
public List<string> ValueNames => _valueNames;

private int FindValueName(string valueName)
{
for (int i = 0; i < _valueNames.Count; i++)
{
if (string.Equals(_valueNames[i], valueName, StringComparison.OrdinalIgnoreCase))
{
return i;
}
}

return -1;
}

private static int FindBraceIndex(string format, char brace, int startIndex, int endIndex)
{
// Example: {{prefix{{{Argument}}}suffix}}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,41 @@ partial class C
Assert.Empty(diagnostics);
}

[Fact]
public async Task ValidTemplates_WithDuplicatePlaceholders()
{
IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
partial class C
{
[LoggerMessage(EventId = 1, Level = LogLevel.Debug, Message = ""Hello {Name}. How are you {Name}"")]
static partial void M1(ILogger logger, string name);

[LoggerMessage(EventId = 2, Level = LogLevel.Debug, Message = ""Hello {Name}. You are {Age} years old. How are you {Name}"")]
static partial void M2(ILogger logger, string name, int age);

[LoggerMessage(EventId = 3, Level = LogLevel.Debug, Message = ""{Age} {Name} {Age}"")]
static partial void M3(ILogger logger, int age, string name);

[LoggerMessage(EventId = 4, Level = LogLevel.Debug, Message = ""{Name} {Name} {Name}"")]
static partial void M4(ILogger logger, string name);

[LoggerMessage(EventId = 5, Level = LogLevel.Debug, Message = ""Age: {Age}, Name: {Name}, Age: {Age}, Name: {Name}"")]
static partial void M5(ILogger logger, int age, string name);

[LoggerMessage(EventId = 6, Level = LogLevel.Debug, Message = ""Hello {Name}. How are you {name}"")]
static partial void M6(ILogger logger, string name);

[LoggerMessage(EventId = 7, Level = LogLevel.Debug, Message = ""{Name} {NAME} {name}"")]
static partial void M7(ILogger logger, string name);

[LoggerMessage(EventId = 8, Level = LogLevel.Debug, Message = ""Hello {Name}. You are {age} years old. How are you {NAME}"")]
static partial void M8(ILogger logger, string name, int age);
}
");

Assert.Empty(diagnostics);
}

[Fact]
public async Task Cancellation()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,56 @@ public void FormatsEnumerableValues(string messageFormat, object[] arguments, st
Assert.Equal(expected, logValues.ToString());
}

[Theory]
[InlineData("Hello David. How are you David", "Hello {Name}. How are you {Name}", new object[] { "David" })]
[InlineData("Hello David. You are 100 years old. How are you David", "Hello {Name}. You are {Age} years old. How are you {Name}", new object[] { "David", 100 })]
[InlineData("100 David 100", "{Age} {Name} {Age}", new object[] { 100, "David" })]
[InlineData("David David David", "{Name} {Name} {Name}", new object[] { "David" })]
[InlineData("Age: 100, Name: David, Age: 100, Name: David", "Age: {Age}, Name: {Name}, Age: {Age}, Name: {Name}", new object[] { 100, "David" })]
[InlineData("Hello David. How are you David", "Hello {Name}. How are you {name}", new object[] { "David" })]
[InlineData("David David David", "{Name} {NAME} {name}", new object[] { "David" })]
[InlineData("Hello David. You are 100 years old. How are you David", "Hello {Name}. You are {age} years old. How are you {NAME}", new object[] { "David", 100 })]
public void LogValues_WithDuplicatePlaceholders(string expected, string format, object[] args)
{
var logValues = new FormattedLogValues(format, args);
Assert.Equal(expected, logValues.ToString());

// Original format is expected to be returned from GetValues.
Assert.Equal(format, logValues.First(v => v.Key == "{OriginalFormat}").Value);
}

[Fact]
public void LogValues_WithDuplicatePlaceholders_CorrectKeyValuePairs()
{
var format = "Hello {Name}. How are you {Name}";
var name = "David";
var logValues = new FormattedLogValues(format, name);

var state = logValues.ToArray();
Assert.Equal(new[]
{
new KeyValuePair<string, object>("Name", name),
new KeyValuePair<string, object>("{OriginalFormat}", format),
}, state);
}

[Fact]
public void LogValues_WithMultipleDuplicatePlaceholders_CorrectKeyValuePairs()
{
var format = "Hello {Name}. You are {Age} years old. How are you {Name}";
var name = "David";
var age = 100;
var logValues = new FormattedLogValues(format, name, age);

var state = logValues.ToArray();
Assert.Equal(new[]
{
new KeyValuePair<string, object>("Name", name),
new KeyValuePair<string, object>("Age", age),
new KeyValuePair<string, object>("{OriginalFormat}", format),
}, state);
}

private class MediaType
{
public MediaType(string type, string subType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,5 +514,111 @@ public void LogValues_OutOfRangeAccess_ThrowsIndexOutOfRangeExceptionWithDefault
Assert.NotEqual("index", exception.Message);
Assert.False(string.IsNullOrEmpty(exception.Message));
}

[Fact]
public void LoggerMessage_Define_WithDuplicatePlaceholder_SingleParameter()
{
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var action = LoggerMessage.Define<string>(LogLevel.Information, new EventId(0, "LogSomething"), "Hello {Name}. How are you {Name}");

action(testLogger, "David", null);

Assert.Single(testSink.Writes);
var writeContext = testSink.Writes.First();
var actualLogValues = Assert.IsAssignableFrom<IReadOnlyList<KeyValuePair<string, object>>>(writeContext.State);

Assert.Equal("Hello David. How are you David", actualLogValues.ToString());
Assert.Equal("Hello {Name}. How are you {Name}", actualLogValues.First(v => v.Key == "{OriginalFormat}").Value);
}

[Fact]
public void LoggerMessage_Define_WithDuplicatePlaceholder_MultipleParameters()
{
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var action = LoggerMessage.Define<string, int>(LogLevel.Information, new EventId(0, "LogSomething"), "Hello {Name}. You are {Age} years old. How are you {Name}");

action(testLogger, "David", 100, null);

Assert.Single(testSink.Writes);
var writeContext = testSink.Writes.First();
var actualLogValues = Assert.IsAssignableFrom<IReadOnlyList<KeyValuePair<string, object>>>(writeContext.State);

Assert.Equal("Hello David. You are 100 years old. How are you David", actualLogValues.ToString());
Assert.Equal("Hello {Name}. You are {Age} years old. How are you {Name}", actualLogValues.First(v => v.Key == "{OriginalFormat}").Value);
}

[Fact]
public void LoggerMessage_Define_WithAllDuplicatePlaceholders()
{
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var action = LoggerMessage.Define<string>(LogLevel.Information, new EventId(0, "LogSomething"), "{Name} {Name} {Name}");

action(testLogger, "David", null);

Assert.Single(testSink.Writes);
var writeContext = testSink.Writes.First();
var actualLogValues = Assert.IsAssignableFrom<IReadOnlyList<KeyValuePair<string, object>>>(writeContext.State);

Assert.Equal("David David David", actualLogValues.ToString());
Assert.Equal("{Name} {Name} {Name}", actualLogValues.First(v => v.Key == "{OriginalFormat}").Value);
}

[Fact]
public void LoggerMessage_DefineScope_WithDuplicatePlaceholder()
{
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var scopeFunc = LoggerMessage.DefineScope<string>("Hello {Name}. How are you {Name}");

using (scopeFunc(testLogger, "David"))
{
Assert.Single(testSink.Scopes);
var scopeContext = testSink.Scopes.First();
var actualLogValues = Assert.IsAssignableFrom<IReadOnlyList<KeyValuePair<string, object>>>(scopeContext.Scope);

Assert.Equal("Hello David. How are you David", actualLogValues.ToString());
Assert.Equal("Hello {Name}. How are you {Name}", actualLogValues.First(v => v.Key == "{OriginalFormat}").Value);
}
}

[Fact]
public void LoggerMessage_Define_WithDuplicatePlaceholder_MatchingArgCount_Throws()
{
Assert.Throws<ArgumentException>(() =>
LoggerMessage.Define<string, string>(LogLevel.Information, new EventId(0, "LogSomething"), "Hello {Name}. How are you {Name}"));
}

[Fact]
public void LoggerMessage_Define_WithDuplicatePlaceholder_DifferentCasing()
{
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var action = LoggerMessage.Define<string>(LogLevel.Information, new EventId(0, "LogSomething"), "Hello {Name}. How are you {name}");

action(testLogger, "David", null);

Assert.Single(testSink.Writes);
var writeContext = testSink.Writes.First();
var actualLogValues = Assert.IsAssignableFrom<IReadOnlyList<KeyValuePair<string, object>>>(writeContext.State);
Assert.Equal("Hello David. How are you David", actualLogValues.ToString());
}

[Fact]
public void LoggerMessage_Define_WithAllDuplicatePlaceholders_DifferentCasing()
{
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var action = LoggerMessage.Define<string>(LogLevel.Information, new EventId(0, "LogSomething"), "{Name} {NAME} {name}");

action(testLogger, "David", null);

Assert.Single(testSink.Writes);
var writeContext = testSink.Writes.First();
var actualLogValues = Assert.IsAssignableFrom<IReadOnlyList<KeyValuePair<string, object>>>(writeContext.State);
Assert.Equal("David David David", actualLogValues.ToString());
}
}
}
Loading