Fixed various issues reported by EnableNETAnalyzers #6007
Conversation
|
Warning Rate limit exceeded@snakefoot has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughStandardizes string comparisons and culture-invariant formatting, narrows several collection types to concrete List/Dictionary, introduces conditional span-based write paths for newer TFMs, adjusts small parsing/substring behaviors and minor initializations. No public API signature changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/NLog/Internal/XmlParser.cs (1)
592-599: Avoid allocations when parsing hex entities
string.Contains+int.Parseallocates a temporary string on every hex digit and still callschar.ToLowerwith the current culture. We can keep the new invariant semantics, skip the transient allocations, and stay culture-agnostic by working directly on the char values.- if ("abcdef".Contains(char.ToLower(_xmlSource.Current))) - unicode += int.Parse(_xmlSource.Current.ToString(), System.Globalization.NumberStyles.HexNumber, System.Globalization.CultureInfo.InvariantCulture); - else if (_xmlSource.Current < '0' || _xmlSource.Current > '9') - throw new XmlParserException("Invalid XML document. Cannot parse unicode-char hex-value"); - else - unicode += _xmlSource.Current - '0'; + var chrLower = char.ToLowerInvariant(_xmlSource.Current); + if (chrLower >= 'a' && chrLower <= 'f') + { + unicode += chrLower - 'a' + 10; + } + else if (_xmlSource.Current >= '0' && _xmlSource.Current <= '9') + { + unicode += _xmlSource.Current - '0'; + } + else + { + throw new XmlParserException("Invalid XML document. Cannot parse unicode-char hex-value"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/NLog/Common/InternalLogger.cs(1 hunks)src/NLog/Config/PropertyTypeConverter.cs(1 hunks)src/NLog/Internal/CollectionExtensions.cs(1 hunks)src/NLog/Internal/ObjectReflectionCache.cs(1 hunks)src/NLog/Internal/ScopeContextAsyncState.cs(1 hunks)src/NLog/Internal/StackTraceUsageUtils.cs(1 hunks)src/NLog/Internal/StringBuilderExt.cs(1 hunks)src/NLog/Internal/XmlParser.cs(1 hunks)src/NLog/LayoutRenderers/CounterLayoutRenderer.cs(1 hunks)src/NLog/LayoutRenderers/DateLayoutRenderer.cs(1 hunks)src/NLog/LayoutRenderers/ScopeContextNestedStatesLayoutRenderer.cs(1 hunks)src/NLog/LayoutRenderers/Wrappers/WrapLineLayoutRendererWrapper.cs(1 hunks)src/NLog/LayoutRenderers/Wrappers/XmlEncodeLayoutRendererWrapper.cs(1 hunks)src/NLog/LogLevel.cs(1 hunks)src/NLog/MessageTemplates/MessageTemplateParameters.cs(1 hunks)src/NLog/ScopeContext.cs(1 hunks)src/NLog/Targets/ColoredConsoleAnsiPrinter.cs(1 hunks)src/NLog/Targets/ColoredConsoleSystemPrinter.cs(1 hunks)src/NLog/Targets/ConsoleWordHighlightingRule.cs(1 hunks)src/NLog/Targets/EventLogTarget.cs(1 hunks)src/NLog/Targets/FileArchiveHandlers/BaseFileArchiveHandler.cs(1 hunks)src/NLog/Targets/FileTarget.cs(1 hunks)src/NLog/Targets/TargetWithContext.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
src/NLog/Internal/StringBuilderExt.cs (2)
src/NLog/LayoutRenderers/LayoutRenderer.cs (1)
CultureInfo(232-235)tests/NLog.UnitTests/NLogTestBase.cs (1)
CultureInfo(360-363)
src/NLog/LayoutRenderers/DateLayoutRenderer.cs (1)
src/NLog/LayoutRenderers/ShortDateLayoutRenderer.cs (2)
CachedDateFormatted(105-115)CachedDateFormatted(107-111)
src/NLog/LayoutRenderers/CounterLayoutRenderer.cs (3)
src/NLog/Targets/LineEndingMode.cs (2)
Equals(188-191)Equals(196-199)src/NLog/LogFactory.cs (2)
Equals(1113-1116)Equals(1118-1121)src/NLog/Filters/WhenRepeatedFilter.cs (2)
Equals(391-407)Equals(409-412)
src/NLog/Internal/XmlParser.cs (1)
src/NLog/LayoutRenderers/LayoutRenderer.cs (1)
CultureInfo(232-235)
src/NLog/Targets/ColoredConsoleSystemPrinter.cs (2)
tests/NLog.UnitTests/Targets/ColoredConsoleTargetTests.cs (1)
Write(384-387)src/NLog/Internal/SimpleStringReader.cs (1)
Substring(122-125)
src/NLog/Targets/ColoredConsoleAnsiPrinter.cs (1)
tests/NLog.UnitTests/Targets/ColoredConsoleTargetTests.cs (1)
Write(384-387)
src/NLog/Internal/ScopeContextAsyncState.cs (1)
src/NLog/ScopeContext.cs (5)
Dictionary(493-511)Dictionary(730-744)Dictionary(747-754)Dictionary(776-787)Dictionary(826-829)
src/NLog/Targets/EventLogTarget.cs (1)
src/NLog/Layouts/SimpleLayout.cs (5)
SimpleLayout(70-73)SimpleLayout(79-82)SimpleLayout(89-92)SimpleLayout(100-104)SimpleLayout(106-140)
src/NLog/MessageTemplates/MessageTemplateParameters.cs (1)
src/NLog/MessageTemplates/MessageTemplateParameter.cs (2)
MessageTemplateParameter(105-111)MessageTemplateParameter(120-126)
src/NLog/LayoutRenderers/ScopeContextNestedStatesLayoutRenderer.cs (2)
src/NLog/LayoutRenderers/DateLayoutRenderer.cs (1)
Append(110-122)src/NLog/LayoutRenderers/AllEventPropertiesLayoutRenderer.cs (1)
Append(165-204)
src/NLog/Common/InternalLogger.cs (3)
src/NLog/Internal/TargetWithFilterChain.cs (2)
Equals(404-407)Equals(409-414)src/NLog/Targets/LineEndingMode.cs (2)
Equals(188-191)Equals(196-199)src/NLog/LogFactory.cs (2)
Equals(1113-1116)Equals(1118-1121)
src/NLog/Config/PropertyTypeConverter.cs (1)
src/NLog/LayoutRenderers/LayoutRenderer.cs (1)
CultureInfo(232-235)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (8)
src/NLog/LayoutRenderers/DateLayoutRenderer.cs (1)
90-91: Nice touch on nullable field.Annotating
_cachedDateFormattedas nullable lines up with how the cache is managed and removes redundant initialization. Clean and clear.src/NLog/LogLevel.cs (1)
273-274: Message formatting consistency looks good.Switching to the interpolated string keeps the exception readable without changing behavior. All good here.
src/NLog/Internal/ObjectReflectionCache.cs (1)
683-698: Static helper reduces hidden captures.Making
BuildEnumeratorstatic avoids any unintended coupling with the instance and keeps the intent obvious. Looks solid.src/NLog/Targets/EventLogTarget.cs (1)
362-365: Ordinal comparison is the right call.Using
StringComparison.Ordinalfor the fixed machine-name check prevents culture surprises and aligns with the rest of the PR’s tightening. Nicely done.src/NLog/Internal/StringBuilderExt.cs (1)
409-409: Invariant padding keeps digits stable.Formatting the cached two-digit strings with
CultureInfo.InvariantCultureremoves locale-dependent surprises—exactly what we want for these helpers.src/NLog/Internal/StackTraceUsageUtils.cs (1)
89-92: Preserves full local function name. Switching toSubstring(1)keeps async/local function names intact while still trimming the extra compiler prefix. Good catch.src/NLog/Targets/ColoredConsoleSystemPrinter.cs (1)
89-93: Span-based write avoids allocation. Leveraging theReadOnlySpan<char>overload on newer targets while retaining the old fallback is a clean way to cut substring churn without dropping compatibility.src/NLog/Targets/ColoredConsoleAnsiPrinter.cs (1)
88-94: Nice consistency with the system printer. Matching the span-based path here gives the same allocation win for the ANSI writer, and the conditional keeps older TFMs happy.
0782c4d to
053418b
Compare
|
Apply
AsSpan()instead ofSubString(), when doing word-highlighting in ColoredConsoleTarget to reduce memory allocations