SONARJAVA-1360 Fix IllegalArgumentException when BytecodeVisitor rais…

…e an issue at file level
SonarSource · Nov 9, 2015 · f978f2f · f978f2f
1 parent 2345ae8
commit f978f2f
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 5 deletions.
diff --git a/its/plugin/projects/squid/src/main/java/package2/Class2.java b/its/plugin/projects/squid/src/main/java/package2/Class2.java
@@ -6,4 +6,8 @@ public class Class2 {
   public Class1 method() {
     return null;
   }
+
+  enum EmptyEnum {
+
+  }
 }
diff --git a/its/plugin/tests/src/test/java/com/sonar/it/java/suite/SquidTest.java b/its/plugin/tests/src/test/java/com/sonar/it/java/suite/SquidTest.java
@@ -21,7 +21,6 @@
 
 import com.sonar.orchestrator.Orchestrator;
 import com.sonar.orchestrator.build.MavenBuild;
-import org.junit.Assume;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
@@ -71,4 +70,13 @@ public void should_detect_missing_package_info() throws Exception {
     issues = issueClient.find(IssueQuery.create().components("com.sonarsource.it.samples:squid:src/test/java/package1")).list();
     assertThat(issues).isEmpty();
   }
+
+  @Test
+  public void should_not_fail_on_bytecode_visitor_issue_on_file() throws Exception {
+    IssueClient issueClient = orchestrator.getServer().wsClient().issueClient();
+    List<Issue> issues = issueClient.find(IssueQuery.create().components(JavaTestSuite.keyFor("com.sonarsource.it.samples:squid", "package2/", "Class2.java"))).list();
+    assertThat(issues).hasSize(1);
+    assertThat(issues.get(0).ruleKey()).isEqualTo("squid:UnusedPrivateMethod");
+    assertThat(issues.get(0).line()).isNull();
+  }
 }
diff --git a/its/plugin/tests/src/test/resources/com/sonar/it/java/SquidTest/squid-backup.xml b/its/plugin/tests/src/test/resources/com/sonar/it/java/SquidTest/squid-backup.xml
@@ -13,5 +13,10 @@
       <key>S1228</key>
       <priority>MAJOR</priority>
     </rule>
+    <rule>
+      <repositoryKey>squid</repositoryKey>
+      <key>UnusedPrivateMethod</key>
+      <priority>MAJOR</priority>
+    </rule>
   </rules>
-</profile>
+</profile>
diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedPrivateMethodCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedPrivateMethodCheck.java
@@ -71,7 +71,7 @@ public void visitMethod(AsmMethod asmMethod) {
         messageStr += Joiner.on(",").join(params) + ")' is never used.";
       }
       int line = getMethodLineNumber(asmMethod);
-      getContext().reportIssue(this, getSourceFile(asmClass), messageStr, line > 0 ? line : 0);
+      getContext().reportIssue(this, getSourceFile(asmClass), messageStr, line);
     }
   }
 

diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedProtectedMethodCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedProtectedMethodCheck.java
@@ -49,7 +49,7 @@ public void visitClass(AsmClass asmClass) {
   public void visitMethod(AsmMethod asmMethod) {
     if (isUnusedNonOverridenProtectedMethod(asmMethod) && !asmClass.isAbstract() && !SerializableContract.methodMatch(asmMethod)) {
       int methodLineNumber = getMethodLineNumber(asmMethod);
-      getContext().reportIssue(this, getSourceFile(asmClass), "Protected method '" + asmMethod.getName() + "(...)' is never used.", methodLineNumber > 0 ? methodLineNumber : 0);
+      getContext().reportIssue(this, getSourceFile(asmClass), "Protected method '" + asmMethod.getName() + "(...)' is never used.", methodLineNumber);
     }
   }
 

diff --git a/java-squid/src/main/java/org/sonar/java/SonarComponents.java b/java-squid/src/main/java/org/sonar/java/SonarComponents.java
@@ -192,7 +192,8 @@ public void addIssue(File file, JavaCheck check, int line, String message, @Null
         Issuable.IssueBuilder issueBuilder = issuable.newIssueBuilder()
           .ruleKey(key)
           .message(message);
-        if (line > -1) {
+        if (line > 0) {
+          // Optional line index, starting from 1. It must not be zero or negative.
           issueBuilder.line(line);
         }
         if (cost == null) {