Skip to content

Exclude test classes from CommentDefaultAccessModifier rule#3

Merged
lithium147 merged 2 commits into
mainfrom
devin/1772670103-exclude-test-classes-comment-default-access
Mar 5, 2026
Merged

Exclude test classes from CommentDefaultAccessModifier rule#3
lithium147 merged 2 commits into
mainfrom
devin/1772670103-exclude-test-classes-comment-default-access

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Exclude test classes from CommentDefaultAccessModifier PMD rule

Summary

Adds a violationSuppressXPath property to the CommentDefaultAccessModifier rule in pmd7-rules.xml to suppress all violations inside test classes. This resolves the existing TODO comment ("ignore constructors of test classes") by excluding test classes entirely.

The XPath matches classes using the same naming pattern already defined in the ClassNamingConventions rule: Test*, IT*, *Test, *Tests, *TestCase, *IT, *ITCase.

The previous approach only ignored specific test-related annotations (e.g. @Test, @BeforeEach). This change broadens the exclusion to cover everything inside a test class — including fields, constructors, and helper methods that lack those annotations.

Review & Testing Checklist for Human

  • Verify XPath AST node names are correct for PMD 7: The expression uses ClassDeclaration and @SimpleName, which are PMD 7 AST names (PMD 6 used ClassOrInterfaceDeclaration / @Image). Confirm these are valid for PMD 7.22.0.
  • Test against an actual project: Run PMD with this ruleset against a project that has test classes with default-access members to confirm violations are actually suppressed. CI alone may not cover this.
  • Check if the broad exclusion is desired: ancestor-or-self::ClassDeclaration means nested classes inside test classes are also excluded. Verify this is the intended behavior.
  • Verify test class name pattern coverage: Confirm the regex covers all test class naming conventions used across your codebase (currently: ^(Test|IT).*$|^[A-Z][a-zA-Z0-9]*Test(s|Case)?$|^[A-Z][a-zA-Z0-9]*IT(Case)?$).

Notes

  • Could not run Maven locally to functionally verify the rule change — only validated XML syntax.
  • Devin Session
  • Requested by: @lithium147

Open with Devin

Add violationSuppressXPath property to suppress CommentDefaultAccessModifier
violations in test classes (matching pattern: Test*, IT*, *Test, *Tests,
*TestCase, *IT, *ITCase).

Co-Authored-By: lithium147@gmail.com <timlwalters@yahoo.co.uk>
@devin-ai-integration

Copy link
Copy Markdown
Contributor 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 and CI monitoring

@lithium147
lithium147 merged commit c0863e7 into main Mar 5, 2026
@lithium147
lithium147 deleted the devin/1772670103-exclude-test-classes-comment-default-access branch March 5, 2026 01:16

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

Copy link
Copy Markdown
Contributor 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 2 additional findings.

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