Skip to content

Commit d786530

Browse files
mpaladinmpaladin
committed
SONARJAVA-1285 JavaCheckVerifier: Add support for effortToFix and precise location assertions
1 parent 6788e93 commit d786530

8 files changed

Lines changed: 294 additions & 26 deletions

‎java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/JavaCheckVerifier.java‎

Lines changed: 143 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,18 @@
2020
package org.sonar.java.checks.verifier;
2121

2222
import com.google.common.base.Preconditions;
23+
import com.google.common.base.Splitter;
2324
import com.google.common.collect.ArrayListMultimap;
25+
import com.google.common.collect.HashMultiset;
2426
import com.google.common.collect.ImmutableList;
27+
import com.google.common.collect.ImmutableMap;
2528
import com.google.common.collect.Iterables;
2629
import com.google.common.collect.Lists;
30+
import com.google.common.collect.Multiset;
2731
import org.apache.commons.io.FileUtils;
2832
import org.apache.commons.lang.StringUtils;
2933
import org.fest.assertions.Fail;
34+
import org.sonar.java.AnalyzerMessage;
3035
import org.sonar.java.ast.JavaAstScanner;
3136
import org.sonar.java.ast.visitors.SubscriptionVisitor;
3237
import org.sonar.java.model.VisitorsBridge;
@@ -38,10 +43,13 @@
3843
import org.sonar.squidbridge.api.SourceCode;
3944

4045
import java.io.File;
46+
import java.util.ArrayList;
4147
import java.util.Collection;
4248
import java.util.Collections;
49+
import java.util.EnumMap;
4350
import java.util.List;
4451
import java.util.Locale;
52+
import java.util.Map;
4553
import java.util.Set;
4654

