Skip to content

[LANG-1341] Clarify superclass equality in EqualsBuilder Javadoc - #1849

Closed
rys888 wants to merge 2 commits into
apache:masterfrom
rys888:LANG-1341-equals-builder-javadoc
Closed

rys888 wants to merge 2 commits into
apache:masterfrom
rys888:LANG-1341-equals-builder-javadoc

Conversation

@rys888

@rys888 rys888 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Related to LANG-1341.

The class-level EqualsBuilder example calls appendSuper(super.equals(obj)) unconditionally. For a class whose superclass uses Object.equals, this compares identity and makes distinct instances unequal even when all their fields match.

This documentation-only change removes that call from the general example and explains when to include superclass equality, both after the example and in appendSuper(boolean)'s Javadoc. No runtime code or imports change.

The earlier PR #343 was closed for lack of follow-up. This addresses its remaining review request to explain Object.equals in the method documentation as well.

Validation (JDK 21, Maven 3.9.16):

  • The full default Maven goals passed: 92,172 tests, 0 failures, 0 errors, 18 skipped, plus RAT, Checkstyle, japicmp, SpotBugs, PMD and Javadoc.
  • Command: mvn -B -ntp -U -s ../commons-lang-LANG-1341-validation/maven-settings.xml. The temporary settings file only points the existing mirror at Maven Central. The initial run with the local mirror failed to obtain the parent site descriptor and stopped while rendering the PMD report (skin cannot be null); no project configuration was changed and no checks were skipped.
  • Extracted and compiled the corrected example locally with Java 8 targeting: distinct instances with equal fields compare equal; identity, null and unrelated-class checks also pass. No additional upstream tests are needed for this documentation-only change.

Checklist:

  • Read the contribution guidelines.
  • Read the ASF Generative Tooling Guidance.
  • AI assistance: OpenAI Codex helped inspect the issue and prior discussion, draft the documentation, and run validation. This is an AI-assisted contribution.
  • Run a successful build using the default Maven goals.
  • Behavioral tests: not applicable; no runtime behavior changes. Existing tests pass.
  • Provide a description of what changes and why.
  • Use a meaningful commit subject and body, including an Assisted-By trailer.

Review follow-up (2026-11-01): Removed the changes.xml entry as requested. The PR remains a draft pending review of LANG-1341 and alignment between the implementation and Javadoc. The follow-up passed the documented non-test checks on JDK 21: mvn -B -ntp -DskipTests -DskipITs -s ../commons-lang-LANG-1341-validation/maven-settings.xml. Tests were skipped in this follow-up run; the full test results above describe the original submission.

Remove appendSuper from the general example and explain why Object.equals makes distinct instances unequal even when all compared fields match.

Assisted-By: OpenAI Codex
Comment thread src/changes/changes.xml Outdated
<action type="fix" dev="ggregory" due-to="alhuda, Gary Gregory">Keep RandomStringUtils.random rejection cap from failing valid ranges (#1843).</action>
<action type="fix" dev="ggregory" due-to="Pendela Phanindra Rao, Gary Gregory">[javadoc] Correct hexadecimal output examples in Conversion (#1844).</action>
<action type="fix" dev="ggregory" due-to="Mohd Irshad, Gary Gregory">Fix Conversion hex method Javadoc (#1845).</action>
<action issue="LANG-1341" type="fix" due-to="rys888">Clarify when to include superclass equality in EqualsBuilder Javadoc.</action>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not edit this file.

@garydgregory
garydgregory marked this pull request as draft November 1, 2026 11:26
@garydgregory

Copy link
Copy Markdown
Member

Setting to draft. The Jira needs review to see how code and Javadoc should sync up.

Revert the manually added changes.xml entry as requested by the maintainer. Keep the Javadoc changes for Jira design review.

Assisted-By: OpenAI Codex
garydgregory added a commit that referenced this pull request Nov 1, 2026
Remove appendSuper(super.equals(obj)) from the general example.
When super.equals(obj) invokes Object.equals(Object), distinct
instances compare unequal even when all their fields are equal.

PR #1849 correctly addresses this documentation problem, but its
wording leaves inherited overrides implicit and describes the builder
as returning false without identifying isEquals().

Make the usage guidance explicit, cover inherited overrides, and
document that false makes the equality result false while true leaves
the current result unchanged. Explain when to omit identity comparison
for field-based equality and correct the super.equals(obj) examples.

References:
- https://issues.apache.org/jira/browse/LANG-1341
- #1849
@garydgregory

Copy link
Copy Markdown
Member

@rys888

Thank you for the PR and addressing LANG-1341. The PR is basically correct but the text needs to be more precise.

I addressed this in commit a0b7121. You are credited in changes.xml.

Notes:

  • “The superclass overrides” leaves inherited overrides implicit. The guidance should explicitly cover an equals(Object) implementation inherited from an ancestor.
  • “Makes this builder return false” should identify isEquals(), since appendSuper() itself returns the builder.
  • The method documentation should explain both boolean cases: false makes the equality result false, while true preserves the current result, including an existing false result.
  • The warning should explain when to omit identity comparison when implementing field-based equality.

The PR identifies the right problem, but I wanted the resulting documentation to give users explicit usage guidance and an accurate description of the method’s behavior. The commit supersedes this PR; no runtime implementation change was needed.

Closing PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants