You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "hxperl (via GitHub)" <gi...@apache.org> on 2026/09/15 01:09:31 UTC
[PR] Make the @param names in BaseNCodec match the parameters they document [commons-codec]
hxperl opened a new pull request, #444:
URL: https://github.com/apache/commons-codec/pull/444
Three `@param` tags in `BaseNCodec` name a parameter their method does not have. javadoc does not render a `@param` it cannot bind, so instead of documenting the argument, each of these caused that argument's documentation to be **dropped** from the generated docs. Nothing fails loudly — the method just renders with an undocumented parameter.
Reproduced on `master` at `f49b96388c2d`, before changing anything, with nothing but the JDK:
```
$ cd src/main/java
$ javadoc -Xdoclint:reference,syntax -private -quiet -d /tmp/jd \
org/apache/commons/codec/binary/BaseNCodec.java
org/apache/commons/codec/binary/BaseNCodec.java:483: error: @param name not found
* @param bigInt {@code BigInteger} to be converted.
org/apache/commons/codec/binary/BaseNCodec.java:693: error: @param name not found
* @param offset initial offset of the subarray.
org/apache/commons/codec/binary/BaseNCodec.java:774: error: @param name not found
* @param offset initial offset of the subarray.
3 errors
```
After the change the same command reports nothing.
Note these are doclint **errors**, not warnings, so they would break a build that enables `-Xdoclint:reference`.
### The three, and why they are two different fixes
**`toUnsignedBytes` (line 483)** — the signature is `toUnsignedBytes(final BigInteger value)`, the tag said `bigInt`. The tag is simply wrong, so it becomes `@param value`. No ambiguity here.
**`decode` and `encode` (lines 693 and 774)** — these two are the judgement call, because here it is the *parameter name* that looks like the mistake rather than the tag:
```java
* @param array A byte array containing Base-N character data.
* @param offset initial offset of the subarray.
* @param length length of the subarray.
abstract void decode(byte[] array, int i, int length, Context context);
```
Four separate things in the codebase already call this argument `offset`:
1. the tag itself,
2. its description, "initial offset of the subarray",
3. the internal caller — `encode(byte[] array, int offset, int length)` passes a local literally named `offset` into `encode(array, offset, length, context)`,
4. the `Base16` and `Base58` overrides, which declare `final int offset`.
`i` also makes an incoherent pair with its neighbour `length`, where `offset`/`length` is the conventional one. So I renamed the parameter to `offset` in both abstract declarations rather than rewriting the tags to `@param i`.
**The alternative was to leave the signatures alone and change the tags to `@param i`.** That is a strictly comment-only diff, which is the more conservative choice, but it would publish "i — initial offset of the subarray", making the docs accurate and worse at the same time. I went the other way; if you would rather this PR touch only comments, say so and I will flip it — it is a one-line change either way.
The rename is behaviour-free: these are abstract, package-private methods, parameter names are not part of the binary signature, and overrides are free to name them whatever they already do (`inPos`, `offset`, ...). `japicmp:cmp` in the default goal agrees. The other four overrides (`Base32`, `Base45`, `Base64`, `Base16`'s `encode`) are untouched and still compile, since an override never has to match the name.
### Deliberately not included
`BaseNCodec` also has 26 `-Xdoclint:missing` warnings, including `no @param for context` on these same two methods. Those are a separate, pre-existing, file-wide gap rather than a mis-bound tag, so I left every one of them alone instead of widening the diff. Happy to file that separately if it is wanted.
### Checklist
- [X] Read the [contribution guidelines](CONTRIBUTING.md) for this project.
- [X] Read the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) if you use Artificial Intelligence (AI).
- [X] **I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?**
Disclosing per the box above and per CONTRIBUTING. The tool was **Claude Opus 5, via Claude Code (Anthropic)**, and it was used for all of it: running the `javadoc -Xdoclint` scan that found the three sites, reading the overrides and callers to decide `offset` over `i`, running the build, and drafting this description. The commit carries a `Generated-by:` token as the guidance recommends.
On the conditions in that guidance: the contributed change is three identifier renames — `bigInt` to `value`, and `i` to `offset` twice — each dictated by a name already present in this file. It is not copyrightable subject matter, and no third-party material is incorporated, so conditions 2.1 and 2.2 are both met rather than relying on the unsettled one alone. I have reviewed every line and take responsibility for it.
- [X] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself.
- [ ] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
No test added, and I do not think one is possible here: the change is a comment and two parameter names, with no runtime behaviour for a test to pin. The check that would fail without it is the `javadoc -Xdoclint` run above.
- [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [X] Each commit in the pull request should have a meaningful subject line and body.
No JIRA ticket, per CONTRIBUTING: "For changes of a trivial nature to comments and documentation, it is not always necessary to create a new ticket in JIRA." Glad to open one if you would rather have it in the changelog.
### What I verified, and where it stops
macOS arm64, Temurin/Homebrew OpenJDK 17.0.16, Maven 3.9.9, against `master` at `f49b96388c2d`. The full default goal — `clean verify apache-rat:check japicmp:cmp pmd:check checkstyle:check spotbugs:check javadoc:javadoc`:
```
[INFO] --- surefire:3.6.0:test (default-test) @ commons-codec ---
[INFO] Tests run: 19103, Failures: 0, Errors: 0, Skipped: 21
[INFO] --- japicmp:0.26.2:cmp (default-cli) @ commons-codec ---
[INFO] --- pmd:3.28.0:check (default-cli) @ commons-codec ---
[INFO] --- checkstyle:3.6.0:check (default-cli) @ commons-codec ---
[INFO] --- spotbugs:4.10.4.0:check (default-cli) @ commons-codec ---
[INFO] --- javadoc:3.12.0:javadoc (default-cli) @ commons-codec ---
[INFO] BUILD SUCCESS
[INFO] Total time: 01:39 min
```
The build emits 11 `[WARNING]` lines; I checked each and they are all pre-existing or environmental (PMD ruleset deprecations, skipped tests, and a `jrt-fs.jar` auxClasspath note from SpotBugs on this JDK). None involve `BaseNCodec` or this diff.
That is one JDK on one platform. CI will need to confirm the rest of the matrix, though for a change of this shape I would be surprised by a difference.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01GuW8Eie2iuc8mqnqZaCLWR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make the @param names in BaseNCodec match the parameters they document [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #444:
URL: https://github.com/apache/commons-codec/pull/444
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make the @param names in BaseNCodec match the parameters they document [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #444:
URL: https://github.com/apache/commons-codec/pull/444#issuecomment-5742548512
Thank you @hxperl , merged 🚀
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org