src/NLog/Config/LoggingConfigurationParser.cs (1)

550-558: Consider renaming variable for clarity.

The variable targetsOptions is used within the rules validation context, which is confusing. Consider renaming it to ruleOption, option, or ruleAttribute to better reflect its purpose.

Apply this diff:

         if (rulesElement.Values.Count > 0)
         {
-            foreach (var targetsOptions in rulesElement.Values)
+            foreach (var ruleOption in rulesElement.Values)
             {
-                var configException = new NLogConfigurationException($"Unrecognized value '{targetsOptions.Key}'='{targetsOptions.Value}' in section '{rulesElement.Name}'");
+                var configException = new NLogConfigurationException($"Unrecognized value '{ruleOption.Key}'='{ruleOption.Value}' in section '{rulesElement.Name}'");
                 if (MustThrowConfigException(configException))
                     throw configException;
             }
         }

tests/NLog.UnitTests/Config/XmlConfigTests.cs (1)

197-223: Good test coverage for graceful degradation.

The test appropriately validates that unknown attributes on <targets> and <rules> elements don't break logging functionality when exception throwing is disabled. This aligns well with the new validation logic in LoggingConfigurationParser.cs.

Optional enhancement: Consider asserting that warnings about the unknown attributes (option='unknown' and bingo='tada') are logged to the InternalLogger, similar to how XmlConfig_attributes_shouldNotLogWarningsToInternalLog captures internal log output. This would provide more complete validation that the configuration parser properly detects and reports these issues.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

tests/NLog.UnitTests/Config/XmlConfigTests.cs (1)

197-223: Consider adding test coverage for the exception-throwing scenario.

The current test validates that unknown attributes don't break logging when exceptions are suppressed. However, there's no corresponding test that verifies exceptions ARE thrown when throwExceptions='true' or throwConfigExceptions='true' is set.

Following the pattern of ShoudThrowExceptionOnDuplicateAttributeWhenOptionIsEnabledTest (line 433), consider adding a similar test for unknown attributes on targets and rules elements.

Example test structure:

[Fact]
public void ShouldThrowExceptionOnUnknownAttributeWhenOptionIsEnabledTest()
{
    Assert.Throws<NLogConfigurationException>(() =>
    {
        new LogFactory().Setup().LoadConfigurationFromXml(@"
        <nlog throwExceptions='true'>
            <targets option='unknown'>
                <target name='debug' type='Debug' layout='${message}' />
            </targets>
            <rules>
                <logger name='*' minlevel='info' appendto='debug' />
            </rules>
        </nlog>");
    });

    Assert.Throws<NLogConfigurationException>(() =>
    {
        new LogFactory().Setup().LoadConfigurationFromXml(@"
        <nlog throwConfigExceptions='true'>
            <targets>
                <target name='debug' type='Debug' layout='${message}' />
            </targets>
            <rules bingo='tada'>
                <logger name='*' minlevel='info' appendto='debug' />
            </rules>
        </nlog>");
    });
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro