Hardening work

Clip.Analyzers.Tests/InvalidFieldsArgumentAnalyzerTests.cs

@@ -152,6 +152,52 @@ await Verify.VerifyAnalyzerAsync(test,
             Verify.Diagnostic(DiagnosticIds.InvalidFieldsArgument).WithLocation(0).WithArguments("int"));
     }
 
+    [Fact]
+    public async Task MethodCall_ReturningPrimitive_Diagnostic()
+    {
+        const string test = """
+                            using Clip;
+                            class C {
+                                int GetCount() => 42;
+                                void M(ILogger logger) {
+                                    logger.Info("msg", {|#0:GetCount()|});
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test,
+            Verify.Diagnostic(DiagnosticIds.InvalidFieldsArgument).WithLocation(0).WithArguments("int"));
+    }
+
+    [Fact]
+    public async Task TernaryExpression_BothBranchesString_Diagnostic()
+    {
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger, bool flag) {
+                                    logger.Info("msg", {|#0:flag ? "yes" : "no"|});
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test,
+            Verify.Diagnostic(DiagnosticIds.InvalidFieldsArgument).WithLocation(0).WithArguments("string"));
+    }
+
+    [Fact]
+    public async Task ArrayIndexer_PrimitiveElement_Diagnostic()
+    {
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger, int[] arr) {
+                                    logger.Info("msg", {|#0:arr[0]|});
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test,
+            Verify.Diagnostic(DiagnosticIds.InvalidFieldsArgument).WithLocation(0).WithArguments("int"));
+    }
+
     [Fact]
     public async Task ErrorWithException_NoDiagnostic()
     {

Clip.Analyzers.Tests/LowercaseMessageAnalyzerTests.cs

@@ -74,4 +74,51 @@ void M(ILogger logger) {
                             """;
         await Verify.VerifyAnalyzerAsync(test);
     }
+
+    [Fact]
+    public async Task NonAsciiLowercaseStart_Diagnostic()
+    {
+        // char.IsLower returns true for Unicode lowercase too — German "über" should be
+        // flagged just like ASCII "user".
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger) {
+                                    logger.Info({|#0:"über die brücke"|});
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test,
+            Verify.Diagnostic(DiagnosticIds.LowercaseMessage).WithLocation(0));
+    }
+
+    [Fact]
+    public async Task NonAsciiUppercaseStart_NoDiagnostic()
+    {
+        // Already capitalized — no diagnostic.
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger) {
+                                    logger.Info("Über die Brücke");
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task SymbolStartMessage_NoDiagnostic()
+    {
+        // Punctuation isn't lowercase — no diagnostic.
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger) {
+                                    logger.Info("[startup] ready");
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test);
+    }
 }

Clip.Analyzers.Tests/MessageTemplateSyntaxAnalyzerTests.cs

