[LANG-1815] Fix AnnotationUtils.equals for package-private annotations.

[kzhunmax]

Jira ticket is LANG-1815
This PR closes #1587 by reusing facilities introduced in #1558

[Copilot]

Pull request overview

This PR addresses LANG-1815 by making AnnotationUtils.equals(Annotation, Annotation) resilient to Java reflective access restrictions (e.g., when the annotation type is package-private and therefore not reflectively accessible from org.apache.commons.lang3). It does so by reusing the AbstractReflection “force accessible” facility introduced previously, and adds a regression test outside the org.apache.commons.lang3 package to reproduce the original failure mode.

Changes:

• Add an external-package regression test that reproduces the package-private accessibility scenario and verifies exception wrapping behavior.
• Update AnnotationUtils.equals(...) to force accessibility on annotation member methods and to wrap ReflectiveOperationException instead of silently returning false.
• Generalize and expose AbstractReflection helpers so AnnotationUtils can reuse the force-accessible behavior.

Reviewed changes

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

File	Description
src/test/java/org/apache/commons/lang3/external/AnnotationEqualsTest.java	Adds regression coverage for package-private annotations and reflective exception wrapping from outside org.apache.commons.lang3.
src/main/java/org/apache/commons/lang3/builder/AbstractReflection.java	Exposes/generalizes force-accessible helpers so they can be reused outside the builder package.
src/main/java/org/apache/commons/lang3/AnnotationUtils.java	Forces accessibility for annotation member access in equals(...) and changes reflective-failure handling to throw an unchecked exception.

Comments suppressed due to low confidence (2)

src/main/java/org/apache/commons/lang3/builder/AbstractReflection.java:145

• The Javadoc for setAccessibleTrue(...) still refers to a "field" and mentions a "forceAccessible" flag that is not a parameter of this method, which makes the contract unclear when reading the code.

    /**
     * Sets the field as accessible by calling {@link AccessibleObject#setAccessible(boolean) AccessibleObject#setAccessible(true)} but <em>only</em> if a field
     * is not already accessible.
     *
     * @param accessibleObject The accessibleObject to set, may be null.

src/main/java/org/apache/commons/lang3/AnnotationUtils.java:226

• equals(...) now throws an IllegalStateException when reflective access fails (instead of returning false). The method-level Javadoc still only describes the boolean return semantics and does not document this unchecked exception, which can surprise API consumers.

        } catch (final ReflectiveOperationException ex) {
            throw new IllegalStateException(ex);
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[garydgregory]

Hello @kzhunmax
Thank you for the PR. Please review Copliot's comments. In the future, please request a Copilot review and address obvious issues. TY!

[kzhunmax]

Hello @garydgregory. I reviewed Copilot's comments. Fix available in commits: d046ada and b2947ab

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/main/java/org/apache/commons/lang3/AnnotationUtils.java:226

• AnnotationUtils.equals(Annotation, Annotation) now wraps ReflectiveOperationException in IllegalStateException, while hashCode() and toString() wrap the same failure mode in UncheckedException. Using different wrappers for the same underlying reflection failure makes the API inconsistent and harder to handle uniformly. Consider using UncheckedException here as well (and update the regression test accordingly).

        } catch (final ReflectiveOperationException ex) {
            throw new IllegalStateException(ex);
        }

src/main/java/org/apache/commons/lang3/builder/AbstractReflection.java:142

• setAccessible(boolean, Field) previously existed (package-private) and was changed to setAccessible(boolean, AccessibleObject). Even though the old method wasn’t public API, removing it can still cause NoSuchMethodError for any compiled code that happens to live in the same package (e.g., shaded/embedded usage). Adding a small overload that keeps the old signature and delegates would preserve binary compatibility while still allowing the generalized AccessibleObject variant.

    public static boolean setAccessible(final boolean forceAccessible, final AccessibleObject accessibleObject) {
        if (accessibleObject == null) {
            return false;
        }
        if (accessibleObject.isAccessible()) {

src/test/java/org/apache/commons/lang3/external/AnnotationEqualsTest.java:88

• This test currently expects IllegalStateException, but if AnnotationUtils.equals() uses the same reflection-failure wrapper as hashCode()/toString() (i.e., UncheckedException), the assertion should be updated to match. Using the fully qualified class name avoids adding a new import.

        final IllegalStateException ex =
                assertThrows(IllegalStateException.class, () -> AnnotationUtils.equals(tagA, tagB));
        assertInstanceOf(InvocationTargetException.class, ex.getCause());