diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8947.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8947.json new file mode 100644 index 00000000000..c97b49feafd --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8947.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8947", + "hasTruePositives": false, + "falseNegatives": 7, + "falsePositives": 0 +} diff --git a/java-checks-test-sources/default/src/main/java/checks/JpaEntityFinalCheckJakartaSample.java b/java-checks-test-sources/default/src/main/java/checks/JpaEntityFinalCheckJakartaSample.java new file mode 100644 index 00000000000..05cbacb3b48 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/JpaEntityFinalCheckJakartaSample.java @@ -0,0 +1,99 @@ +package checks; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.MappedSuperclass; + +@Entity +final class JpaEntityFinalCheckJakartaFinalEntity { // Noncompliant {{Remove this "final" modifier from this JPA entity class.}} +//^[sc=1;ec=5] + @Id + private Long id; + + public Long getId() { + return id; + } +} + +@MappedSuperclass +final class JpaEntityFinalCheckJakartaFinalMappedSuperclass { // Noncompliant {{Remove this "final" modifier from this JPA entity class.}} +//^[sc=1;ec=5] + @Id + private Long id; +} + +@Entity +class JpaEntityFinalCheckJakartaEntityWithFinalMethod { // Compliant - class itself is not final + @Id + private Long id; + + public final Long getId() { // Noncompliant {{Remove this "final" modifier from this JPA entity method.}} + // ^^^^^ + return id; + } + + public Long getIdCompliant() { // Compliant + return id; + } + + private final Long getIdPrivate() { // Compliant - private methods cannot be overridden by proxies + return id; + } + + public static final Long getIdStatic() { // Compliant - static methods cannot be overridden by proxies + return 0L; + } +} + +@MappedSuperclass +class JpaEntityFinalCheckJakartaMappedSuperclassWithFinalMethod { // Compliant - class itself is not final + @Id + private Long id; + + public final Long getId() { // Noncompliant {{Remove this "final" modifier from this JPA entity method.}} + // ^^^^^ + return id; + } +} + +interface JpaEntityFinalCheckJakartaEntityInterface { + Long getId(); +} + +@Entity +final class JpaEntityFinalCheckJakartaFinalEntityWithInterface implements JpaEntityFinalCheckJakartaEntityInterface { // Compliant - implements interface, proxy can be interface-based + @Id + private Long id; + + @Override + public Long getId() { + return id; + } +} + +@Entity +class JpaEntityFinalCheckJakartaCompliantEntity { // Compliant + @Id + private Long id; + + public Long getId() { + return id; + } +} + +@MappedSuperclass +class JpaEntityFinalCheckJakartaCompliantMappedSuperclass { // Compliant + @Id + private Long id; +} + +class JpaEntityFinalCheckJakartaNotAnEntity { // Compliant - not a JPA entity +} + +final class JpaEntityFinalCheckJakartaFinalNotAnEntity { // Compliant - not a JPA entity +} + +class JpaEntityFinalCheckJakartaNotAnEntityWithFinalMethod { // Compliant - not a JPA entity + public final void doSomething() { // Compliant - not a JPA entity + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/JpaEntityFinalCheckJavaxSample.java b/java-checks-test-sources/default/src/main/java/checks/JpaEntityFinalCheckJavaxSample.java new file mode 100644 index 00000000000..b5c0b06f2eb --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/JpaEntityFinalCheckJavaxSample.java @@ -0,0 +1,67 @@ +package checks; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.MappedSuperclass; + +@Entity +final class JpaEntityFinalCheckJavaxFinalEntity { // Noncompliant {{Remove this "final" modifier from this JPA entity class.}} +//^[sc=1;ec=5] + @Id + private Long id; + + public Long getId() { + return id; + } +} + +@MappedSuperclass +final class JpaEntityFinalCheckJavaxFinalMappedSuperclass { // Noncompliant {{Remove this "final" modifier from this JPA entity class.}} +//^[sc=1;ec=5] + @Id + private Long id; +} + +@Entity +class JpaEntityFinalCheckJavaxEntityWithFinalMethod { // Compliant - class itself is not final + @Id + private Long id; + + public final Long getId() { // Noncompliant {{Remove this "final" modifier from this JPA entity method.}} + // ^^^^^ + return id; + } + + private final Long getIdPrivate() { // Compliant - private methods cannot be overridden by proxies + return id; + } + + public static final Long getIdStatic() { // Compliant - static methods cannot be overridden by proxies + return 0L; + } +} + +interface JpaEntityFinalCheckJavaxEntityInterface { + Long getId(); +} + +@Entity +final class JpaEntityFinalCheckJavaxFinalEntityWithInterface implements JpaEntityFinalCheckJavaxEntityInterface { // Compliant - implements interface, proxy can be interface-based + @Id + private Long id; + + @Override + public Long getId() { + return id; + } +} + +@Entity +class JpaEntityFinalCheckJavaxCompliantEntity { // Compliant + @Id + private Long id; + + public Long getId() { + return id; + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/JpaEntityFinalCheck.java b/java-checks/src/main/java/org/sonar/java/checks/JpaEntityFinalCheck.java new file mode 100644 index 00000000000..69eeba4a858 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/JpaEntityFinalCheck.java @@ -0,0 +1,88 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.java.model.ModifiersUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.SymbolMetadata; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Modifier; +import org.sonar.plugins.java.api.tree.ModifierKeywordTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8947") +public class JpaEntityFinalCheck extends IssuableSubscriptionVisitor { + + private static final List ENTITY_ANNOTATIONS = List.of( + "javax.persistence.Entity", + "jakarta.persistence.Entity", + "javax.persistence.MappedSuperclass", + "jakarta.persistence.MappedSuperclass" + ); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS, Tree.Kind.METHOD); + } + + @Override + public void visitNode(Tree tree) { + if (tree.is(Tree.Kind.CLASS)) { + visitClass((ClassTree) tree); + } else { + visitMethod((MethodTree) tree); + } + } + + private void visitClass(ClassTree classTree) { + if (!isJpaEntity(classTree.symbol().metadata())) { + return; + } + // Hibernate allows final entity classes that implement an interface declaring all attribute getters/setters. + // To avoid false positives, we skip classes that implement any interface. + if (!classTree.superInterfaces().isEmpty()) { + return; + } + ModifierKeywordTree finalModifier = ModifiersUtils.getModifier(classTree.modifiers(), Modifier.FINAL); + if (finalModifier != null) { + reportIssue(finalModifier, "Remove this \"final\" modifier from this JPA entity class."); + } + } + + 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(); + if (enclosingClass != null && isJpaEntity(enclosingClass.metadata())) { + reportIssue(finalModifier, "Remove this \"final\" modifier from this JPA entity method."); + } + } + + private static boolean isJpaEntity(SymbolMetadata metadata) { + return ENTITY_ANNOTATIONS.stream().anyMatch(metadata::isAnnotatedWith); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/JpaEntityFinalCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/JpaEntityFinalCheckTest.java new file mode 100644 index 00000000000..847f1ac0d39 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/JpaEntityFinalCheckTest.java @@ -0,0 +1,41 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class JpaEntityFinalCheckTest { + + @Test + void testWithJakarta() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/JpaEntityFinalCheckJakartaSample.java")) + .withCheck(new JpaEntityFinalCheck()) + .verifyIssues(); + } + + @Test + void testWithJavax() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/JpaEntityFinalCheckJavaxSample.java")) + .withCheck(new JpaEntityFinalCheck()) + .verifyIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html new file mode 100644 index 00000000000..caf81663c13 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.html @@ -0,0 +1,76 @@ +

This issue arises when Java classes annotated with @Entity or @MappedSuperclass are declared final, or when +methods within these classes are declared final. JPA providers require the ability to create proxy subclasses for lazy loading and other +runtime optimizations, which is prevented by the final modifier.

+

Why is this an issue?

+

JPA (Java Persistence API) providers like Hibernate rely on runtime proxy generation to implement several key features:

+ +

To create these proxies, the JPA provider needs to generate a subclass of your entity class at runtime. This subclass overrides methods to add the +lazy loading and tracking behavior.

+

When you declare a class or method as final, you prevent inheritance and method overriding. This breaks the proxy mechanism:

+ +

Without working proxies, lazy loading fails. Instead of loading data on demand, the JPA provider may fall back to eager loading, which can cause +significant performance problems. In some cases, it may even cause runtime exceptions.

+

What is the potential impact?

+

When JPA entities or their methods are marked as final, the application can experience:

+ +

How to fix it

+

Remove the final modifier from the entity class declaration or its methods. This allows the JPA provider to create proxy subclasses +for lazy loading and other optimizations. If you are using Hibernate, you can also implement an interface that declares all the attribute +getters/setters.

+

Code examples

+

Noncompliant code example

+
+@Entity
+public final class User { // Noncompliant
+    @Id
+    private Long id;
+
+    private String username;
+
+    @OneToMany(fetch = FetchType.LAZY)
+    private List<Order> orders;
+
+    // getters and setters
+}
+
+

Compliant solution

+
+@Entity
+public class User {
+    @Id
+    private Long id;
+
+    private String username;
+
+    @OneToMany(fetch = FetchType.LAZY)
+    private List<Order> orders;
+
+    // getters and setters
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.json new file mode 100644 index 00000000000..d8c1bf07621 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8947.json @@ -0,0 +1,26 @@ +{ + "title": "JPA entity classes and methods should not be final", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "jpa", + "hibernate", + "pitfall" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-8947", + "sqKey": "S8947", + "scope": "Main", + "quickfix": "unknown", + "code": { + "impacts": { + "RELIABILITY": "HIGH", + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "LOGICAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 43c82fe9248..dc43b833313 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -537,6 +537,7 @@ "S8909", "S8911", "S8924", + "S8947", "S8948" ] }