4755
import static org.fest.assertions.Assertions.assertThat;
@@ -55,6 +63,18 @@
5563
* // Noncompliant@+1 {{do not import "java.util.List"}}
5664
* import java.util.List;
5765
* </pre>
66+
* Full syntax:
67+
* <pre>
68+
* // Noncompliant@+1 [[startColumn=1;endLine=+1;endColumn=2;effortToFix=4;secondary=3,4]] {{issue message}}
69+
* </pre>
70+
* Attributes between [[]] are optional:
71+
* <ul>
72+
* <li>startColumn: column where the highlight starts</li>
73+
* <li>endLine: relative endLine where the highlight ends (i.e. +1), same line if omitted</li>
74+
* <li>endColumn: column where the highlight ends</li>
75+
* <li>effortToFix: the cost to fix as integer</li>
76+
* <li>secondary: a comma separated list of integers identifying the lines of secondary locations if any</li>
77+
* </ul>
5878
*/
5979
public class JavaCheckVerifier extends SubscriptionVisitor {
6080

@@ -63,12 +83,33 @@ public class JavaCheckVerifier extends SubscriptionVisitor {
6383
*/
6484
private static final String DEFAULT_TEST_JARS_DIRECTORY = "target/test-jars";
6585
private static final String TRIGGER = "// Noncompliant";
66-
private ArrayListMultimap<Integer, String> expected = ArrayListMultimap.create();
86+
private final ArrayListMultimap<Integer, Map<IssueAttribute, String>> expected = ArrayListMultimap.create();
6787
private String testJarsDirectory;
6888
private boolean expectNoIssues = false;
6989
private String expectFileIssue;
7090
private Integer expectFileIssueOnline;
7191

92+
private static final Map<String, IssueAttribute> ATTRIBUTE_MAP = ImmutableMap.<String, IssueAttribute>builder()
93+
.put("message", IssueAttribute.MESSAGE)
94+
.put("effortToFix", IssueAttribute.EFFORT_TO_FIX)
95+
.put("sc", IssueAttribute.START_COLUMN)
96+
.put("startColumn", IssueAttribute.START_COLUMN)
97+
.put("el", IssueAttribute.END_LINE)
98+
.put("endLine", IssueAttribute.END_LINE)
99+
.put("ec", IssueAttribute.END_COLUMN)
100+
.put("endColumn", IssueAttribute.END_COLUMN)
101+
.put("secondary", IssueAttribute.SECONDARY_LOCATIONS)
102+
.build();
103+
104+
enum IssueAttribute {
105+
MESSAGE,
106+
START_COLUMN,
107+
END_COLUMN,
108+
END_LINE,
109+
EFFORT_TO_FIX,
110+
SECONDARY_LOCATIONS
111+
}
112+
72113
private JavaCheckVerifier() {
73114
this.testJarsDirectory = DEFAULT_TEST_JARS_DIRECTORY;
74115
}
@@ -147,7 +188,7 @@ private static void scanFile(String filename, JavaFileScanner check, JavaCheckVe
147188
if (testJars.exists()) {
148189
classpath = FileUtils.listFiles(testJars, new String[]{"jar", "zip"}, true);
149190
} else if (!DEFAULT_TEST_JARS_DIRECTORY.equals(javaCheckVerifier.testJarsDirectory)) {
150-
throw Fail.fail("The directory to be used to extend class path does not exists (" + testJars.getAbsolutePath() + ").");
191+
Fail.fail("The directory to be used to extend class path does not exists (" + testJars.getAbsolutePath() + ").");
151192
}
152193
classpath.add(new File("target/test-classes"));
153194
JavaAstScanner.scanSingleFile(new File(filename), new VisitorsBridge(Lists.newArrayList(check, javaCheckVerifier), Lists.newArrayList(classpath), null));
@@ -171,10 +212,15 @@ private void collectExpectedIssues(String comment, int line) {
171212
if (comment.startsWith(TRIGGER)) {
172213
String cleanedComment = StringUtils.remove(comment, TRIGGER);
173214

215+
EnumMap<IssueAttribute, String> attr = new EnumMap<>(IssueAttribute.class);
174216
String expectedMessage = StringUtils.substringBetween(cleanedComment, "{{", "}}");
217+
if (StringUtils.isNotEmpty(expectedMessage)) {
218+
attr.put(IssueAttribute.MESSAGE, expectedMessage);
219+
}
175220
int expectedLine = line;
221+
String attributesSubstr = extractAttributes(comment, attr);
176222

177-
cleanedComment = StringUtils.stripEnd(StringUtils.remove(cleanedComment, "{{" + expectedMessage + "}}"), " \t");
223+
cleanedComment = StringUtils.stripEnd(StringUtils.remove(StringUtils.remove(cleanedComment, "[[" + attributesSubstr + "]]"), "{{" + expectedMessage + "}}"), " \t");
178224
if (StringUtils.startsWith(cleanedComment, "@")) {
179225
final int lineAdjustment;
180226
final char firstChar = cleanedComment.charAt(1);
@@ -189,13 +235,42 @@ private void collectExpectedIssues(String comment, int line) {
189235
} else if (firstChar == '-') {
190236
expectedLine -= lineAdjustment;
191237
} else {
192-
throw Fail.fail("Use only '@+N' or '@-N' to shifts messages.");
238+
Fail.fail("Use only '@+N' or '@-N' to shifts messages.");
193239
}
194240
}
195-
expected.put(expectedLine, expectedMessage);
241+
updateEndLine(expectedLine, attr);
242+
expected.put(expectedLine, attr);
243+
}
244+
}
245+
246+
private static void updateEndLine(int expectedLine, EnumMap<IssueAttribute, String> attr) {
247+
if (attr.containsKey(IssueAttribute.END_LINE)) {
248+
String endLineStr = attr.get(IssueAttribute.END_LINE);
249+
if (endLineStr.startsWith("+")) {
250+
int endLine = Integer.parseInt(endLineStr);
251+
attr.put(IssueAttribute.END_LINE, Integer.toString(expectedLine + endLine));
252+
} else {
253+
Fail.fail("endLine attribute should be relative to the line and must be +N with N integer");
254+
}
196255
}
197256
}
198257

258+
private static String extractAttributes(String comment, Map<IssueAttribute, String> attr) {
259+
String attributesSubstr = StringUtils.substringBetween(comment, "[[", "]]");
260+
if (!StringUtils.isEmpty(attributesSubstr)) {
261+
Iterable<String> attributes = Splitter.on(";").split(attributesSubstr);
262+
for (String attribute : attributes) {
263+
String[] split = StringUtils.split(attribute, '=');
264+
if (split.length == 2 && ATTRIBUTE_MAP.containsKey(split[0])) {
265+
attr.put(ATTRIBUTE_MAP.get(split[0]), split[1]);
266+
} else {
267+
Fail.fail("// Noncompliant attributes not valid: " + attributesSubstr);
268+
}
269+
}
270+
}
271+
return attributesSubstr;
272+
}
273+
199274
@Override
200275
public void scanFile(JavaFileScannerContext context) {
201276
expected.clear();
@@ -220,22 +295,74 @@ private void assertMultipleIssue(SourceCode sourceCode) throws AssertionError {
220295
Preconditions.checkState(sourceCode.hasCheckMessages(), "At least one issue expected");
221296
List<Integer> unexpectedLines = Lists.newLinkedList();
222297
for (CheckMessage checkMessage : sourceCode.getCheckMessages()) {
223-
int line = checkMessage.getLine();
224-
if (!expected.containsKey(line)) {
225-
unexpectedLines.add(line);
226-
} else {
227-
List<String> list = expected.get(line);
228-
String expectedMessage = list.remove(list.size() - 1);
229-
if (expectedMessage != null) {
230-
assertThat(checkMessage.getText(Locale.US)).isEqualTo(expectedMessage);
231-
}
232-
}
298+
validateIssue(unexpectedLines, checkMessage);
233299
}
234300
if (!expected.isEmpty() || !unexpectedLines.isEmpty()) {
235301
Collections.sort(unexpectedLines);
236302
String expectedMsg = !expected.isEmpty() ? ("Expected " + expected) : "";
237303
String unexpectedMsg = !unexpectedLines.isEmpty() ? ((expectedMsg.isEmpty() ? "" : ", ") + "Unexpected at " + unexpectedLines) : "";
238-
throw Fail.fail(expectedMsg + unexpectedMsg);
304+
Fail.fail(expectedMsg + unexpectedMsg);
305+
}
306+
}
307+
308+
private void validateIssue(List<Integer> unexpectedLines, CheckMessage checkMessage) {
309+
int line = checkMessage.getLine();
310+
if (expected.containsKey(line)) {
311+
Map<IssueAttribute, String> attrs = Iterables.getLast(expected.get(line));
312+
assertEquals(checkMessage.getText(Locale.US), attrs, IssueAttribute.MESSAGE);
313+
Double cost = checkMessage.getCost();
314+
if (cost != null) {
315+
assertEquals(Integer.toString(cost.intValue()), attrs, IssueAttribute.EFFORT_TO_FIX);
316+
}
317+
if (checkMessage instanceof org.sonar.java.JavaCheckMessage) {
318+
AnalyzerMessage analyzerMessage = ((org.sonar.java.JavaCheckMessage) checkMessage).getAnalyzerMessage();
319+
if (analyzerMessage != null) {
320+
validateAnalyzerMessage(attrs, analyzerMessage);
321+
}
322+
}
323+
expected.remove(line, attrs);
324+
} else {
325+
unexpectedLines.add(line);
326+
}
327+
}
328+
329+
private static void validateAnalyzerMessage(Map<IssueAttribute, String> attrs, AnalyzerMessage analyzerMessage) {
330+
Double effortToFix = analyzerMessage.getCost();
331+
if (effortToFix != null) {
332+
assertEquals(Integer.toString(effortToFix.intValue()), attrs, IssueAttribute.EFFORT_TO_FIX);
333+
}
334+
AnalyzerMessage.TextSpan textSpan = analyzerMessage.primaryLocation();
335+
assertEquals(normalizeColumn(textSpan.startCharacter), attrs, IssueAttribute.START_COLUMN);
336+
assertEquals(Integer.toString(textSpan.endLine), attrs, IssueAttribute.END_LINE);
337+
assertEquals(normalizeColumn(textSpan.endCharacter), attrs, IssueAttribute.END_COLUMN);
338+
if (attrs.containsKey(IssueAttribute.SECONDARY_LOCATIONS)) {
339+
List<AnalyzerMessage> secondaryLocations = analyzerMessage.secondaryLocations;
340+
Multiset<String> actualLines = HashMultiset.create();
341+
for (AnalyzerMessage secondaryLocation : secondaryLocations) {
342+
actualLines.add(Integer.toString(secondaryLocation.getLine()));
343+
}
344+
List<String> expected = Lists.newArrayList(Splitter.on(",").omitEmptyStrings().trimResults().split(attrs.get(IssueAttribute.SECONDARY_LOCATIONS)));
345+
List<String> unexpected = new ArrayList<>();
346+
for (String actualLine : actualLines) {
347+
if (expected.contains(actualLine)) {
348+
expected.remove(actualLine);
349+
} else {
350+
unexpected.add(actualLine);
351+
}
352+
}
353+
if (!expected.isEmpty() || !unexpected.isEmpty()) {
354+
Fail.fail("Secondary locations: expected: " + expected + " unexpected:" + unexpected);
355+
}
356+
}
357+
}
358+
359+
private static String normalizeColumn(int startCharacter) {
360+
return Integer.toString(startCharacter + 1);
361+
}
362+
363+
private static void assertEquals(String value, Map<IssueAttribute, String> attributes, IssueAttribute attribute) {
364+
if (attributes.containsKey(attribute)) {
365+
assertThat(value).as("attribute mismatch for " + attribute + ": " + attributes).isEqualTo(attributes.get(attribute));
239366
}
240367
}
241368

‎java-checks-testkit/src/test/files/JavaCheckVerifier.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ void foo() { // test method
77
int j;
88
int k;
99
// Noncompliant@-1 {{message3}}
10-
int l; // Noncompliant {{message4}}
11-
int m; // Noncompliant
10+
int l; // Noncompliant {{message4}} [[sc=9;endColumn=10;secondary=3,4]] bla bla bla
11+
int m; // Noncompliant [[effortToFix=4]]
12+
int m; // Noncompliant [[effortToFix=4]]
13+
// Noncompliant@-5
14+
System // Noncompliant [[sc==5;el=+1;ec=11]]
15+
.out.println();
16+
// Noncompliant@+1 blabla
17+
int z;
1218
}
1319
}

‎java-checks-testkit/src/test/files/JavaCheckVerifierIncorrectAttribute.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
class A { // Noncompliant [[invalid=1]] {{message}}
2+
}

‎java-checks-testkit/src/test/files/JavaCheckVerifierIncorrectAttribute2.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
class A { // Noncompliant [[invalid=1=2]] {{message}}
2+
}

‎java-checks-testkit/src/test/files/JavaCheckVerifierIncorrectEndLine.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
class A { // Noncompliant [[endLine=-1]] {{message}}
2+
}

‎java-checks-testkit/src/test/files/JavaCheckVerifierIncorrectSecondaryLocation.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
class A { // Noncompliant {{message}}
2+
3+
int i; // Noncompliant {{message1}}
4+
5+
void foo() { // test method
6+
// Noncompliant@+1 {{message2}}
7+
int j;
8+
int k;
9+
// Noncompliant@-1 {{message3}}
10+
int l; // Noncompliant {{message4}} [[sc=9;endColumn=10;secondary=4]] bla bla bla
11+
int m; // Noncompliant [[effortToFix=4]]
12+
int m; // Noncompliant [[effortToFix=4]]
13+
// Noncompliant@-5
14+
System // Noncompliant [[sc==5;el=+1;ec=11]]
15+
.out.println();
16+
// Noncompliant@+1 blabla
17+
int z;
18+
}
19+
}

‎java-checks-testkit/src/test/files/JavaCheckVerifierIncorrectSecondaryLocation2.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
class A { // Noncompliant {{message}}
2+
3+
int i; // Noncompliant {{message1}}
4+
5+
void foo() { // test method
6+
// Noncompliant@+1 {{message2}}
7+
int j;
8+
int k;
9+
// Noncompliant@-1 {{message3}}
10+
int l; // Noncompliant {{message4}} [[sc=9;endColumn=10;secondary=3,4,5]] bla bla bla
11+
int m; // Noncompliant [[effortToFix=4]]
12+
int m; // Noncompliant [[effortToFix=4]]
13+
// Noncompliant@-5
14+
System // Noncompliant [[sc==5;el=+1;ec=11]]
15+
.out.println();
16+
// Noncompliant@+1 blabla
17+
int z;
18+
}
19+
}

0 commit comments

Comments
 (0)