Issue checkstyle#199: Refactor CheckstyleAuditListener.getLine()

Return 1 if the Checkstyle event contained anything other than a real line number. Do not use null anymore for this.
checkstyle-addons · Mar 21, 2019 · d8cb1a1 · d8cb1a1
1 parent 6b0d17b
commit d8cb1a1
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 18 deletions.
diff --git a/...tyle-sonar-plugin/src/main/java/org/sonar/plugins/checkstyle/CheckstyleAuditListener.java b/...tyle-sonar-plugin/src/main/java/org/sonar/plugins/checkstyle/CheckstyleAuditListener.java
@@ -100,15 +100,10 @@ public void addError(AuditEvent event) {
             final NewIssue issue = context.newIssue();
             final ActiveRule rule = ruleFinder.find(
                     RuleKey.of(CheckstyleConstants.REPOSITORY_KEY, ruleKey));
-            final Integer lineId = getLineId(event);
-            int lineNo = 1;
-            if (lineId != null) {
-                lineNo = lineId;
-            }
             if (Objects.nonNull(issue) && Objects.nonNull(rule)) {
                 final NewIssueLocation location = issue.newLocation()
                         .on(currentResource)
-                        .at(currentResource.selectLine(lineNo))
+                        .at(currentResource.selectLine(getLineId(event)))
                         .message(message);
                 issue.forRule(rule.ruleKey())
                         .at(location)
@@ -162,14 +157,12 @@ static String getMessage(AuditEvent event) {
     }
 
     @VisibleForTesting
-    static Integer getLineId(AuditEvent event) {
-        Integer result = null;
+    static int getLineId(AuditEvent event) {
+        int result = 1;
         try {
-            final int line = event.getLine();
-            // checkstyle returns 0 if there is no relation to a file content,
-            // but we use null
-            if (line != 0) {
-                result = line;
+            final int eventLine = event.getLine();
+            if (eventLine > 0) {
+                result = eventLine;
             }
         }
         catch (Exception ex) {

diff --git a/...-sonar-plugin/src/test/java/org/sonar/plugins/checkstyle/CheckstyleAuditListenerTest.java b/...-sonar-plugin/src/test/java/org/sonar/plugins/checkstyle/CheckstyleAuditListenerTest.java
@@ -86,7 +86,7 @@ public void testUtilityMethods() {
 
         eventTest = new AuditEvent(this, "", new LocalizedMessage(0, "", "", null, "",
                 CheckstyleAuditListenerTest.class, "msg"));
-        assertThat(CheckstyleAuditListener.getLineId(eventTest)).isNull();
+        assertThat(CheckstyleAuditListener.getLineId(eventTest)).isEqualTo(1);
         assertThat(CheckstyleAuditListener.getMessage(eventTest)).isEqualTo("msg");
         assertThat(CheckstyleAuditListener.getRuleKey(eventTest)).isEqualTo(
                 CheckstyleAuditListenerTest.class.getName());
@@ -99,19 +99,33 @@ public void testUtilityMethods() {
                 CheckstyleAuditListenerTest.class.getName());
 
         eventTest = new AuditEvent(this);
-        assertThat(CheckstyleAuditListener.getLineId(eventTest)).isNull();
+        assertThat(CheckstyleAuditListener.getLineId(eventTest)).isEqualTo(1);
         assertThat(CheckstyleAuditListener.getMessage(eventTest)).isNull();
         assertThat(CheckstyleAuditListener.getRuleKey(eventTest)).isNull();
 
         eventTest = new AuditEvent(this, "", new LocalizedMessage(0, "", "", null, "module",
                 CheckstyleAuditListenerTest.class, "msg"));
-        assertThat(CheckstyleAuditListener.getLineId(eventTest)).isNull();
+        assertThat(CheckstyleAuditListener.getLineId(eventTest)).isEqualTo(1);
         assertThat(CheckstyleAuditListener.getMessage(eventTest)).isEqualTo("msg");
         assertThat(CheckstyleAuditListener.getRuleKey(eventTest)).isEqualTo("module");
     }
 
     @Test
     public void addErrorTest() {
+        addErrorTestForLine(42);
+    }
+
+    @Test
+    public void addErrorLine0Test() {
+        addErrorTestForLine(0);
+    }
+
+    @Test
+    public void addErrorLine1Test() {
+        addErrorTestForLine(1);
+    }
+
+    private void addErrorTestForLine(final int pLineNo) {
         final ActiveRule rule = setupRule("repo", "key");
 
         final NewIssue newIssue = mock(NewIssue.class);
@@ -125,9 +139,12 @@ public void addErrorTest() {
         when(newLocation.message(anyString())).thenReturn(newLocation);
 
         when(inputFile.selectLine(anyInt())).thenReturn(new DefaultTextRange(
-                new DefaultTextPointer(1, 1), new DefaultTextPointer(1, 2)));
+            new DefaultTextPointer(1, 1), new DefaultTextPointer(1, 2)));
 
-        addErrorToListener(event);
+        final AuditEvent eventAdded = new AuditEvent(this, file.getAbsolutePath(),
+            new LocalizedMessage(pLineNo, "", "", null, "", CheckstyleAuditListenerTest.class,
+				"msg"));
+        addErrorToListener(eventAdded);
 
         verify(newIssue, times(1)).save();
         verify(newIssue, times(1)).forRule(rule.ruleKey());