Skip to content

Commit ae437c3

Browse files
romainbrenguierromainbrenguier
authored
SONARJAVA-5494 Add rule for redundant new class name argument in transformClass call (#5220)
1 parent e2ad440 commit ae437c3

File tree

10 files changed

+407
-2
lines changed

10 files changed

+407
-2
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ Make sure that the `java-checks-test-sources` module has been compiled (ie: the
164164

165165
In doubt, go the [`java-checks-test-sources`](java-checks-tests-sources) module and run:
166166
```shell
167-
# Use java 23!
167+
# Use java 24!
168168
mvn clean compile
169169
```
170170

‎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(14);
202+
softly.assertThat(rulesNotReporting).hasSize(15);
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
@@ -2848,5 +2848,11 @@
28482848
"hasTruePositives": true,
28492849
"falseNegatives": 0,
28502850
"falsePositives": 0
2851+
},
2852+
{
2853+
"ruleKey": "7477",
2854+
"hasTruePositives": false,
2855+
"falseNegatives": 0,
2856+
"falsePositives": 0
28512857
}
28522858
]

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

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

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package checks;
2+
3+
import java.io.IOException;
4+
import java.lang.classfile.ClassFile;
5+
import java.lang.classfile.ClassModel;
6+
import java.lang.classfile.ClassTransform;
7+
import java.lang.classfile.MethodModel;
8+
import java.lang.constant.ClassDesc;
9+
import java.nio.file.Path;
10+
import java.util.Optional;
11+
12+
public class ClassNameInClassTransformCheckSample {
13+
ClassTransform classTransform = (classBuilder, classElement) -> {
14+
if (!(classElement instanceof MethodModel methodModel &&
15+
methodModel.methodName().stringValue().startsWith("debug"))) {
16+
classBuilder.with(classElement);
17+
}
18+
};
19+
20+
public byte[] transformClassFileNonCompliantDesc(Path path) throws IOException {
21+
ClassFile classFile = ClassFile.of();
22+
ClassModel classModel = classFile.parse(path);
23+
return classFile.transformClass(classModel,
24+
classModel.thisClass(), // Noncompliant {{Use `transformClass` overload without the class name.}}
25+
// ^^^^^^^^^^^^^^^^^^^^^^
26+
classTransform);
27+
}
28+
29+
public byte[] transformClassFileNonCompliant(Path path) throws IOException {
30+
ClassFile classFile = ClassFile.of();
31+
ClassModel classModel = classFile.parse(path);
32+
return classFile.transformClass(classModel,
33+
classModel.thisClass().asSymbol(), // Noncompliant {{Use `transformClass` overload without the class name.}}
34+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
35+
classTransform);
36+
}
37+
38+
public byte[] transformClassFileNonCompliantInternalName(Path path) throws IOException {
39+
ClassFile classFile = ClassFile.of();
40+
ClassModel classModel = classFile.parse(path);
41+
return classFile.transformClass(classModel,
42+
ClassDesc.ofInternalName(classModel.thisClass().asInternalName()), // Noncompliant {{Use `transformClass` overload without the class name.}}
43+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44+
classTransform);
45+
}
46+
47+
public byte[] transformClassFileNonCompliantName(Path path) throws IOException {
48+
ClassFile classFile = ClassFile.of();
49+
ClassModel classModel = classFile.parse(path);
50+
return classFile.transformClass(classModel,
51+
ClassDesc.ofInternalName(classModel.thisClass().name().stringValue()), // Noncompliant {{Use `transformClass` overload without the class name.}}
52+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
53+
classTransform);
54+
}
55+
56+
public byte[] transformClassFileNonCompliantDesc2(Path path) throws IOException {
57+
ClassFile classFile = ClassFile.of();
58+
ClassModel classModel = classFile.parse(path);
59+
return classFile.transformClass(classModel,
60+
ClassDesc.ofDescriptor(classModel.thisClass().asSymbol().descriptorString()), // Noncompliant {{Use `transformClass` overload without the class name.}}
61+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
62+
classTransform);
63+
}
64+
65+
public byte[] transformClassFile(Path path) throws IOException {
66+
ClassFile classFile = ClassFile.of();
67+
ClassModel classModel = classFile.parse(path);
68+
return classFile.transformClass(classModel, // Compliant
69+
classTransform);
70+
}
71+
72+
public byte[] transformClassFile_changedName(Path path) throws IOException {
73+
ClassFile classFile = ClassFile.of();
74+
ClassModel classModel = classFile.parse(path);
75+
return classFile.transformClass(classModel,
76+
ClassDesc.ofInternalName(classModel.thisClass().asInternalName() + "Copy"), // Compliant
77+
classTransform);
78+
}
79+
80+
public byte[] transformClassFile_nonVariable(Path path) throws IOException {
81+
ClassFile classFile = ClassFile.of();
82+
Optional<ClassModel> classModel = Optional.of(classFile.parse(path));
83+
return classFile.transformClass(classModel.get(),
84+
classModel.get().thisClass(), // False negative: the case where the classModel is not a variable is not handled
85+
classTransform);
86+
}
87+
}

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

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
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 java.util.function.Predicate;
21+
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
24+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
25+
import org.sonar.plugins.java.api.tree.ExpressionTree;
26+
import org.sonar.plugins.java.api.tree.IdentifierTree;
27+
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
28+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
29+
import org.sonar.plugins.java.api.tree.Tree;
30+
31+
import javax.annotation.Nullable;
32+
33+
@Rule(key = "S7477")
34+
public class ClassNameInClassTransformCheck extends IssuableSubscriptionVisitor {
35+
private static final String CLASS_ENTRY_CLASSNAME = "java.lang.classfile.constantpool.ClassEntry";
36+
private static final String CLASS_MODEL_CLASSNAME = "java.lang.classfile.ClassModel";
37+
private static final String CLASS_DESC_CLASSNAME = "java.lang.constant.ClassDesc";
38+
39+
MethodMatchers classTransformMatcher = MethodMatchers.create()
40+
.ofTypes("java.lang.classfile.ClassFile")
41+
.names("transformClass")
42+
.addParametersMatcher(
43+
CLASS_MODEL_CLASSNAME,
44+
CLASS_DESC_CLASSNAME,
45+
"java.lang.classfile.ClassTransform")
46+
.addParametersMatcher(
47+
CLASS_MODEL_CLASSNAME,
48+
CLASS_ENTRY_CLASSNAME,
49+
"java.lang.classfile.ClassTransform")
50+
.build();
51+
52+
MethodMatchers thisClassMatcher = MethodMatchers.create()
53+
.ofTypes(CLASS_MODEL_CLASSNAME)
54+
.names("thisClass")
55+
.addWithoutParametersMatcher()
56+
.build();
57+
58+
MethodMatchers internalNameMatcher = MethodMatchers.create()
59+
.ofTypes(CLASS_ENTRY_CLASSNAME)
60+
.names("asInternalName")
61+
.addWithoutParametersMatcher()
62+
.build();
63+
64+
MethodMatchers asSymbolMatcher = MethodMatchers.create()
65+
.ofTypes(CLASS_ENTRY_CLASSNAME)
66+
.names("asSymbol")
67+
.addWithoutParametersMatcher()
68+
.build();
69+
70+
MethodMatchers nameMatcher = MethodMatchers.create()
71+
.ofTypes(CLASS_ENTRY_CLASSNAME)
72+
.names("name")
73+
.addWithoutParametersMatcher()
74+
.build();
75+
76+
MethodMatchers stringValueMatcher = MethodMatchers.create()
77+
.ofTypes("java.lang.classfile.constantpool.Utf8Entry")
78+
.names("stringValue")
79+
.addWithoutParametersMatcher()
80+
.build();
81+
82+
MethodMatchers ofDescriptorMatcher = MethodMatchers.create()
83+
.ofTypes(CLASS_DESC_CLASSNAME)
84+
.names("ofDescriptor")
85+
.addParametersMatcher("java.lang.String")
86+
.build();
87+
88+
MethodMatchers ofInternalNameMatcher = MethodMatchers.create()
89+
.ofTypes(CLASS_DESC_CLASSNAME)
90+
.names("ofInternalName")
91+
.addParametersMatcher("java.lang.String")
92+
.build();
93+
94+
MethodMatchers descriptorStringMatcher = MethodMatchers.create()
95+
.ofTypes(CLASS_DESC_CLASSNAME)
96+
.names("descriptorString")
97+
.addWithoutParametersMatcher()
98+
.build();
99+
100+
static class ExpressionMatcher {
101+
@Nullable
102+
private final ExpressionTree expressionTree;
103+
104+
ExpressionMatcher(@Nullable ExpressionTree expressionTree) {
105+
this.expressionTree = expressionTree;
106+
}
107+
108+
CallMatcher calls(MethodMatchers methodMatchers) {
109+
if (expressionTree instanceof MethodInvocationTree mit && methodMatchers.matches(mit)) {
110+
return new CallMatcher(mit);
111+
}
112+
return new CallMatcher(null);
113+
}
114+
115+
boolean isIdentifier(IdentifierTree identifier) {
116+
if (expressionTree instanceof IdentifierTree id) {
117+
return id.symbol().equals(identifier.symbol());
118+
}
119+
return false;
120+
}
121+
122+
boolean matches(Predicate<ExpressionTree> predicate) {
123+
return expressionTree != null && predicate.test(expressionTree);
124+
}
125+
}
126+
127+
static class CallMatcher {
128+
129+
/** When {@link #methodInvocationTree} is null, the matcher doesn't match anything. */
130+
@Nullable
131+
private final MethodInvocationTree methodInvocationTree;
132+
133+
CallMatcher(@Nullable MethodInvocationTree methodInvocationTree) {
134+
this.methodInvocationTree = methodInvocationTree;
135+
}
136+
137+
ExpressionMatcher onExpression() {
138+
if (methodInvocationTree != null && methodInvocationTree.methodSelect() instanceof MemberSelectExpressionTree mset) {
139+
return new ExpressionMatcher(mset.expression());
140+
}
141+
return new ExpressionMatcher(null);
142+
}
143+
144+
ExpressionMatcher withArgument(int argumentIndex) {
145+
if (methodInvocationTree != null && methodInvocationTree.arguments().size() >= argumentIndex) {
146+
return new ExpressionMatcher(methodInvocationTree.arguments().get(argumentIndex));
147+
}
148+
return new ExpressionMatcher(null);
149+
}
150+
}
151+
152+
static ExpressionMatcher check(ExpressionTree expression) {
153+
return new ExpressionMatcher(expression);
154+
}
155+
156+
@Override
157+
public List<Tree.Kind> nodesToVisit() {
158+
return List.of(Tree.Kind.METHOD_INVOCATION);
159+
}
160+
161+
@Override
162+
public void visitNode(Tree tree) {
163+
MethodInvocationTree mit = (MethodInvocationTree) tree;
164+
if (classTransformMatcher.matches(mit)) {
165+
ExpressionTree classModel = mit.arguments().get(0);
166+
ExpressionTree classDesc = mit.arguments().get(1);
167+
if (classModel instanceof IdentifierTree classModelId &&
168+
(isThisClassOf(classModelId).test(classDesc) || isDescriptorOf(classModelId).test(classDesc))) {
169+
context.reportIssue(this, classDesc, "Use `transformClass` overload without the class name.");
170+
}
171+
}
172+
}
173+
174+
/** The expression may be of the form `model.thisClass().asInternalName()` or `model.thisClass().name().stringValue(). */
175+
private Predicate<ExpressionTree> isInternalNameOf(IdentifierTree classModel) {
176+
return expression -> check(expression).calls(internalNameMatcher).onExpression().matches(isThisClassOf(classModel))
177+
|| check(expression).calls(stringValueMatcher).onExpression().calls(nameMatcher).onExpression().matches(isThisClassOf(classModel));
178+
}
179+
180+
/**
181+
* The expression may be of the form:
182+
* - `model.thisClass().asSymbol()`
183+
* - or `Desc.ofInternalName(internalName)` where `internalName` is the result of `model.thisClass().asInternalName()`
184+
* - or `Desc.ofDescriptorString(desc.descriptorString())` where `desc` itself has the form of a descriptor. */
185+
private Predicate<ExpressionTree> isDescriptorOf(IdentifierTree classModel) {
186+
return descTree -> check(descTree).calls(asSymbolMatcher).onExpression().matches(isThisClassOf(classModel))
187+
|| check(descTree).calls(ofInternalNameMatcher).withArgument(0).matches(isInternalNameOf(classModel))
188+
|| check(descTree).calls(ofDescriptorMatcher).withArgument(0)
189+
.calls(descriptorStringMatcher).onExpression().matches(isDescriptorOf(classModel));
190+
}
191+
192+
/** Check whether the given expression is of the form: `classModel.thisClass()`. */
193+
private Predicate<ExpressionTree> isThisClassOf(IdentifierTree classModel) {
194+
return expression -> check(expression).calls(thisClassMatcher).onExpression().isIdentifier(classModel);
195+
}
196+
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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 ClassNameInClassTransformCheckTest {
24+
25+
@Test
26+
void test() {
27+
CheckVerifier.newVerifier()
28+
.onFile(TestUtils.mainCodeSourcesPath("checks/ClassNameInClassTransformCheckSample.java"))
29+
.withCheck(new ClassNameInClassTransformCheck())
30+
.verifyIssues();
31+
}
32+
33+
@Test
34+
void testWithoutSemantic() {
35+
CheckVerifier.newVerifier()
36+
.onFile(TestUtils.mainCodeSourcesPath("checks/ClassNameInClassTransformCheckSample.java"))
37+
.withCheck(new ClassNameInClassTransformCheck())
38+
.withoutSemantic()
39+
.verifyIssues();
40+
}
41+
}

0 commit comments

Comments
 (0)