Skip to content

SONARJAVA-6539 Create rule S8947: JPA entity classes should not be final#5719

Merged
NoemieBenard merged 7 commits into
masterfrom
nb/sonarjava-6539-implement-S8947
Jul 2, 2026
Merged

SONARJAVA-6539 Create rule S8947: JPA entity classes should not be final#5719
NoemieBenard merged 7 commits into
masterfrom
nb/sonarjava-6539-implement-S8947

Conversation

@NoemieBenard

@NoemieBenard NoemieBenard commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary by Gitar

  • New Rule Implementation:
    • Added S8947 to detect and flag final JPA entity classes.
  • Test Infrastructure:
    • Added test cases in S8947Test.java to verify rule detection on compliant and non-compliant code.
    • Added sample Java files in src/test/files/checks/S8947.java to support testing logic.

This will update automatically on new commits.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6539

@NoemieBenard NoemieBenard marked this pull request as ready for review July 1, 2026 12:40
Comment thread sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html Outdated
@NoemieBenard NoemieBenard force-pushed the nb/sonarjava-6539-implement-S8947 branch from 5ee08d4 to 4268b8f Compare July 1, 2026 15:08
@sonarqube-next

sonarqube-next Bot commented Jul 2, 2026

Copy link
Copy Markdown

@NoemieBenard NoemieBenard merged commit dbdfcdd into master Jul 2, 2026
16 checks passed
@NoemieBenard NoemieBenard deleted the nb/sonarjava-6539-implement-S8947 branch July 2, 2026 13:15
@gitar-bot

gitar-bot Bot commented Jul 2, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 3 resolved / 4 findings

Implements rule S8947 to flag final JPA entity classes, resolving false positives on static and private methods. Please remove the unrelated inclusions of S8908, S8909, and S8948 from the Sonar_way profile before merging.

💡 Quality: Sonar_way profile adds rules unrelated to S8947

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json:536-541

This PR is scoped to introducing rule S8947 (JPA entities should not be final), yet the delta commit adds three additional, unrelated rules to the default Sonar_way profile: S8908, S8909, and S8948. The corresponding rule JSON/HTML files exist in the repo, so this won't break the build, but activating unrelated rules in the default profile inside an S8947 PR is almost certainly an accidental inclusion (likely a rebase/merge artifact). Activating rules by default has real impact on all users of the Sonar way profile. Recommend reverting the profile change to add only S8947, and let the owning PRs enable S8908/S8909/S8948 separately.

✅ 3 resolved
Quality: Rule doc claims final-method detection the check doesn't implement

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html:1 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html:14 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html:17 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html:22 📄 java-checks/src/main/java/org/sonar/java/checks/JpaEntityFinalCheck.java:40-54
The S8947 rule description (S8947.html) repeatedly states that the issue arises both when JPA entity classes are declared final AND when methods within these classes are declared final (e.g. "...or when methods within these classes are declared final", "A final method cannot be overridden, so the JPA provider cannot intercept calls to implement lazy loading", "When JPA entities or their methods are marked as final..."). However, JpaEntityFinalCheck only visits Tree.Kind.CLASS and only reports the class-level final modifier — it never inspects methods. This mismatch will mislead users who expect final methods on entities to be flagged, and it produces false negatives relative to the documented behavior (Hibernate does require non-final methods for proxy interception).

Fix by either (a) extending the check to also report final methods on entity classes (visit methods / iterate members and report Modifier.FINAL method modifiers), or (b) trimming the HTML so it only describes final classes, matching the implementation and the rule title "JPA entity classes should not be final".

Edge Case: Final static/private methods flagged as false positives

📄 java-checks/src/main/java/org/sonar/java/checks/JpaEntityFinalCheck.java:65-74 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html:12-17
visitMethod reports an issue on any method carrying the final modifier inside a JPA entity/mapped-superclass. However, the rule's rationale (see S8947.html lines 12-17) is that JPA providers create proxy subclasses that override methods; only overridable methods matter. static and private methods are never overridden by a proxy, so static final and private final methods are perfectly safe. Flagging them produces false positives.

Consider excluding methods that are static or private before reporting, e.g.:

private void visitMethod(MethodTree methodTree) {
  ModifierKeywordTree finalModifier = ModifiersUtils.getModifier(methodTree.modifiers(), Modifier.FINAL);
  if (finalModifier == null) {
    return;
  }
  Symbol methodSymbol = methodTree.symbol();
  if (methodSymbol.isStatic() || methodSymbol.isPrivate()) {
    return;
  }
  Symbol.TypeSymbol enclosingClass = methodSymbol.enclosingClass();
  ...
}

Adding test cases for static final and private final methods (expected Compliant) would lock in this behavior.

Quality: 'How to fix it' section omits final methods

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html:33-35
The rule now also detects final methods, and the intro/why sections were updated accordingly. However the 'How to fix it' section (S8947.html line 33-35) still only mentions removing the final modifier from the entity class declaration and does not mention methods. Update this text (and ideally add a method-based code example) so the documentation fully matches the implemented behavior.

🤖 Prompt for agents
Code Review: Implements rule S8947 to flag final JPA entity classes, resolving false positives on static and private methods. Please remove the unrelated inclusions of S8908, S8909, and S8948 from the Sonar_way profile before merging.

1. 💡 Quality: Sonar_way profile adds rules unrelated to S8947
   Files: sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json:536-541

   This PR is scoped to introducing rule S8947 (JPA entities should not be final), yet the delta commit adds three additional, unrelated rules to the default Sonar_way profile: `S8908`, `S8909`, and `S8948`. The corresponding rule JSON/HTML files exist in the repo, so this won't break the build, but activating unrelated rules in the default profile inside an S8947 PR is almost certainly an accidental inclusion (likely a rebase/merge artifact). Activating rules by default has real impact on all users of the Sonar way profile. Recommend reverting the profile change to add only `S8947`, and let the owning PRs enable `S8908`/`S8909`/`S8948` separately.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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.

2 participants