Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🦺 Remove Non-Null Invariant from Findings #30

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
```

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
57 changes: 57 additions & 0 deletions src/main/java/io/codemodder/codetf/Finding.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
24 changes: 6 additions & 18 deletions src/main/java/io/codemodder/codetf/FixedFinding.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
36 changes: 17 additions & 19 deletions src/main/java/io/codemodder/codetf/UnfixedFinding.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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;
}
}
2 changes: 0 additions & 2 deletions src/test/java/io/codemodder/codetf/CodeTFResultTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/io/codemodder/codetf/EqualsAndHashcodeTests.java
Original file line number Diff line number Diff line change
@@ -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");
}
}
25 changes: 25 additions & 0 deletions src/test/java/io/codemodder/codetf/FixedFindingTest.java
Original file line number Diff line number Diff line change
@@ -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"));
}
}
26 changes: 26 additions & 0 deletions src/test/java/io/codemodder/codetf/UnfixedFindingTest.java
Original file line number Diff line number Diff line change
@@ -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");
}
}