Introduce new attribute callback interface

[nilmerg]

We all know the limitations the current attribute callbacks have:

• There is no way other than using reflection (the infamous BaseFormElement::getValueOfNameAttribute() comes to mind) or Attributes::render() to get a result of a registered attribute get callback
• Set callbacks are only used in Attributes::set(), but not in ::add() which sets a native attribute as if no callback is involved, so you accidentally end up with two attribute states

A previous discussion took place in #3, but a solution was never concluded.

I want to propose two solutions to this now:

1. There must be an officially supported signature to get a value of a get callback
2. No set callbacks anymore: Attributes::registerAttributeCallback() is deprecated and the alternative only supports get callbacks

Let me explain my reasoning for each solution:

Officially supported signature

Since the current supported way to evaluate get callbacks is to render attributes, workarounds like BaseFormElement::getValueOfNameAttribute() evolved. Luckily, this is an isolated workaround for form elements. And now we know the exact use cases in which the result of a callback is needed. For form elements, it all comes down to elements that are part of a fieldset and it always is about the name of an element. And the name of an element, is and will ever be, available using a get callback. So, let's keep it that way: If anyone requires the value of a get callback, let them access it using:

Attributes::call(string $name): ImmutableAttribute

A new type of attribute is needed for this: ImmutableAttribute. It must not be possible to assume that the result of a callback can be altered. The method ::call() will not store the callback's result in any way, it is nothing else than a proxy to the underlying callback.

The reason I went for a new method, rather than teaching ::get() about callbacks, was to avoid having to deal with the question what happens if there is a native attribute with the same name. And guess what, this is also something that evolved up until now. There are implementations that require a native attribute plus a get callback, there exists even a test case for this: AttributesTest::testNativeAttributesAndCallbacks()

Another reason is that requiring access to a callback's result is highly specific. There never was a need to transparently get callback results. Instead, there's a documented case where a clear distinction between native and callback is required. And, as I mentioned already, form elements that expect to be part of a fieldset also only require access to the callback result, while knowing the difference.

So: ::get() is for native attributes, like it ever was. ::call() is for get callbacks only.

No set callbacks anymore

This might be hard for some to understand. For me, this is also about how set callbacks evolved being used: Form element options. Proxying the population of such through attributes is just strange, sorry. So my reasoning here is simple and plain:

We've used them for form options, let's keep that but change how it's implemented, given that we recently raised the minimum PHP version requirement to 8.2 and have access to another variant of attributes!

Attributes::setCallback(string $name, callable $callback): static is the alternative for the now deprecated ::registerAttributeCallback().

The alternative for set callbacks is: Icinga/ipl-stdlib#59 in combination with #157

Regarding compatibility

Attributes::setCallback('name', $this->getName(...)) is not the same as Attributes::registerAttributeCallback('name', $this->getName(...)), as the latter cannot be accessed by Attributes::call(). Except error handling, that's different for the new callbacks, both behave equally.

None of the callbacks, new or legacy, are preserved in cloned Attributes now. My opinion on this is that this must be discouraged and not be supported. Callbacks hold references to $this and others, if not cloned properly (which they weren't till now), the declaring object cannot be garbage collected as early as it could be without such references. It may not lead to a significant memory leak, but still causes potential peaks in memory usage. I also found no occurrence where this is clearly wanted, so I deemed it acceptable to break this.

[Copilot]

Pull Request Overview

This PR introduces a new attribute callback interface to address limitations in the current attribute callback system. The main changes include implementing immutable attributes, adding a new callback method that only supports get operations, and deprecating the old callback system while maintaining backward compatibility.

• Introduces ImmutableAttribute class and BaseAttribute common parent class for better type safety
• Adds new setCallback() and call() methods for simplified attribute callback handling
• Deprecates registerAttributeCallback() in favor of the new get-only callback approach

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/HtmlElementTest.php	Adds test for HtmlElement accepting Attributes instances with callbacks
tests/AttributesTest.php	Comprehensive test coverage for new callback system and ImmutableAttribute functionality
tests/AttributeTest.php	Tests for ImmutableAttribute behavior and attribute cloning
src/ImmutableAttribute.php	New immutable attribute class implementation
src/HtmlElement.php	Updates constructor to directly assign attributes instead of merging
src/Common/BaseAttribute.php	New base class extracting common attribute functionality
src/Attributes.php	Core implementation of new callback system with legacy compatibility
src/Attribute.php	Refactored to extend BaseAttribute with mutable behavior

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}