kennknowles commented on a change in pull request #12366:
URL: https://github.com/apache/beam/pull/12366#discussion_r460355720



##########
File path: 
sdks/java/extensions/sql/datacatalog/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/DataCatalogBigQueryIT.java
##########
@@ -66,12 +66,15 @@
           });
     }
 
+    @SuppressWarnings("initialization.fields.uninitialized")
     @Parameterized.Parameter(0)
     public String dialectName;
 
+    @SuppressWarnings("initialization.fields.uninitialized")
     @Parameterized.Parameter(1)
     public Class<? extends QueryPlanner> queryPlanner;
 
+    @SuppressWarnings("nullness")

Review comment:
       The checkerframework nullness analysis is not a heuristic warning 
framework like spotbugs/findbugs. It is a new and improved type system, so the 
idea is to always satisfy it by the right types everywhere.
   
   In this case, the [javadoc for 
Class#getCanonicalName](https://github.com/typetools/jdk/blob/af987068c65767e101a3c58919cbae15ad08218b/src/java.base/share/classes/java/lang/Class.java#L1648)
 says "the canonical name of the class if it exists, otherwise null". So the 
checkerframework team has put a `@Nullable` annotation on the method. I 
actually did not realize this method was nullable. That is pretty annoying. So 
we might want to switch to just `getName` sometimes or otherwise use 
`MoreObjects.firstNonNull` to give a default string.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to