Allow Javadoc tag description to begin with an acronym#471
Conversation
…cutive uppercase letters (e.g., "JNI", "HTML", "URL") instead of reporting a badCase violation. Single capitalized words are still rejected. Closes spring-iogh-468 Signed-off-by: Venkata Naga Sai Srikanth Gollapudi <42247688+GollapudiSrikanth@users.noreply.github.com>
| } | ||
|
|
||
| } | ||
| return upperCount >= 2; |
There was a problem hiding this comment.
This will allow a description that starts with something like STRaNGE which is probably more than we need to allow. For now at least, we should only relax things to accept an upper-case first letter if all subsequent letters until the first space are also upper-case.
| private boolean startsWithUppercase(String description) { | ||
| return description.length() > 0 && Character.isUpperCase(description.charAt(0)); | ||
| if(description.length() > 0 || Character.isUpperCase(description.charAt(0))) { | ||
| return false; |
There was a problem hiding this comment.
The logic's now wrong as a description that starts with an upper-case character will return false. This is evident if you run the module's tests – something that you should do before submitting a PR – as SpringChecksTests now has two failures.
It also prevents startWithAcronym from being reached whenever the description starts with an upper-case character.
There was a problem hiding this comment.
Thanks for pointing this out — you're right. I ran the tests on my local main branch but missed running them against my PR branch after the changes.
| if(description.length() > 0 || Character.isUpperCase(description.charAt(0))) { | ||
| return false; | ||
| } | ||
| return !startWithAcronym(description); |
There was a problem hiding this comment.
I don't think this check belongs in a method named startsWithUppercase. I would either:
- create a new method named something like
hasCorrectCasethat checks that the description either starts with an acronym or does not start with an upper-case character - update the code that currently calls
startsWithUppercaseto call bothstartsWithAcronymandstartsWithUppercase
There was a problem hiding this comment.
Thanks for the detailed feedback — this was really helpful.
I’ve updated the implementation to address all the points:
Refined the acronym check to ensure that only fully upper-case words (e.g. URL, HTML, JNI) are allowed, avoiding cases like STRaNGE
Fixed the logic so that acronym detection is evaluated before the uppercase check, ensuring valid cases are not incorrectly rejected and existing tests pass
Refactored the logic to improve clarity by separating concerns
I’ve also run the full test suite on this branch and confirmed everything is passing
Please let me know if you'd prefer a different structure or further refinements.
Closes spring-iogh-468 Signed-off-by: Venkata Naga Sai Srikanth Gollapudi <42247688+GollapudiSrikanth@users.noreply.github.com>
2 participants
This Change updates the case check to allow descriptions that start with 2 or more consecutive uppercase letters(acronym), while still rejecting single capitalized words like "The" or "Url"
Examples Allowed: "HTML","URL","JNI"
Closes #468