From 9006e7087bcefaecaf4c80489cb8c9e7a796d583 Mon Sep 17 00:00:00 2001 From: Matthias Andreas Benkard Date: Tue, 1 Mar 2022 13:43:50 +0100 Subject: Ensure nullness correctness using Checker Framework. Change-Id: Ie5a7749194313664a206e44597091a62afca9bdb --- .../main/java/eu/mulk/jgvariant/core/Decoder.java | 63 +++++++++++----------- .../java/eu/mulk/jgvariant/core/Signature.java | 3 +- .../java/eu/mulk/jgvariant/ostree/ByteString.java | 3 +- .../eu/mulk/jgvariant/ostree/DeltaSuperblock.java | 28 +++++----- .../mulk/jgvariant/ostree/OstreeDecoderTest.java | 1 + jgvariant-parent/pom.xml | 40 +++++++++++++- 6 files changed, 91 insertions(+), 47 deletions(-) diff --git a/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java b/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java index 1dbd1fa..8a3741b 100644 --- a/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java +++ b/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java @@ -26,6 +26,7 @@ import java.util.function.Predicate; import java.util.function.UnaryOperator; import org.apiguardian.api.API; import org.apiguardian.api.API.Status; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** @@ -75,12 +76,11 @@ public abstract class Decoder { * data. * @throws IllegalArgumentException if the serialized GVariant is ill-formed */ - public abstract T decode(ByteBuffer byteSlice); + public abstract @NotNull T decode(ByteBuffer byteSlice); abstract byte alignment(); - @Nullable - abstract Integer fixedSize(); + abstract @Nullable Integer fixedSize(); final boolean hasFixedSize() { return fixedSize() != null; @@ -103,7 +103,7 @@ public abstract class Decoder { * @return a new, decorated {@link Decoder}. * @see java.util.stream.Stream#map */ - public final Decoder map(Function function) { + public final Decoder map(Function<@NotNull T, @NotNull U> function) { return new MappingDecoder<>(function); } @@ -348,12 +348,12 @@ public abstract class Decoder { @Override @Nullable - Integer fixedSize() { + public Integer fixedSize() { return null; } @Override - public List decode(ByteBuffer byteSlice) { + public @NotNull List decode(ByteBuffer byteSlice) { List elements; var elementSize = elementDecoder.fixedSize(); @@ -406,12 +406,13 @@ public abstract class Decoder { } @Override + @Nullable public Integer fixedSize() { return entryArrayDecoder.fixedSize(); } @Override - public Map decode(ByteBuffer byteSlice) { + public @NotNull Map decode(ByteBuffer byteSlice) { List> entries = entryArrayDecoder.decode(byteSlice); return entries.stream().collect(toMap(Entry::getKey, Entry::getValue)); } @@ -433,7 +434,7 @@ public abstract class Decoder { } @Override - public byte[] decode(ByteBuffer byteSlice) { + public byte @NotNull [] decode(ByteBuffer byteSlice) { // A simple C-style array. byte[] elements = new byte[byteSlice.limit() / ELEMENT_SIZE]; byteSlice.get(elements); @@ -461,7 +462,7 @@ public abstract class Decoder { } @Override - public Optional decode(ByteBuffer byteSlice) { + public @NotNull Optional decode(ByteBuffer byteSlice) { if (!byteSlice.hasRemaining()) { return Optional.empty(); } else { @@ -498,12 +499,12 @@ public abstract class Decoder { } @Override - public Integer fixedSize() { + public @Nullable Integer fixedSize() { return tupleDecoder.fixedSize(); } @Override - public U decode(ByteBuffer byteSlice) { + public @NotNull U decode(ByteBuffer byteSlice) { Object[] recordConstructorArguments = tupleDecoder.decode(byteSlice); try { @@ -536,7 +537,7 @@ public abstract class Decoder { } @Override - public Integer fixedSize() { + public @Nullable Integer fixedSize() { int position = 0; for (var componentDecoder : componentDecoders) { var fixedComponentSize = componentDecoder.fixedSize(); @@ -556,7 +557,7 @@ public abstract class Decoder { } @Override - public Object[] decode(ByteBuffer byteSlice) { + public Object @NotNull [] decode(ByteBuffer byteSlice) { int framingOffsetSize = byteCount(byteSlice.limit()); var objects = new Object[componentDecoders.length]; @@ -616,13 +617,13 @@ public abstract class Decoder { } @Override - public Integer fixedSize() { + public @Nullable Integer fixedSize() { return tupleDecoder.fixedSize(); } @Override @SuppressWarnings("unchecked") - public Map.Entry decode(ByteBuffer byteSlice) { + public Map.@NotNull Entry decode(ByteBuffer byteSlice) { Object[] components = tupleDecoder.decode(byteSlice); return new SimpleImmutableEntry<>((K) components[0], (V) components[1]); } @@ -642,7 +643,7 @@ public abstract class Decoder { } @Override - public Variant decode(ByteBuffer byteSlice) { + public @NotNull Variant decode(ByteBuffer byteSlice) { for (int i = byteSlice.limit() - 1; i >= 0; --i) { if (byteSlice.get(i) != 0) { continue; @@ -678,7 +679,7 @@ public abstract class Decoder { } @Override - public Boolean decode(ByteBuffer byteSlice) { + public @NotNull Boolean decode(ByteBuffer byteSlice) { return byteSlice.get() != 0; } } @@ -696,7 +697,7 @@ public abstract class Decoder { } @Override - public Byte decode(ByteBuffer byteSlice) { + public @NotNull Byte decode(ByteBuffer byteSlice) { return byteSlice.get(); } } @@ -714,7 +715,7 @@ public abstract class Decoder { } @Override - public Short decode(ByteBuffer byteSlice) { + public @NotNull Short decode(ByteBuffer byteSlice) { return byteSlice.getShort(); } } @@ -732,7 +733,7 @@ public abstract class Decoder { } @Override - public Integer decode(ByteBuffer byteSlice) { + public @NotNull Integer decode(ByteBuffer byteSlice) { return byteSlice.getInt(); } } @@ -750,7 +751,7 @@ public abstract class Decoder { } @Override - public Long decode(ByteBuffer byteSlice) { + public @NotNull Long decode(ByteBuffer byteSlice) { return byteSlice.getLong(); } } @@ -768,7 +769,7 @@ public abstract class Decoder { } @Override - public Double decode(ByteBuffer byteSlice) { + public @NotNull Double decode(ByteBuffer byteSlice) { return byteSlice.getDouble(); } } @@ -793,7 +794,7 @@ public abstract class Decoder { } @Override - public String decode(ByteBuffer byteSlice) { + public @NotNull String decode(ByteBuffer byteSlice) { byteSlice.limit(byteSlice.limit() - 1); return charset.decode(byteSlice).toString(); } @@ -801,9 +802,9 @@ public abstract class Decoder { private class MappingDecoder extends Decoder { - private final Function function; + private final Function<@NotNull T, @NotNull U> function; - MappingDecoder(Function function) { + MappingDecoder(Function<@NotNull T, @NotNull U> function) { this.function = function; } @@ -818,7 +819,7 @@ public abstract class Decoder { } @Override - public U decode(ByteBuffer byteSlice) { + public @NotNull U decode(ByteBuffer byteSlice) { return function.apply(Decoder.this.decode(byteSlice)); } } @@ -842,7 +843,7 @@ public abstract class Decoder { } @Override - public T decode(ByteBuffer byteSlice) { + public @NotNull T decode(ByteBuffer byteSlice) { var transformedBuffer = function.apply(byteSlice.asReadOnlyBuffer().order(byteSlice.order())); return Decoder.this.decode(transformedBuffer); } @@ -867,7 +868,7 @@ public abstract class Decoder { } @Override - public T decode(ByteBuffer byteSlice) { + public @NotNull T decode(ByteBuffer byteSlice) { var newByteSlice = byteSlice.duplicate(); newByteSlice.order(byteOrder); return Decoder.this.decode(newByteSlice); @@ -898,7 +899,9 @@ public abstract class Decoder { if (!Objects.equals(thenDecoder.fixedSize(), elseDecoder.fixedSize())) { throw new IllegalArgumentException( "incompatible sizes in predicate branches: then=%s, else=%s" - .formatted(thenDecoder.fixedSize(), elseDecoder.fixedSize())); + .formatted( + Objects.requireNonNullElse(thenDecoder.fixedSize(), "(null)"), + Objects.requireNonNullElse(elseDecoder.fixedSize(), "(null)"))); } } @@ -913,7 +916,7 @@ public abstract class Decoder { } @Override - public U decode(ByteBuffer byteSlice) { + public @NotNull U decode(ByteBuffer byteSlice) { var b = selector.test(byteSlice); byteSlice.rewind(); return b ? thenDecoder.decode(byteSlice) : elseDecoder.decode(byteSlice); diff --git a/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Signature.java b/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Signature.java index d61b9d1..cc9674d 100644 --- a/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Signature.java +++ b/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Signature.java @@ -12,6 +12,7 @@ import java.util.List; import java.util.Objects; import org.apiguardian.api.API; import org.apiguardian.api.API.Status; +import org.jetbrains.annotations.Nullable; /** * A GVariant signature string. @@ -73,7 +74,7 @@ public final class Signature { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { return (o instanceof Signature signature) && Objects.equals(signatureString, signature.signatureString); } diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/ByteString.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/ByteString.java index 3a8185c..cfe3635 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/ByteString.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/ByteString.java @@ -10,6 +10,7 @@ import java.util.Base64; import java.util.HexFormat; import java.util.stream.IntStream; import java.util.stream.Stream; +import org.jetbrains.annotations.Nullable; /** * A wrapper for a {@code byte[]} that implements {@link #equals(Object)}, {@link #hashCode()}, and @@ -31,7 +32,7 @@ public record ByteString(byte[] bytes) { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { return (o instanceof ByteString byteString) && Arrays.equals(bytes, byteString.bytes); } diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java index 52b970f..50da203 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java @@ -70,20 +70,20 @@ public record DeltaSuperblock( Decoder.ofByteArray().map(DeltaSuperblock::parseDeltaNameList), Decoder.ofArray(DeltaMetaEntry.decoder()).withByteOrder(ByteOrder.LITTLE_ENDIAN), Decoder.ofArray(DeltaFallback.decoder()).withByteOrder(ByteOrder.LITTLE_ENDIAN)) - .map( - deltaSuperblock -> { - // Fix up the endianness of the 'entries' and 'fallbacks' fields, which have - // unspecified byte order. - var endiannessMetadatum = - deltaSuperblock.metadata().fields().get("ostree.endianness"); - if (endiannessMetadatum != null - && endiannessMetadatum.value() instanceof Byte endiannessByte - && endiannessByte == (byte) 'B') { - return deltaSuperblock.byteSwapped(); - } else { - return deltaSuperblock; - } - }); + .map(DeltaSuperblock::byteSwappedIfBigEndian); + + private DeltaSuperblock byteSwappedIfBigEndian() { + // Fix up the endianness of the 'entries' and 'fallbacks' fields, which have + // unspecified byte order. + var endiannessMetadatum = metadata().fields().get("ostree.endianness"); + if (endiannessMetadatum != null + && endiannessMetadatum.value() instanceof Byte endiannessByte + && endiannessByte == (byte) 'B') { + return byteSwapped(); + } else { + return this; + } + } private DeltaSuperblock byteSwapped() { return new DeltaSuperblock( diff --git a/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java b/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java index 5c1dd79..d8ad271 100644 --- a/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java +++ b/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java @@ -17,6 +17,7 @@ import java.util.Map; import org.junit.jupiter.api.Test; @TestWithResources +@SuppressWarnings("initialization.field.uninitialized") class OstreeDecoderTest { @GivenBinaryResource("/ostree/summary") diff --git a/jgvariant-parent/pom.xml b/jgvariant-parent/pom.xml index fe0c1ed..92b7e07 100644 --- a/jgvariant-parent/pom.xml +++ b/jgvariant-parent/pom.xml @@ -74,6 +74,7 @@ SPDX-License-Identifier: LGPL-3.0-or-later 22.0.0 5.8.2 1.9 + 3.21.1 @@ -102,6 +103,18 @@ SPDX-License-Identifier: LGPL-3.0-or-later ${apiguardian.version} + + + org.checkerframework + checker + ${checker-framework.version} + + + org.checkerframework + checker-qual + ${checker-framework.version} + + org.tukaani @@ -131,6 +144,22 @@ SPDX-License-Identifier: LGPL-3.0-or-later + + + + org.checkerframework + checker + provided + true + + + org.checkerframework + checker-qual + provided + true + + + @@ -166,8 +195,9 @@ SPDX-License-Identifier: LGPL-3.0-or-later true -XDcompilePolicy=simple - -Xplugin:ErrorProne + -Xplugin:ErrorProne -Xep:InvalidParam:OFF -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED @@ -184,7 +214,15 @@ SPDX-License-Identifier: LGPL-3.0-or-later error_prone_core ${errorprone.version} + + org.checkerframework + checker + ${checker-framework.version} + + + org.checkerframework.checker.nullness.NullnessChecker + -- cgit v1.2.3