[Log]: honor global LogParameters/NoLog when aspect comes from Apply rule

packages/ZibStack.NET.Aop/src/ZibStack.NET.Aop/Generator/AopParser.cs

@@ -164,16 +164,32 @@ public static class AopParser
     /// Builds the parameter/return-type parts of the method model given a pre-resolved
     /// set of aspects. Factored out so <see cref="ParseInterfaceProxy"/> can reuse it.
     /// </summary>
-    private static InterceptedMethodModel? BuildMethodModel(IMethodSymbol method, List<AspectInfo> aspects)
+    /// <param name="attributeOverlay">
+    /// When non-null, parameter and return-type attributes from this method are merged
+    /// with <paramref name="method"/>'s own. Used by <see cref="ParseInterfaceProxy"/>:
+    /// the emitted signature has to use the interface's parameter list, but [NoLog] /
+    /// [Sensitive] / [return: NoLog] are typically declared on the concrete impl — so
+    /// we need to see attributes from both sides or the proxy logs data the impl
+    /// marked as redacted.
+    /// </param>
+    private static InterceptedMethodModel? BuildMethodModel(
+        IMethodSymbol method,
+        List<AspectInfo> aspects,
+        IMethodSymbol? attributeOverlay = null)
     {
         // Parse parameters
         var parameters = new List<InterceptedParameterModel>();
-        foreach (var param in method.Parameters)
+        for (int pi = 0; pi < method.Parameters.Length; pi++)
         {
-            bool isSensitive = param.GetAttributes()
-                .Any(a => a.AttributeClass?.ToDisplayString() == SensitiveAttributeName);
-            bool isNoLog = param.GetAttributes()
-                .Any(a => a.AttributeClass?.ToDisplayString() == NoLogAttributeName);
+            var param = method.Parameters[pi];
+            var overlayParam = attributeOverlay is not null && pi < attributeOverlay.Parameters.Length
+                ? attributeOverlay.Parameters[pi]
+                : null;
+
+            bool isSensitive = HasAttr(param, SensitiveAttributeName)
+                || (overlayParam is not null && HasAttr(overlayParam, SensitiveAttributeName));
+            bool isNoLog = HasAttr(param, NoLogAttributeName)
+                || (overlayParam is not null && HasAttr(overlayParam, NoLogAttributeName));
             bool isComplex = IsComplexType(param.Type);
 
             SanitizedTypeModel? sanitizedType = null;
@@ -405,34 +421,37 @@ private static IReadOnlyList<TypeParameterModel> ExtractMethodTypeParameters(IMe
                 continue;
 
             var openMethod = (IMethodSymbol)closedMethod.OriginalDefinition;
+            var implMethod = implClassSymbol.FindImplementationForInterfaceMember(closedMethod) as IMethodSymbol;
 
             var aspects = new List<AspectInfo>();
             var seenAspectTypes = new HashSet<string>();
 
             // Method-level aspects declared on the interface member itself (rare but valid).
-            CollectAspects(openMethod.GetAttributes(), openMethod, aspects, seenAspectTypes);
+            // Passing implMethod as the attribute-overlay lets [return: NoLog]/[return: Sensitive]
+            // declared on the concrete impl flow into the proxy too.
+            CollectAspects(openMethod.GetAttributes(), openMethod, aspects, seenAspectTypes, implMethod);
 
             // Method-level aspects on the concrete implementation of THIS interface member.
             // This is what makes `[Log]` on an impl method intercept calls made via the
             // interface reference too.
-            if (implClassSymbol.FindImplementationForInterfaceMember(closedMethod) is IMethodSymbol implMethod)
-                CollectAspects(implMethod.GetAttributes(), openMethod, aspects, seenAspectTypes);
+            if (implMethod is not null)
+                CollectAspects(implMethod.GetAttributes(), openMethod, aspects, seenAspectTypes, implMethod);
 
             // Impl class-level aspects (e.g. `[Log]` on the class) apply to every public method.
             if (classLevelAspectAttrs.Length > 0)
-                CollectAspects(classLevelAspectAttrs, openMethod, aspects, seenAspectTypes);
+                CollectAspects(classLevelAspectAttrs, openMethod, aspects, seenAspectTypes, implMethod);
 
             // Apply() rules from IAopConfigurator — match against the IMPL method so that
             // selectors like ClassesWhere(c => c.Name.StartsWith("Order")) resolve against
             // the concrete class, not the interface.
-            if (implClassSymbol.FindImplementationForInterfaceMember(closedMethod) is IMethodSymbol implMethodForApply
+            if (implMethod is not null
                 && (implClassSymbol.ContainingAssembly as ISourceAssemblySymbol)?.Compilation is { } compilation)
             {
                 var config = AopConfiguratorParser.ReadAll(compilation);
                 foreach (var rule in config.ApplyRules)
                 {
-                    if (!seenAspectTypes.Contains(rule.AspectFqn) && MatchesApplyRule(rule, implMethodForApply))
-                        CollectVirtualAspect(rule, openMethod, aspects, seenAspectTypes, compilation);
+                    if (!seenAspectTypes.Contains(rule.AspectFqn) && MatchesApplyRule(rule, implMethod))
+                        CollectVirtualAspect(rule, openMethod, aspects, seenAspectTypes, compilation, implMethod);
                 }
             }
 
@@ -441,7 +460,10 @@ private static IReadOnlyList<TypeParameterModel> ExtractMethodTypeParameters(IMe
 
             aspects.Sort((a, b) => a.Order.CompareTo(b.Order));
 
-            var model = BuildMethodModel(openMethod, aspects);
+            // attributeOverlay = implMethod so [NoLog]/[Sensitive] on the concrete impl's
+            // parameters flow through the interface proxy (the proxy otherwise only sees
+            // interface-declared parameter attributes).
+            var model = BuildMethodModel(openMethod, aspects, implMethod);
             if (model != null)
                 methods.Add(model);
         }
@@ -615,12 +637,19 @@ internal static bool MatchesApplyRule(AopConfiguratorParser.ApplyRule rule, IMet
     /// Creates an AspectInfo from an Apply rule — equivalent to having the attribute
     /// on the method, but driven from the fluent config.
     /// </summary>
+    /// <param name="attributeOverlay">
+    /// Additional symbol whose return-type attributes are merged with
+    /// <paramref name="method"/>'s. Used by <see cref="ParseInterfaceProxy"/> so that
+    /// <c>[return: NoLog]</c> declared on the concrete impl propagates through the
+    /// synthesized interface proxy.
+    /// </param>
     private static void CollectVirtualAspect(
         AopConfiguratorParser.ApplyRule rule,
         IMethodSymbol method,
         List<AspectInfo> aspects,
         HashSet<string> seenTypes,
-        Compilation compilation)
+        Compilation compilation,
+        IMethodSymbol? attributeOverlay = null)
     {
         if (!seenTypes.Add(rule.AspectFqn)) return;
 
@@ -674,23 +703,45 @@ private static void CollectVirtualAspect(
             }
         }
 
+        var returnAttrs = method.GetReturnTypeAttributes();
+        if (attributeOverlay is not null)
+            returnAttrs = returnAttrs.AddRange(attributeOverlay.GetReturnTypeAttributes());
+        bool sensitiveReturn = returnAttrs.Any(a => a.AttributeClass?.ToDisplayString() == SensitiveAttributeName);
+        bool noLogReturn = returnAttrs.Any(a => a.AttributeClass?.ToDisplayString() == NoLogAttributeName);
+
         aspects.Add(new AspectInfo(
             rule.AspectFqn,
             order: 0,
             properties: props,
             handlerTypeName: handlerTypeName,
-            hasSyncHandler: hasSyncHandler,
             isAsyncHandler: isAsyncHandler,
             isAroundHandler: isAroundHandler,
             isAsyncAroundHandler: isAsyncAroundHandler,
-            genericAroundTypeArg: genericAroundTypeArg));
+            sensitiveReturn: sensitiveReturn,
+            noLogReturn: noLogReturn,
+            genericAroundTypeArg: genericAroundTypeArg,
+            hasSyncHandler: hasSyncHandler));
     }
 
+    /// <summary>
+    /// Resolution order for aspect properties (highest wins):
+    /// <list type="number">
+    ///   <item>method-level attribute: <c>[Log(LogParameters=false)]</c></item>
+    ///   <item>class-level attribute: <c>[Log]</c> on the class</item>
+    ///   <item><c>b.Apply&lt;LogAttribute&gt;(..., a =&gt; a.LogParameters = false)</c></item>
+    ///   <item><c>ILogConfigurator.Defaults(d =&gt; d.LogParameters = false)</c></item>
+    ///   <item>hard-coded generator default</item>
+    /// </list>
+    /// This is realised by populating <see cref="AspectInfo.Properties"/> from sources
+    /// (1)–(3) (earlier wins) and <c>InterceptedClassModel.AspectClassData</c> from (4).
+    /// The <c>P(...)</c> helper in each emitter consults them in the same order.
+    /// </summary>
     private static void CollectAspects(
         System.Collections.Immutable.ImmutableArray<AttributeData> attributes,
         IMethodSymbol method,
         List<AspectInfo> aspects,
-        HashSet<string> seenTypes)
+        HashSet<string> seenTypes,
+        IMethodSymbol? attributeOverlay = null)
     {
         foreach (var attr in attributes)
         {
@@ -779,6 +830,8 @@ private static void CollectAspects(
             }
 
             var returnAttrs = method.GetReturnTypeAttributes();
+            if (attributeOverlay is not null)
+                returnAttrs = returnAttrs.AddRange(attributeOverlay.GetReturnTypeAttributes());
             bool sensitiveReturn = returnAttrs.Any(a => a.AttributeClass?.ToDisplayString() == SensitiveAttributeName);
             bool noLogReturn = returnAttrs.Any(a => a.AttributeClass?.ToDisplayString() == NoLogAttributeName);
 
@@ -799,6 +852,9 @@ private static bool DerivesFromAspectAttribute(INamedTypeSymbol? type)
         return false;
     }
 
+    private static bool HasAttr(IParameterSymbol param, string attrFqn) =>
+        param.GetAttributes().Any(a => a.AttributeClass?.ToDisplayString() == attrFqn);
+
     /// <summary>
     /// Returns true if a generated interceptor — which lives in a separate `__X_Aop` static
     /// class and invokes `@this.Method(...)` — can legally call a method with this

packages/ZibStack.NET.Log/src/ZibStack.NET.Log/Generator/LogAspectEmitter.cs

@@ -461,26 +461,58 @@ private static void EmitDictEntries(StringBuilder sb, SanitizedTypeModel model,
 /// </summary>
 internal sealed class LogClassDataProvider : IClassDataProvider
 {
-    public string AttributeFullName => "ZibStack.NET.Log.LogAttribute";
+    private const string LogAttributeFqn = "ZibStack.NET.Log.LogAttribute";
+
+    public string AttributeFullName => LogAttributeFqn;
 
     // Compilation isn't plumbed through the IClassDataProvider contract, but the
     // containing assembly's compilation is reachable via the first syntax reference's
     // semantic model on the class symbol. Close enough for parser discovery.
     public IReadOnlyDictionary<string, object?>? ExtractClassData(INamedTypeSymbol classSymbol)
     {
-        // Check if class has any [Log] methods
-        bool hasLog = classSymbol.GetMembers().OfType<IMethodSymbol>()
-            .Any(m => m.GetAttributes().Any(a => a.AttributeClass?.ToDisplayString() == "ZibStack.NET.Log.LogAttribute"));
-        if (!hasLog) return null;
-
         var compilation = GetCompilation(classSymbol);
         if (compilation is null) return null;
 
+        // [Log] reaches a class through three paths — method-level attribute,
+        // class-level attribute, or an IAopConfigurator.Apply<LogAttribute>(...) rule
+        // matching any of its methods. Historically we only checked the first one, so
+        // global ILogConfigurator defaults (LogParameters=false etc.) silently dropped
+        // on the floor for the other two paths — the emitter then fell back to the
+        // hardcoded `true` and kept logging parameters anyway.
+        if (!HasAnyLogAspect(classSymbol, compilation)) return null;
+
         var defaults = LogConfiguratorParser.Read(compilation);
         var data = defaults.ToDefaultsDictionary();
         return data.Count > 0 ? data : null;
     }
 
+    private static bool HasAnyLogAspect(INamedTypeSymbol classSymbol, Compilation compilation)
+    {
+        if (classSymbol.GetAttributes().Any(IsLogAttr))
+            return true;
+
+        foreach (var member in classSymbol.GetMembers())
+        {
+            if (member is IMethodSymbol m && m.GetAttributes().Any(IsLogAttr))
+                return true;
+        }
+
+        var logRules = AopConfiguratorParser.ReadAll(compilation).ApplyRules
+            .Where(r => r.AspectFqn == LogAttributeFqn)
+            .ToList();
+        if (logRules.Count == 0) return false;
+
+        foreach (var member in classSymbol.GetMembers())
+        {
+            if (member is not IMethodSymbol m) continue;
+            if (logRules.Any(r => AopParser.MatchesApplyRule(r, m))) return true;
+        }
+        return false;
+    }
+
+    private static bool IsLogAttr(AttributeData a) =>
+        a.AttributeClass?.ToDisplayString() == LogAttributeFqn;
+
     private static Compilation? GetCompilation(INamedTypeSymbol classSymbol)
     {
         // Classes reaching this provider are declared in the user's source (the generator

packages/ZibStack.NET.Log/tests/ZibStack.NET.Log.Tests/ApplyLogTests.cs

@@ -138,4 +138,51 @@ public void ApplyLog_InterfaceDispatch_ReturnValueLogged()
         var exitLog = entries.First(e => e.Message.Contains("Exited") && e.Message.Contains("GetBalance"));
         Assert.Contains("300", exitLog.Message);
     }
+
+    // ── Apply(selector, configure) — aspect configuration at the rule level ──
+    // Verifies that the second lambda on b.Apply<LogAttribute>() actually feeds
+    // LogParameters=false into the emitted interceptor. Historically the flag
+    // was ignored because LogClassDataProvider only wired up global defaults
+    // when the class carried a method-level [Log] attribute.
+
+    [Fact]
+    public void ApplyLog_ConfigureLambda_LogParametersFalse_OmitsArgs()
+    {
+        var svc = new NoParamsService();
+        svc.Charge("Alice", 99.99m);
+
+        var entries = _logProvider.AllEntries;
+        var entryLog = entries.First(e => e.Message.Contains("Entering") && e.Message.Contains("Charge"));
+        Assert.DoesNotContain("Alice", entryLog.Message);
+        Assert.DoesNotContain("99.99", entryLog.Message);
+    }
+
+    // ── [NoLog] parameter via interface dispatch ─────────────────────────────
+    // Covers both placements of the attribute: on the interface parameter and
+    // on the impl parameter. With the bug fix, both paths redact the value
+    // from the proxy's entry log.
+
+    [Fact]
+    public void NoLogOnInterfaceParam_InterfaceDispatch_Redacted()
+    {
+        INoLogIfaceParamService svc = new NoLogIfaceParamServiceImpl();
+        _ = svc.DoThing("ok", "topsecret");
+
+        var entry = _logProvider.AllEntries.First(e =>
+            e.Message.Contains("Entering") && e.Message.Contains("DoThing"));
+        Assert.Contains("ok", entry.Message);
+        Assert.DoesNotContain("topsecret", entry.Message);
+    }
+
+    [Fact]
+    public void NoLogOnImplParam_InterfaceDispatch_Redacted()
+    {
+        INoLogImplParamService svc = new NoLogImplParamServiceImpl();
+        _ = svc.DoThing("ok", "topsecret");
+
+        var entry = _logProvider.AllEntries.First(e =>
+            e.Message.Contains("Entering") && e.Message.Contains("DoThing"));
+        Assert.Contains("ok", entry.Message);
+        Assert.DoesNotContain("topsecret", entry.Message);
+    }
 }

packages/ZibStack.NET.Log/tests/ZibStack.NET.Log.Tests/Fixtures/LogTestServices.cs

@@ -168,5 +168,47 @@ public void Configure(ZibStack.NET.Aop.IAopBuilder b)
             .ClassesWhere(c => c.Name.StartsWith("E2e"))
             .PublicMethods()
         );
+
+        // Same class-matcher style but with a configure lambda — exercises the
+        // "Apply(selector, configure)" overload and verifies that the aspect
+        // property (LogParameters=false) flows down into the generated interceptor.
+        b.Apply<LogAttribute>(
+            to => to.ClassesWhere(c => c.Name.StartsWith("NoParams")),
+            a => a.LogParameters = false
+        );
     }
 }
+
+// Service whose params should NOT appear in the entry log, because the Apply rule
+// carries `a => a.LogParameters = false`. Covers hierarchy level 3 (Apply-config).
+public class NoParamsService
+{
+    public string Charge(string customer, decimal amount) => $"charged-{customer}-{amount}";
+}
+
+// ── [NoLog] on parameters via interface dispatch ────────────────────────────
+// Two shapes: [NoLog] declared on the interface parameter only, and [NoLog]
+// declared on the impl parameter only. Both should redact the value when the
+// call is made through the interface (i.e. goes through the synthesized proxy).
+
+public interface INoLogIfaceParamService
+{
+    string DoThing(string visible, [NoLog] string secret);
+}
+
+public class NoLogIfaceParamServiceImpl : INoLogIfaceParamService
+{
+    [Log]
+    public string DoThing(string visible, string secret) => $"{visible}-{secret}";
+}
+
+public interface INoLogImplParamService
+{
+    string DoThing(string visible, string secret);
+}
+
+public class NoLogImplParamServiceImpl : INoLogImplParamService
+{
+    [Log]
+    public string DoThing(string visible, [NoLog] string secret) => $"{visible}-{secret}";
+}