[KYUUBI #7697] Reject hiddenImpl on a DynConstructors builder without a base class

[LuciferYang]

Why are the changes needed?

Closes #7697.

DynConstructors.builder() leaves baseClass null, and the zero-argument hiddenImpl(Class<?>... types) forwarded that field into hiddenImpl(Class, Class...), which calls targetClass.getDeclaredConstructor(types) and catches only SecurityException and NoSuchMethodException. The null escaped as a bare NPE from inside the builder chain, with no mention of a missing base class. A builder that never named a class asking for a hidden constructor in its base class has no correct reading, so the patch rejects the call. AGENTS.md asks for a throw on unsupported usage rather than a silent skip, and IllegalStateException is what this file already uses when a call does not fit the receiver's state: Ctor.bind throws it for "Cannot bind constructors".

A loose Class argument does not reach that overload: builder().hiddenImpl(Foo.class) binds hiddenImpl(Class, Class...) with Foo as the target. What reaches it is a call with no arguments, or one passing an explicit Class<?>[], which is what a Scala : _* splat compiles to and is the shape ReflectUtils already uses against the DynMethods builder.

Nothing in Kyuubi reaches it either. hiddenImpl() has no caller outside the new tests, and the twenty-odd production sites that build without a class all continue with one of the impl overloads. This is hygiene on a retained upstream API, not a field failure being fixed.

There is a second option a maintainer may prefer: drop the overload, as iceberg-common did in 1.7.0 (apache/iceberg#10818) after deprecating it in 1.6.0 over the same varargs conflict. That is not free, though: a caller passing an explicit array, or a Scala splat, binds here today, so removal is a source break for them even if nothing in this repository notices. Keeping the overload means carrying this guard and its explanation indefinitely. Removing a public method from a published module is a maintainer call rather than mine; #7689 put the same choice to reviewers for DynMethods.ctorImpl.

Two scope notes. The guard sits below the ctor != null short-circuit that opens every other impl and hiddenImpl overload, so a class-less builder that already matched keeps working: builder().impl("java.lang.String").hiddenImpl() builds before this patch and still builds after. Placed above the short-circuit it would start throwing. And the generic Cannot find constructor for null that a class-less builder reports when all its name-based lookups miss is untouched; hiddenImpl() no longer reaches it, and improving that message belongs with the problems diagnostics work.

How was this patch tested?

Two tests in DynConstructorsTest, one per half of the contract.

testHiddenImplWithoutBaseClassFailsFast asserts that builder().hiddenImpl() throws IllegalStateException whose message says the builder has no base class. Against origin/master's DynConstructors it goes red with Unexpected exception type thrown, expected: <java.lang.IllegalStateException> but was: <java.lang.NullPointerException>, that NPE being Cannot invoke "java.lang.Class.getDeclaredConstructor(java.lang.Class[])" because "targetClass" is null.

testHiddenImplWithoutBaseClassIsSkippedOnceFound builds builder().impl("java.lang.String").hiddenImpl().buildChecked() and asserts the constructed class is String. It pins the guard's placement rather than restating the first test: with the null check moved above the ctor != null short-circuit, it fails with IllegalStateException.

build/mvn -o test -pl kyuubi-util -am -DwildcardSuites=none -Dtest=DynConstructorsTest
build/mvn -o spotless:check -pl kyuubi-util

6 of 6 green and spotless clean. Both revert checks above ran against a copy of the pre-fix class compiled outside the repository.

Was this patch assisted by generative AI tooling?

Assisted-by: Claude Opus 5