Skip to content

Commit bdc7758

Browse files
dfa1claude
andauthored
chore: clear SonarCloud code smells (78 -> 0) (#42)
All open issues were CODE_SMELL (0 bugs, 0 vulns; gate already green). Fix the genuine ones and suppress the rules that conflict with mandatory FFM idiom or an enforced project convention: Fixed: - S6213 (28) rename variables matching restricted identifiers - S5838 (6) use AssertJ hasSize/hasSizeLessThan - S5853 (3) chain assertions on the same actual (left the side-effecting InputStream.read() triple alone — chaining would change reads) - S1066 (1) merge nested if in ZstdInputStream.produce() (behavior-identical) - S7467 (1) unnamed pattern for an unused binding Suppressed (with justification): - S7474 (34) pom-level ignore: the rule wants {@link}/HTML javadoc, which the project forbids (mandates `///` Markdown + [Class#method] refs, checkstyle-enforced) - S112 (3) @SuppressWarnings on the invokeExact wrapper interfaces - S1181 (1) catching Throwable is mandatory: MethodHandle.invokeExact throws it - S3776 (1) produce() complexity is inherent to the decode/EOF loop and tested No production behavior change. validate + test green. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8ff19b0 commit bdc7758

17 files changed

Lines changed: 162 additions & 144 deletions

integration-tests/src/test/java/io/github/dfa1/zstd/it/GoldenCorpusTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,9 @@ void dictIdRidesWithFrame(Path file) {
254254
ZstdDictionaryId frameDictId = ZstdFrame.dictId(frame);
255255

256256
// Then
257-
assertThat(frameDictId).isEqualTo(dict.id());
258-
assertThat(frameDictId).isNotEqualTo(ZstdDictionaryId.NONE);
257+
assertThat(frameDictId)
258+
.isEqualTo(dict.id())
259+
.isNotEqualTo(ZstdDictionaryId.NONE);
259260
}
260261
}
261262
}

integration-tests/src/test/java/io/github/dfa1/zstd/it/ZstdInteropExtrasTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,25 +217,26 @@ void javaReadsDictIdFromJniDictFrame() {
217217
ZstdDictionary dict = trainDict();
218218
var jniDict = new com.github.luben.zstd.ZstdDictCompress(
219219
dict.toByteArray(), Zstd.defaultCompressionLevel());
220-
byte[] frame = com.github.luben.zstd.Zstd.compress(record(7), jniDict);
220+
byte[] frame = com.github.luben.zstd.Zstd.compress(sample(7), jniDict);
221221

222222
// When
223223
ZstdDictionaryId dictId = ZstdFrame.dictId(frame);
224224

225225
// Then
226-
assertThat(dictId).isEqualTo(dict.id());
227-
assertThat(dictId).isNotEqualTo(ZstdDictionaryId.NONE);
226+
assertThat(dictId)
227+
.isEqualTo(dict.id())
228+
.isNotEqualTo(ZstdDictionaryId.NONE);
228229
}
229230

230231
private ZstdDictionary trainDict() {
231232
List<byte[]> samples = new ArrayList<>();
232233
for (int i = 0; i < 3000; i++) {
233-
samples.add(record(i));
234+
samples.add(sample(i));
234235
}
235236
return ZstdDictionary.train(samples, 8 * 1024);
236237
}
237238

238-
private byte[] record(int i) {
239+
private byte[] sample(int i) {
239240
return ("{\"id\":" + i + ",\"user\":\"u" + (i % 30) + "\",\"event\":\"click\"}")
240241
.getBytes(StandardCharsets.UTF_8);
241242
}

integration-tests/src/test/java/io/github/dfa1/zstd/it/ZstdJniInteropTest.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -100,29 +100,29 @@ class Dictionary {
100100
@Test
101101
void javaDictCompressJniDictDecompress() {
102102
ZstdDictionary dict = trainDict();
103-
byte[] record = record(11);
103+
byte[] sample = sample(11);
104104

105105
byte[] frame;
106106
try (ZstdCompressCtx ctx = new ZstdCompressCtx()) {
107-
frame = ctx.compress(record, dict);
107+
frame = ctx.compress(sample, dict);
108108
}
109109
ZstdDictDecompress jniDict = new ZstdDictDecompress(dict.toByteArray());
110-
assertThat(com.github.luben.zstd.Zstd.decompress(frame, jniDict, record.length)).isEqualTo(record);
110+
assertThat(com.github.luben.zstd.Zstd.decompress(frame, jniDict, sample.length)).isEqualTo(sample);
111111
}
112112

113113
@Test
114114
void jniDictCompressJavaDictDecompress() {
115115
ZstdDictionary dict = trainDict();
116-
byte[] record = record(22);
116+
byte[] sample = sample(22);
117117

118118
ZstdDictCompress jniDict = new ZstdDictCompress(dict.toByteArray(), Zstd.defaultCompressionLevel());
119-
byte[] frame = com.github.luben.zstd.Zstd.compress(record, jniDict);
119+
byte[] frame = com.github.luben.zstd.Zstd.compress(sample, jniDict);
120120

121121
byte[] restored;
122122
try (ZstdDecompressCtx ctx = new ZstdDecompressCtx()) {
123-
restored = ctx.decompress(frame, record.length, dict);
123+
restored = ctx.decompress(frame, sample.length, dict);
124124
}
125-
assertThat(restored).isEqualTo(record);
125+
assertThat(restored).isEqualTo(sample);
126126
}
127127

128128
@Test
@@ -131,42 +131,42 @@ void javaLoadedDictWithChecksumJniDictDecompress() {
131131
// (checksum) — the COMPRESS2 path — must still produce a frame zstd-jni
132132
// decodes against the same dictionary.
133133
ZstdDictionary dict = trainDict();
134-
byte[] record = record(33);
134+
byte[] sample = sample(33);
135135

136136
byte[] frame;
137137
try (ZstdCompressCtx ctx = new ZstdCompressCtx().checksum(true)) {
138138
ctx.loadDictionary(dict);
139-
frame = ctx.compress(record);
139+
frame = ctx.compress(sample);
140140
}
141141
ZstdDictDecompress jniDict = new ZstdDictDecompress(dict.toByteArray());
142-
assertThat(com.github.luben.zstd.Zstd.decompress(frame, jniDict, record.length)).isEqualTo(record);
142+
assertThat(com.github.luben.zstd.Zstd.decompress(frame, jniDict, sample.length)).isEqualTo(sample);
143143
}
144144

145145
@Test
146146
void javaReferencedDigestedDictJniDictDecompress() {
147147
// A frame from a context referencing a digested CDict must decode in zstd-jni.
148148
ZstdDictionary dict = trainDict();
149-
byte[] record = record(44);
149+
byte[] sample = sample(44);
150150

151151
byte[] frame;
152152
try (ZstdCompressDict cdict = new ZstdCompressDict(dict, Zstd.defaultCompressionLevel());
153153
ZstdCompressCtx ctx = new ZstdCompressCtx()) {
154154
ctx.refDictionary(cdict);
155-
frame = ctx.compress(record);
155+
frame = ctx.compress(sample);
156156
}
157157
ZstdDictDecompress jniDict = new ZstdDictDecompress(dict.toByteArray());
158-
assertThat(com.github.luben.zstd.Zstd.decompress(frame, jniDict, record.length)).isEqualTo(record);
158+
assertThat(com.github.luben.zstd.Zstd.decompress(frame, jniDict, sample.length)).isEqualTo(sample);
159159
}
160160

161161
private ZstdDictionary trainDict() {
162162
List<byte[]> samples = new ArrayList<>();
163163
for (int i = 0; i < 3000; i++) {
164-
samples.add(record(i));
164+
samples.add(sample(i));
165165
}
166166
return ZstdDictionary.train(samples, 8 * 1024);
167167
}
168168

169-
private byte[] record(int i) {
169+
private byte[] sample(int i) {
170170
return ("{\"id\":" + i + ",\"user\":\"u" + (i % 30) + "\",\"event\":\"click\"}")
171171
.getBytes(StandardCharsets.UTF_8);
172172
}

pom.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@
7272
<sonar.exclusions>
7373
**/benchmark/**
7474
</sonar.exclusions>
75+
<!-- S7474 ("Markdown, HTML and Javadoc tags should be consistent") flags our
76+
JEP 467 Markdown reference links ([ClassName#method]) and wants the
77+
{@link}/HTML form instead. That form is forbidden here: the project
78+
mandates `///` Markdown javadoc with [ClassName] reference links and
79+
checkstyle bans {@link}. The rule therefore conflicts with a mandated,
80+
enforced convention rather than catching a real defect — ignore it. -->
81+
<sonar.issue.ignore.multicriteria>jep467</sonar.issue.ignore.multicriteria>
82+
<sonar.issue.ignore.multicriteria.jep467.ruleKey>java:S7474</sonar.issue.ignore.multicriteria.jep467.ruleKey>
83+
<sonar.issue.ignore.multicriteria.jep467.resourceKey>**/*.java</sonar.issue.ignore.multicriteria.jep467.resourceKey>
7584
<sonar.coverage.exclusions>
7685
**/benchmark/**
7786
</sonar.coverage.exclusions>

zstd/src/main/java/io/github/dfa1/zstd/NativeCall.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ final class NativeCall {
1212

1313
/// A native call returning a zstd `size_t` status that may encode an error.
1414
@FunctionalInterface
15+
@SuppressWarnings("java:S112") // wraps MethodHandle.invokeExact, which is declared to throw Throwable
1516
interface ZstdCall {
1617
long run() throws Throwable;
1718
}
@@ -33,6 +34,7 @@ static long checkReturnValue(ZstdCall c) {
3334

3435
/// A native factory call returning a freshly allocated object pointer.
3536
@FunctionalInterface
37+
@SuppressWarnings("java:S112") // wraps MethodHandle.invokeExact, which is declared to throw Throwable
3638
interface NativeFactory {
3739
MemorySegment create() throws Throwable;
3840
}

zstd/src/main/java/io/github/dfa1/zstd/NativeObject.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,6 @@ public final void close() {
4747
///
4848
/// @param ptr the non-NULL native pointer to free
4949
/// @throws Throwable if the native free call fails; the exception is swallowed by [#close()]
50+
@SuppressWarnings("java:S112") // implementations wrap MethodHandle.invokeExact, declared to throw Throwable
5051
protected abstract void tryClose(MemorySegment ptr) throws Throwable;
5152
}

zstd/src/main/java/io/github/dfa1/zstd/ZstdDecompressStream.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public ZstdDecompressStream() {
2626
/// Creates a streaming decompressor for frames built with `dictionary`.
2727
///
2828
/// @param dictionary the dictionary the frames were compressed against, or `null` for none
29+
@SuppressWarnings("java:S1181") // loadDictionary wraps MethodHandle.invokeExact (throws Throwable); must catch Throwable
2930
public ZstdDecompressStream(ZstdDictionary dictionary) {
3031
// Own the context first, so any failure setting it up is cleaned up by
3132
// close() — one release path, no leak on a half-built stream.

zstd/src/main/java/io/github/dfa1/zstd/ZstdInputStream.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ public int read(byte[] b, int off, int len) throws IOException {
127127
}
128128

129129
/// Decodes the next slice of output into `hold`. Returns false at end of stream.
130+
// The branching here (input refill, truncation detection, no-progress guard) is
131+
// essential to correct EOF/truncation handling and is covered by tests; splitting
132+
// it would obscure the decode loop rather than clarify it.
133+
@SuppressWarnings("java:S3776") // cognitive complexity is inherent to the streaming decode/EOF logic
130134
private boolean produce() throws IOException {
131135
while (true) {
132136
if (inBuf.pos() == inBuf.size()) {
@@ -157,15 +161,13 @@ private boolean produce() throws IOException {
157161
}
158162
// Nothing produced. If the decoder neither advanced its input nor wants
159163
// more, it cannot make progress on this input — stop to avoid spinning.
160-
if (inBuf.pos() == inBuf.size()) {
161-
if (inputEof) {
162-
if (lastHint != 0) {
163-
throw new ZstdException("truncated zstd stream: " + lastHint
164-
+ " more input byte(s) expected");
165-
}
166-
return false;
164+
// (Input drained but not at EOF: fall through and loop to refill from `in`.)
165+
if (inBuf.pos() == inBuf.size() && inputEof) {
166+
if (lastHint != 0) {
167+
throw new ZstdException("truncated zstd stream: " + lastHint
168+
+ " more input byte(s) expected");
167169
}
168-
// input drained but frame wants more: loop to refill from `in`.
170+
return false;
169171
}
170172
}
171173
}

zstd/src/test/java/io/github/dfa1/zstd/NativeObjectTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
1616
import org.junit.jupiter.api.Test;
1717

18-
class NativeObjectTest {
18+
class NativeObjectTest {
1919

2020
// A non-NULL stand-in pointer; never dereferenced, only compared by identity.
2121
private static final MemorySegment POINTER = MemorySegment.ofAddress(0x1234);

zstd/src/test/java/io/github/dfa1/zstd/RefPrefixTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ void prefixIsAppliedAndRequiredToDecode() {
7979
MemorySegment out = arena.allocate(random.length);
8080
long m = dctx.decompress(out, segmentOf(arena, frame));
8181
reproduced = Arrays.equals(bytesOf(out, (int) m), random);
82-
} catch (ZstdException e) {
82+
} catch (ZstdException _) {
8383
reproduced = false;
8484
}
8585
assertThat(reproduced).isFalse();

0 commit comments

Comments
 (0)