Skip to content

Commit b7553c5

Browse files
romainbrenguierromainbrenguier
authored
SONARJAVA-5493 Implement check for uses of ClassFile.build instead of transformClass (#5222)
1 parent 6430576 commit b7553c5

File tree

12 files changed

+744
-20
lines changed

12 files changed

+744
-20
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(15);
202+
softly.assertThat(rulesNotReporting).hasSize(16);
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
@@ -2854,5 +2854,11 @@
28542854
"hasTruePositives": false,
28552855
"falseNegatives": 0,
28562856
"falsePositives": 0
2857+
},
2858+
{
2859+
"ruleKey": "7478",
2860+
"hasTruePositives": false,
2861+
"falseNegatives": 0,
2862+
"falsePositives": 0
28572863
}
28582864
]

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

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

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package checks;
2+
3+
import java.io.IOException;
4+
import java.lang.classfile.ClassElement;
5+
import java.lang.classfile.ClassFile;
6+
import java.lang.classfile.ClassModel;
7+
import java.lang.classfile.MethodModel;
8+
import java.lang.classfile.constantpool.ConstantPool;
9+
import java.lang.classfile.constantpool.ConstantPoolBuilder;
10+
import java.nio.file.Files;
11+
import java.nio.file.Path;
12+
13+
public class UseTransformClassInsteadOfBuildCheckSample {
14+
public static void transformClassFile(Path path) throws IOException {
15+
ClassFile classFile = ClassFile.of();
16+
ClassModel classModel = classFile.parse(path);
17+
byte[] newBytes = classFile.build( // Noncompliant {{Replace this 'build()' call with 'transformClass()'.}}
18+
// ^^^^^^^^^^^^^^^
19+
classModel.thisClass().asSymbol(), classBuilder -> {
20+
for (ClassElement classElement : classModel) {
21+
if (!(classElement instanceof MethodModel methodModel &&
22+
methodModel.methodName().stringValue().startsWith("debug"))) {
23+
classBuilder.with(classElement);
24+
25+
}
26+
}
27+
});
28+
Files.write(path, newBytes);
29+
}
30+
31+
public static void transformClassFileWithConstantPool(Path path) throws IOException {
32+
ClassFile classFile = ClassFile.of();
33+
ClassModel classModel = classFile.parse(path);
34+
ConstantPoolBuilder constantPoolBuilder = ConstantPoolBuilder.of(classModel);
35+
byte[] newBytes = classFile.build( // Noncompliant
36+
// ^^^^^^^^^^^^^^^
37+
classModel.thisClass(),
38+
constantPoolBuilder,
39+
classBuilder -> {
40+
for (ClassElement classElement : classModel) {
41+
if (!(classElement instanceof MethodModel methodModel &&
42+
methodModel.methodName().stringValue().startsWith("debug"))) {
43+
classBuilder.with(classElement);
44+
45+
}
46+
}
47+
});
48+
Files.write(path, newBytes);
49+
}
50+
51+
public static void transformClassFileCompliant(Path path) throws IOException {
52+
ClassFile classFile = ClassFile.of();
53+
ClassModel classModel = classFile.parse(path);
54+
byte[] newBytes = classFile.transformClass(
55+
classModel, (classBuilder, classElement) -> {
56+
if (!(classElement instanceof MethodModel methodModel &&
57+
methodModel.methodName().stringValue().startsWith("debug"))) {
58+
classBuilder.with(classElement);
59+
}
60+
});
61+
Files.write(path, newBytes);
62+
}
63+
64+
public static void transformClassFileFalseNegative(Path path) throws IOException {
65+
var classFile = ClassFile.of();
66+
var classModel = classFile.parse(path);
67+
byte[] newBytes = classFile.build(
68+
// False negative. We don't detect that we could use `transformClass` instead of
69+
// `build` because the lambda contains several statements.
70+
classModel.thisClass().asSymbol(), classBuilder -> {
71+
var methodName = "debug";
72+
for (ClassElement classElement : classModel) {
73+
if (!(classElement instanceof MethodModel methodModel &&
74+
methodModel.methodName().stringValue().startsWith(methodName))) {
75+
classBuilder.with(classElement);
76+
}
77+
}
78+
});
79+
Files.write(path, newBytes);
80+
}
81+
}

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

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.function.Predicate;
2121

