Skip to content

Commit 414978c

Browse files
nathsounathsou
committed
fix-ci: extract comparison dispatch into utilities to cut duplication
The quality gate failed on 6.3% duplicated new code (limit 3%) because S9148 and S9354 still shared the same visitNode shape after dropping the abstract check class. ComparisonMethodUtils now owns that dispatch.
1 parent 4b5a4c0 commit 414978c

3 files changed

Lines changed: 52 additions & 61 deletions

File tree

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

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,13 @@
1616
*/
1717
package org.sonar.java.checks;
1818

19-
import java.util.Arrays;
2019
import java.util.List;
2120
import org.sonar.check.Rule;
2221
import org.sonar.java.checks.helpers.ComparisonMethodUtils;
2322
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
2423
import org.sonar.plugins.java.api.semantic.Type;
25-
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
2624
import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
27-
import org.sonar.plugins.java.api.tree.ClassTree;
2825
import org.sonar.plugins.java.api.tree.ExpressionTree;
29-
import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
30-
import org.sonar.plugins.java.api.tree.MethodTree;
3126
import org.sonar.plugins.java.api.tree.Tree;
3227

3328
@Rule(key = "S9148")
@@ -37,33 +32,22 @@ public class FloatingPointComparisonCheck extends IssuableSubscriptionVisitor {
3732

3833
@Override
3934
public List<Tree.Kind> nodesToVisit() {
40-
return Arrays.asList(Tree.Kind.METHOD, Tree.Kind.LAMBDA_EXPRESSION);
35+
return ComparisonMethodUtils.nodesToVisit();
4136
}
4237

4338
@Override
4439
public void visitNode(Tree tree) {
45-
if (context.getSemanticModel() == null) {
46-
return;
47-
}
48-
if (tree.is(Tree.Kind.METHOD)) {
49-
MethodTree methodTree = (MethodTree) tree;
50-
if (ComparisonMethodUtils.isCompareMethod(methodTree)) {
51-
methodTree.block().accept(new FloatingPointComparisonVisitor());
52-
}
53-
} else {
54-
LambdaExpressionTree lambda = (LambdaExpressionTree) tree;
55-
if (ComparisonMethodUtils.isComparatorLambda(lambda)) {
56-
lambda.body().accept(new FloatingPointComparisonVisitor());
57-
}
58-
}
40+
ComparisonMethodUtils.visitComparisonNode(context, tree,
41+
methodTree -> methodTree.block().accept(new FloatingPointComparisonVisitor()),
42+
lambda -> lambda.body().accept(new FloatingPointComparisonVisitor()));
5943
}
6044

6145
private static boolean hasFloatingType(ExpressionTree tree) {
6246
return tree.symbolType().isPrimitive(Type.Primitives.FLOAT)
6347
|| tree.symbolType().isPrimitive(Type.Primitives.DOUBLE);
6448
}
6549

66-
private class FloatingPointComparisonVisitor extends BaseTreeVisitor {
50+
private class FloatingPointComparisonVisitor extends ComparisonMethodUtils.SkipNestedTypesVisitor {
6751

6852
@Override
6953
public void visitBinaryExpression(BinaryExpressionTree tree) {
@@ -78,15 +62,5 @@ && hasFloatingOperand(tree)) {
7862
private boolean hasFloatingOperand(BinaryExpressionTree tree) {
7963
return hasFloatingType(tree.leftOperand()) || hasFloatingType(tree.rightOperand());
8064
}
81-
82-
@Override
83-
public void visitClass(ClassTree tree) {
84-
// Do not visit inner classes
85-
}
86-
87-
@Override
88-
public void visitLambdaExpression(LambdaExpressionTree tree) {
89-
// Do not visit nested lambdas
90-
}
9165
}
9266
}

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

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,14 @@
1616
*/
1717
package org.sonar.java.checks;
1818

19-
import java.util.Arrays;
2019
import java.util.List;
2120
import org.sonar.check.Rule;
2221
import org.sonar.java.checks.helpers.ComparisonMethodUtils;
2322
import org.sonar.java.model.ExpressionUtils;
2423
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
2524
import org.sonar.plugins.java.api.semantic.Type;
26-
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
2725
import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
28-
import org.sonar.plugins.java.api.tree.ClassTree;
2926
import org.sonar.plugins.java.api.tree.ExpressionTree;
30-
import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
31-
import org.sonar.plugins.java.api.tree.MethodTree;
3227
import org.sonar.plugins.java.api.tree.ReturnStatementTree;
3328
import org.sonar.plugins.java.api.tree.Tree;
3429
import org.sonar.plugins.java.api.tree.TypeCastTree;
@@ -40,34 +35,25 @@ public class IntegerSubtractionInComparisonCheck extends IssuableSubscriptionVis
4035

4136
@Override
4237
public List<Tree.Kind> nodesToVisit() {
43-
return Arrays.asList(Tree.Kind.METHOD, Tree.Kind.LAMBDA_EXPRESSION);
38+
return ComparisonMethodUtils.nodesToVisit();
4439
}
4540

4641
@Override
4742
public void visitNode(Tree tree) {
48-
if (context.getSemanticModel() == null) {
49-
return;
50-
}
51-
if (tree.is(Tree.Kind.METHOD)) {
52-
MethodTree methodTree = (MethodTree) tree;
53-
if (ComparisonMethodUtils.isCompareMethod(methodTree)) {
54-
methodTree.block().accept(new ComparisonResultVisitor(methodTree.simpleName().name()));
55-
}
56-
} else {
57-
LambdaExpressionTree lambda = (LambdaExpressionTree) tree;
58-
if (ComparisonMethodUtils.isComparatorLambda(lambda)) {
43+
ComparisonMethodUtils.visitComparisonNode(context, tree,
44+
methodTree -> methodTree.block().accept(new ComparisonResultVisitor(methodTree.simpleName().name())),
45+
lambda -> {
5946
Tree body = lambda.body();
6047
ComparisonResultVisitor visitor = new ComparisonResultVisitor("compare");
6148
if (body.is(Tree.Kind.BLOCK)) {
6249
body.accept(visitor);
6350
} else {
6451
visitor.checkComparisonResult((ExpressionTree) body);
6552
}
66-
}
67-
}
53+
});
6854
}
6955

70-
private class ComparisonResultVisitor extends BaseTreeVisitor {
56+
private class ComparisonResultVisitor extends ComparisonMethodUtils.SkipNestedTypesVisitor {
7157

7258
private final String enclosingMethodName;
7359

@@ -93,16 +79,6 @@ private void checkComparisonResult(ExpressionTree expression) {
9379
reportIssue(((BinaryExpressionTree) unwrapped).operatorToken(), String.format(MESSAGE, enclosingMethodName, replacement));
9480
}
9581
}
96-
97-
@Override
98-
public void visitClass(ClassTree tree) {
99-
// Do not visit inner classes
100-
}
101-
102-
@Override
103-
public void visitLambdaExpression(LambdaExpressionTree tree) {
104-
// Do not visit nested lambdas
105-
}
10682
}
10783

10884
private static ExpressionTree skipParenthesesAndIntCasts(ExpressionTree expression) {

‎java-checks/src/main/java/org/sonar/java/checks/helpers/ComparisonMethodUtils.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,16 @@
1616
*/
1717
package org.sonar.java.checks.helpers;
1818

19+
import java.util.Arrays;
20+
import java.util.List;
21+
import java.util.function.Consumer;
22+
import org.sonar.plugins.java.api.JavaFileScannerContext;
1923
import org.sonar.plugins.java.api.semantic.MethodMatchers;
24+
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
25+
import org.sonar.plugins.java.api.tree.ClassTree;
2026
import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
2127
import org.sonar.plugins.java.api.tree.MethodTree;
28+
import org.sonar.plugins.java.api.tree.Tree;
2229

2330
public final class ComparisonMethodUtils {
2431

@@ -37,11 +44,45 @@ public final class ComparisonMethodUtils {
3744
private ComparisonMethodUtils() {
3845
}
3946

47+
public static List<Tree.Kind> nodesToVisit() {
48+
return Arrays.asList(Tree.Kind.METHOD, Tree.Kind.LAMBDA_EXPRESSION);
49+
}
50+
51+
public static void visitComparisonNode(JavaFileScannerContext context, Tree tree,
52+
Consumer<MethodTree> onCompareMethod, Consumer<LambdaExpressionTree> onComparatorLambda) {
53+
if (context.getSemanticModel() == null) {
54+
return;
55+
}
56+
if (tree.is(Tree.Kind.METHOD)) {
57+
MethodTree methodTree = (MethodTree) tree;
58+
if (isCompareMethod(methodTree)) {
59+
onCompareMethod.accept(methodTree);
60+
}
61+
} else {
62+
LambdaExpressionTree lambda = (LambdaExpressionTree) tree;
63+
if (isComparatorLambda(lambda)) {
64+
onComparatorLambda.accept(lambda);
65+
}
66+
}
67+
}
68+
4069
public static boolean isCompareMethod(MethodTree methodTree) {
4170
return methodTree.block() != null && COMPARE_METHODS.matches(methodTree);
4271
}
4372

4473
public static boolean isComparatorLambda(LambdaExpressionTree lambda) {
4574
return lambda.symbolType().isSubtypeOf("java.util.Comparator");
4675
}
76+
77+
public static class SkipNestedTypesVisitor extends BaseTreeVisitor {
78+
@Override
79+
public void visitClass(ClassTree tree) {
80+
// Do not visit inner classes
81+
}
82+
83+
@Override
84+
public void visitLambdaExpression(LambdaExpressionTree tree) {
85+
// Do not visit nested lambdas
86+
}
87+
}
4788
}

0 commit comments

Comments
 (0)