Skip to content

Commit eba217b

Browse files
mpaladinmpaladin
committed
SONARJAVA-1429 FP in IndentationCheck when case contains a block
1 parent 6c3a23d commit eba217b

3 files changed

Lines changed: 95 additions & 38 deletions

File tree

‎its/ruling/src/test/resources/guava/squid-IndentationCheck.json‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12659,11 +12659,9 @@
1265912659
85,
1266012660
87,
1266112661
91,
12662-
93,
1266312662
94,
1266412663
96,
1266512664
98,
12666-
102,
1266712665
103,
1266812666
105,
1266912667
107,

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

Lines changed: 71 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.sonar.java.checks;
2121

2222
import com.google.common.collect.ImmutableList;
23+
import com.google.common.collect.Iterables;
2324
import org.sonar.api.server.rule.RulesDefinition;
2425
import org.sonar.check.Priority;
2526
import org.sonar.check.Rule;
@@ -32,6 +33,7 @@
3233
import org.sonar.plugins.java.api.tree.CaseGroupTree;
3334
import org.sonar.plugins.java.api.tree.CaseLabelTree;
3435
import org.sonar.plugins.java.api.tree.ClassTree;
36+
import org.sonar.plugins.java.api.tree.StatementTree;
3537
import org.sonar.plugins.java.api.tree.SyntaxToken;
3638
import org.sonar.plugins.java.api.tree.Tree;
3739
import org.sonar.plugins.java.api.tree.Tree.Kind;
@@ -104,57 +106,93 @@ public void visitNode(Tree tree) {
104106
}
105107
expectedLevel += indentationLevel;
106108
isBlockAlreadyReported = false;
107-
checkCaseGroup(tree);
108-
checkClassTree(tree);
109-
checkBlock(tree);
109+
110+
switch (tree.kind()) {
111+
case CLASS:
112+
case ENUM:
113+
case INTERFACE:
114+
case ANNOTATION_TYPE:
115+
checkClass((ClassTree) tree);
116+
break;
117+
case CASE_GROUP:
118+
checkCaseGroup((CaseGroupTree) tree);
119+
break;
120+
case BLOCK:
121+
checkBlock((BlockTree) tree);
122+
break;
123+
default:
124+
break;
125+
}
110126
}
111127