@@ -89,4 +89,50 @@ void M(ILogger logger, string msg) {
                             """;
         await Verify.VerifyAnalyzerAsync(test);
     }
+
+    [Fact]
+    public async Task EmptyBraces_NoDiagnostic()
+    {
+        // {} has no identifier — must not match the placeholder pattern.
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger) {
+                                    logger.Info("Curly braces: {}");
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task LoneOpenBrace_NoDiagnostic()
+    {
+        // Unmatched `{` with no closing brace must not be flagged.
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger) {
+                                    logger.Info("Trailing brace: {prefix");
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task EscapedBracesAroundIdentifier_NoDiagnostic()
+    {
+        // `{{x}}` is the C# composite-format escape — renders as literal `{x}` and is not
+        // a template placeholder. The regex's negative lookarounds must reject it.
+        const string test = """
+                            using Clip;
+                            class C {
+                                void M(ILogger logger) {
+                                    logger.Info("Escaped: {{x}}");
+                                }
+                            }
+                            """;
+        await Verify.VerifyAnalyzerAsync(test);
+    }
 }

Clip.Extensions.Logging.Tests/MelIntegrationTests.cs

@@ -256,6 +256,176 @@ public void CategoryLevelFiltering_ViaOptions()
         Assert.Single(listSink.Records);
     }
 
+    //
+    // Scope state shape variants
+    //
+
+    [Fact]
+    public void BeginScope_WithStringState_AddedAsScopeField()
+    {
+        // Non-KVP scope state takes the fallback path: a single "Scope" field is added
+        // with the raw state as the value.
+        var (factory, sink) = CreateFactory();
+        var logger = factory.CreateLogger("Test");
+
+        using (logger.BeginScope("Processing batch 5"))
+            logger.LogInformation("inside scope");
+
+        var fields = sink.Records[0].Fields;
+        Assert.Contains(fields, f => f.Key == "Scope" && (string)f.RefValue! == "Processing batch 5");
+    }
+
+    [Fact]
+    public void BeginScope_WithPocoState_AddedAsSingleScopeField()
+    {
+        // An anonymous object is not IReadOnlyList<KVP>, so it goes to the fallback path.
+        // The whole POCO ends up as the value of a single "Scope" field — properties are
+        // *not* expanded individually (that would require ad-hoc reflection on the hot path).
+        var (factory, sink) = CreateFactory();
+        var logger = factory.CreateLogger("Test");
+
+        using (logger.BeginScope(new { RequestId = "abc" }))
+            logger.LogInformation("inside scope");
+
+        var fields = sink.Records[0].Fields;
+        Assert.Contains(fields, f => f.Key == "Scope");
+        Assert.DoesNotContain(fields, f => f.Key == "RequestId");
+    }
+
+    //
+    // EventId edge cases
+    //
+
+    [Fact]
+    public void EventId_ZeroIdWithName_EventNameStillEmitted()
+    {
+        // EventId and EventName are independent — a caller that supplies only Name should
+        // still see the name in the output. The Id=0 case is only meaningful when *both*
+        // Id and Name are unset (the implicit default from log-macro overloads that take
+        // no EventId), which is tested via the no-EventId logger calls above.
+        var (factory, sink) = CreateFactory();
+        var logger = factory.CreateLogger("Test");
+
+        logger.Log(MelLogLevel.Information, new EventId(0, "MyEvent"), "msg");
+
+        var fields = sink.Records[0].Fields;
+        Assert.DoesNotContain(fields, f => f.Key == "EventId");
+        Assert.Contains(fields, f =>
+        {
+            if (f.Key != "EventName") return false;
+            return (string)f.RefValue! == "MyEvent";
+        });
+    }
+
+    [Fact]
+    public void EventId_NonZeroIdWithoutName_OnlyEventIdEmitted()
+    {
+        var (factory, sink) = CreateFactory();
+        var logger = factory.CreateLogger("Test");
+
+        logger.Log(MelLogLevel.Information, new EventId(42), "msg");
+
+        var fields = sink.Records[0].Fields;
+        Assert.Contains(fields, f => f is { Key: "EventId", IntValue: 42 });
+        Assert.DoesNotContain(fields, f => f.Key == "EventName");
+    }
+
+    [Fact]
+    public void EventId_DefaultZeroIdNullName_NeitherFieldEmitted()
+    {
+        // The implicit default — no EventId argument at all. Neither field appears.
+        var (factory, sink) = CreateFactory();
+        var logger = factory.CreateLogger("Test");
+
+        logger.LogInformation("msg");
+
+        var fields = sink.Records[0].Fields;
+        Assert.DoesNotContain(fields, f => f.Key == "EventId");
+        Assert.DoesNotContain(fields, f => f.Key == "EventName");
+    }
+
+    //
+    // External scope provider (ISupportExternalScope)
+    //
+
+    [Fact]
+    public void ExternalScope_KvpFields_VisibleInLog()
+    {
+        // The host's LoggerFactory builds a unified IExternalScopeProvider and pushes it
+        // into every ISupportExternalScope provider. Scopes pushed through the *factory*
+        // (e.g. via factory.CreateLogger(...).BeginScope, or by other providers like
+        // ASP.NET Core's HostingApplication for HTTP requests) must end up in Clip's fields.
+        var listSink = new ListSink();
+        var services = new ServiceCollection();
+        services.AddLogging(builder =>
+        {
+            builder.SetMinimumLevel(MelLogLevel.Trace);
+            builder.AddClip(opts =>
+            {
+                opts.ConfigureLogger = c => c
+                    .MinimumLevel(ClipLogLevel.Trace)
+                    .WriteTo.Sink(listSink);
+            });
+        });
+
+        var sp = services.BuildServiceProvider();
+        var factory = sp.GetRequiredService<ILoggerFactory>();
+        var logger = factory.CreateLogger("Test");
+
+        // BeginScope on the factory-produced logger flows through the unified scope provider.
+        using (logger.BeginScope(new[] { new KeyValuePair<string, object?>("RequestId", "abc-123") }))
+            logger.LogInformation("scoped");
+
+        var fields = listSink.Records[0].Fields;
+        Assert.Contains(fields, f => f.Key == "RequestId" && (string)f.RefValue! == "abc-123");
+    }
+
+    [Fact]
+    public void ExternalScope_NonKvpState_AddedAsScopeField()
+    {
+        // Non-KVP scope states (POCOs, strings) collected via the external provider must
+        // also surface — same fallback contract as ClipLogger.BeginScope.
+        var listSink = new ListSink();
+        var services = new ServiceCollection();
+        services.AddLogging(builder =>
+        {
+            builder.SetMinimumLevel(MelLogLevel.Trace);
+            builder.AddClip(opts =>
+            {
+                opts.ConfigureLogger = c => c
+                    .MinimumLevel(ClipLogLevel.Trace)
+                    .WriteTo.Sink(listSink);
+            });
+        });
+
+        var factory = services.BuildServiceProvider().GetRequiredService<ILoggerFactory>();
+        var logger = factory.CreateLogger("Test");
+
+        using (logger.BeginScope("processing-batch-5"))
+            logger.LogInformation("scoped");
+
+        var fields = listSink.Records[0].Fields;
+        Assert.Contains(fields, f =>
+            f.Key == "Scope" && (string)f.RefValue! == "processing-batch-5");
+    }
+
+    [Fact]
+    public void NoExternalScopeProvider_DirectProviderUse_DoesNotCrash()
+    {
+        // Constructing the provider directly (without going through LoggerFactory) leaves
+        // _scopeProvider null. The Log path must take the fast no-scope branch and not NRE.
+        var listSink = new ListSink();
+        var clip = Logger.Create(c => c
+            .MinimumLevel(ClipLogLevel.Trace)
+            .WriteTo.Sink(listSink));
+        using var provider = new ClipLoggerProvider(clip);
+        var logger = provider.CreateLogger("Test");
+
+        var ex = Record.Exception(() => logger.LogInformation("no scope provider"));
+        Assert.Null(ex);
+        Assert.Single(listSink.Records);
+    }
+
     //
     // Nested scopes
     //

Clip.Extensions.Logging/ClipLogger.cs

@@ -1,4 +1,5 @@
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using Microsoft.Extensions.Logging;
 using ClipLogLevel = Clip.LogLevel;
 using MelLogLevel = Microsoft.Extensions.Logging.LogLevel;
@@ -10,11 +11,14 @@ internal sealed class ClipLogger : Microsoft.Extensions.Logging.ILogger
     private readonly Logger _inner;
     private readonly string _categoryName;
     private readonly MelLogLevel _effectiveMelLevel;
+    private IExternalScopeProvider? _scopeProvider;
 
-    internal ClipLogger(Logger inner, string categoryName, ClipLogLevel effectiveLevel)
+    internal ClipLogger(Logger inner, string categoryName, ClipLogLevel effectiveLevel,
+        IExternalScopeProvider? scopeProvider = null)
     {
         _inner = inner;
         _categoryName = categoryName;
+        _scopeProvider = scopeProvider;
 
         // Precompute the effective minimum as a MelLogLevel so IsEnabled
         // becomes a single integer comparison — no enum conversion, no
@@ -28,6 +32,8 @@ internal ClipLogger(Logger inner, string categoryName, ClipLogLevel effectiveLev
         _effectiveMelLevel = LevelMapping.ToMel(effectiveClip);
     }
 
+    internal void SetScopeProvider(IExternalScopeProvider scopeProvider) => _scopeProvider = scopeProvider;
+
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public bool IsEnabled(MelLogLevel logLevel)
     {
@@ -47,7 +53,28 @@ public void Log<TState>(
         var clipLevel = LevelMapping.ToClip(logLevel);
         var stateFields = MelFieldAdapter.ExtractFields(_categoryName, state, eventId);
 
-        _inner.Log(clipLevel, message, stateFields, exception);
+        if (_scopeProvider is null)
+        {
+            _inner.Log(clipLevel, message, stateFields, exception);
+            return;
+        }
+
+        // Merge external-scope fields with state fields. Allocates a small list per call,
+        // but only when an external scope provider is wired up — a Clip-only setup pays nothing.
+        var merged = new List<Field>(stateFields.Length + 4);
+        merged.AddRange(stateFields);
+        _scopeProvider.ForEachScope(
+            static (scope, list) =>
+            {
+                if (scope is IReadOnlyList<KeyValuePair<string, object?>> kvps)
+                    for (var i = 0; i < kvps.Count; i++)
+                        list.Add(MelFieldAdapter.CreateFieldFromKvp(kvps[i].Key, kvps[i].Value));
+                else if (scope is not null)
+                    list.Add(new Field("Scope", scope));
+            },
+            merged);
+
+        _inner.Log(clipLevel, message, CollectionsMarshal.AsSpan(merged), exception);
     }
 
     public IDisposable BeginScope<TState>(TState state) where TState : notnull