Skip to content

Commit 65fbe7c

Browse files
committed
fix: Exclude ExpressionTemplate contexts from property-argument highlighting
Property-argument highlighting no longer triggers on ExpressionTemplate built-in properties like {@t}, {@l}, {@m}, {@x} since these don't have corresponding arguments to highlight. Adds test to verify the fix. Also refactored to use SerilogCallDetector and StringLiteralParser utilities to eliminate duplicate regex patterns
1 parent ff3795f commit 65fbe7c

File tree

3 files changed

+116
-54
lines changed

3 files changed

+116
-54
lines changed

SerilogSyntax.Tests/Tagging/PropertyArgumentHighlighterTests.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,4 +991,43 @@ public void HighlightComplexNestedArguments()
991991
// Assert
992992
Assert.Equal(2, tags.Count); // Should highlight items.Count() and {Count}
993993
}
994+
995+
[Fact]
996+
public void ExpressionTemplate_BuiltInProperties_ShouldNotBeHighlighted()
997+
{
998+
// Arrange - ExpressionTemplate with built-in properties that don't have arguments
999+
var text = @".WriteTo.Console(new ExpressionTemplate(
1000+
""[{@t:HH:mm:ss} {@l:u3}] {#if SourceContext is not null}[{Substring(SourceContext, LastIndexOf(SourceContext, '.') + 1)}]{#end} {@m}\n{@x}""))";
1001+
var buffer = MockTextBuffer.Create(text);
1002+
var view = new MockTextView(buffer);
1003+
var highlighter = new PropertyArgumentHighlighter(view, buffer);
1004+
1005+
// Act - position cursor on the opening brace of {@t:HH:mm:ss}
1006+
var cursorPosition = text.IndexOf("{@t:");
1007+
view.Caret.MoveTo(new SnapshotPoint(buffer.CurrentSnapshot, cursorPosition));
1008+
1009+
var tags = highlighter.GetTags(new NormalizedSnapshotSpanCollection(
1010+
new SnapshotSpan(buffer.CurrentSnapshot, 0, buffer.CurrentSnapshot.Length))).ToList();
1011+
1012+
// Assert - No tags should be created since {@t:HH:mm:ss} is not a property with an argument
1013+
Assert.Empty(tags);
1014+
1015+
// Also test cursor on {@l:u3}
1016+
cursorPosition = text.IndexOf("{@l:");
1017+
view.Caret.MoveTo(new SnapshotPoint(buffer.CurrentSnapshot, cursorPosition));
1018+
1019+
tags = highlighter.GetTags(new NormalizedSnapshotSpanCollection(
1020+
new SnapshotSpan(buffer.CurrentSnapshot, 0, buffer.CurrentSnapshot.Length))).ToList();
1021+
1022+
Assert.Empty(tags);
1023+
1024+
// Also test cursor on {@m}
1025+
cursorPosition = text.IndexOf("{@m}");
1026+
view.Caret.MoveTo(new SnapshotPoint(buffer.CurrentSnapshot, cursorPosition));
1027+
1028+
tags = highlighter.GetTags(new NormalizedSnapshotSpanCollection(
1029+
new SnapshotSpan(buffer.CurrentSnapshot, 0, buffer.CurrentSnapshot.Length))).ToList();
1030+
1031+
Assert.Empty(tags);
1032+
}
9941033
}

SerilogSyntax/Tagging/PropertyArgumentHighlighter.cs

