src/NLog/Common/InternalLogger.cs (1)

477-478: Optional: align token variable naming for readability.

localapplicationdatadir breaks the naming pattern used by nearby locals. Consider localApplicationDataDirToken for consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Common/InternalLogger.cs` around lines 477 - 478, The local variable
name localapplicationdatadir used in the ContainsSubStringIgnoreCase call and
subsequent Replace should be renamed to follow the surrounding naming style
(e.g., localApplicationDataDirToken) for readability and consistency; update all
references in the block where internalLogFile is processed (the
ContainsSubStringIgnoreCase call, the out variable declaration, and the Replace
call) so the symbol names remain correct and compile.

src/NLog/Layouts/JSON/JsonAttribute.cs (1)

221-228: Consider keeping Layout visible when Encode=false.

Current output drops layout details in that branch, which makes diagnostics less informative.

♻️ Suggested adjustment

 public override string ToString()
 {
-    if (Encode)
-        return $"{Name}={Layout}";
-    else
-        return $"{Name} (Encode=false)";
+    if (Encode)
+        return $"{Name}={Layout}";
+    else
+        return $"{Name}={Layout} (Encode=false)";
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Layouts/JSON/JsonAttribute.cs` around lines 221 - 228, The
ToString() implementation of JsonAttribute hides Layout when Encode is false;
update JsonAttribute.ToString() so both branches include the Layout for
diagnostics (keep the existing Encode=true behavior but change the Encode=false
branch to also include Layout, e.g. include Name, Encode=false and the Layout
value), referencing the JsonAttribute class and its Encode, Name and Layout
properties so the layout remains visible in logs.

src/NLog/Common/InternalLogger.cs (1)

465-478: Consider consolidating token replacement to reduce drift risk.

The repeated ContainsSubStringIgnoreCase(..., out var token) && token != null blocks are now fairly repetitive. A tiny helper would make future token additions safer and easier to maintain.

Refactor sketch