112-
private void checkCaseGroup(Tree tree) {
113-
if (tree.is(Kind.CASE_GROUP)) {
114-
List<CaseLabelTree> labels = ((CaseGroupTree) tree).labels();
115-
if (labels.size() >= 2) {
116-
CaseLabelTree previousCaseLabelTree = labels.get(labels.size() - 2);
117-
lastCheckedLine = LastSyntaxTokenFinder.lastSyntaxToken(previousCaseLabelTree).line();
118-
}
119-
checkIndentation(((CaseGroupTree) tree).body());
128+
private void checkClass(ClassTree classTree) {
129+
// Exclude anonymous classes
130+
if (classTree.simpleName() != null) {
131+
checkIndentation(classTree.members());
120132
}
121133
}
122134

123-
private void checkClassTree(Tree tree) {
124-
if (isClassTree(tree)) {
125-
ClassTree classTree = (ClassTree) tree;
126-
// Exclude anonymous classes
127-
if (classTree.simpleName() != null) {
128-
checkIndentation(classTree.members());
129-
}
135+
private void checkBlock(BlockTree blockTree) {
136+
adjustBlockForExceptionalParents(blockTree.parent());
137+
checkIndentation(blockTree.body());
138+
}
139+
140+
private void checkCaseGroup(CaseGroupTree tree) {
141+
List<CaseLabelTree> labels = tree.labels();
142+
if (labels.size() >= 2) {
143+
CaseLabelTree previousCaseLabelTree = labels.get(labels.size() - 2);
144+
lastCheckedLine = LastSyntaxTokenFinder.lastSyntaxToken(previousCaseLabelTree).line();
145+
}
146+
List<StatementTree> body = tree.body();
147+
List<StatementTree> newBody = body;
148+
int bodySize = body.size();
149+
if (bodySize > 0 && body.get(0).is(Kind.BLOCK)) {
150+
expectedLevel -= indentationLevel;
151+
checkIndentation(body.get(0), Iterables.getLast(labels).colonToken().column() + 2);
152+
newBody = body.subList(1, bodySize);
153+
}
154+
checkIndentation(newBody);
155+
if (bodySize > 0 && body.get(0).is(Kind.BLOCK)) {
156+
expectedLevel += indentationLevel;
130157
}
131158
}
132159

133-
private void checkBlock(Tree tree) {
134-
if (tree.is(Kind.BLOCK)) {
135-
if (tree.parent().is(Kind.LAMBDA_EXPRESSION)) {
136-
expectedLevel += indentationLevel;
137-
}
138-
checkIndentation(((BlockTree) tree).body());
139-
if (tree.parent().is(Kind.LAMBDA_EXPRESSION)) {
140-
expectedLevel -= indentationLevel;
141-
}
160+
private void adjustBlockForExceptionalParents(Tree parent) {
161+
if (parent.is(Kind.CASE_GROUP)) {
162+
expectedLevel -= indentationLevel;
163+
} else if (parent.is(Kind.LAMBDA_EXPRESSION)) {
164+
expectedLevel += indentationLevel;
165+
}
166+
}
167+
168+
private void restoreBlockForExceptionalParents(Tree parent) {
169+
if (parent.is(Kind.CASE_GROUP)) {
170+
expectedLevel += indentationLevel;
171+
} else if (parent.is(Kind.LAMBDA_EXPRESSION)) {
172+
expectedLevel -= indentationLevel;
142173
}
143174
}
144175

145176
private void checkIndentation(List<? extends Tree> trees) {
146177
for (Tree tree : trees) {
147-
SyntaxToken firstSyntaxToken = FirstSyntaxTokenFinder.firstSyntaxToken(tree);
148-
if (firstSyntaxToken.column() != expectedLevel && !isExcluded(tree, firstSyntaxToken.line())) {
149-
addIssue(tree, "Make this line start at column " + (expectedLevel + 1) + ".");
150-
isBlockAlreadyReported = true;
151-
}
152-
lastCheckedLine = LastSyntaxTokenFinder.lastSyntaxToken(tree).line();
178+
checkIndentation(tree, expectedLevel);
153179
}
154180
}
155181

182+
private void checkIndentation(Tree tree, int expectedLevel) {
183+
SyntaxToken firstSyntaxToken = FirstSyntaxTokenFinder.firstSyntaxToken(tree);
184+
if (firstSyntaxToken.column() != expectedLevel && !isExcluded(tree, firstSyntaxToken.line())) {
185+
addIssue(tree, "Make this line start at column " + (expectedLevel + 1) + ".");
186+
isBlockAlreadyReported = true;
187+
}
188+
lastCheckedLine = LastSyntaxTokenFinder.lastSyntaxToken(tree).line();
189+
}
190+
156191
@Override
157192
public void leaveNode(Tree tree) {
193+
if (tree.is(Kind.BLOCK)) {
194+
restoreBlockForExceptionalParents(tree.parent());
195+
}
158196
expectedLevel -= indentationLevel;
159197
isBlockAlreadyReported = false;
160198
lastCheckedLine = LastSyntaxTokenFinder.lastSyntaxToken(tree).line();

‎java-checks/src/test/files/checks/IndentationCheck_default.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ void foo() {
4747
IntStream
4848
.range(1, 5)
4949
.map(a -> {
50+
if (a == 5) {
51+
a--;
52+
}
5053
a += 1; // Noncompliant {{Make this line start at column 9.}}
5154
return a + 1;
5255
});
@@ -106,12 +109,30 @@ public void foo() {
106109
switch (foo) { // Compliant
107110
}
108111

109-
switch (foo) { // Compliant
112+
switch (foo) {
110113
case 0:
111114
case 1:
112-
case 2:
113-
case 3:
115+
case 2: {
116+
new Object().toString();
117+
new Object().toString(); // Noncompliant {{Make this line start at column 9.}}
118+
break;
119+
}
120+
case 3: {
121+
new Object().toString();
122+
}
123+
new Object().toString();
124+
break;
125+
case 4:
126+
{ // Noncompliant {{Make this line start at column 15.}}
127+
new Object().toString();
114128
break;
129+
}
130+
case 5:
131+
if (abs(x - z) == 0.5) {
132+
return x + copySign(0.5, x);
133+
} else {
134+
return z;
135+
}
115136
}
116137

117138
switch (foo) {

0 commit comments

Comments
 (0)