Skip to content

Commit 53e29e6

Browse files
SONARJAVA-5975 S6856: FP when the ModelAttribute is a class (#5515)
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent e6043db commit 53e29e6

File tree

12 files changed

+612
-248
lines changed

12 files changed

+612
-248
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S6856",
33
"hasTruePositives": false,
4-
"falseNegatives": 47,
4+
"falseNegatives": 59,
55
"falsePositives": 0
66
}

java-checks-common/src/main/java/org/sonar/java/checks/helpers/ExpressionsHelper.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,16 @@
5151

5252
public class ExpressionsHelper {
5353

54+
private static final Set<String> SIMPLE_TYPES = Set.of(
55+
"java.lang.String",
56+
"java.lang.Integer",
57+
"java.lang.Long",
58+
"java.lang.Double",
59+
"java.lang.Float",
60+
"java.lang.Boolean",
61+
"java.util.Optional"
62+
);
63+
5464
private ExpressionsHelper() {
5565
}
5666

@@ -277,4 +287,13 @@ public static List<ExpressionTree> getIdentifierAssignments(IdentifierTree ident
277287
return assignments;
278288
}
279289

290+
public static boolean isStandardDataType(Type type) {
291+
if (type.isPrimitive()) {
292+
return true;
293+
}
294+
295+
String fqn = type.fullyQualifiedName();
296+
return SIMPLE_TYPES.contains(fqn);
297+
}
298+
280299
}

java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,17 @@
2121
import org.junit.jupiter.api.Test;
2222
import org.junit.jupiter.params.ParameterizedTest;
2323
import org.junit.jupiter.params.provider.CsvSource;
24+
import org.junit.jupiter.params.provider.ValueSource;
2425
import org.sonar.plugins.java.api.semantic.Symbol;
26+
import org.sonar.plugins.java.api.semantic.Type;
2527
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
2628
import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
2729
import org.sonar.plugins.java.api.tree.ExpressionTree;
2830
import org.sonar.plugins.java.api.tree.IdentifierTree;
2931
import org.sonar.plugins.java.api.tree.ImportTree;
3032
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
3133
import org.sonar.plugins.java.api.tree.MethodTree;
34+
import org.sonar.plugins.java.api.tree.VariableTree;
3235

3336
import static org.assertj.core.api.Assertions.assertThat;
3437
import static org.mockito.Mockito.mock;
@@ -232,4 +235,35 @@ private static ExpressionTree getCallArgument(String code) {
232235
var methodInvocationTree = (MethodInvocationTree) exprStmtTree.expression();
233236
return methodInvocationTree.arguments().get(0);
234237
}
238+
239+
@ParameterizedTest
240+
@ValueSource(strings = {"byte", "short", "int", "long", "float", "double", "boolean", "char",
241+
"String", "Integer", "Long", "Double", "Float", "Boolean", "java.util.Optional<String>"})
242+
void isStandardDataType_basicTypes(String type) {
243+
String code = newCode(
244+
"void f() {",
245+
" " + type + " x;",
246+
"}"
247+
);
248+
MethodTree method = methodTree(code);
249+
VariableTree variable = (VariableTree) method.block().body().get(0);
250+
Type variableType = variable.type().symbolType();
251+
assertThat(ExpressionsHelper.isStandardDataType(variableType)).isTrue();
252+
}
253+
254+
@Test
255+
void isStandardDataType_customClass() {
256+
String code = newCode(
257+
"static class CustomClass {}",
258+
"void f() {",
259+
" CustomClass x;",
260+
"}"
261+
);
262+
// The method is at index 1 (after the CustomClass at index 0)
263+
MethodTree method = (MethodTree) classTree(code).members().get(1);
264+
VariableTree variable = (VariableTree) method.block().body().get(0);
265+
Type type = variable.type().symbolType();
266+
assertThat(ExpressionsHelper.isStandardDataType(type)).isFalse();
267+
}
268+
235269
}
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package checks.spring.s6856;
2+
3+
import lombok.Data;
4+
import lombok.Setter;
5+
6+
class ExtractSetterPropertiesTestData {
7+
8+
// Class with explicit setters
9+
static class ExplicitSetters {
10+
private String name;
11+
private int age;
12+
private boolean active;
13+
14+
public void setName(String name) {
15+
this.name = name;
16+
}
17+
18+
public void setAge(int age) {
19+
this.age = age;
20+
}
21+
22+
public void setActive(boolean active) {
23+
this.active = active;
24+
}
25+
26+
// Not a setter - has wrong return type
27+
public String setInvalid(String value) {
28+
return value;
29+
}
30+
31+
// Not a setter - has no parameters
32+
public void setEmpty() {
33+
}
34+
35+
// Not a setter - has multiple parameters
36+
public void setMultiple(String a, String b) {
37+
}
38+
39+
// Not a setter - is private
40+
private void setPrivate(String value) {
41+
}
42+
43+
// Not a setter - is static
44+
public static void setStatic(String value) {
45+
}
46+
}
47+
48+
// Class with Lombok @Data (generates setters for all non-final fields)
49+
@Data
50+
static class LombokData {
51+
private String project;
52+
private int year;
53+
private String month;
54+
private final String constant = "CONST"; // Should be excluded (final)
55+
private static String staticField; // Should be excluded (static)
56+
}
57+
58+
// Class with Lombok @Setter at class level
59+
@Setter
60+
static class LombokClassLevelSetter {
61+
private String firstName;
62+
private String lastName;
63+
private final String id = "ID"; // Should be excluded (final)
64+
private static int count; // Should be excluded (static)
65+
}
66+
67+
// Class with Lombok @Setter at field level
68+
static class LombokFieldLevelSetter {
69+
@Setter
70+
private String email;
71+
@Setter
72+
private int score;
73+
private String noSetter; // No @Setter, should be excluded
74+
private static String staticField; // Should be excluded (static)
75+
}
76+
77+
// Mixed: explicit setters + Lombok field-level @Setter
78+
static class MixedSetters {
79+
@Setter
80+
private String lombokField;
81+
private String explicitField;
82+
83+
public void setExplicitField(String value) {
84+
this.explicitField = value;
85+
}
86+
}
87+
88+
// Class with no setters
89+
static class NoSetters {
90+
private String field;
91+
92+
public String getField() {
93+
return field;
94+
}
95+
}
96+
97+
// Empty class
98+
static class EmptyClass {
99+
}
100+
101+
// Class with only getters
102+
static class OnlyGetters {
103+
private String name;
104+
private int age;
105+
106+
public String getName() {
107+
return name;
108+
}
109+
110+
public int getAge() {
111+
return age;
112+
}
113+
}
114+
}

0 commit comments

Comments
 (0)