Lines changed: 74 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Microsoft.VisualStudio.Text.Editor;
33
using Microsoft.VisualStudio.Text.Tagging;
44
using SerilogSyntax.Parsing;
5+
using SerilogSyntax.Utilities;
56
using System;
67
using System.Collections.Generic;
78
using System.Linq;
@@ -14,9 +15,13 @@ namespace SerilogSyntax.Tagging;
1415
/// </summary>
1516
internal sealed class PropertyArgumentHighlighter : ITagger<TextMarkerTag>, IDisposable
1617
{
18+
// Note: We use SerilogCallDetector for finding Serilog calls and StringLiteralParser for string extraction
19+
// to avoid duplicating regex patterns and parsing logic
20+
1721
private readonly ITextView _view;
1822
private readonly ITextBuffer _buffer;
1923
private readonly TemplateParser _parser = new();
24+
private readonly StringLiteralParser _stringParser = new();
2025
private SnapshotPoint? _currentChar;
2126
private readonly List<ITagSpan<TextMarkerTag>> _currentTags = [];
2227
private bool _disposed;
@@ -245,10 +250,8 @@ private SerilogCallInfo FindSerilogCall(SnapshotPoint caretPoint)
245250
var snapshot = caretPoint.Snapshot;
246251
var text = snapshot.GetText();
247252

248-
// Use regex to find all Serilog calls
249-
// This matches both direct calls (logger.Information) and chained calls (.Information after ForContext)
250-
var pattern = @"(?:(?:_?logger|Log|log)\.(?:LogVerbose|LogDebug|LogInformation|LogWarning|LogError|LogCritical|LogFatal|Verbose|Debug|Information|Warning|Error|Critical|Fatal|Write|BeginScope)|\.(?:LogVerbose|LogDebug|LogInformation|LogWarning|LogError|LogCritical|LogFatal|Verbose|Debug|Information|Warning|Error|Critical|Fatal|Write))\s*\(";
251-
var methodMatches = Regex.Matches(text, pattern, RegexOptions.IgnoreCase);
253+
// Use SerilogCallDetector to find all Serilog calls to avoid duplicating regex patterns
254+
var methodMatches = SerilogCallDetector.FindAllSerilogCalls(text);
252255

253256
foreach (Match methodMatch in methodMatches)
254257
{
@@ -284,6 +287,10 @@ private SerilogCallInfo FindSerilogCall(SnapshotPoint caretPoint)
284287
// Extract the call content
285288
var callContent = text.Substring(methodEnd, callEnd - methodEnd);
286289

290+
// Skip ExpressionTemplate calls - they don't have argument mapping
291+
if (methodMatch.Value.Contains("ExpressionTemplate"))
292+
continue;
293+
287294
// Handle LogError special case (first parameter might be exception)
288295
var hasException = methodMatch.Value.Contains("LogError") && HasExceptionFirstParam(callContent);
289296
string adjustedCallContent = callContent;
@@ -344,7 +351,7 @@ private bool HasExceptionFirstParam(string callContent)
344351
if (trimmedContent.StartsWith("\"") ||
345352
trimmedContent.StartsWith("@\"") ||
346353
trimmedContent.StartsWith("$\"") ||
347-
Regex.IsMatch(trimmedContent, @"^@?(""{3,})")) // Raw string literals
354+
(trimmedContent.Length > 2 && trimmedContent[0] == '\"' && trimmedContent[1] == '\"' && trimmedContent[2] == '\"')) // Raw string literals
348355
{
349356
// First parameter is a string template, no exception
350357
return false;
@@ -356,63 +363,83 @@ private bool HasExceptionFirstParam(string callContent)
356363

357364
private TemplateStringInfo ExtractTemplate(string callContent)
358365
{
359-
// Handle different string literal formats
360-
int start = -1;
361-
int end = -1;
362-
string template = null;
363-
364-
// Try raw string literal (""" or more quotes for custom delimiters)
365-
// Match 3 or more quotes at the start and end
366-
var rawMatch = Regex.Match(callContent, @"@?(""{3,})([\s\S]*?)\1");
367-
if (rawMatch.Success)
368-
{
369-
// For raw string literals, the template content starts after the opening quotes
370-
var quoteDelimiter = rawMatch.Groups[1].Value;
371-
var quoteCount = quoteDelimiter.Length;
372-
start = rawMatch.Index + quoteCount;
373-
template = rawMatch.Groups[2].Value;
374-
end = rawMatch.Index + rawMatch.Length;
375-
}
376-
else
366+
// Find the first string literal in the call content
367+
int searchPos = 0;
368+
369+
// Skip whitespace at the beginning
370+
while (searchPos < callContent.Length && char.IsWhiteSpace(callContent[searchPos]))
371+
searchPos++;
372+
373+
if (searchPos >= callContent.Length)
374+
return null;
375+
376+
// Try to parse a string literal using StringLiteralParser
377+
if (_stringParser.TryParseStringLiteral(callContent, searchPos, out var result))
377378
{
378-
// Try verbatim string (@")
379-
var verbatimMatch = Regex.Match(callContent, @"@""([^""]*(?:""""[^""]*)*)""");
380-
if (verbatimMatch.Success)
379+
// Determine if it's a verbatim string
380+
bool isVerbatim = searchPos < callContent.Length - 1 &&
381+
callContent[searchPos] == '@' &&
382+
callContent[searchPos + 1] == '"';
383+
384+
// For verbatim strings, we need to store the original content with "" for position mapping
385+
if (isVerbatim)
381386
{
382-
start = verbatimMatch.Index + 2; // Skip @"
383-
template = verbatimMatch.Groups[1].Value.Replace("\"\"", "\"");
384-
end = verbatimMatch.Index + verbatimMatch.Length;
387+
// Extract the original content with escaped quotes intact
388+
var originalContent = callContent.Substring(result.Start + 2, result.End - result.Start - 2);
389+
var cleanedContent = originalContent.Replace("\"\"", "\"");
385390

386-
// For verbatim strings, store the original for position adjustment
387391
return new TemplateStringInfo
388392
{
389-
Template = template,
390-
RelativeStart = start,
391-
RelativeEnd = end,
392-
OriginalTemplate = verbatimMatch.Groups[1].Value,
393+
Template = cleanedContent,
394+
RelativeStart = result.Start + 2, // Skip @"
395+
RelativeEnd = result.End + 1,
396+
OriginalTemplate = originalContent,
393397
IsVerbatimString = true
394398
};
395399
}
396-
else
400+
401+
// For raw strings, check quote count
402+
int quoteCount = 0;
403+
if (callContent[searchPos] == '"')
397404
{
398-
// Try regular string
399-
var regularMatch = Regex.Match(callContent, @"""([^""\\]*(?:\\.[^""\\]*)*)""");
400-
if (regularMatch.Success)
405+
int pos = searchPos;
406+
while (pos < callContent.Length && callContent[pos] == '"')
401407
{
402-
start = regularMatch.Index + 1; // Skip opening quote
403-
template = Regex.Unescape(regularMatch.Groups[1].Value);
404-
end = regularMatch.Index + regularMatch.Length;
408+
quoteCount++;
409+
pos++;
410+
}
411+
}
412+
413+
// For raw strings (3+ quotes), adjust start position
414+
if (quoteCount >= 3)
415+
{
416+
return new TemplateStringInfo
417+
{
418+
Template = result.Content,
419+
RelativeStart = result.Start + quoteCount,
420+
RelativeEnd = result.End + 1
421+
};
422+
}
423+
424+
// For regular strings, unescape the content
425+
string unescapedContent = result.Content;
426+
if (quoteCount == 1) // Regular string with escape sequences
427+
{
428+
try
429+
{
430+
unescapedContent = Regex.Unescape(result.Content);
431+
}
432+
catch
433+
{
434+
// If unescaping fails, use content as-is
405435
}
406436
}
407-
}
408437

409-
if (template != null)
410-
{
411438
return new TemplateStringInfo
412439
{
413-
Template = template,
414-
RelativeStart = start,
415-
RelativeEnd = end
440+
Template = unescapedContent,
441+
RelativeStart = result.Start + 1, // Skip opening quote
442+
RelativeEnd = result.End + 1
416443
};
417444
}
418445

SerilogSyntax/Tagging/SerilogBraceMatcher.cs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -417,20 +417,16 @@ public IEnumerable<ITagSpan<TextMarkerTag>> GetTags(NormalizedSnapshotSpanCollec
417417
bool inExpressionTemplate = IsInsideExpressionTemplate(currentChar);
418418

419419
// For single-line strings, check if we're in a Serilog context
420-
// This includes checking the previous line for configuration methods like outputTemplate
421420
bool inSerilogContext = IsSerilogCall(lineText);
422421

423422
// If not found on current line, check previous line for configuration patterns
423+
// Use SerilogCallDetector for more robust detection
424424
if (!inSerilogContext && currentLine.LineNumber > 0)
425425
{
426426
var prevLine = snapshot.GetLineFromLineNumber(currentLine.LineNumber - 1);
427427
var prevText = prevLine.GetText();
428-
// Check if previous line has outputTemplate or other configuration methods
429-
if (prevText.Contains("outputTemplate") || prevText.Contains("WriteTo.") ||
430-
prevText.Contains("Enrich.") || prevText.Contains("Filter."))
431-
{
432-
inSerilogContext = true;
433-
}
428+
// Use the centralized detector which has more precise pattern matching
429+
inSerilogContext = SerilogCallDetector.IsSerilogCall(prevText);
434430
}
435431

436432
if (!inMultiLineString && !inExpressionTemplate && !inSerilogContext)

0 commit comments

Comments
 (0)