Ensure that cost to fix violation cannot be set to negative or NaN value

2 files changed, 25 insertions, 5 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/Violation.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/Violation.java
index 46f9dcaf428..4cca0ad934b 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/Violation.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/Violation.java
@@ -194,8 +194,12 @@ public class Violation {
    * @since 2.4
    */
   public Violation setCost(Double d) {
-    this.cost = d;
-    return this;
+    if (d >= 0) {
+      this.cost = d;
+      return this;
+    } else {
+      throw new IllegalArgumentException("Cost to fix violation can't be negative or NaN");
+    }
   }
 
   /**
@@ -228,7 +232,7 @@ public class Violation {
   }
 
   /**
-   * Tells wether this violation is ON or OFF.
+   * Tells whether this violation is ON or OFF.
    * 
    * @since 2.8
    * @return true if the violation has been switched off
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/rules/ViolationTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/rules/ViolationTest.java
index a77bcd44cc9..2ba51cfbf7c 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/rules/ViolationTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/rules/ViolationTest.java
@@ -23,16 +23,22 @@ import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertThat;
 
+import org.junit.Before;
 import org.junit.Test;
 
 public class ViolationTest {
+  private Violation violation;
+
+  @Before
+  public void setUp() {
+    violation = Violation.create((Rule) null, null);
+  }
+
   /**
    * See http://jira.codehaus.org/browse/SONAR-2386
    */
   @Test
   public void testLineIdContract() {
-    Violation violation = Violation.create((Rule) null, null);
-
     violation.setLineId(null);
     assertThat(violation.hasLineId(), is(false));
     assertThat(violation.getLineId(), nullValue());
@@ -45,4 +51,14 @@ public class ViolationTest {
     assertThat(violation.hasLineId(), is(true));
     assertThat(violation.getLineId(), is(1));
   }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCostContract_NaN() {
+    violation.setCost(Double.NaN);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCostContract_Negative() {
+    violation.setCost(-1.0);
+  }
 }