🦺 Remove Non-Null Invariant from Findings

See [CodeTF specification update](pixee/codemodder-specs#38)

README.md

@@ -17,15 +17,15 @@ To deserialize a CodeTF file using these objects, simply deserialize with Jackso
 
 ## Gradle
 ```kotlin
-implementation("io.codemodder:codetf-java:4.2.0")
+implementation("io.codemodder:codetf-java:4.2.1")
 ```
 
 ## Maven
 ```xml
 <dependency>
   <groupId>io.codemodder</groupId>
   <artifactId>codetf-java</artifactId>
-  <version>4.2.0</version>
+  <version>4.2.1</version>
 </dependency>
 ```
 

pom.xml

@@ -4,7 +4,7 @@
 
     <groupId>io.codemodder</groupId>
     <artifactId>codetf-java</artifactId>
-    <version>4.2.0</version>
+    <version>4.2.1</version>
 
     <name>codetf-java</name>
     <description>Jackson bindings for the Code Transformation Format (CodeTF)</description>

src/main/java/io/codemodder/codetf/Finding.java

@@ -0,0 +1,57 @@
+package io.codemodder.codetf;
+
+import java.util.Objects;
+
+/**
+ * Describes a detected finding that served as input to the codemod.
+ *
+ * <p>When a codemod is able to fix a finding, it should create a {@link FixedFinding} instance. If
+ * a codemod would typically fix findings of this type but cannot, it can create an {@link
+ * UnfixedFinding} instance to explain why.
+ *
+ * <p>Findings typically have some ID specified in the detector results.
+ */
+public abstract sealed class Finding permits FixedFinding, UnfixedFinding {
+
+  private final String id;
+  private final DetectorRule rule;
+
+  Finding(final String id, final DetectorRule rule) {
+    this.id = id;
+    this.rule = Objects.requireNonNull(rule);
+  }
+
+  Finding(final DetectorRule rule) {
+    this(null, rule);
+  }
+
+  /**
+   * @return the ID of the finding, or {@code null} if the finding has no ID
+   */
+  public String getId() {
+    return id;
+  }
+
+  /**
+   * @return the rule that detected the finding
+   */
+  public DetectorRule getRule() {
+    return rule;
+  }
+
+  @Override
+  public boolean equals(final Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+
+    final Finding finding = (Finding) o;
+    return Objects.equals(id, finding.id) && rule.equals(finding.rule);
+  }
+
+  @Override
+  public int hashCode() {
+    int result = Objects.hashCode(id);
+    result = 31 * result + rule.hashCode();
+    return result;
+  }
+}

src/main/java/io/codemodder/codetf/FixedFinding.java

@@ -2,40 +2,28 @@
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import java.util.Objects;
 
 /** Describes a fixed finding. */
-public final class FixedFinding {
-
-  private final String id;
-  private final DetectorRule rule;
+public final class FixedFinding extends Finding {
 
   @JsonCreator
   public FixedFinding(
       @JsonProperty(value = "id", index = 1) final String id,
       @JsonProperty(value = "rule", index = 2) final DetectorRule rule) {
-    this.id = CodeTFValidator.requireNonBlank(id);
-    this.rule = Objects.requireNonNull(rule);
-  }
-
-  public String getId() {
-    return id;
+    super(id, rule);
   }
 
-  public DetectorRule getRule() {
-    return rule;
+  public FixedFinding(final DetectorRule rule) {
+    super(rule);
   }
 
   @Override
   public boolean equals(final Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-    FixedFinding that = (FixedFinding) o;
-    return Objects.equals(id, that.id) && Objects.equals(rule, that.rule);
+    return super.equals(o);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(id, rule);
+    return super.hashCode();
   }
 }

src/main/java/io/codemodder/codetf/UnfixedFinding.java

@@ -5,10 +5,8 @@
 import java.util.Objects;
 
 /** Describes an unfixed finding. */
-public final class UnfixedFinding {
+public final class UnfixedFinding extends Finding {
 
-  private final String id;
-  private final DetectorRule rule;
   private final String path;
   private final Integer line;
   private final String reason;
@@ -20,15 +18,18 @@ public UnfixedFinding(
       @JsonProperty(value = "path", index = 3) final String path,
       @JsonProperty(value = "line", index = 4) final Integer line,
       @JsonProperty(value = "reason", index = 5) final String reason) {
-    this.id = CodeTFValidator.requireNonBlank(id);
-    this.rule = Objects.requireNonNull(rule);
+    super(id, rule);
     this.path = CodeTFValidator.requireNonBlank(path);
     this.line = line;
     this.reason = CodeTFValidator.requireNonBlank(reason);
   }
 
-  public String getId() {
-    return id;
+  public UnfixedFinding(
+      @JsonProperty(value = "rule", index = 2) final DetectorRule rule,
+      @JsonProperty(value = "path", index = 3) final String path,
+      @JsonProperty(value = "line", index = 4) final Integer line,
+      @JsonProperty(value = "reason", index = 5) final String reason) {
+    this(null, rule, path, line, reason);
   }
 
   public String getPath() {
@@ -39,28 +40,25 @@ public String getReason() {
     return reason;
   }
 
-  public DetectorRule getRule() {
-    return rule;
-  }
-
   public Integer getLine() {
     return line;
   }
 
   @Override
   public boolean equals(final Object o) {
     if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-    UnfixedFinding that = (UnfixedFinding) o;
-    return Objects.equals(id, that.id)
-        && Objects.equals(rule, that.rule)
-        && Objects.equals(path, that.path)
-        && Objects.equals(line, that.line)
-        && Objects.equals(reason, that.reason);
+    if (!(o instanceof final UnfixedFinding that)) return false;
+    if (!super.equals(o)) return false;
+
+    return path.equals(that.path) && Objects.equals(line, that.line) && reason.equals(that.reason);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(id, rule, path, line, reason);
+    int result = super.hashCode();
+    result = 31 * result + path.hashCode();
+    result = 31 * result + Objects.hashCode(line);
+    result = 31 * result + reason.hashCode();
+    return result;
   }
 }

src/test/java/io/codemodder/codetf/CodeTFResultTest.java

@@ -112,8 +112,6 @@ void it_creates_finding() {
     FixedFinding finding = new FixedFinding("finding", rule);
     assertEquals("finding", finding.getId());
     assertThrows(NullPointerException.class, () -> new FixedFinding("finding", null));
-    assertThrows(IllegalArgumentException.class, () -> new FixedFinding("", rule));
-    assertThrows(IllegalArgumentException.class, () -> new FixedFinding(null, rule));
   }
 
   @Test

src/test/java/io/codemodder/codetf/EqualsAndHashcodeTests.java

@@ -0,0 +1,50 @@
+package io.codemodder.codetf;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+import org.junit.jupiter.api.Test;
+
+public interface EqualsAndHashcodeTests<T> {
+
+  /**
+   * @return a new instance of the class under test
+   */
+  T createInstance();
+
+  /**
+   * @return a new instance of the class under test that is equal to the instance returned by {@link
+   *     #createInstance()}
+   */
+  default T createEqualInstance() {
+    return createInstance();
+  }
+
+  /**
+   * @return a new instance of the class under test that is different from the instance returned by
+   *     {@link #createInstance()}
+   */
+  T createDifferentInstance();
+
+  @Test
+  default void testEquals() {
+    final T instance = createInstance();
+    final T equalInstance = createEqualInstance();
+    final T differentInstance = createDifferentInstance();
+
+    assertEquals(instance, equalInstance, "Instances should be equal");
+    assertNotEquals(instance, differentInstance, "Instances should not be equal");
+    assertNotEquals(equalInstance, differentInstance, "Instances should not be equal");
+  }
+
+  @Test
+  default void testHashCode() {
+    final T instance = createInstance();
+    final T equalInstance = createEqualInstance();
+    final T differentInstance = createDifferentInstance();
+
+    assertEquals(instance.hashCode(), equalInstance.hashCode(), "Hash codes should be equal");
+    assertNotEquals(
+        instance.hashCode(), differentInstance.hashCode(), "Hash codes should not be equal");
+  }
+}

src/test/java/io/codemodder/codetf/FixedFindingTest.java

@@ -0,0 +1,25 @@
+package io.codemodder.codetf;
+
+/** Unit tests for {@link FixedFinding}. */
+final class FixedFindingTest implements EqualsAndHashcodeTests<FixedFinding> {
+
+  @Override
+  public FixedFinding createInstance() {
+    final var rule =
+        new DetectorRule(
+            "xxe",
+            "XML External Entities",
+            "https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing");
+    return new FixedFinding(rule);
+  }
+
+  @Override
+  public FixedFinding createDifferentInstance() {
+    return new FixedFinding(
+        "sql-injection/foo",
+        new DetectorRule(
+            "sqli",
+            "SQL Injection",
+            "https://owasp.org/www-community/vulnerabilities/SQL_Injection"));
+  }
+}

src/test/java/io/codemodder/codetf/UnfixedFindingTest.java

@@ -0,0 +1,26 @@
+package io.codemodder.codetf;
+
+/** Unit tests for {@link UnfixedFinding}. */
+final class UnfixedFindingTest implements EqualsAndHashcodeTests<UnfixedFinding> {
+
+  @Override
+  public UnfixedFinding createInstance() {
+    final var rule =
+        new DetectorRule(
+            "xxe",
+            "XML External Entities",
+            "https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing");
+    return new UnfixedFinding(rule, "src/main/java/com/acme/Foo.java", 42, "This is bad");
+  }
+
+  @Override
+  public UnfixedFinding createDifferentInstance() {
+    final var rule =
+        new DetectorRule(
+            "sqli",
+            "SQL Injection",
+            "https://owasp.org/www-community/vulnerabilities/SQL_Injection");
+    return new UnfixedFinding(
+        "sql-injection/foo", rule, "src/main/java/com/acme/Bar.java", 84, "This is also bad");
+  }
+}