Fix #124: Validate empty tag name in widget setters

[WarLikeLaux]

Q	A
Is bugfix?	✔️
New feature?	❌
Tests added?	✔️
Breaks BC?	❌
Fixed issues	#124

What does this PR do?

Validates empty tag names in the setters of Alert, Dropdown, and Menu so an empty string fails fast with InvalidArgumentException instead of slipping through to render time.

Before this, the tag setters in Dropdown and Menu accepted any string and only blew up later inside the render methods. The check now lives in the setter, next to the value it rejects, which matches what Alert::bodyTag() and Breadcrumbs::containerTag() already do. Alert::headerTag() used empty(), which also rejected the string "0" - a valid, if odd, tag name - so it now uses a strict === '' check.

With the setters guarding the value, the tag properties are annotated non-empty-string (the type Html::normalTag() expects), so the old render-time checks are redundant and removed.

Menu::tagName() had the same render-time check as the rest, so it's included here too even though the issue didn't list it.

Breadcrumbs::tag() is left alone on purpose: there an empty string means "don't render a wrapping tag", so it's a real option, not a mistake.

The exception type and message are unchanged. An empty tag never produced valid markup: for tags that always render it already threw at render(), and for the conditionally rendered ones (a divider, a sub-dropdown header, a before/after wrapper) it used to slip by as dead configuration until that branch was hit. Both cases now fail at the setter. This is the fail-fast behavior Alert and Breadcrumbs::containerTag() already had.

Summary by CodeRabbit

• Bug Fixes

  • Strengthened validation for HTML tag names in Alert, Dropdown, and Menu widgets to consistently reject empty string values.
  • Moved validation checks from render-time to setter-time for improved error detection during widget configuration.

• Tests

  • Updated test cases to verify empty tag name validation occurs at setter invocation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a602b53) to head (653e30b).

Additional details and impacted files

@@             Coverage Diff             @@
##              master      #158   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       316       316           
===========================================
  Files              8         8           
  Lines           1005      1005           
===========================================
  Hits            1005      1005           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[WarLikeLaux]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98e95222-2553-4d93-907c-c3d2fc2b5174

📥 Commits

Reviewing files that changed from the base of the PR and between a602b53 and 653e30b.

📒 Files selected for processing (8)

• CHANGELOG.md
• src/Alert.php
• src/Dropdown.php
• src/Menu.php
• tests/Dropdown/ExceptionTest.php
• tests/Dropdown/ImmutableTest.php
• tests/Menu/ExceptionTest.php
• tests/Menu/ImmutableTest.php

---

📝 Walkthrough

Walkthrough

This PR enforces empty HTML tag name validation at the setter level across three widget classes (Alert, Dropdown, Menu), moving validation from render-time to construction-time. Dropdowns and Menus gain Psalm type annotations and setter validation; render methods lose redundant checks. Tests are updated to call setters directly and use non-empty values in immutability assertions.

Changes

Empty tag name validation enforcement

Layer / File(s)	Summary
Alert validation strictness src/Alert.php	Alert::headerTag() validation changes from empty($value) to strict empty-string equality (=== ''), allowing string "0" while still rejecting empty strings.
Dropdown setter validation src/Dropdown.php	Dropdown adds @psalm-var non-empty-string annotations to six tag properties and enforces empty-string rejection in their fluent setters (containerTag, dividerTag, headerTag, itemContainerTag, itemTag, itemsContainerTag).
Dropdown render-time check removal src/Dropdown.php	Dropdown removes redundant empty-string validation checks from private render methods, relying on the setter invariant established at construction time.
Dropdown test alignment tests/Dropdown/ExceptionTest.php, tests/Dropdown/ImmutableTest.php	Exception tests simplify to directly invoke empty-string setters without full render sequences; immutability tests switch from empty-string to 'div' arguments to bypass the new setter validation.
Menu setter validation src/Menu.php	Menu adds @psalm-var non-empty-string annotations to six tag properties and enforces empty-string rejection in their fluent setters (afterTag, beforeTag, dropdownContainerTag, itemsTag, linkTag, tagName).
Menu render-time check removal src/Menu.php	Menu removes redundant empty-string validation checks from private render methods, relying on the setter invariant.
Menu test alignment tests/Menu/ExceptionTest.php, tests/Menu/ImmutableTest.php	Exception tests simplify to directly invoke empty-string setters; immutability tests switch from empty-string to 'div' arguments to bypass setter validation.
Changelog update CHANGELOG.md	Documents Bug #158: setters across Alert, Dropdown, and Menu now validate empty tag names.

Possibly related issues

• #124: This PR directly implements the suggestion from issue #124 by changing Alert::headerTag validation to use === '' and adding empty-string validation to the same Dropdown and Menu tag setters, shifting validation from render-time to setter-time.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Tags now validate when you set them fast,
No empty strings shall ever pass,
Alert grows strict, Dropdown and Menu too,
A rabbit's hop—Bug #158 is through!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.90% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: validating empty tag names in widget setters to fix issue #124.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this 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

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.1.55)

PHP Parse error: syntax error, unexpected token "->", expecting ";" in /vendor/php-standard-library/php-standard-library/packages/class/src/Psl/Class/has_constant.php on line 18
Parse error: syntax error, unexpected token "->", expecting ";" in /vendor/php-standard-library/php-standard-library/packages/class/src/Psl/Class/has_constant.php on line 18

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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