Repository navigation
feat: report unused Java private members - #165
Conversation
|
Hello @devill , I would like to let you know that this PR is ready to review when you have the time. 😀 |
|
Thanks! I'll review this tomorrow. |
|
Sorry for the late response. This PR is a bit harder to review, because it seems to do more than one thing, and also surfaced pre existing issues. Some of the issues are pre existing anti-patterns that got repeated. I'll leave a few comments. |
There was a problem hiding this comment.
Please consider the suggestions.
|
|
||
| The unused members: | ||
|
|
||
| {% for issue in issues -%} |
There was a problem hiding this comment.
Should be using the line level issues include here. Seems like the plugins sensor isn't faithfully following the interface defined by core, making this custom template necessary. Please make sure the plugin follows the pre defined interface.
There was a problem hiding this comment.
Thanks for the guidance. In this pull request, I moved the unused-class-member guide from the TypeScript plugin to the generic plugin so that Java and TypeScript could share it. Following your feedback, the generic guide now uses includes/line_level_issues.md. I also updated both the PMD and Knip sensors to emit details.content, which allows both languages to follow the core interface. Is this the scope you intended? Or would you prefer that I restore the TypeScript guide and Knip sensor, and apply the interface change only to the Java PMD sensor?
There was a problem hiding this comment.
Yes, that is the scope I intended, thanks!
There was a problem hiding this comment.
Yes, that is the scope I intended, thanks!
Thank you too :)
e10bc4a to
ae9ad31
Compare
Thank you. I addressed the suggestions. I asked a question here to better understand one of yours suggestions: #165 (comment) Thank you for your time 😀 |
|
PR merged, thanks for the contribution! (An sorry for the slow response. I've been taking time off) |
You are welcome 😄 ! All good 👍 |
What this changes
This PR adds Java detection for unused private fields and methods. It enables PMD's
UnusedPrivateFieldandUnusedPrivateMethodrules in the bundled fallback ruleset and maps both results to the existingunused-class-membersmell.The Java integration uses the source file as the issue key. The
keyanddetails.filefields identify that file. Each issue also preserves the line where PMD found the problem, the complete diagnostic message, and the distinct sourcepmd:UnusedPrivateFieldorpmd:UnusedPrivateMethod.When a file contains several violations, they continue to share the snooze behavior for that entire file. PMD remains responsible for deciding whether a member is reachable, applying exclusions and properties, and producing custom messages. A ruleset supplied by the project still replaces the bundled fallback ruleset instead of being combined with it.
This PR also moves
unused-class-member.mdfrom the TypeScript plugin intogeneric. There is now one shared source of guidance that applies across programming languages. The guide covers fields, methods, properties, reflection, annotations and decorators, serialization, lifecycle hooks, and other uses that static analysis may not detect.The guide asks readers to investigate the member before changing it. Depending on what they find, they can connect the intended caller, extract a cohesive unit, or remove a member that is confirmed to be unused. The guide does not suggest automatic deletion.
Why the guide selects
nameormessageKnip and PMD provide different data. Knip identifies the member through
details.name. PMD provides the complete description in its JSON report, but it does not provide a separate field dedicated to the member name.Extracting a name from PMD text would make custom messages fragile. It would also couple the adapter to wording that PMD does not guarantee as part of its detector contract.
The shared template therefore uses
details.namewhen that field is defined. Otherwise, it usesdetails.message. If both fields are present,nametakes precedence. The mapper does not inspect the programming language, interpret diagnostic text, or add another display field.Moving the guide does not turn
genericinto a detector, and it does not enablegenericautomatically. Whengenericis omitted, language sensors still report the smell and the existing fallback without coaching remains in effect.Documentation and decisions
The smell vocabulary now records both Java mappings, the behavior of the bundled fallback ruleset, and the authority of a ruleset supplied by the project. It also explains that Java issues identify the source file, while TypeScript issues identify the member by name.
The decision record preserves the previous ownership by the TypeScript plugin as historical context and explains why #164 moves the single shared guide to
generic. The executable guide specifications demonstrate how the displayed value is selected, and the Java plugin specifications demonstrate the behavior of the real PMD integration.Validation
ignoredFieldNames.1070 passed, 4 skipped.138 passed.7 passed. These tests include both Java rules and confirm that only thegenericplugin packages the shared guide.uv run ruff check --config pyproject.toml src tests pluginspassed.uv run habit-hooks --allcompleted with zero findings.The enforced severity remains unchanged. Projects that use the bundled Java fallback ruleset may now see unused private members that were not reported before. Projects that supply their own PMD ruleset retain full control, and this change never modifies a reported member automatically.
Closes #164
Any feedback on the contracts, examples, or guidance is very welcome.