From 2113095efa0eafbec64d4392715efc871d2d5275 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Fri, 21 Aug 2015 13:21:29 +0200 Subject: [PATCH] SONAR-6792 Analysis fails with a meaningless ArrayIndexOutOfBounds exception if a rule try to log an issue on a non-existing line --- .../main/java/org/sonar/xoo/XooPlugin.java | 1 + .../xoo/rule/InvalidIssueLineSensor.java | 80 +++++++++++++++++++ .../sonar/batch/issue/DefaultIssuable.java | 34 +++++++- .../sonar/batch/issue/IssuableFactory.java | 8 +- .../org/sonar/batch/issue/ModuleIssues.java | 3 + .../batch/issue/DefaultIssuableTest.java | 58 +++++++++++++- .../batch/issue/IssuableFactoryTest.java | 9 ++- .../issues/InvalidIssueLineMediumTest.java | 69 ++++++++++++++++ 8 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/InvalidIssueLineSensor.java create mode 100644 sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/InvalidIssueLineMediumTest.java diff --git a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/XooPlugin.java b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/XooPlugin.java index 8aa011ef873..5f7b6385641 100644 --- a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/XooPlugin.java +++ b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/XooPlugin.java @@ -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 index 00000000000..94199f65556 --- /dev/null +++ b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/InvalidIssueLineSensor.java @@ -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); + } +} diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java index 578bb2bb49e..f4850177971 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java @@ -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 resolvedIssues() { List 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 issues() { List result = Lists.newArrayList(); for (DefaultIssue issue : cache.byComponent(component.key())) { - if (issue.resolution()==null) { + if (issue.resolution() == null) { result.add(issue); } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java index 7491bbb7814..5f060d99e27 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java @@ -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 { 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 { 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; } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java index 64f7e8d2446..330d559e9bd 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java @@ -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; diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssuableTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssuableTest.java index 26d1fccd060..6b71578a345 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssuableTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssuableTest.java @@ -19,6 +19,11 @@ */ 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 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 issues = perspective.resolvedIssues(); assertThat(issues).containsOnly(resolved); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java index 514a6d05a1b..7d9fa121847 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java @@ -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 index 00000000000..668799d6350 --- /dev/null +++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/InvalidIssueLineMediumTest.java @@ -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(); + } +} -- 2.39.5