Repository: jhy/jsoup
Pull request #1467 (2020-12-20 01:12:29)

Add initial nullability annotations
Using JSR-305, which IntelliJ and Kotlin and other checkers are compatible with to annotate nullability. 

Includes initial annotations for ReturnsNonnull and ParametersNonnull for Jsoup.class. Over time, the goal will be to add those to all packages, and only explicitly specify nullable returns / params / fields.




Patch:
diff --git a/CHANGES b/CHANGES
index 491fda12..610a2682 100644
--- a/CHANGES
+++ b/CHANGES
@@ -22,6 +22,9 @@ jsoup changelog
 
   * Build Improvement: updated Jetty (used for integration tests; not bundled) to 9.4.35.v2020112.
 
+  * Build Improvement: added nullability annotations and initial settings.
+    <https://github.com/jhy/jsoup/pull/1467>
+
   * Bugfix: when parsing HTML, could throw NPEs on some tags (isindex or table>input).
     <https://github.com/jhy/jsoup/issues/1404>
 
diff --git a/pom.xml b/pom.xml
index c7328e05..7de140e3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -31,6 +31,10 @@
     <url>https://jhy.io/</url>
   </organization>
 
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+  </properties>
+
   <build>
     <plugins>
       <plugin>
@@ -311,7 +315,6 @@
       <scope>test</scope>
     </dependency>
 
-
     <dependency>
       <!-- jetty for webserver integration tests -->
       <groupId>org.eclipse.jetty</groupId>
@@ -320,6 +323,14 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <!-- javax.annotations.nonnull, with Apache 2 (not GPL) license. Not distributed. -->
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>jsr305</artifactId>
+      <version>3.0.2</version>
+      <scope>compile</scope>
+    </dependency>
+
   </dependencies>
 
   <dependencyManagement>
@@ -327,10 +338,6 @@
     </dependencies>
   </dependencyManagement>
 
-  <properties>
-    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-  </properties>
-
   <developers>
     <developer>
       <id>jhy</id>
diff --git a/src/main/java/org/jsoup/Jsoup.java b/src/main/java/org/jsoup/Jsoup.java
index 5fe62861..066978a6 100644
--- a/src/main/java/org/jsoup/Jsoup.java
+++ b/src/main/java/org/jsoup/Jsoup.java
@@ -2,12 +2,15 @@ package org.jsoup;
 
 import org.jsoup.helper.DataUtil;
 import org.jsoup.helper.HttpConnection;
+import org.jsoup.internal.ReturnsAreNonnullByDefault;
 import org.jsoup.nodes.Document;
 import org.jsoup.parser.Parser;
 import org.jsoup.safety.Cleaner;
 import org.jsoup.safety.Safelist;
 import org.jsoup.safety.Whitelist;
 
+import javax.annotation.Nullable;
+import javax.annotation.ParametersAreNonnullByDefault;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -17,6 +20,8 @@ import java.net.URL;
  The core public access point to the jsoup functionality.
 
  @author Jonathan Hedley */