+        private static string ReplacePathToken(string path, string tokenName, string replacement)
+        {
+            if (ContainsSubStringIgnoreCase(path, tokenName, out var token) && token != null)
+                return path.Replace(token, replacement);
+            return path;
+        }
+
         private static string ExpandFilePathVariables(string internalLogFile)
         {
             try
             {
                 if (internalLogFile.IndexOf('%') >= 0)
                     internalLogFile = Environment.ExpandEnvironmentVariables(internalLogFile);
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${currentdir}", out var currentDirToken) && currentDirToken != null)
-                    internalLogFile = internalLogFile.Replace(currentDirToken, System.IO.Directory.GetCurrentDirectory() + System.IO.Path.DirectorySeparatorChar.ToString());
+                internalLogFile = ReplacePathToken(internalLogFile, "${currentdir}", System.IO.Directory.GetCurrentDirectory() + System.IO.Path.DirectorySeparatorChar);
                 // ...same pattern for remaining tokens...

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Common/InternalLogger.cs` around lines 465 - 478, Replace the
repeated token-replacement blocks in InternalLogger.cs with a small helper that
centralizes the ContainsSubStringIgnoreCase check and replacement to avoid
drift; create a method (e.g., ReplaceTokenIfPresent) that accepts the token name
(like "${currentdir}"), the out-token capture logic (uses
ContainsSubStringIgnoreCase) and a replacement string/provider, and call it for
each token instead of duplicating the pattern around internalLogFile and
ContainsSubStringIgnoreCase; reference the existing ContainsSubStringIgnoreCase
function and the internalLogFile variable so callers like
ReplaceTokenIfPresent("${basedir}", () =>
LogManager.LogFactory.CurrentAppEnvironment.AppDomainBaseDirectory +
Path.DirectorySeparatorChar.ToString()) perform the same replacement behavior.

src/NLog/Layouts/XML/XmlAttribute.cs (1)

200-208: Consider including Layout in all cases for fuller diagnostic output.

When Encode=false and Layout is not a SimpleLayout, the current implementation omits the Layout value entirely. This could reduce diagnostic utility when debugging configuration issues.

💡 Optional: Include Layout value in all branches

 public override string ToString()
 {
     if (Encode || Layout is SimpleLayout)
         return $"{Name}={Layout}";
     else
-        return $"{Name} (Encode=false)";
+        return $"{Name}={Layout} (Encode=false)";
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Layouts/XML/XmlAttribute.cs` around lines 200 - 208, The ToString
override in XmlAttribute omits the Layout when Encode is false and Layout is not
a SimpleLayout; update XmlAttribute.ToString to always include the Layout value
for fuller diagnostics (e.g., include Layout.ToString() or Layout?.ToString()
regardless of Encode) while still indicating the Encode flag and Name so the
output contains Name, Encode state, and the Layout representation; refer to the
XmlAttribute class, its ToString method, the Encode and Layout properties, and
Name to implement this change.

src/NLog/Common/InternalLogger.cs (1)

467-480: Consider using [NotNullWhen(true)] to eliminate redundant null guards.

The && token != null checks are logically redundant since ContainsSubStringIgnoreCase returns true only when result is non-null. Adding the [NotNullWhen(true)] attribute to the method's out parameter would let the compiler infer this, allowing removal of the redundant checks.

♻️ Proposed refactor

Update the method signature at line 492:

-private static bool ContainsSubStringIgnoreCase(string haystack, string needle, out string? result)
+private static bool ContainsSubStringIgnoreCase(string haystack, string needle, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out string? result)

Then simplify the null guards:

-if (ContainsSubStringIgnoreCase(internalLogFile, "${currentdir}", out var currentDirToken) && currentDirToken != null)
+if (ContainsSubStringIgnoreCase(internalLogFile, "${currentdir}", out var currentDirToken))
     internalLogFile = internalLogFile.Replace(currentDirToken, System.IO.Directory.GetCurrentDirectory() + System.IO.Path.DirectorySeparatorChar.ToString());

(Apply similar simplification to all other token replacement lines)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Common/InternalLogger.cs` around lines 467 - 480, The repeated "&&
token != null" guards are redundant because ContainsSubStringIgnoreCase only
returns true when the out token is non-null; update the
ContainsSubStringIgnoreCase method signature to annotate the out parameter with
[NotNullWhen(true)] so the compiler knows the token is non-null on true, then
remove the redundant "&& ... != null" checks from each replacement line (e.g.,
currentDirToken, baseDirToken, tempDirToken, processDirToken,
commonAppDataDirToken, appDataDirToken, localapplicationdataToken) so the if
statements simply check the boolean result and perform Replace when true.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/NLog/Common/InternalLogger.cs (1)

465-480: The && token != null checks are redundant.

ContainsSubStringIgnoreCase returns true only when the out parameter is non-null (line 496: return result != null). The additional null checks are unnecessary.

However, this is a minor verbosity issue and the code is functionally correct. The checks do satisfy the nullable analyzer and provide defensive clarity, so keeping them is acceptable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Common/InternalLogger.cs` around lines 465 - 480, The if-conditions
in InternalLogger.cs redundantly check both ContainsSubStringIgnoreCase(...) and
the out-token for null (e.g., currentDirToken, baseDirToken, tempDirToken,
processDirToken, commonAppDataDirToken, appDataDirToken,
localapplicationdataToken); remove the unnecessary "&& <token> != null"
fragments because ContainsSubStringIgnoreCase already guarantees a non-null out
value when it returns true—update the conditions around internalLogFile
replacements (the blocks that call ContainsSubStringIgnoreCase and then Replace)
to rely solely on the ContainsSubStringIgnoreCase(...) result while leaving the
replacement logic unchanged.

src/NLog/Common/InternalLogger.cs (1)

467-480: Consider extracting token replacement into a helper to reduce repetition.

The logic is correct, but this block is repetitive and harder to maintain as token count grows.

♻️ Proposed refactor

-                if (ContainsSubStringIgnoreCase(internalLogFile, "${currentdir}", out var currentDirToken) && currentDirToken != null)
-                    internalLogFile = internalLogFile.Replace(currentDirToken, System.IO.Directory.GetCurrentDirectory() + System.IO.Path.DirectorySeparatorChar.ToString());
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${basedir}", out var baseDirToken) && baseDirToken != null)
-                    internalLogFile = internalLogFile.Replace(baseDirToken, LogManager.LogFactory.CurrentAppEnvironment.AppDomainBaseDirectory + System.IO.Path.DirectorySeparatorChar.ToString());
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${tempdir}", out var tempDirToken) && tempDirToken != null)
-                    internalLogFile = internalLogFile.Replace(tempDirToken, LogManager.LogFactory.CurrentAppEnvironment.UserTempFilePath + System.IO.Path.DirectorySeparatorChar.ToString());
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${processdir}", out var processDirToken) && processDirToken != null)
-                    internalLogFile = internalLogFile.Replace(processDirToken, System.IO.Path.GetDirectoryName(LogManager.LogFactory.CurrentAppEnvironment.CurrentProcessFilePath) + System.IO.Path.DirectorySeparatorChar.ToString());
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${commonApplicationDataDir}", out var commonAppDataDirToken) && commonAppDataDirToken != null)
-                    internalLogFile = internalLogFile.Replace(commonAppDataDirToken, NLog.LayoutRenderers.SpecialFolderLayoutRenderer.GetFolderPath(Environment.SpecialFolder.CommonApplicationData) + System.IO.Path.DirectorySeparatorChar.ToString());
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${userApplicationDataDir}", out var appDataDirToken) && appDataDirToken != null)
-                    internalLogFile = internalLogFile.Replace(appDataDirToken, NLog.LayoutRenderers.SpecialFolderLayoutRenderer.GetFolderPath(Environment.SpecialFolder.ApplicationData) + System.IO.Path.DirectorySeparatorChar.ToString());
-                if (ContainsSubStringIgnoreCase(internalLogFile, "${userLocalApplicationDataDir}", out var localapplicationdataToken) && localapplicationdataToken != null)
-                    internalLogFile = internalLogFile.Replace(localapplicationdataToken, NLog.LayoutRenderers.SpecialFolderLayoutRenderer.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) + System.IO.Path.DirectorySeparatorChar.ToString());
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${currentdir}", System.IO.Directory.GetCurrentDirectory() + System.IO.Path.DirectorySeparatorChar);
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${basedir}", LogManager.LogFactory.CurrentAppEnvironment.AppDomainBaseDirectory + System.IO.Path.DirectorySeparatorChar);
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${tempdir}", LogManager.LogFactory.CurrentAppEnvironment.UserTempFilePath + System.IO.Path.DirectorySeparatorChar);
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${processdir}", System.IO.Path.GetDirectoryName(LogManager.LogFactory.CurrentAppEnvironment.CurrentProcessFilePath) + System.IO.Path.DirectorySeparatorChar);
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${commonApplicationDataDir}", NLog.LayoutRenderers.SpecialFolderLayoutRenderer.GetFolderPath(Environment.SpecialFolder.CommonApplicationData) + System.IO.Path.DirectorySeparatorChar);
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${userApplicationDataDir}", NLog.LayoutRenderers.SpecialFolderLayoutRenderer.GetFolderPath(Environment.SpecialFolder.ApplicationData) + System.IO.Path.DirectorySeparatorChar);
+                internalLogFile = ReplaceTokenIfFoundIgnoreCase(internalLogFile, "${userLocalApplicationDataDir}", NLog.LayoutRenderers.SpecialFolderLayoutRenderer.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) + System.IO.Path.DirectorySeparatorChar);

private static string ReplaceTokenIfFoundIgnoreCase(string value, string needle, string replacement)
{
    if (ContainsSubStringIgnoreCase(value, needle, out var token) && token != null)
        return value.Replace(token, replacement);

    return value;
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Common/InternalLogger.cs` around lines 467 - 480, The repeated
token-replacement blocks in InternalLogger.cs make maintenance hard; create a
helper like ReplaceTokenIfFoundIgnoreCase(value, needle, replacement) that uses
ContainsSubStringIgnoreCase and returns the replaced string or original, then
replace each in the method that computes internalLogFile (the blocks handling
"${currentdir}", "${basedir}", "${tempdir}", "${processdir}",
"${commonApplicationDataDir}", "${userApplicationDataDir}",
"${userLocalApplicationDataDir}") by calling this helper with the appropriate
replacement expressions (e.g., System.IO.Directory.GetCurrentDirectory() +
Path.DirectorySeparatorChar,
LogManager.LogFactory.CurrentAppEnvironment.AppDomainBaseDirectory +
Path.DirectorySeparatorChar, etc.), keeping existing symbol names
ContainsSubStringIgnoreCase and LogManager.LogFactory.CurrentAppEnvironment
referenced exactly as they are.