Skip to content

Layout.IsNullOrEmpty similar to string.IsNullOrEmpty#6123

Merged
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:LayoutIsNullOrEmpty
Mar 21, 2026
Merged

Layout.IsNullOrEmpty similar to string.IsNullOrEmpty#6123
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:LayoutIsNullOrEmpty

Conversation

@snakefoot
Copy link
Copy Markdown
Contributor

No description provided.

@snakefoot snakefoot added the enhancement Improvement on existing feature label Mar 20, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: bfbd4967-0111-44a5-81c4-9aae364c4fbf

📥 Commits

Reviewing files that changed from the base of the PR and between eb1c7e2 and eb94d78.

📒 Files selected for processing (6)
  • src/NLog/LayoutRenderers/Wrappers/WhenEmptyLayoutRendererWrapper.cs
  • src/NLog/Layouts/Layout.cs
  • src/NLog/Targets/EventLogTarget.cs
  • src/NLog/Targets/FileTarget.cs
  • src/NLog/Targets/Wrappers/GroupByTargetWrapper.cs
  • tests/NLog.UnitTests/Layouts/SimpleLayoutParserTests.cs
✅ Files skipped from review due to trivial changes (1)
  • tests/NLog.UnitTests/Layouts/SimpleLayoutParserTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/NLog/Layouts/Layout.cs

Walkthrough

Introduced Layout.IsNullOrEmpty(Layout? layout) and replaced several manual null/empty reference checks with this helper across layout wrappers and targets; added a unit test verifying behavior for null, empty, Layout.Empty, and a non-empty layout. (47 words)

Changes

Cohort / File(s) Summary
Layout Helper
src/NLog/Layouts/Layout.cs
Added public static bool IsNullOrEmpty(Layout? layout) that returns true for null or when ReferenceEquals(layout, Layout.Empty), otherwise false.
Targets & Wrappers
src/NLog/Targets/EventLogTarget.cs, src/NLog/Targets/FileTarget.cs, src/NLog/Targets/Wrappers/GroupByTargetWrapper.cs, src/NLog/LayoutRenderers/Wrappers/WhenEmptyLayoutRendererWrapper.cs
Replaced ad-hoc null/ReferenceEquals(..., Layout.Empty) checks with Layout.IsNullOrEmpty(...). Validation logic and thrown NLogConfigurationException remain, but the set of treated-as-empty layouts is broadened.
Unit Test
tests/NLog.UnitTests/Layouts/SimpleLayoutParserTests.cs
Added IsNullOrEmpty xUnit test asserting true for "", null, and Layout.Empty, and false for a SimpleLayout from "${message}".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped along the code today,
Found empty layouts in my way,
One little helper now takes the test,
Null or empty — it knows best,
Hooray for tidy checks and less dismay! 🎉

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to evaluate whether a description relates to the changeset. Add a pull request description explaining the purpose and benefits of the Layout.IsNullOrEmpty helper method and its usage across the codebase.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: introducing a Layout.IsNullOrEmpty helper method analogous to string.IsNullOrEmpty, which is implemented in the pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/NLog/Layouts/Layout.cs`:
- Around line 105-107: IsNullOrEmpty currently only checks reference equality
with Layout.Empty, missing other empty instances (e.g., new SimpleLayout("")).
Update Layout.IsNullOrEmpty to return true when layout is null OR
ReferenceEquals(layout, Layout.Empty) OR the layout renders to an empty string
for a default/no-op event; for example, call the layout's Render method on a
default LogEventInfo (or equivalent neutral event) and treat a null/empty result
as empty. Reference the Layout.IsNullOrEmpty method and ensure new
SimpleLayout("") and other empty-equivalent Layout implementations are covered
by this additional render-based emptiness check.

In `@tests/NLog.UnitTests/Layouts/SimpleLayoutParserTests.cs`:
- Around line 68-75: Add an assertion that directly constructs an empty
SimpleLayout and verifies Layout.IsNullOrEmpty returns true; specifically, in
the IsNullOrEmpty test method add Assert.True(Layout.IsNullOrEmpty(new
SimpleLayout(""))) so the code covers the direct empty SimpleLayout construction
and prevents regressions in empty-layout semantics for the SimpleLayout class
and Layout.IsNullOrEmpty helper.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 31ac3eea-3f74-4c1b-822c-289f1501eb1e

📥 Commits

Reviewing files that changed from the base of the PR and between fdd6646 and eb1c7e2.

📒 Files selected for processing (2)
  • src/NLog/Layouts/Layout.cs
  • tests/NLog.UnitTests/Layouts/SimpleLayoutParserTests.cs

@snakefoot snakefoot force-pushed the LayoutIsNullOrEmpty branch from eb1c7e2 to eb94d78 Compare March 20, 2026 19:33
@snakefoot snakefoot closed this Mar 20, 2026
@snakefoot snakefoot reopened this Mar 20, 2026
@sonarqubecloud
Copy link
Copy Markdown

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement on existing feature size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant