Skip to content

Re-enable checkstyle enforcement - #3

Open
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1786550501-checkstyle-javadoc
Open

devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1786550501-checkstyle-javadoc

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

checkstyle.fail.on.violation has been false since the JDK 21 refresh (walmartlabs#159), which reported ~157 (currently 316) javadoc violations as "pre-existing debt to fix separately". Almost none of it is real debt: the Checkstyle 10 migration dropped properties from the JavadocMethod module without replacing them, silently turning on @param/@return requirements the project never had. Restoring the original intent leaves 22 genuine violations, which are fixed here, so enforcement goes back on.

 <module name="JavadocMethod">
-    <property name="validateThrows" value="false"/>
+    <property name="accessModifiers" value="public"/>     <!-- was scope=public -->
+    <property name="allowMissingParamTags" value="true"/>
+    <property name="allowMissingReturnTag" value="true"/>
+    <property name="validateThrows" value="false"/>
+    <property name="allowedAnnotations" value="Override, Test, BeforeEach, ..."/>
 </module>

Compare against the rules as originally committed in c6c6bb4 — scope=public, allowMissingParamTags, allowMissingThrowsTags, allowMissingReturnTag, allowedAnnotations, etc. scope was renamed to accessModifiers; minLineCount, allowThrowsTagsForSubclasses and allowMissingPropertyJavadoc no longer exist and have no equivalent, but they only ever loosened the check further. allowedAnnotations is also updated for the JUnit 5 lifecycle annotation names.

That drops 316 → 22. The remainder are real and fixed individually:

  • Stale @param names that no longer match the signature — @param transactionParsers on registerTransactionSetParser(TransactionSetParser txParser), likewise in AbstractTransactionSetParserChainable, and @param errorDetail → errorDetails in AbstractX12TransactionSetWithLoop.
  • Two malformed @throws tags that made Checkstyle's javadoc parser bail on the whole comment (surfacing as confusing AtclauseOrder / JavadocTagContinuationIndentation parse errors): @throws @{link PatternSyntaxException} (a typo'd {@link) and @throws {@link X12ParserException}. @throws wants an exception name, not a link.
  • One static-import ordering nit in SegmentIteratorTest.

The three RightCurly violations were all } while (...) closings, which cannot satisfy option=alone, so LITERAL_DO is dropped from that module's token list rather than reformatting valid do-while loops.

No behavior change — only javadoc text, one import reorder, and build config. Verified: mvn clean test → You have 0 Checkstyle violations. and 412 tests pass. I also confirmed the gate actually bites by temporarily adding a second top-level class, which fails the build with OneTopLevelClass.

Link to Devin session: https://app.devin.ai/sessions/5280d76c516144ad88d3910c89b9ab3b
Requested by: @AllenGithu8

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@AllenGithu8 AllenGithu8 self-assigned this Aug 12, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

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.

1 participant