Skip to content

Commit 8decf1a

Browse files
romainbrenguierromainbrenguier
authored
SONARJAVA-5500 Implement rule S7479 (#5230)
1 parent 358e567 commit 8decf1a

File tree

11 files changed

+290
-1
lines changed

11 files changed

+290
-1
lines changed

‎its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public void javaCheckTestSources() throws Exception {
199199
softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
200200
softly.assertThat(newTotal).isEqualTo(knownTotal);
201201
softly.assertThat(rulesCausingFPs).hasSize(9);
202-
softly.assertThat(rulesNotReporting).hasSize(16);
202+
softly.assertThat(rulesNotReporting).hasSize(17);
203203

204204
/**
205205
* 4. Check total number of differences (FPs + FNs)

‎its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2860,5 +2860,11 @@
28602860
"hasTruePositives": false,
28612861
"falseNegatives": 0,
28622862
"falsePositives": 0
2863+
},
2864+
{
2865+
"ruleKey": "7479",
2866+
"hasTruePositives": false,
2867+
"falseNegatives": 0,
2868+
"falsePositives": 0
28632869
}
28642870
]

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S7479.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S7479",
3+
"hasTruePositives": false,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}

‎java-checks-test-sources/default/src/main/java/checks/ClassBuilderWithMethodCheckSample.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package checks;
2+
3+
import java.lang.classfile.ClassBuilder;
4+
import java.lang.constant.ClassDesc;
5+
6+
import static java.lang.classfile.ClassFile.ACC_PUBLIC;
7+
import static java.lang.classfile.ClassFile.ACC_STATIC;
8+
import static java.lang.constant.ConstantDescs.MTD_void;
9+
10+
public class ClassBuilderWithMethodCheckSample {
11+
12+
private static final String CLASSNAME = "java.io.PrintStream";
13+
14+
ClassBuilder addMethod(ClassBuilder builder) {
15+
return builder
16+
.withMethod("foo", MTD_void, ACC_PUBLIC | ACC_STATIC, methodBuilder -> { // Noncompliant {{Replace call with `ClassBuilder.withMethodBody`.}}
17+
// ^^^^^^^^^^
18+
methodBuilder.withCode(codeBuilder -> codeBuilder.getstatic(ClassDesc.of("java.lang.System"), "out", ClassDesc.of(CLASSNAME))
19+
.ldc("Hello World")
20+
.invokevirtual(ClassDesc.of(CLASSNAME), "println", MTD_void)
21+
.return_());
22+
});
23+
}
24+
25+
ClassBuilder addMethodWithoutCurlyBraces(ClassBuilder builder) {
26+
return builder
27+
.withMethod("foo", MTD_void, ACC_PUBLIC | ACC_STATIC, methodBuilder -> // Noncompliant {{Replace call with `ClassBuilder.withMethodBody`.}}
28+
// ^^^^^^^^^^
29+
methodBuilder.withCode(codeBuilder -> codeBuilder.getstatic(ClassDesc.of("java.lang.System"), "out", ClassDesc.of("java.io.PrintStream"))
30+
.ldc("Hello World")
31+
.invokevirtual(ClassDesc.of("java.io.PrintStream"), "println", MTD_void)
32+
.return_())
33+
);
34+
}
35+
36+
ClassBuilder addMethodCompliant(ClassBuilder builder) {
37+
return builder
38+
.withMethod("foo", MTD_void, ACC_PUBLIC | ACC_STATIC, methodBuilder ->
39+
{
40+
// Compliant because `withCode` is not the only statement in the lambda
41+
methodBuilder.constantPool().intEntry(121);
42+
methodBuilder.withCode(codeBuilder -> codeBuilder.getstatic(ClassDesc.of("java.lang.System"), "out", ClassDesc.of("java.io.PrintStream"))
43+
.ldc("Hello World")
44+
.invokevirtual(ClassDesc.of("java.io.PrintStream"), "println", MTD_void)
45+
.return_());
46+
});
47+
}
48+
49+
ClassBuilder addMethodCompliantWithMethodBody(ClassBuilder builder) {
50+
return builder
51+
.withMethodBody("foo", MTD_void, ACC_PUBLIC | ACC_STATIC, codeBuilder -> codeBuilder.getstatic(ClassDesc.of("java.lang.System"), "out", ClassDesc.of("java.io.PrintStream"))
52+
.ldc("Hello World")
53+
.invokevirtual(ClassDesc.of("java.io.PrintStream"), "println", MTD_void)
54+
.return_());
55+
}
56+
}

‎java-checks/src/main/java/org/sonar/java/checks/ClassBuilderWithMethodCheck.java‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import java.util.List;
20+
import org.sonar.check.Rule;
21+
import org.sonar.java.matcher.TreeMatcher;
22+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
23+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
24+
import org.sonar.plugins.java.api.tree.ExpressionTree;
25+
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
26+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
27+
import org.sonar.plugins.java.api.tree.Tree;
28+
29+
import static org.sonar.java.matcher.TreeMatcher.any;
30+
import static org.sonar.java.matcher.TreeMatcher.calls;
31+
import static org.sonar.java.matcher.TreeMatcher.hasSize;
32+
import static org.sonar.java.matcher.TreeMatcher.isExpression;
33+
import static org.sonar.java.matcher.TreeMatcher.isInvocationOf;
34+
import static org.sonar.java.matcher.TreeMatcher.statementAt;
35+
import static org.sonar.java.matcher.TreeMatcher.withBody;
36+
37+
@Rule(key = "S7479")
38+
public class ClassBuilderWithMethodCheck extends IssuableSubscriptionVisitor {
39+
40+
private final MethodMatchers withMethod = MethodMatchers.create()
41+
.ofTypes("java.lang.classfile.ClassBuilder")
42+
.names("withMethod")
43+
.withAnyParameters()
44+
.build();
45+
46+
private final MethodMatchers withCode = MethodMatchers.create()
47+
.ofTypes("java.lang.classfile.MethodBuilder")
48+
.names("withCode")
49+
.withAnyParameters()
50+
.build();
51+
52+
/** Matches lambda expression composed of just a call to `MethodBuilder.withBody`. */
53+
private final TreeMatcher<ExpressionTree> matcher = TreeMatcher
54+
.isLambdaExpression(
55+
withBody(
56+
// Case with curly braces
57+
hasSize(1)
58+
.and(
59+
statementAt(0, isInvocationOf(withCode, any())))
60+
// Case with no curly braces
61+
.or(isExpression(calls(withCode, any())))));
62+
63+
@Override
64+
public List<Tree.Kind> nodesToVisit() {
65+
return List.of(Tree.Kind.METHOD_INVOCATION);
66+
}
67+
68+
/** Raise if last argument of withMethod is a lambda which only calls `MethodBuilder.withCode` */
69+
@Override
70+
public void visitNode(Tree tree) {
71+
MethodInvocationTree invocation = (MethodInvocationTree) tree;
72+
if (withMethod.matches(invocation)) {
73+
ExpressionTree lastArgument = invocation.arguments().get(invocation.arguments().size() - 1);
74+
if (matcher.check(lastArgument)) {
75+
reportIssue(findLocation(invocation), "Replace call with `ClassBuilder.withMethodBody`.");
76+
}
77+
}
78+
}
79+
80+
private static Tree findLocation(MethodInvocationTree invocation) {
81+
ExpressionTree methodSelect = invocation.methodSelect();
82+
return ((MemberSelectExpressionTree) methodSelect).identifier();
83+
}
84+
85+
}

‎java-checks/src/test/java/org/sonar/java/checks/ClassBuilderWithMethodCheckTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
import org.sonar.java.checks.verifier.TestUtils;
22+
23+
class ClassBuilderWithMethodCheckTest {
24+
25+
@Test
26+
void test() {
27+
CheckVerifier.newVerifier()
28+
.onFile(TestUtils.mainCodeSourcesPath("checks/ClassBuilderWithMethodCheckSample.java"))
29+
.withCheck(new ClassBuilderWithMethodCheck())
30+
.verifyIssues();
31+
}
32+
33+
@Test
34+
void testWithoutSemantic() {
35+
CheckVerifier.newVerifier()
36+
.onFile(TestUtils.mainCodeSourcesPath("checks/ClassBuilderWithMethodCheckSample.java"))
37+
.withCheck(new ClassBuilderWithMethodCheck())
38+
.withoutSemantic()
39+
.verifyIssues();
40+
}
41+
42+
}

‎java-frontend/src/main/java/org/sonar/java/matcher/TreeMatcher.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ public static <U extends Tree> TreeMatcher<U> matching(Predicate<U> predicate) {
6565
return new TreeMatcher<>(predicate);
6666
}
6767

68+
public static <U extends Tree> TreeMatcher<U> any() {
69+
return new TreeMatcher<>(x -> true);
70+
}
71+
6872
public Predicate<T> asPredicate() {
6973
return predicate;
7074
}

‎java-frontend/src/test/java/org/sonar/java/matcher/TreeMatcherTest.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import static org.junit.jupiter.api.Assertions.assertTrue;
4040
import static org.mockito.Mockito.mock;
4141
import static org.mockito.Mockito.when;
42+
import static org.sonar.java.matcher.TreeMatcher.any;
4243
import static org.sonar.java.matcher.TreeMatcher.forEachStatement;
4344
import static org.sonar.java.matcher.TreeMatcher.hasSize;
4445
import static org.sonar.java.matcher.TreeMatcher.invokedOn;
@@ -107,6 +108,23 @@ void foo(String s) {}
107108
assertFalse(forEachInLambdaMatcher.check(lambda2));
108109
}
109110