+
+@ParametersAreNonnullByDefault @ReturnsAreNonnullByDefault
 public class Jsoup {
     private Jsoup() {}
 
@@ -85,7 +90,7 @@ public class Jsoup {
 
      @throws IOException if the file could not be found, or read, or if the charsetName is invalid.
      */
-    public static Document parse(File in, String charsetName, String baseUri) throws IOException {
+    public static Document parse(File in, @Nullable String charsetName, String baseUri) throws IOException {
         return DataUtil.load(in, charsetName, baseUri);
     }
 
@@ -100,7 +105,7 @@ public class Jsoup {
      @throws IOException if the file could not be found, or read, or if the charsetName is invalid.
      @see #parse(File, String, String)
      */
-    public static Document parse(File in, String charsetName) throws IOException {
+    public static Document parse(File in, @Nullable String charsetName) throws IOException {
         return DataUtil.load(in, charsetName, in.getAbsolutePath());
     }
 
@@ -115,7 +120,7 @@ public class Jsoup {
 
      @throws IOException if the file could not be found, or read, or if the charsetName is invalid.
      */
-    public static Document parse(InputStream in, String charsetName, String baseUri) throws IOException {
+    public static Document parse(InputStream in, @Nullable String charsetName, String baseUri) throws IOException {
         return DataUtil.load(in, charsetName, baseUri);
     }
 
@@ -132,7 +137,7 @@ public class Jsoup {
 
      @throws IOException if the file could not be found, or read, or if the charsetName is invalid.
      */
-    public static Document parse(InputStream in, String charsetName, String baseUri, Parser parser) throws IOException {
+    public static Document parse(InputStream in, @Nullable String charsetName, String baseUri, Parser parser) throws IOException {
         return DataUtil.load(in, charsetName, baseUri, parser);
     }
 
diff --git a/src/main/java/org/jsoup/internal/ReturnsAreNonnullByDefault.java b/src/main/java/org/jsoup/internal/ReturnsAreNonnullByDefault.java
new file mode 100644
index 00000000..3f004a68
--- /dev/null
+++ b/src/main/java/org/jsoup/internal/ReturnsAreNonnullByDefault.java
@@ -0,0 +1,20 @@
+package org.jsoup.internal;
+
+import javax.annotation.Nonnull;
+import javax.annotation.meta.TypeQualifierDefault;
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Documented
+@Nonnull
+@TypeQualifierDefault(ElementType.METHOD)
+@Retention(value = RetentionPolicy.RUNTIME)
+
+/**
+ Indicates return types are not nullable, unless otherwise specified by @Nullable.
+ @see javax.annotation.ParametersAreNonnullByDefault
+ */
+public @interface ReturnsAreNonnullByDefault {
+}
diff --git a/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java b/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java
index 39520ed5..cb398a2c 100644
--- a/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java
+++ b/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java
@@ -12,6 +12,7 @@ import org.jsoup.nodes.Node;
 import org.jsoup.nodes.TextNode;
 import org.jsoup.select.Elements;
 
+import javax.annotation.ParametersAreNonnullByDefault;
 import java.io.Reader;
 import java.io.StringReader;
 import java.util.ArrayList;
@@ -60,7 +61,7 @@ public class HtmlTreeBuilder extends TreeBuilder {
         return ParseSettings.htmlDefault;
     }
 
-    @Override
+    @Override @ParametersAreNonnullByDefault
     protected void initialiseParse(Reader input, String baseUri, Parser parser) {
         super.initialiseParse(input, baseUri, parser);
 
diff --git a/src/main/java/org/jsoup/parser/TreeBuilder.java b/src/main/java/org/jsoup/parser/TreeBuilder.java
index 5292705c..5c1ecfbf 100644
--- a/src/main/java/org/jsoup/parser/TreeBuilder.java
+++ b/src/main/java/org/jsoup/parser/TreeBuilder.java
@@ -6,6 +6,7 @@ import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;
 import org.jsoup.nodes.Node;
 
+import javax.annotation.ParametersAreNonnullByDefault;
 import java.io.Reader;
 import java.util.ArrayList;
 import java.util.List;
@@ -27,9 +28,11 @@ abstract class TreeBuilder {
     private Token.EndTag end  = new Token.EndTag();
     abstract ParseSettings defaultSettings();
 
+    @ParametersAreNonnullByDefault
     protected void initialiseParse(Reader input, String baseUri, Parser parser) {
         Validate.notNull(input, "String input must not be null");
         Validate.notNull(baseUri, "BaseURI must not be null");
+        Validate.notNull(parser);
 
         doc = new Document(baseUri);
         doc.parser(parser);
@@ -42,6 +45,7 @@ abstract class TreeBuilder {
         this.baseUri = baseUri;
     }
 
+    @ParametersAreNonnullByDefault
     Document parse(Reader input, String baseUri, Parser parser) {
         initialiseParse(input, baseUri, parser);
         runParser();
diff --git a/src/main/java/org/jsoup/parser/XmlTreeBuilder.java b/src/main/java/org/jsoup/parser/XmlTreeBuilder.java
index d68156dc..1be8bf1e 100644
--- a/src/main/java/org/jsoup/parser/XmlTreeBuilder.java
+++ b/src/main/java/org/jsoup/parser/XmlTreeBuilder.java
@@ -10,6 +10,7 @@ import org.jsoup.nodes.Node;
 import org.jsoup.nodes.TextNode;
 import org.jsoup.nodes.XmlDeclaration;
 
+import javax.annotation.ParametersAreNonnullByDefault;
 import java.io.Reader;
 import java.io.StringReader;
 import java.util.List;
@@ -26,7 +27,7 @@ public class XmlTreeBuilder extends TreeBuilder {
         return ParseSettings.preserveCase;
     }
 
-    @Override
+    @Override @ParametersAreNonnullByDefault
     protected void initialiseParse(Reader input, String baseUri, Parser parser) {
         super.initialiseParse(input, baseUri, parser);
         stack.add(doc); // place the document onto the stack. differs from HtmlTreeBuilder (not on stack)


Test patch:
diff --git a/src/test/java/org/jsoup/helper/ValidateTest.java b/src/test/java/org/jsoup/helper/ValidateTest.java
new file mode 100644
index 00000000..138e5324
--- /dev/null
+++ b/src/test/java/org/jsoup/helper/ValidateTest.java
@@ -0,0 +1,19 @@
+package org.jsoup.helper;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class ValidateTest {
+
+    @Test
+    public void testNotNull() {
+        Validate.notNull("foo");
+        boolean threw = false;
+        try {
+            Validate.notNull(null);
+        } catch (IllegalArgumentException e) {
+            threw = true;
+        }
+        Assertions.assertTrue(threw);
+    }
+}
diff --git a/src/test/java/org/jsoup/parser/HtmlTreeBuilderTest.java b/src/test/java/org/jsoup/parser/HtmlTreeBuilderTest.java
index 04630dc9..086a1c2e 100644
--- a/src/test/java/org/jsoup/parser/HtmlTreeBuilderTest.java
+++ b/src/test/java/org/jsoup/parser/HtmlTreeBuilderTest.java
@@ -3,9 +3,14 @@ package org.jsoup.parser;
 
 import org.junit.jupiter.api.Test;
 
+import javax.annotation.Nonnull;
+import javax.annotation.ParametersAreNonnullByDefault;
+import java.io.Reader;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
 import java.util.Arrays;
 
-import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.*;
 
 public class HtmlTreeBuilderTest {
     @Test
@@ -26,4 +31,28 @@ public class HtmlTreeBuilderTest {
             assertArrayEquals(array, copy);
         }
     }
+
+    @Test
+    public void nonnull() {
+        assertThrows(IllegalArgumentException.class, () -> {
+                HtmlTreeBuilder treeBuilder = new HtmlTreeBuilder();
+                treeBuilder.parse(null, null, null); // not sure how to test that these visual warnings actually appear! - test below checks for method annotation
+            }
+        ); // I'm not convinced that this lambda is easier to read than the old Junit 4 @Test(expected=IEA.class)...
+    }
+
+    @Test public void nonnullAssertions() throws NoSuchMethodException {
+        Method parseMethod = TreeBuilder.class.getDeclaredMethod("parse", Reader.class, String.class, Parser.class);
+        assertNotNull(parseMethod);
+        Annotation[] declaredAnnotations = parseMethod.getDeclaredAnnotations();
+        boolean seen = false;
+        for (Annotation annotation : declaredAnnotations) {
+            if (annotation.annotationType().isAssignableFrom(ParametersAreNonnullByDefault.class))
+                seen = true;
+        }
+
+        // would need to rework this if/when that annotation moves from the method to the class / package.
+        assertTrue(seen);
+
+    }
 }