Skip to content

Commit 926a7ec

Browse files
carlosu7carlosu7
authored
Sonar codemod for removing unused private method codemod (#266)
example https://sonarcloud.io/project/issues?fileUuids=AYvtrjqILCzGLicz7AZw&resolved=false&types=CODE_SMELL&id=nahsra_WebGoat_10_23&open=AYx_HQhaOVCWpCEOKEAO reference: https://rules.sonarsource.com/java/RSPEC-1144/
1 parent 2145da2 commit 926a7ec

File tree

8 files changed

+390
-1
lines changed

8 files changed

+390
-1
lines changed

‎core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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.MethodDeclaration;
6+
import com.github.javaparser.ast.expr.SimpleName;
7+
import io.codemodder.Codemod;
8+
import io.codemodder.CodemodExecutionPriority;
9+
import io.codemodder.CodemodInvocationContext;
10+
import io.codemodder.ReviewGuidance;
11+
import io.codemodder.providers.sonar.ProvidedSonarScan;
12+
import io.codemodder.providers.sonar.RuleIssues;
13+
import io.codemodder.providers.sonar.SonarPluginJavaParserChanger;
14+
import io.codemodder.providers.sonar.api.Issue;
15+
import java.util.Optional;
16+
import javax.inject.Inject;
17+
18+
/** A codemod for removing unused private methods. */
19+
@Codemod(
20+
id = "sonar:java/remove-unused-private-method-s1144",
21+
reviewGuidance = ReviewGuidance.MERGE_AFTER_REVIEW,
22+
executionPriority = CodemodExecutionPriority.HIGH)
23+
public final class RemoveUnusedPrivateMethodCodemod
24+
extends SonarPluginJavaParserChanger<SimpleName> {
25+
26+
@Inject
27+
public RemoveUnusedPrivateMethodCodemod(
28+
@ProvidedSonarScan(ruleId = "java:S1144") final RuleIssues issues) {
29+
super(issues, SimpleName.class);
30+
}
31+
32+
@Override
33+
public boolean onIssueFound(
34+
final CodemodInvocationContext context,
35+
final CompilationUnit cu,
36+
final SimpleName node,
37+
final Issue issue) {
38+
39+
final Optional<Node> methodDeclarationOptional = node.getParentNode();
40+
41+
if (methodDeclarationOptional.isEmpty()) {
42+
return false;
43+
}
44+
45+
final MethodDeclaration methodDeclaration = (MethodDeclaration) methodDeclarationOptional.get();
46+
47+
methodDeclaration.removeForced();
48+
49+
return true;
50+
}
51+
}

‎core-codemods/src/main/resources/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod/description.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
This change removes unused `private` methods. Dead code can cause confusion and increase the mental load of maintainers. It can increase your maintenance burden as you have to keep that unused code compiling when you make sweeping changes to the APIs used within the method.
2+
3+
Our changes look something like this:
4+
5+
```diff
6+
- private String getUuid(){
7+
- return uuid;
8+
- }
9+
```

‎core-codemods/src/main/resources/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod/report.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"summary" : "Removed unused private method (Sonar).",
3+
"change" : "Removed unused private method.",
4+
"references" : [
5+
"https://rules.sonarsource.com/java/RSPEC-1144/",
6+
"https://understandlegacycode.com/blog/delete-unused-code/"
7+
]
8+
}

‎core-codemods/src/main/resources/io/codemodder/codemods/ReplaceStreamCollectorsToListCodemod/description.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
This change modernizes the a stream's `List` creation to be driven from the simple, and more readable [`Stream#toList()`](https://docs.oracle.com/javase/16/docs/api/java.base/java/util/stream/Collectors.html#toList()) method.
1+
This change modernizes a stream's `List` creation to be driven from the simple, and more readable [`Stream#toList()`](https://docs.oracle.com/javase/16/docs/api/java.base/java/util/stream/Collectors.html#toList()) method.
22

33
Our changes look something like this:
44

