diff --git a/README.md b/README.md index 74b9247..2f68928 100644 --- a/README.md +++ b/README.md @@ -225,3 +225,7 @@ Unresolvable methods (no coverage information available): 1. usage of immutable data structures only 1. for collections defensive copies and `Collections.unmodifiableXzy(...)` in constructor 1. other fields or elements in collections have to be immutable +1. usage of `Optional` (was actually designed for method return types only, but is the only JDK built-in way to indicate a nullable value (all `@Nullable` annotations are from third-party libraries)) + 1. never pass or return `null`, use `Optional` instead (this eliminates the need for `null` checks everywhere) + 1. but prefer method overloading or usage of a builder over `Optional` method parameters + 1. even use `Optional` for class fields (avoids unnecessary `Optional.ofNullable(...)` calls in getter of immutable objects) diff --git a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/OptionalTypeAdapter.java b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/OptionalTypeAdapter.java new file mode 100644 index 0000000..7bd068d --- /dev/null +++ b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/OptionalTypeAdapter.java @@ -0,0 +1,31 @@ +package com.scheible.testgapanalysis.maven; + +import com.google.gson.JsonDeserializationContext; +import com.google.gson.JsonDeserializer; +import com.google.gson.JsonElement; +import com.google.gson.JsonParseException; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; +import java.lang.reflect.Type; +import java.util.Optional; + +/** + * Unfortunately Gson has no built-in support for Optional. + * + * @author sj + */ +class OptionalTypeAdapter implements JsonDeserializer>, JsonSerializer> { + + OptionalTypeAdapter() { + } + + @Override + public Optional deserialize(final JsonElement element, final Type type, final JsonDeserializationContext context) throws JsonParseException { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public JsonElement serialize(final Optional value, final Type type, final JsonSerializationContext context) { + return context.serialize(value.orElse(null)); + } +} diff --git a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java index 60eecf7..0d2e2a2 100644 --- a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java +++ b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java @@ -105,7 +105,8 @@ private void logReport(final TestGapReport report) { private void writeJsonReport(final TestGapReport report) throws MojoExecutionException { final File reportFile = new File(buildDir, "test-gap-report.json"); - final Gson gson = new GsonBuilder().disableHtmlEscaping().setPrettyPrinting().create(); + final Gson gson = new GsonBuilder().disableHtmlEscaping().setPrettyPrinting() + .registerTypeAdapter(Optional.class, new OptionalTypeAdapter()).create(); try { Files.write(reportFile.toPath(), gson.toJson(report).getBytes()); @@ -114,3 +115,4 @@ private void writeJsonReport(final TestGapReport report) throws MojoExecutionExc } } } + diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/testgap/TestGapReport.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/testgap/TestGapReport.java index 1e706a5..382f3e1 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/testgap/TestGapReport.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/testgap/TestGapReport.java @@ -58,8 +58,8 @@ public static class TestGapMethod { private final int sourceLine; private final int sourceColumn; - private final String coveredMethodName; - private final Integer coveredMethodLine; + private final Optional coveredMethodName; + private final Optional coveredMethodLine; private TestGapMethod(final String topLevelTypeFqn, final String description, final int sourceLine, final int sourceColumn, final String coveredMethodName, final Integer coveredMethodLine) { @@ -68,8 +68,8 @@ private TestGapMethod(final String topLevelTypeFqn, final String description, fi this.sourceLine = sourceLine; this.sourceColumn = sourceColumn; - this.coveredMethodName = coveredMethodName; - this.coveredMethodLine = coveredMethodLine; + this.coveredMethodName = Optional.ofNullable(coveredMethodName); + this.coveredMethodLine = Optional.ofNullable(coveredMethodLine); } public TestGapMethod(final String topLevelTypeFqn, final String description, final int sourceLine, @@ -100,17 +100,17 @@ public int getSourceColumn() { } public Optional getCoveredMethodName() { - return Optional.ofNullable(coveredMethodName); + return coveredMethodName; } public Optional getCoveredMethodLine() { - return Optional.ofNullable(coveredMethodLine); + return coveredMethodLine; } @Override public String toString() { - final String coverageInfo = coveredMethodName != null && coveredMethodLine != null - ? String.format(" resolved to '%s' with line %d", coveredMethodName, coveredMethodLine) + final String coverageInfo = coveredMethodName.isPresent() && coveredMethodLine.isPresent() + ? String.format(" resolved to '%s' with line %d", coveredMethodName.get(), coveredMethodLine.get()) : ""; return String.format("%s%s at %d:%d%s", topLevelTypeFqn, description, sourceLine, sourceColumn, coverageInfo); @@ -144,7 +144,7 @@ public String toString() { private final String workDir; private final String oldCommitHash; - private final String newCommitHash; + private final Optional newCommitHash; private final Boolean compareWithWorkingCopyChanges; private final Set jaCoCoReportFiles; @@ -176,7 +176,7 @@ public static WorkDirStep builder() { this.workDir = builder.workDir; this.oldCommitHash = builder.oldCommitHash; - this.newCommitHash = builder.newCommitHash.orElse(null); + this.newCommitHash = builder.newCommitHash; compareWithWorkingCopyChanges = builder.newCommitHash.isPresent() ? null : Boolean.TRUE; this.jaCoCoReportFiles = Collections.unmodifiableSet(new HashSet<>(builder.jaCoCoReportFiles)); @@ -210,7 +210,7 @@ public String getOldCommitHash() { } public Optional getNewCommitHash() { - return Optional.ofNullable(newCommitHash); + return newCommitHash; } public Boolean getCompareWithWorkingCopyChanges() { diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java index 6484c66..ed3f7ca 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java @@ -39,7 +39,7 @@ public enum MethodType { private final int argumentCount; private final List argumentTypes; private final Map typeParameters; - private final String outerDeclaringType; + private final Optional outerDeclaringType; public static MethodTypeStep builder() { return new BuilderImpl(); @@ -65,7 +65,7 @@ public static MethodTypeStep builder() { argumentCount = argumentTypes.size(); this.typeParameters = Collections .unmodifiableMap(builder.typeParameters != null ? new HashMap<>(builder.typeParameters) : emptyMap()); - this.outerDeclaringType = builder.outerDeclaringType.orElse(null); + this.outerDeclaringType = builder.outerDeclaringType; } public MethodType getMethodType() { @@ -175,7 +175,7 @@ public Map getTypeParameters() { } public Optional getOuterDeclaringType() { - return Optional.of(outerDeclaringType); + return outerDeclaringType; } public String getDescription() { @@ -238,6 +238,6 @@ public String toString() { + ", empty=" + empty + (!argumentTypes.isEmpty() ? ", parameterTypes=" + argumentTypes : "") + ", argumentCount=" + argumentCount + (!typeParameters.isEmpty() ? ", typeParameters=" + typeParameters : "") - + (outerDeclaringType != null ? ", outerDeclaringType=" + outerDeclaringType : "") + "]"; + + (outerDeclaringType.isPresent() ? ", outerDeclaringType=" + outerDeclaringType.get() : "") + "]"; } }