Skip to content

chore: enable PMD GuardLogStatement rule and clean up TODO comments#1284

Draft
joaodinissf wants to merge 8 commits intodsldevkit:masterfrom
joaodinissf:chore/pmd-todo-cleanup
Draft

chore: enable PMD GuardLogStatement rule and clean up TODO comments#1284
joaodinissf wants to merge 8 commits intodsldevkit:masterfrom
joaodinissf:chore/pmd-todo-cleanup

Conversation

@joaodinissf
Copy link
Copy Markdown
Collaborator

Summary

  • Replace bare TODO comments in PMD ruleset with actionable descriptions
  • Enable GuardLogStatement rule for debug and trace log levels
  • Convert string concatenation in log calls to parameterized logging ({} placeholders)
  • Use Supplier/method-refs for debug and trace log arguments where appropriate
  • Suppress false positives with // NOPMD GuardLogStatement

Test plan

  • mvn compile pmd:pmd pmd:check passes with no violations
  • CI build passes

🤖 Generated with Claude Code

joaodinissf and others added 5 commits March 7, 2026 14:35
…riptions

Replace <!--TODO--> on three excluded PMD rules with descriptive
comments explaining what needs to happen before each rule can be enabled:
- GuardLogStatement: ~35 log calls need parameterized logging
- PreserveStackTrace: catch blocks need cause-chaining audit
- UseTryWithResources: manual try/finally needs conversion audit

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Configure the rule to only flag unguarded debug/trace log calls where
arguments involve method calls (eager evaluation). Info/warn/error
levels are excluded since those are typically always enabled.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…logging

Replace `+` concatenation and MessageFormat.format/NLS.bind in log
statements with Log4j2 parameterized `{}` placeholders. This avoids
unnecessary string construction when the log level is disabled.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert debug/trace log calls to use Log4j2 Supplier overloads
(method references and lambdas) for lazy argument evaluation. Remove
superfluous isDebugEnabled/isTraceEnabled guards that are no longer
needed when all arguments are Suppliers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add NOPMD suppression for SWTBot's SLF4J logger (no Supplier support),
ITraceSet.trace() misidentified as a log call, and Log4j2 method-ref
Suppliers not recognized by PMD.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joaodinissf joaodinissf marked this pull request as draft March 7, 2026 15:16
joaodinissf and others added 3 commits March 7, 2026 17:12
Checkstyle MultipleStringLiteralsCheck flagged parameterized log
messages that appeared more than once in the same file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The log4j util.Supplier package is not available in the OSGi test
runtime. Use direct parameterized logging instead of lambdas for
info-level messages that are almost always evaluated.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add org.apache.logging.log4j.util to Import-Package in OSGi manifests
so that Supplier lambdas in log calls resolve at runtime. This reverts
the de-lambda workaround (318edc2) and fixes the root cause instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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