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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
