]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-6792 Analysis fails with a meaningless ArrayIndexOutOfBounds exception if a...
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Fri, 21 Aug 2015 11:21:29 +0000 (13:21 +0200)
committerDuarte Meneses <duarte.meneses@sonarsource.com>
Fri, 21 Aug 2015 11:21:29 +0000 (13:21 +0200)
plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/XooPlugin.java
plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/InvalidIssueLineSensor.java [new file with mode: 0644]
sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java
sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java
sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssuableTest.java
sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java
sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/InvalidIssueLineMediumTest.java [new file with mode: 0644]

index 8aa011ef8736b1b01bda2f3ef94776477bbf8075..5f7b6385641a8d4de2677d2fa977d66c9476ff6a 100644 (file)
@@ -64,6 +64,7 @@ public class XooPlugin extends SonarPlugin {
       ChecksSensor.class,
       RandomAccessSensor.class,
       DeprecatedResourceApiSensor.class,
+      InvalidIssueLineSensor.class,
 
       OneIssuePerLineSensor.class,
       OneIssueOnDirPerFileSensor.class,
diff --git a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/InvalidIssueLineSensor.java b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/InvalidIssueLineSensor.java
new file mode 100644 (file)
index 0000000..94199f6
--- /dev/null
@@ -0,0 +1,80 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.xoo.rule;
+
+import org.sonar.api.issue.Issue;
+
+import org.sonar.api.component.ResourcePerspectives;
+import org.sonar.api.issue.Issuable;
+import org.sonar.xoo.Xoo;
+import org.sonar.api.batch.fs.FilePredicates;
+import org.sonar.api.batch.fs.FileSystem;
+import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.fs.InputFile.Type;
+import org.sonar.api.batch.sensor.Sensor;
+import org.sonar.api.batch.sensor.SensorContext;
+import org.sonar.api.batch.sensor.SensorDescriptor;
+import org.sonar.api.rule.RuleKey;
+
+public class InvalidIssueLineSensor implements Sensor {
+
+  public static final String RULE_KEY = "InvalidIssueLine";
+  public static final String FORCE_SEVERITY_PROPERTY = "sonar.oneIssuePerLine.forceSeverity";
+  private final ResourcePerspectives perspectives;
+  private static final RuleKey RULE_KEY_DEF = RuleKey.of(XooRulesDefinition.XOO_REPOSITORY, RULE_KEY);
+
+  public InvalidIssueLineSensor(ResourcePerspectives perspectives) {
+    this.perspectives = perspectives;
+  }
+
+  @Override
+  public void describe(SensorDescriptor descriptor) {
+    descriptor
+      .name("Invalid issue line")
+      .onlyOnLanguages(Xoo.KEY)
+      .createIssuesForRuleRepositories(XooRulesDefinition.XOO_REPOSITORY);
+  }
+
+  @Override
+  public void execute(SensorContext context) {
+    if(context.activeRules().find(RULE_KEY_DEF) == null) {
+      return;
+    }
+    
+    FileSystem fs = context.fileSystem();
+    FilePredicates p = fs.predicates();
+    for (InputFile file : fs.inputFiles(p.and(p.hasLanguages(Xoo.KEY), p.hasType(Type.MAIN)))) {
+      createIssuesOld(file, context);
+    }
+  }
+
+  private void createIssuesOld(InputFile file, SensorContext context) {
+    Issuable issuable = perspectives.as(Issuable.class, file);
+
+    Issue issue = issuable.newIssueBuilder()
+      .ruleKey(RULE_KEY_DEF)
+      .line(file.lines() + 10)
+      .message("This issue is generated on an invalid line using the old API")
+      .effortToFix(10.0)
+      .build();
+
+    issuable.addIssue(issue);
+  }
+}
index 578bb2bb49ef97af435c0b7ee2985ae9f47d534f..f485017797159ab4cae6f8b7a81bcf45b17d2645 100644 (file)
@@ -19,6 +19,9 @@
  */
 package org.sonar.batch.issue;
 
+import org.sonar.batch.scan.filesystem.InputPathCache;
+
+import org.sonar.api.batch.fs.InputFile;
 import com.google.common.collect.Lists;
 import org.sonar.api.component.Component;
 import org.sonar.api.issue.Issuable;
@@ -38,12 +41,14 @@ public class DefaultIssuable implements Issuable {
   private final IssueCache cache;
   private final Component component;
   private final Project project;
+  private final InputPathCache inputPathCache;
 
-  DefaultIssuable(Component component, Project project, ModuleIssues moduleIssues, IssueCache cache) {
+  DefaultIssuable(Component component, Project project, ModuleIssues moduleIssues, IssueCache cache, InputPathCache inputPathCache) {
     this.component = component;
     this.project = project;
     this.moduleIssues = moduleIssues;
     this.cache = cache;
+    this.inputPathCache = inputPathCache;
   }
 
   @Override
@@ -53,15 +58,38 @@ public class DefaultIssuable implements Issuable {
 
   @Override
   public boolean addIssue(Issue issue) {
+    validateIssueLine(issue);
     return moduleIssues.initAndAddIssue((DefaultIssue) issue);
   }
 
+  private void validateIssueLine(Issue issue) {
+    if (issue.line() == null) {
+      return;
+    }
+    String moduleKey = issue.projectKey();
+    String path = component.path();
+
+    InputFile file = inputPathCache.getFile(moduleKey, path);
+
+    if (issue.line() <= 0) {
+      invalidateIssue(issue, "must be > 0");
+    }
+
+    if (file != null && issue.line() > file.lines()) {
+      invalidateIssue(issue, "must be <= " + file.lines());
+    }
+  }
+
+  private void invalidateIssue(Issue issue, String msg) {
+    throw new IllegalStateException(String.format("Invalid line %s (%s) in issue for '%s' created by the rule '%s'", issue.line(), msg, issue.componentKey(), issue.ruleKey()));
+  }
+
   @SuppressWarnings("unchecked")
   @Override
   public List<Issue> resolvedIssues() {
     List<Issue> result = Lists.newArrayList();
     for (DefaultIssue issue : cache.byComponent(component.key())) {
-      if (issue.resolution()!=null) {
+      if (issue.resolution() != null) {
         result.add(issue);
       }
     }
@@ -73,7 +101,7 @@ public class DefaultIssuable implements Issuable {
   public List<Issue> issues() {
     List<Issue> result = Lists.newArrayList();
     for (DefaultIssue issue : cache.byComponent(component.key())) {
-      if (issue.resolution()==null) {
+      if (issue.resolution() == null) {
         result.add(issue);
       }
     }
index 7491bbb78144e523232b3ae4923da67cf8ad08b4..5f060d99e2712b87c36d7e9fb68bfdbcdb6c3cfc 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.batch.issue;
 
+import org.sonar.batch.scan.filesystem.InputPathCache;
+
 import org.sonar.api.component.Component;
 import org.sonar.api.issue.Issuable;
 import org.sonar.api.resources.Scopes;
@@ -37,12 +39,14 @@ public class IssuableFactory extends PerspectiveBuilder<Issuable> {
   private final ModuleIssues moduleIssues;
   private final IssueCache cache;
   private final ProjectTree projectTree;
+  private final InputPathCache inputPathCache;
 
-  public IssuableFactory(ModuleIssues moduleIssues, IssueCache cache, ProjectTree projectTree) {
+  public IssuableFactory(ModuleIssues moduleIssues, IssueCache cache, ProjectTree projectTree, InputPathCache inputPathCache) {
     super(Issuable.class);
     this.moduleIssues = moduleIssues;
     this.cache = cache;
     this.projectTree = projectTree;
+    this.inputPathCache = inputPathCache;
   }
 
   @CheckForNull
@@ -52,6 +56,6 @@ public class IssuableFactory extends PerspectiveBuilder<Issuable> {
     if (component instanceof ResourceComponent) {
       supported = Scopes.isHigherThanOrEquals(((ResourceComponent) component).scope(), Scopes.FILE);
     }
-    return supported ? new DefaultIssuable(component, projectTree.getRootProject(), moduleIssues, cache) : null;
+    return supported ? new DefaultIssuable(component, projectTree.getRootProject(), moduleIssues, cache, inputPathCache) : null;
   }
 }
index 64f7e8d2446f31038ef97969a879d7527155ab2a..330d559e9bd55c57939295d3e5f5b1246c8fc229 100644 (file)
@@ -19,6 +19,9 @@
  */
 package org.sonar.batch.issue;
 
+import com.google.common.base.Preconditions;
+import org.sonar.api.batch.fs.InputFile;
+
 import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import org.sonar.api.batch.debt.DebtRemediationFunction;
index 26d1fccd0601dfe58aedb56e8145744fc4dae92d..6b71578a345313576d431b4422199fc4c000c5ba 100644 (file)
  */
 package org.sonar.batch.issue;
 
+import org.junit.rules.ExpectedException;
+import org.junit.Rule;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.api.batch.fs.InputFile;
+import org.sonar.batch.scan.filesystem.InputPathCache;
 import org.junit.Test;
 import org.sonar.api.component.Component;
 import org.sonar.api.issue.Issue;
@@ -28,16 +33,22 @@ import org.sonar.api.resources.Project;
 import java.util.Arrays;
 import java.util.List;
 
+import static org.mockito.Mockito.verify;
+
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class DefaultIssuableTest {
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
 
   ModuleIssues moduleIssues = mock(ModuleIssues.class);
   IssueCache cache = mock(IssueCache.class);
   Project project = mock(Project.class);
   Component component = mock(Component.class);
+  InputPathCache inputPathCache = mock(InputPathCache.class);
 
   @Test
   public void test_unresolved_issues() throws Exception {
@@ -46,12 +57,55 @@ public class DefaultIssuableTest {
     DefaultIssue unresolved = new DefaultIssue();
     when(cache.byComponent("struts:org.apache.Action")).thenReturn(Arrays.asList(resolved, unresolved));
 
-    DefaultIssuable perspective = new DefaultIssuable(component, project, moduleIssues, cache);
+    DefaultIssuable perspective = new DefaultIssuable(component, project, moduleIssues, cache, inputPathCache);
 
     List<Issue> issues = perspective.issues();
     assertThat(issues).containsOnly(unresolved);
   }
 
+  @Test
+  public void validateIssueLine() {
+    InputFile file = mock(InputFile.class);
+
+    when(file.lines()).thenReturn(999);
+    when(component.path()).thenReturn("file");
+    when(inputPathCache.getFile("module", "file")).thenReturn(file);
+
+    DefaultIssue issue = new DefaultIssue()
+      .setLine(1000)
+      .setProjectKey("module")
+      .setRuleKey(RuleKey.of("repo", "rule"))
+      .setComponentKey("componentKey");
+    DefaultIssuable issuable = new DefaultIssuable(component, project, moduleIssues, cache, inputPathCache);
+
+    exception.expect(IllegalStateException.class);
+    exception.expectMessage("Invalid line 1000 (must be <= 999) in issue for 'componentKey' created by the rule 'repo:rule'");
+    issuable.addIssue(issue);
+  }
+
+  @Test
+  public void dontValidateNullLine() {
+    DefaultIssuable issuable = new DefaultIssuable(component, project, moduleIssues, cache, inputPathCache);
+
+    DefaultIssue issue = new DefaultIssue()
+      .setLine(null);
+    issuable.addIssue(issue);
+    verifyNoMoreInteractions(inputPathCache);
+  }
+
+  @Test
+  public void dontValidateUnknownFiles() {
+    DefaultIssuable issuable = new DefaultIssuable(component, project, moduleIssues, cache, inputPathCache);
+    when(component.path()).thenReturn("file");
+
+    DefaultIssue issue = new DefaultIssue()
+      .setLine(1505)
+      .setProjectKey("module");
+
+    issuable.addIssue(issue);
+    verify(inputPathCache).getFile("module", "file");
+  }
+
   @Test
   public void test_resolved_issues() throws Exception {
     when(component.key()).thenReturn("struts:org.apache.Action");
@@ -59,7 +113,7 @@ public class DefaultIssuableTest {
     DefaultIssue unresolved = new DefaultIssue();
     when(cache.byComponent("struts:org.apache.Action")).thenReturn(Arrays.asList(resolved, unresolved));
 
-    DefaultIssuable perspective = new DefaultIssuable(component, project, moduleIssues, cache);
+    DefaultIssuable perspective = new DefaultIssuable(component, project, moduleIssues, cache, inputPathCache);
 
     List<Issue> issues = perspective.resolvedIssues();
     assertThat(issues).containsOnly(resolved);
index 514a6d05a1bc91b5d584e5b1402792862baf31a1..7d9fa1218472ddb62d27c85434669e8e2688e728 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.batch.issue;
 
+import org.sonar.batch.scan.filesystem.InputPathCache;
+
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.sonar.api.component.Component;
@@ -27,7 +29,6 @@ import org.sonar.api.resources.File;
 import org.sonar.api.resources.Project;
 import org.sonar.batch.ProjectTree;
 import org.sonar.core.component.ResourceComponent;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 
@@ -36,10 +37,12 @@ public class IssuableFactoryTest {
   ModuleIssues moduleIssues = mock(ModuleIssues.class);
   IssueCache cache = mock(IssueCache.class, Mockito.RETURNS_MOCKS);
   ProjectTree projectTree = mock(ProjectTree.class);
+  InputPathCache inputPathCache = mock(InputPathCache.class);
+  
 
   @Test
   public void file_should_be_issuable() throws Exception {
-    IssuableFactory factory = new IssuableFactory(moduleIssues, cache, projectTree);
+    IssuableFactory factory = new IssuableFactory(moduleIssues, cache, projectTree, inputPathCache);
     Component component = new ResourceComponent(File.create("foo/bar.c").setEffectiveKey("foo/bar.c"));
     Issuable issuable = factory.loadPerspective(Issuable.class, component);
 
@@ -50,7 +53,7 @@ public class IssuableFactoryTest {
 
   @Test
   public void project_should_be_issuable() throws Exception {
-    IssuableFactory factory = new IssuableFactory(moduleIssues, cache, projectTree);
+    IssuableFactory factory = new IssuableFactory(moduleIssues, cache, projectTree, inputPathCache);
     Component component = new ResourceComponent(new Project("Foo").setEffectiveKey("foo"));
     Issuable issuable = factory.loadPerspective(Issuable.class, component);
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/InvalidIssueLineMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/InvalidIssueLineMediumTest.java
new file mode 100644 (file)
index 0000000..668799d
--- /dev/null
@@ -0,0 +1,69 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.batch.mediumtest.issues;
+
+import org.junit.Rule;
+
+import org.junit.rules.ExpectedException;
+
+import java.io.File;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.sonar.batch.mediumtest.BatchMediumTester;
+import org.sonar.batch.protocol.input.ActiveRule;
+import org.sonar.xoo.XooPlugin;
+
+public class InvalidIssueLineMediumTest {
+  @Rule
+  public TemporaryFolder temp = new TemporaryFolder();
+  
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
+  public BatchMediumTester tester = BatchMediumTester.builder()
+    .registerPlugin("xoo", new XooPlugin())
+    .addDefaultQProfile("xoo", "Sonar Way")
+    .activateRule(new ActiveRule("xoo", "InvalidIssueLine", null, "Invalid issue line", "MAJOR", "InvalidIssueLine.internal", "xoo"))
+    .build();
+
+  @Before
+  public void prepare() {
+    tester.start();
+  }
+
+  @After
+  public void stop() {
+    tester.stop();
+  }
+
+  @Test
+  public void testInvalidIssueLine() throws Exception {
+    File projectDir = new File(IssuesMediumTest.class.getResource("/mediumtest/xoo/sample").toURI());
+
+    exception.expect(IllegalStateException.class);
+    exception.expectMessage("Invalid line 18 (must be <= 8) in issue for 'sample:xources/hello/HelloJava.xoo' created by the rule 'xoo:InvalidIssueLine'");
+    tester
+      .newScanTask(new File(projectDir, "sonar-project.properties"))
+      .start();
+  }
+}