‎core-codemods/src/test/java/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemodTest.java‎

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 = RemoveUnusedPrivateMethodCodemod.class,
8+
testResourceDir = "remove-unused-private-method-s1144",
9+
renameTestFile =
10+
"src/main/java/org/owasp/webgoat/lessons/authbypass/AccountVerificationHelper.java",
11+
dependencies = {})
12+
final class RemoveUnusedPrivateMethodCodemodTest implements CodemodTestMixin {}

‎core-codemods/src/test/resources/remove-unused-private-method-s1144/Test.java.after‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
* This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
3+
*
4+
* Copyright (c) 2002 - 2019 Bruce Mayhew
5+
*
6+
* This program is free software; you can redistribute it and/or modify it under the terms of the
7+
* GNU General Public License as published by the Free Software Foundation; either version 2 of the
8+
* License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
11+
* even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12+
* General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License along with this program; if
15+
* not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
16+
* 02111-1307, USA.
17+
*
18+
* Getting Source ==============
19+
*
20+
* Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
21+
*/
22+
23+
package org.owasp.webgoat.lessons.authbypass;
24+
25+
import java.util.HashMap;
26+
import java.util.Map;
27+
28+
/** Created by appsec on 7/18/17. */
29+
public class AccountVerificationHelper {
30+
31+
// simulating database storage of verification credentials
32+
private static final Integer verifyUserId = 1223445;
33+
private static final Map<String, String> userSecQuestions = new HashMap<>();
34+
35+
static {
36+
userSecQuestions.put("secQuestion0", "Dr. Watson");
37+
userSecQuestions.put("secQuestion1", "Baker Street");
38+
}
39+
40+
private static final Map<Integer, Map> secQuestionStore = new HashMap<>();
41+
42+
static {
43+
secQuestionStore.put(verifyUserId, userSecQuestions);
44+
}
45+
46+
// end 'data store set up'
47+
48+
// this is to aid feedback in the attack process and is not intended to be part of the
49+
// 'vulnerable' code
50+
public boolean didUserLikelylCheat(HashMap<String, String> submittedAnswers) {
51+
boolean likely = false;
52+
53+
if (submittedAnswers.size() == secQuestionStore.get(verifyUserId).size()) {
54+
likely = true;
55+
}
56+
57+
if ((submittedAnswers.containsKey("secQuestion0")
58+
&& submittedAnswers
59+
.get("secQuestion0")
60+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion0")))
61+
&& (submittedAnswers.containsKey("secQuestion1")
62+
&& submittedAnswers
63+
.get("secQuestion1")
64+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion1")))) {
65+
likely = true;
66+
} else {
67+
likely = false;
68+
}
69+
70+
return likely;
71+
}
72+
73+
// end of cheating check ... the method below is the one of real interest. Can you find the flaw?
74+
75+
public boolean verifyAccount(Integer userId, HashMap<String, String> submittedQuestions) {
76+
// short circuit if no questions are submitted
77+
if (submittedQuestions.entrySet().size() != secQuestionStore.get(verifyUserId).size()) {
78+
return false;
79+
}
80+
81+
if (submittedQuestions.containsKey("secQuestion0")
82+
&& !submittedQuestions
83+
.get("secQuestion0")
84+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion0"))) {
85+
return false;
86+
}
87+
88+
if (submittedQuestions.containsKey("secQuestion1")
89+
&& !submittedQuestions
90+
.get("secQuestion1")
91+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion1"))) {
92+
return false;
93+
}
94+
95+
unusedSuccess(true, 1);
96+
unusedFailsOverload(1,2,3);
97+
unusedFailsOverload(1, 2);
98+
99+
// else
100+
return true;
101+
}
102+
103+
private void unusedSuccess(boolean a, int b){
104+
System.out.println(b);
105+
}
106+
107+
private void unusedFailsOverload(Integer a, Integer b, Integer c){
108+
System.out.println(a + c);
109+
}
110+
111+
private void unusedFailsOverload(Integer a, Integer b){
112+
unusedFailsOverload(a, b, null);
113+
}
114+
}

‎core-codemods/src/test/resources/remove-unused-private-method-s1144/Test.java.before‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
3+
*
4+
* Copyright (c) 2002 - 2019 Bruce Mayhew
5+
*
6+
* This program is free software; you can redistribute it and/or modify it under the terms of the
7+
* GNU General Public License as published by the Free Software Foundation; either version 2 of the
8+
* License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
11+
* even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12+
* General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License along with this program; if
15+
* not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
16+
* 02111-1307, USA.
17+
*
18+
* Getting Source ==============
19+
*
20+
* Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
21+
*/
22+
23+
package org.owasp.webgoat.lessons.authbypass;
24+
25+
import java.util.HashMap;
26+
import java.util.Map;
27+
28+
/** Created by appsec on 7/18/17. */
29+
public class AccountVerificationHelper {
30+
31+
// simulating database storage of verification credentials
32+
private static final Integer verifyUserId = 1223445;
33+
private static final Map<String, String> userSecQuestions = new HashMap<>();
34+
35+
static {
36+
userSecQuestions.put("secQuestion0", "Dr. Watson");
37+
userSecQuestions.put("secQuestion1", "Baker Street");
38+
}
39+
40+
private static final Map<Integer, Map> secQuestionStore = new HashMap<>();
41+
42+
static {
43+
secQuestionStore.put(verifyUserId, userSecQuestions);
44+
}
45+
46+
// end 'data store set up'
47+
48+
// this is to aid feedback in the attack process and is not intended to be part of the
49+
// 'vulnerable' code
50+
public boolean didUserLikelylCheat(HashMap<String, String> submittedAnswers) {
51+
boolean likely = false;
52+
53+
if (submittedAnswers.size() == secQuestionStore.get(verifyUserId).size()) {
54+
likely = true;
55+
}
56+
57+
if ((submittedAnswers.containsKey("secQuestion0")
58+
&& submittedAnswers
59+
.get("secQuestion0")
60+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion0")))
61+
&& (submittedAnswers.containsKey("secQuestion1")
62+
&& submittedAnswers
63+
.get("secQuestion1")
64+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion1")))) {
65+
likely = true;
66+
} else {
67+
likely = false;
68+
}
69+
70+
return likely;
71+
}
72+
73+
// end of cheating check ... the method below is the one of real interest. Can you find the flaw?
74+
75+
public boolean verifyAccount(Integer userId, HashMap<String, String> submittedQuestions) {
76+
// short circuit if no questions are submitted
77+
if (submittedQuestions.entrySet().size() != secQuestionStore.get(verifyUserId).size()) {
78+
return false;
79+
}
80+
81+
if (submittedQuestions.containsKey("secQuestion0")
82+
&& !submittedQuestions
83+
.get("secQuestion0")
84+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion0"))) {
85+
return false;
86+
}
87+
88+
if (submittedQuestions.containsKey("secQuestion1")
89+
&& !submittedQuestions
90+
.get("secQuestion1")
91+
.equals(secQuestionStore.get(verifyUserId).get("secQuestion1"))) {
92+
return false;
93+
}
94+
95+
unusedSuccess(true, 1);
96+
unusedFailsOverload(1,2,3);
97+
unusedFailsOverload(1, 2);
98+
99+
// else
100+
return true;
101+
}
102+
103+
private void unusedSuccess(boolean a, int b){
104+
System.out.println(b);
105+
}
106+
107+
private void unusedFailsOverload(Integer a, Integer b, Integer c){
108+
System.out.println(a + c);
109+
}
110+
111+
private void unusedFailsOverload(Integer a, Integer b){
112+
unusedFailsOverload(a, b, null);
113+
}
114+
115+
private void unusedMethod(){
116+
System.out.println("I'm unused :(");
117+
}
118+
}

0 commit comments

Comments
 (0)