Message-ID: <gh-pgjdbc-pgjdbc-3396-c2409007249@github.com>
From: "binoverfl0w (@binoverfl0w)" <noreply+binoverfl0w@github.com>
To: "pgjdbc/pgjdbc" <noreply+pgjdbc-pgjdbc@github.com>
Date: Sun, 13 Oct 2024 14:45:39 +0000
Subject: Re: [pgjdbc/pgjdbc] PR #3396: SQLData Support
In-Reply-To: <gh-pgjdbc-pgjdbc-3396@github.com>
References: <gh-pgjdbc-pgjdbc-3396@github.com>
List-Id: <gh-pgjdbc-pgjdbc.github.com>
X-GitHub-Author-Login: binoverfl0w
X-GitHub-Comment-Id: 2409007249
X-GitHub-Comment-Type: issue_comment
X-GitHub-Edited-At: 2024-10-13T23:09:35Z
X-GitHub-Issue: 3396
X-GitHub-Repo: pgjdbc/pgjdbc
X-GitHub-Type: comment
X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/3396#issuecomment-2409007249
Content-Type: text/plain; charset=utf-8

I remember running into this as well. Set **enableCheckerframework** build parameter to **true** as it is false by default. https://github.com/pgjdbc/pgjdbc/blob/fcc13e70e6b6bb64b848df4b4ba6b3566b5e95a3/build-logic/build-parameters/build.gradle.kts#L50

I tried to run ```./gradlew checkstyleAll``` and it gave the same error as the one reported here.
The fixes seem trivial. From a quick look, the patch below gets rid of the failures.

A helpful note on ```@Nullable byte[]``` vs ```byte @Nullable []``` that might be of interest: http://errorprone.info/bugpattern/NullablePrimitiveArray

Since ```@SupressWarnings``` cannot be applied to statements, you may consider moving the following block
```java
          if ("NULL".equals(item)) {
            java.lang.reflect.Array.set(results, i, null);
          } else {
            java.lang.reflect.Array.set(results, i, converter.apply(item));
          }
```
in a separate function and annotate that with ```@SurpressWarnings("nullable")``` instead of annotating ```#getConverter``` with ```@SuppressWarnings({"unchecked", "nullness"})```

Anyway once you face the same errors locally you will be able to better reason about such details.

```
Subject: [PATCH] fix CheckerFramework failures
---
Index: pgjdbc/src/main/java/org/postgresql/jdbc/PgSQLInput.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgSQLInput.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgSQLInput.java
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgSQLInput.java	(revision c664b7d767951d48df94f1a84084e4d86f688e91)
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgSQLInput.java	(date 1728829537425)
@@ -151,7 +151,7 @@
 
   @SuppressWarnings({"override.return", "nullness.on.primitive", "return"})
   @Override
-  public @Nullable byte[] readBytes() throws SQLException {
+  public byte @Nullable [] readBytes() throws SQLException {
     return getNextValue(bytesConv);
   }
 
@@ -187,7 +187,8 @@
 
   @Override
   public InputStream readBinaryStream() throws SQLException {
-    return new ByteArrayInputStream(readBytes());
+    byte[] bytes = readBytes();
+    return new ByteArrayInputStream(bytes == null ? new byte[0] : bytes);
   }
 
   @SuppressWarnings("override.return")
@@ -253,13 +254,13 @@
     throw Driver.notImplemented(this.getClass(), "readNString()");
   }
 
-  @SuppressWarnings("unchecked")
+  @SuppressWarnings({"unchecked", "nullness"})
   private <T> SQLFunction<String, T> getConverter(Class<T> type) throws SQLException {
     if (type.isArray()) {
       Class<?> itemType = type.getComponentType();
       SQLFunction<String, ?> converter = getConverter(itemType);
       return (value) -> {
-        List<String> items = new SQLDataReader().parseArray(value);
+        List<@Nullable String> items = new SQLDataReader().parseArray(value);
         Object results = java.lang.reflect.Array.newInstance(itemType, items.size());
         for (int i = 0; i < items.size(); i++) {
           String item = items.get(i);
```