SONARJAVA-6430 Centralize class-like tree kinds - #5869
SONARJAVA-6430 Centralize class-like tree kinds#5869aurelien-coet-sonarsource wants to merge 3 commits into
Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| public List<Tree.Kind> nodesToVisit() { | ||
| return Arrays.asList(Tree.Kind.INTERFACE, Tree.Kind.CLASS, Tree.Kind.RECORD); | ||
| return Tree.CLASS_KINDS; | ||
| } |
There was a problem hiding this comment.
⚠️ Bug: RedundantNullability rule scope expands to enums/annotations
Previously nodesToVisit() returned only {INTERFACE, CLASS, RECORD} and checkMembers only recursed into {CLASS, INTERFACE, RECORD} members. This commit switches nodesToVisit() to Tree.CLASS_KINDS (adding ENUM, ANNOTATION_TYPE, IMPLICIT_CLASS) and line 81 manually adds ENUM/ANNOTATION_TYPE to the inner-class member check. This is not a behavior-preserving refactor: S6665 will now analyze top-level and nested enums and annotation types for redundant nullability annotations, which can raise new (possibly false-positive) issues on user code. Confirm this expansion is intended and covered by tests; if the rule should keep its original scope, restrict nodesToVisit() to the original kinds rather than CLASS_KINDS.
Was this helpful? React with 👍 / 👎
| public Set<Tree.Kind> excludedNodes() { | ||
| return SetUtils.immutableSetOf(Tree.Kind.METHOD, Tree.Kind.CLASS, Tree.Kind.ENUM, Tree.Kind.INTERFACE, Tree.Kind.NEW_CLASS); | ||
| Set<Tree.Kind> excluded = new HashSet<>(Tree.CLASS_KINDS); | ||
| excluded.add(Tree.Kind.METHOD); | ||
| excluded.add(Tree.Kind.NEW_CLASS); | ||
| return excluded; | ||
| } |
There was a problem hiding this comment.
💡 Quality: HiddenFieldCheck excludedNodes() now also excludes RECORD/ANN/IMPLICIT
VariableList.excludedNodes() previously stopped recursion at {METHOD, CLASS, ENUM, INTERFACE, NEW_CLASS}; it now uses Tree.CLASS_KINDS plus METHOD/NEW_CLASS, additionally excluding RECORD, ANNOTATION_TYPE and IMPLICIT_CLASS. This changes which variables inside static blocks/method bodies are collected as excluded when a local record (or annotation type) is present, altering hidden-field detection in that edge case. Verify this matches the intended behavior or that it is exercised by tests.
Was this helpful? React with 👍 / 👎
| public List<Tree.Kind> nodesToVisit() { | ||
| return List.of(Tree.Kind.COMPILATION_UNIT, Tree.Kind.CLASS, Tree.Kind.ENUM, Tree.Kind.INTERFACE, Tree.Kind.RECORD, Tree.Kind.METHOD_INVOCATION); | ||
| return ListUtils.concat(Tree.CLASS_KINDS, List.of(Tree.Kind.COMPILATION_UNIT, Tree.Kind.METHOD_INVOCATION)); |
There was a problem hiding this comment.
💡 Quality: MockitoStaticImportCheck switch can drift from CLASS_KINDS
nodesToVisit() now derives from Tree.CLASS_KINDS, but the visitNode()/leaveNode() switch statements still hand-list the class-like kinds (CLASS, ENUM, INTERFACE, RECORD, ANNOTATION_TYPE, IMPLICIT_CLASS). If Tree.CLASS_KINDS gains a new kind later, this check would subscribe to it but silently ignore it in the switch (falling into the default branch), leaving the classMethodsStack unbalanced. Consider centralizing the handling to avoid divergence from CLASS_KINDS.
Was this helpful? React with 👍 / 👎
CI failed: Test failures in SanityTest caused by a NullPointerException in RightCurlyBraceStartLineCheck when visiting tokens affected by the centralized class-like tree kinds refactoring.Overview1 test failure pattern found across 1 job analysis, directly caused by changes in tree kind representation introduced in the PR. FailuresSanityTest Failures in RightCurlyBraceStartLineCheck (confidence: high)
Summary
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|
1 participant
Summary by Gitar
Tree.CLASS_KINDSlist containing all class-like node kinds inTree.javaTreeTest.javato verifyCLASS_KINDSmatches all kinds backed byClassTreeThis will update automatically on new commits.