111+
@Test
112+
void testAny() {
113+
CompilationUnitTree t = JParserTestUtils.parse("""
114+
class C {
115+
int i = foo(x);
116+
}
117+
""");
118+
119+
ClassTree c = (ClassTree) t.types().get(0);
120+
VariableTree variableTree = (VariableTree) c.members().get(0);
121+
ExpressionTree expression = variableTree.initializer();
122+
123+
assertTrue(any().check(c));
124+
assertTrue(any().check(variableTree));
125+
assertTrue(any().check(expression));
126+
}
127+
110128
@Test
111129
void testAsPredicate() {
112130
CompilationUnitTree t = JParserTestUtils.parse("""

‎sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7479.html‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>The <code>ClassBuilder</code> API provides multiple ways to declare a method and its body, including <code>withMethod</code> and
3+
<code>withMethodBody</code>. While they can be used in very similar ways, <code>withMethodBody</code> reduces boilerplate code, lowers cognitive
4+
complexity and improves maintainability.</p>
5+
<h3>Exceptions</h3>
6+
<p>The rule will not raise on calls where the method under construction is abstract (eg: using the flag <code>ClassFile.ACC_ABSTRACT</code>).</p>
7+
<h2>How to fix it</h2>
8+
<p>Replace the invocation of <code>withMethod</code> with <code>withMethodBody</code>.</p>
9+
<h3>Code examples</h3>
10+
<h4>Noncompliant code example</h4>
11+
<pre data-diff-id="1" data-diff-type="noncompliant">
12+
ClassBuilder addMethod(ClassBuilder builder) {
13+
return builder
14+
.withMethod("foo", MTD_void, ACC_PUBLIC | ACC_STATIC, methodBuilder -&gt; { // Noncompliant
15+
methodBuilder.withCode(codeBuilder -&gt;
16+
codeBuilder.getstatic(ClassDesc.of("java.lang.System"), "out", ClassDesc.of("java.io.PrintStream"))
17+
.ldc("Hello World")
18+
.invokevirtual(ClassDesc.of("java.io.PrintStream"), "println", MTD_void)
19+
.return_()
20+
);
21+
});
22+
}
23+
</pre>
24+
<h4>Compliant solution</h4>
25+
<pre data-diff-id="1" data-diff-type="compliant">
26+
ClassBuilder addMethod(ClassBuilder builder) {
27+
return builder
28+
.withMethodBody("foo", MTD_void, ACC_PUBLIC | ACC_STATIC, codeBuilder -&gt;
29+
codeBuilder.getstatic(ClassDesc.of("java.lang.System"), "out", ClassDesc.of("java.io.PrintStream"))
30+
.ldc("Hello World")
31+
.invokevirtual(ClassDesc.of("java.io.PrintStream"), "println", MTD_void)
32+
.return_()
33+
);
34+
}
35+
</pre>
36+
<h2>Resources</h2>
37+
<h3>Documentation</h3>
38+
<ul>
39+
<li> <a
40+
href="https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/classfile/package-summary.html#writing-classfiles-heading">Writing
41+
classfiles - classfile Javadoc</a> </li>
42+
<li> <a
43+
href="https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/classfile/ClassBuilder.html#withMethod(java.lang.classfile.constantpool.Utf8Entry,java.lang.classfile.constantpool.Utf8Entry,int,java.util.function.Consumer)">withMethod - ClassBuilder Javadoc</a> </li>
44+
<li> <a
45+
href="https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/classfile/ClassBuilder.html#withMethodBody(java.lang.classfile.constantpool.Utf8Entry,java.lang.classfile.constantpool.Utf8Entry,int,java.util.function.Consumer)">withMethodBody - ClassBuilder Javadoc</a> </li>
46+
<li> <a href="https://openjdk.org/jeps/484">JEP 484: Class-File API</a> </li>
47+
</ul>
48+

‎sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7479.json‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"title": "\"ClassBuilder.withMethodBody\" should be preferred to \"ClassBuilder.withMethod\"",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"java24"
11+
],
12+
"defaultSeverity": "Minor",
13+
"ruleSpecification": "RSPEC-7479",
14+
"sqKey": "S7479",
15+
"scope": "All",
16+
"quickfix": "targeted",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "LOW"
20+
},
21+
"attribute": "CLEAR"
22+
}
23+
}

0 commit comments

Comments
 (0)