Skip to content

Commit f6dcfea

Browse files
authored
Sonar codemod for declaring a variable in a separate line (#268)
Reference: https://rules.sonarsource.com/java/RSPEC-1659/ Example: https://sonarcloud.io/project/issues?languages=java&resolved=false&id=nahsra_WebGoat_10_23&open=AYxkqUZwlAj1v1V7lso3 Declaring a variable can happen as: * A field declaration inside a class-interface * A variable declaration expression inside a block statement DOCS PR -> pixee/docs#115
1 parent cccc23b commit f6dcfea

File tree

12 files changed

+720
-1
lines changed

12 files changed

+720
-1
lines changed
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package io.codemodder.codemods;
2+
3+
import com.github.javaparser.ast.Node;
4+
import com.github.javaparser.ast.NodeList;
5+
import com.github.javaparser.ast.body.VariableDeclarator;
6+
import com.github.javaparser.ast.nodeTypes.NodeWithVariables;
7+
import java.util.ArrayList;
8+
import java.util.List;
9+
import java.util.Objects;
10+
11+
abstract class DeclareVariableOnSeparateLine {
12+
13+
protected final NodeWithVariables<?> parentNode;
14+
15+
protected DeclareVariableOnSeparateLine(final NodeWithVariables<?> parentNode) {
16+
this.parentNode = Objects.requireNonNull(parentNode);
17+
}
18+
19+
/**
20+
* Splits multiple inline variables within a parent node into separate variable declaration
21+
* statements.
22+
*/
23+
protected boolean splitVariablesIntoTheirOwnStatements() {
24+
25+
final List<VariableDeclarator> inlineVariables = parentNode.getVariables().stream().toList();
26+
27+
final List<VariableDeclarator> remainingVariables =
28+
inlineVariables.subList(1, inlineVariables.size());
29+
30+
final List<Node> newVariableNodes = createVariableNodesToAdd(remainingVariables);
31+
32+
if (!addNewNodesToParentNode(newVariableNodes)) {
33+
return false;
34+
}
35+
// Replace parent's node that has all inline variables with only the first variable
36+
parentNode.setVariables(new NodeList<>(inlineVariables.get(0)));
37+
38+
return true;
39+
}
40+
41+
/** Returns a list of nodes created from the list of inline variables. */
42+
protected abstract List<Node> createVariableNodesToAdd(List<VariableDeclarator> inlineVariables);
43+
44+
/** Adds new variable nodes to the parent node. */
45+
protected abstract boolean addNewNodesToParentNode(List<Node> nodesToAdd);
46+
47+
/**
48+
* Inserts a list of nodes after a specified reference node within an original list of nodes. The
49+
* result of this operation is a new list containing the original nodes with the additional nodes
50+
* inserted after the specified reference node while maintaining the original order of elements.
51+
*/
52+
static <T> List<T> insertNodesAfterReference(
53+
final List<T> originalNodes, final Node referenceNode, final List<Node> nodesToAdd) {
54+
55+
// Find the index of the reference node in the original list
56+
final int referenceIndex = originalNodes.indexOf(referenceNode);
57+
58+
// Split the original list into elements before and after the reference node
59+
final List<T> elementsBefore = originalNodes.subList(0, referenceIndex + 1);
60+
final List<T> elementsAfter = originalNodes.subList(referenceIndex + 1, originalNodes.size());
61+
62+
// Create a new list with nodes inserted after the reference node
63+
final List<T> newElements = new ArrayList<>(elementsBefore);
64+
nodesToAdd.forEach(node -> newElements.add((T) node));
65+
newElements.addAll(elementsAfter);
66+
67+
return newElements;
68+
}
69+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package io.codemodder.codemods;
2+
3+
import com.github.javaparser.ast.CompilationUnit;
4+
import com.github.javaparser.ast.Node;
5+
import com.github.javaparser.ast.body.FieldDeclaration;
6+
import com.github.javaparser.ast.body.VariableDeclarator;
7+
import com.github.javaparser.ast.expr.VariableDeclarationExpr;
8+
import com.github.javaparser.ast.nodeTypes.NodeWithVariables;
9+
import io.codemodder.*;
10+
import io.codemodder.providers.sonar.ProvidedSonarScan;
11+
import io.codemodder.providers.sonar.RuleIssues;
12+
import io.codemodder.providers.sonar.SonarPluginJavaParserChanger;
13+
import io.codemodder.providers.sonar.api.Issue;
14+
import java.util.Optional;
15+
import javax.inject.Inject;
16+
17+
/** A codemod for declaring a variable on a separate line. */
18+
@Codemod(
19+
id = "sonar:java/declare-variable-on-separate-line-s1659",
20+
reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW,
21+
executionPriority = CodemodExecutionPriority.HIGH)
22+
public final class DeclareVariableOnSeparateLineCodemod
23+
extends SonarPluginJavaParserChanger<VariableDeclarator> {
24+
@Inject
25+
public DeclareVariableOnSeparateLineCodemod(
26+
@ProvidedSonarScan(ruleId = "java:S1659") final RuleIssues issues) {
27+
super(issues, VariableDeclarator.class);
28+
}
29+
30+
@Override
31+
public boolean onIssueFound(
32+
final CodemodInvocationContext context,
33+
final CompilationUnit cu,
34+
final VariableDeclarator variableDeclarator,
35+
final Issue issue) {
36+
37+
final Optional<Node> parentOptional = variableDeclarator.getParentNode();
38+
39+
if (parentOptional.isEmpty()) {
40+
return false;
41+
}
42+
43+
final NodeWithVariables<?> parentNode = (NodeWithVariables<?>) parentOptional.get();
44+
45+
final DeclareVariableOnSeparateLine declareVariableOnSeparateLine;
46+
47+
if (parentNode instanceof FieldDeclaration fieldDeclaration) {
48+
declareVariableOnSeparateLine =
49+
new DeclareVariableOnSeparateLineForFieldDeclaration(fieldDeclaration);
50+
} else {
51+
declareVariableOnSeparateLine =
52+
new DeclareVariableOnSeparateLineForVariableDeclarationExpr(
53+
(VariableDeclarationExpr) parentNode);
54+
}
55+
56+
return declareVariableOnSeparateLine.splitVariablesIntoTheirOwnStatements();
57+
}
58+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package io.codemodder.codemods;
2+
3+
import com.github.javaparser.ast.Node;
4+
import com.github.javaparser.ast.NodeList;
5+
import com.github.javaparser.ast.body.BodyDeclaration;
6+
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
7+
import com.github.javaparser.ast.body.FieldDeclaration;
8+
import com.github.javaparser.ast.body.VariableDeclarator;
9+
import java.util.ArrayList;
10+
import java.util.List;
11+
import java.util.Objects;
12+
import java.util.Optional;
13+
14+
final class DeclareVariableOnSeparateLineForFieldDeclaration extends DeclareVariableOnSeparateLine {
15+
16+
private final FieldDeclaration fieldDeclaration;
17+
18+
DeclareVariableOnSeparateLineForFieldDeclaration(final FieldDeclaration parentNode) {
19+
super(parentNode);
20+
this.fieldDeclaration = Objects.requireNonNull(parentNode);
21+
}
22+
23+
/**
24+
* Returns a list of {@link FieldDeclaration} nodes created from a list of {@link
25+
* VariableDeclarator}s.
26+
*/
27+
protected List<Node> createVariableNodesToAdd(List<VariableDeclarator> inlineVariables) {
28+
final List<Node> nodesToAdd = new ArrayList<>();
29+
for (VariableDeclarator inlineVariable : inlineVariables) {
30+
31+
final FieldDeclaration newFieldDeclaration =
32+
new FieldDeclaration(
33+
fieldDeclaration.getModifiers(),
34+
((FieldDeclaration) parentNode).getAnnotations(),
35+
new NodeList<>(inlineVariable));
36+
nodesToAdd.add(newFieldDeclaration);
37+
}
38+
return nodesToAdd;
39+
}
40+
41+
/**
42+
* Adds a list of nodes to the parent node {@link ClassOrInterfaceDeclaration} after the
43+
* fieldDeclaration.
44+
*/
45+
protected boolean addNewNodesToParentNode(List<Node> nodesToAdd) {
46+
final Optional<Node> classOrInterfaceDeclarationOptional = fieldDeclaration.getParentNode();
47+
if (classOrInterfaceDeclarationOptional.isPresent()
48+
&& classOrInterfaceDeclarationOptional.get()
49+
instanceof ClassOrInterfaceDeclaration classOrInterfaceDeclaration) {
50+
51+
final List<BodyDeclaration<?>> allMembers =
52+
insertNodesAfterReference(
53+
classOrInterfaceDeclaration.getMembers().stream().toList(),
54+
fieldDeclaration,
55+
nodesToAdd);
56+
57+
classOrInterfaceDeclaration.setMembers(new NodeList<>(allMembers));
58+
return true;
59+
}
60+
61+
return false;
62+
}
63+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package io.codemodder.codemods;
2+
3+
import com.github.javaparser.ast.Node;
4+
import com.github.javaparser.ast.NodeList;
5+
import com.github.javaparser.ast.body.VariableDeclarator;
6+
import com.github.javaparser.ast.expr.VariableDeclarationExpr;
7+
import com.github.javaparser.ast.stmt.BlockStmt;
8+
import com.github.javaparser.ast.stmt.ExpressionStmt;
9+
import com.github.javaparser.ast.stmt.Statement;
10+
import java.util.ArrayList;
11+
import java.util.List;
12+
import java.util.Objects;
13+
import java.util.Optional;
14+
15+
final class DeclareVariableOnSeparateLineForVariableDeclarationExpr
16+
extends DeclareVariableOnSeparateLine {
17+
18+
private final VariableDeclarationExpr variableDeclarationExpr;
19+
20+
DeclareVariableOnSeparateLineForVariableDeclarationExpr(
21+
final VariableDeclarationExpr parentNode) {
22+
super(parentNode);
23+
this.variableDeclarationExpr = Objects.requireNonNull(parentNode);
24+
}
25+
26+
/**
27+
* Returns a list of {@link VariableDeclarationExpr} nodes created from a list of {@link
28+
* VariableDeclarator}s.
29+
*/
30+
protected List<Node> createVariableNodesToAdd(List<VariableDeclarator> inlineVariables) {
31+
final List<Node> nodesToAdd = new ArrayList<>();
32+
for (VariableDeclarator inlineVariable : inlineVariables) {
33+
final VariableDeclarationExpr newVariableDeclarationExpr =
34+
new VariableDeclarationExpr(
35+
variableDeclarationExpr.getModifiers(), new NodeList<>(inlineVariable));
36+
final ExpressionStmt expressionStmt = new ExpressionStmt(newVariableDeclarationExpr);
37+
nodesToAdd.add(expressionStmt);
38+
}
39+
return nodesToAdd;
40+
}
41+
42+
/** Adds a list of nodes to the parent BlockStmt after the expressionStmt. */
43+
protected boolean addNewNodesToParentNode(List<Node> nodesToAdd) {
44+
final Optional<Node> expressionStmtOptional = variableDeclarationExpr.getParentNode();
45+
if (expressionStmtOptional.isPresent()
46+
&& expressionStmtOptional.get() instanceof ExpressionStmt expressionStmt) {
47+
final Optional<Node> blockStmtOptional = expressionStmt.getParentNode();
48+
if (blockStmtOptional.isPresent() && blockStmtOptional.get() instanceof BlockStmt blockStmt) {
49+
50+
final List<Statement> allStmts =
51+
insertNodesAfterReference(blockStmt.getStatements(), expressionStmt, nodesToAdd);
52+
53+
blockStmt.setStatements(new NodeList<>(allStmts));
54+
55+
return true;
56+
}
57+
}
58+
59+
return false;
60+
}
61+
}

core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@ public final class DefaultCodemods {
1414
public static List<Class<? extends CodeChanger>> asList() {
1515
return List.of(
1616
AddClarifyingBracesCodemod.class,
17+
AddMissingOverrideCodemod.class,
18+
AvoidImplicitPublicConstructorCodemod.class,
19+
DeclareVariableOnSeparateLineCodemod.class,
1720
DisableAutomaticDirContextDeserializationCodemod.class,
21+
FixRedundantStaticOnEnumCodemod.class,
1822
HardenJavaDeserializationCodemod.class,
23+
HardenStringParseToPrimitivesCodemod.class,
1924
HardenProcessCreationCodemod.class,
2025
HardenXMLDecoderCodemod.class,
2126
HardenXMLInputFactoryCodemod.class,
@@ -32,12 +37,18 @@ public static List<Class<? extends CodeChanger>> asList() {
3237
OutputResourceLeakCodemod.class,
3338
PreventFileWriterLeakWithFilesCodemod.class,
3439
RandomizeSeedCodemod.class,
40+
RemoveRedundantVariableCreationCodemod.class,
41+
RemoveUnusedLocalVariableCodemod.class,
42+
RemoveUselessParenthesesCodemod.class,
3543
ReplaceDefaultHttpClientCodemod.class,
44+
ReplaceStreamCollectorsToListCodemod.class,
3645
SanitizeApacheMultipartFilenameCodemod.class,
3746
SanitizeHttpHeaderCodemod.class,
3847
SanitizeSpringMultipartFilenameCodemod.class,
3948
SecureRandomCodemod.class,
4049
SemgrepOverlyPermissiveFilePermissionsCodemod.class,
50+
SimplifyRestControllerAnnotationsCodemod.class,
51+
SubstituteReplaceAllCodemod.class,
4152
SQLParameterizerCodemod.class,
4253
SSRFCodemod.class,
4354
StackTraceExposureCodemod.class,
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
This change splits variable assignments onto their own lines. [Many](https://wiki.sei.cmu.edu/confluence/display/java/DCL52-J.+Do+not+declare+more+than+one+variable+per+declaration) [sources](https://rules.sonarsource.com/java/RSPEC-1659/) [believe](https://dart.dev/tools/linter-rules/avoid_multiple_declarations_per_line) it is easier to review code where the variables are separate statements on their own individual line.
2+
3+
Our changes look something like this:
4+
5+
```diff
6+
- int i = 0, limit = 10;
7+
+ int i = 0;
8+
+ int limit = 10;
9+
10+
while (i < limit){
11+
```
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"summary" : "Split variable declarations into their own statements (Sonar)",
3+
"change" : "Split variable declarations into their own statements.",
4+
"references" : [
5+
"https://rules.sonarsource.com/java/RSPEC-1659/",
6+
"https://wiki.sei.cmu.edu/confluence/display/java/DCL52-J.+Do+not+declare+more+than+one+variable+per+declaration"
7+
]
8+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package io.codemodder.codemods;
2+
3+
import io.codemodder.testutils.CodemodTestMixin;
4+
import io.codemodder.testutils.Metadata;
5+
6+
@Metadata(
7+
codemodType = DeclareVariableOnSeparateLineCodemod.class,
8+
testResourceDir = "declare-variable-on-separate-line-s1659",
9+
renameTestFile =
10+
"src/main/java/org/owasp/webgoat/container/assignments/AssignmentEndpoint.java",
11+
dependencies = {})
12+
final class DeclareVariableOnSeparateLineCodemodTest implements CodemodTestMixin {}

0 commit comments

Comments
 (0)