2222
import org.sonar.check.Rule;
23+
import org.sonar.java.matcher.TreeMatcher;
2324
import org.sonar.java.checks.helpers.QuickFixHelper;
2425
import org.sonar.java.reporting.JavaQuickFix;
2526
import org.sonar.java.reporting.JavaTextEdit;
@@ -28,12 +29,16 @@
2829
import org.sonar.plugins.java.api.tree.Arguments;
2930
import org.sonar.plugins.java.api.tree.ExpressionTree;
3031
import org.sonar.plugins.java.api.tree.IdentifierTree;
31-
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
3232
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
3333
import org.sonar.plugins.java.api.tree.Tree;
3434

3535
import javax.annotation.Nullable;
3636

37+
import static org.sonar.java.matcher.TreeMatcher.calls;
38+
import static org.sonar.java.matcher.TreeMatcher.invokedOn;
39+
import static org.sonar.java.matcher.TreeMatcher.isIdentifier;
40+
import static org.sonar.java.matcher.TreeMatcher.withArgument;
41+
3742
@Rule(key = "S7477")
3843
public class ClassNameInClassTransformCheck extends IssuableSubscriptionVisitor {
3944
private static final String CLASS_ENTRY_CLASSNAME = "java.lang.classfile.constantpool.ClassEntry";
@@ -139,13 +144,6 @@ static class CallMatcher {
139144
this.methodInvocationTree = methodInvocationTree;
140145
}
141146

142-
ExpressionMatcher onExpression() {
143-
if (methodInvocationTree != null && methodInvocationTree.methodSelect() instanceof MemberSelectExpressionTree mset) {
144-
return new ExpressionMatcher(mset.expression());
145-
}
146-
return new ExpressionMatcher(null);
147-
}
148-
149147
ExpressionMatcher withArgument(int argumentIndex) {
150148
if (methodInvocationTree != null && methodInvocationTree.arguments().size() >= argumentIndex) {
151149
return new ExpressionMatcher(methodInvocationTree.arguments().get(argumentIndex));
@@ -170,7 +168,7 @@ public void visitNode(Tree tree) {
170168
ExpressionTree classModel = mit.arguments().get(0);
171169
ExpressionTree classDesc = mit.arguments().get(1);
172170
if (classModel instanceof IdentifierTree classModelId &&
173-
(isThisClassOf(classModelId).test(classDesc) || isDescriptorOf(classModelId).test(classDesc))) {
171+
(isThisClassOf(classModelId).or(isDescriptorOf(classModelId)).check(classDesc))) {
174172

175173
QuickFixHelper.newIssue(context)
176174
.forRule(this)
@@ -191,25 +189,26 @@ private static JavaQuickFix computeQuickFix(Arguments arguments) {
191189
}
192190

193191
/** The expression may be of the form `model.thisClass().asInternalName()` or `model.thisClass().name().stringValue(). */
194-
private Predicate<ExpressionTree> isInternalNameOf(IdentifierTree classModel) {
195-
return expression -> check(expression).calls(internalNameMatcher).onExpression().matches(isThisClassOf(classModel))
196-
|| check(expression).calls(stringValueMatcher).onExpression().calls(nameMatcher).onExpression().matches(isThisClassOf(classModel));
192+
private TreeMatcher<ExpressionTree> isInternalNameOf(IdentifierTree classModel) {
193+
return calls(internalNameMatcher, invokedOn(isThisClassOf(classModel)))
194+
.or(calls(stringValueMatcher, invokedOn(calls(nameMatcher, invokedOn(isThisClassOf(classModel))))));
197195
}
198196

199197
/**
200198
* The expression may be of the form:
201199
* - `model.thisClass().asSymbol()`
202200
* - or `Desc.ofInternalName(internalName)` where `internalName` is the result of `model.thisClass().asInternalName()`
203201
* - or `Desc.ofDescriptorString(desc.descriptorString())` where `desc` itself has the form of a descriptor. */
204-
private Predicate<ExpressionTree> isDescriptorOf(IdentifierTree classModel) {
205-
return descTree -> check(descTree).calls(asSymbolMatcher).onExpression().matches(isThisClassOf(classModel))
206-
|| check(descTree).calls(ofInternalNameMatcher).withArgument(0).matches(isInternalNameOf(classModel))
207-
|| check(descTree).calls(ofDescriptorMatcher).withArgument(0)
208-
.calls(descriptorStringMatcher).onExpression().matches(isDescriptorOf(classModel));
202+
private TreeMatcher<ExpressionTree> isDescriptorOf(IdentifierTree classModel) {
203+
return TreeMatcher.recursive(self ->
204+
calls(asSymbolMatcher, invokedOn(isThisClassOf(classModel)))
205+
.or(calls(ofInternalNameMatcher, withArgument(0, isInternalNameOf(classModel))))
206+
.or(calls(ofDescriptorMatcher,
207+
withArgument(0, calls(descriptorStringMatcher, invokedOn(self))))));
209208
}
210209

211210
/** Check whether the given expression is of the form: `classModel.thisClass()`. */
212-
private Predicate<ExpressionTree> isThisClassOf(IdentifierTree classModel) {
213-
return expression -> check(expression).calls(thisClassMatcher).onExpression().isIdentifier(classModel);
211+
private TreeMatcher<ExpressionTree> isThisClassOf(IdentifierTree classModel) {
212+
return calls(thisClassMatcher, invokedOn(isIdentifier(classModel)));
214213
}
215214
}

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
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.MethodInvocationTree;
26+
import org.sonar.plugins.java.api.tree.Tree;
27+
28+
import static org.sonar.java.matcher.TreeMatcher.forEachStatement;
29+
import static org.sonar.java.matcher.TreeMatcher.hasSize;
30+
import static org.sonar.java.matcher.TreeMatcher.matching;
31+
import static org.sonar.java.matcher.TreeMatcher.statementAt;
32+
import static org.sonar.java.matcher.TreeMatcher.withBody;
33+
import static org.sonar.java.matcher.TreeMatcher.withExpression;
34+
35+
@Rule(key = "S7478")
36+
public class UseTransformClassInsteadOfBuildCheck extends IssuableSubscriptionVisitor {
37+
38+
private static final String CLASS_MODEL_CLASSNAME = "java.lang.classfile.ClassModel";
39+
private static final String CLASS_DESC_CLASSNAME = "java.lang.constant.ClassDesc";
40+
private static final String CONSUMER_CLASSNAME = "java.util.function.Consumer";
41+
private static final String CLASS_ENTRY_CLASSNAME = "java.lang.classfile.constantpool.ClassEntry";
42+
43+
private final MethodMatchers classFileBuildMatcher = MethodMatchers.create()
44+
.ofTypes("java.lang.classfile.ClassFile")
45+
.names("build")
46+
.addParametersMatcher(
47+
CLASS_DESC_CLASSNAME,
48+
CONSUMER_CLASSNAME)
49+
.addParametersMatcher(
50+
CLASS_ENTRY_CLASSNAME,
51+
"java.lang.classfile.constantpool.ConstantPoolBuilder",
52+
CONSUMER_CLASSNAME)
53+
.build();
54+
55+
/**
56+
* Matcher for expressions of the form `x -> { for (ClassModel cm : m) { ... } }`.
57+
* Note that we handle only lambda expressions here, not method references.
58+
*/
59+
private final TreeMatcher<ExpressionTree> treeMatcher = TreeMatcher
60+
.isLambdaExpression(
61+
withBody(
62+
hasSize(1).and(
63+
statementAt(0,
64+
forEachStatement(
65+
withExpression(
66+
matching(e -> e.symbolType().fullyQualifiedName().equals(CLASS_MODEL_CLASSNAME))))))));
67+
68+
@Override
69+
public List<Tree.Kind> nodesToVisit() {
70+
return List.of(Tree.Kind.METHOD_INVOCATION);
71+
}
72+
73+
@Override
74+
public void visitNode(Tree tree) {
75+
MethodInvocationTree methodInvocation = (MethodInvocationTree) tree;
76+
if (classFileBuildMatcher.matches(methodInvocation) && !methodInvocation.arguments().isEmpty()) {
77+
ExpressionTree lastArgument = methodInvocation.arguments().get(methodInvocation.arguments().size() - 1);
78+
if (treeMatcher.check(lastArgument)) {
79+
reportIssue(methodInvocation.methodSelect(), "Replace this 'build()' call with 'transformClass()'.");
80+
}
81+
}
82+
}
83+
}

‎java-checks/src/test/java/org/sonar/java/checks/UseTransformClassInsteadOfBuildCheckTest.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 UseTransformClassInsteadOfBuildCheckTest {
24+
25+
@Test
26+
void test() {
27+
CheckVerifier.newVerifier()
28+
.onFile(TestUtils.mainCodeSourcesPath("checks/UseTransformClassInsteadOfBuildCheckSample.java"))
29+
.withCheck(new UseTransformClassInsteadOfBuildCheck())
30+
.verifyIssues();
31+
}
32+
33+
@Test
34+
void testWithoutSemantic() {
35+
CheckVerifier.newVerifier()
36+
.onFile(TestUtils.mainCodeSourcesPath("checks/UseTransformClassInsteadOfBuildCheckSample.java"))
37+
.withCheck(new UseTransformClassInsteadOfBuildCheck())
38+
.withoutSemantic()
39+
.verifyIssues();
40+
}
41+
42+
}

0 commit comments

Comments
 (0)