Checker Framework integration (PaperMC#126)

CONTRIBUTING.md

@@ -0,0 +1,35 @@
+Thanks for taking the time to submit a contribution to Velocity! Your support
+is greatly appreciated.
+
+In this document, we'll give you some tips on making it more likely your
+contribution will be pulled.
+
+# Setting up a development environment
+
+This isn't as difficult as you may be led to believe. All you need to do is
+clone the Velocity repository in your favorite IDE and have your backend test
+servers set up to run behind Velocity.
+
+# Actually working on the code
+
+It is strongly recommended that you are familiar with the Minecraft protocol,
+proficient with using Java, and have familiarity with the libraries used in
+Velocity (particularly [Netty](https://netty.io), [Google Guava](https://github.com/google/guava),
+and the [Checker Framework annotations](https://checkerframework.org/)).
+While you can certainly work with the Velocity codebase without knowing any
+of this, it can be risky to proceed.
+
+Velocity does not currently obey any one general code style at the moment.
+Plans are [in the works](https://github.com/VelocityPowered/Velocity/issues/125)
+to define the code style the project will follow.
+
+# Notes on the build
+
+To reduce bugs and ensure code quality, we run the following tools on all commits
+and pull requests:
+
+* [Checker Framework](https://checkerframework.org/): an enhancement to Java's type
+  system that is designed to help catch bugs. Velocity runs the _Nullness Checker_
+  and the _Optional Checker_.
+* [Checkstyle](http://checkstyle.sourceforge.net/) (not currently in use): ensures
+  that your code is correctly formatted.

api/build.gradle

@@ -4,6 +4,8 @@ plugins {
     id 'maven-publish'
 }
 
+apply from: '../gradle/checkerframework.gradle'
+
 sourceSets {
     ap {
         compileClasspath += main.compileClasspath + main.output
@@ -17,7 +19,7 @@ dependencies {
     compile 'com.moandjiezana.toml:toml4j:0.7.2'
     compile "org.slf4j:slf4j-api:${slf4jVersion}"
     compile 'com.google.inject:guice:4.2.0'
-    compile 'org.checkerframework:checker-qual:2.5.4'
+    compile "org.checkerframework:checker-qual:${checkerFrameworkVersion}"
 
     testCompile "org.junit.jupiter:junit-jupiter-api:${junitVersion}"
     testCompile "org.junit.jupiter:junit-jupiter-engine:${junitVersion}"

api/src/ap/java/com/velocitypowered/api/plugin/ap/SerializedPluginDescription.java

@@ -2,6 +2,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
 import com.velocitypowered.api.plugin.Plugin;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
@@ -29,8 +30,8 @@ public SerializedPluginDescription(String id, String name, String version, Strin
         this.version = Strings.emptyToNull(version);
         this.description = Strings.emptyToNull(description);
         this.url = Strings.emptyToNull(url);
-        this.authors = authors == null || authors.isEmpty() ? null : authors;
-        this.dependencies = dependencies == null || dependencies.isEmpty() ? null : dependencies;
+        this.authors = authors == null || authors.isEmpty() ? ImmutableList.of() : authors;
+        this.dependencies = dependencies == null || dependencies.isEmpty() ? ImmutableList.of() : dependencies;
         this.main = Preconditions.checkNotNull(main, "main");
     }
 
@@ -63,12 +64,12 @@ public String getId() {
         return url;
     }
 
-    public @Nullable List<String> getAuthors() {
-        return authors;
+    public List<String> getAuthors() {
+        return authors == null ? ImmutableList.of() : authors;
     }
 
-    public @Nullable List<Dependency> getDependencies() {
-        return dependencies;
+    public List<Dependency> getDependencies() {
+        return dependencies == null ? ImmutableList.of() : dependencies;
     }
 
     public String getMain() {

api/src/main/java/com/velocitypowered/api/command/Command.java

@@ -15,15 +15,15 @@ public interface Command {
      * @param source the source of this command
      * @param args the arguments for this command
      */
-    void execute(@NonNull CommandSource source, @NonNull String[] args);
+    void execute(CommandSource source, String @NonNull [] args);
 
     /**
      * Provides tab complete suggestions for a command for a specified {@link CommandSource}.
      * @param source the source to run the command for
      * @param currentArgs the current, partial arguments for this command
      * @return tab complete suggestions
      */
-    default List<String> suggest(@NonNull CommandSource source, @NonNull String[] currentArgs) {
+    default List<String> suggest(CommandSource source, String @NonNull [] currentArgs) {
         return ImmutableList.of();
     }
 
@@ -38,7 +38,7 @@ default List<String> suggest(@NonNull CommandSource source, @NonNull String[] cu
      * @param args the arguments for this command
      * @return whether the source has permission
      */
-    default boolean hasPermission(@NonNull CommandSource source, @NonNull String[] args) {
+    default boolean hasPermission(CommandSource source, String @NonNull [] args) {
         return true;
     }
 }

api/src/main/java/com/velocitypowered/api/command/CommandManager.java

@@ -1,7 +1,5 @@
 package com.velocitypowered.api.command;
 
-import org.checkerframework.checker.nullness.qual.NonNull;
-
 /**
  * Represents an interface to register a command executor with the proxy.
  */
@@ -11,19 +9,19 @@ public interface CommandManager {
      * @param command the command to register
      * @param aliases the alias to use
      */
-    void register(@NonNull Command command, String... aliases);
+    void register(Command command, String... aliases);
 
     /**
      * Unregisters a command.
      * @param alias the command alias to unregister
      */
-    void unregister(@NonNull String alias);
+    void unregister(String alias);
 
     /**
      * Attempts to execute a command from the specified {@code cmdLine}.
      * @param source the command's source
      * @param cmdLine the command to run
      * @return true if the command was found and executed, false if it was not
      */
-    boolean execute(@NonNull CommandSource source, @NonNull String cmdLine);
+    boolean execute(CommandSource source, String cmdLine);
 }

api/src/main/java/com/velocitypowered/api/command/CommandSource.java

@@ -2,7 +2,6 @@
 
 import com.velocitypowered.api.permission.PermissionSubject;
 import net.kyori.text.Component;
-import org.checkerframework.checker.nullness.qual.NonNull;
 
 /**
  * Represents something that can be used to run a {@link Command}.
@@ -12,5 +11,5 @@ public interface CommandSource extends PermissionSubject {
      * Sends the specified {@code component} to the invoker.
      * @param component the text component to send
      */
-    void sendMessage(@NonNull Component component);
+    void sendMessage(Component component);
 }

api/src/main/java/com/velocitypowered/api/event/EventHandler.java

@@ -1,12 +1,10 @@
 package com.velocitypowered.api.event;
 
-import org.checkerframework.checker.nullness.qual.NonNull;
-
 /**
  * Represents an interface to perform direct dispatch of an event. This makes integration easier to achieve with platforms
  * such as RxJava.
  */
 @FunctionalInterface
 public interface EventHandler<E> {
-    void execute(@NonNull E event);
+    void execute(E event);
 }

api/src/main/java/com/velocitypowered/api/event/EventManager.java

@@ -1,7 +1,5 @@
 package com.velocitypowered.api.event;
 
-import org.checkerframework.checker.nullness.qual.NonNull;
-
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -13,7 +11,7 @@ public interface EventManager {
      * @param plugin the plugin to associate with the listener
      * @param listener the listener to register
      */
-    void register(@NonNull Object plugin, @NonNull Object listener);
+    void register(Object plugin, Object listener);
 
     /**
      * Requests that the specified {@code handler} listen for events and associate it with the {@code plugin}.
@@ -22,7 +20,7 @@ public interface EventManager {
      * @param handler the handler to register
      * @param <E> the event type to handle
      */
-    default <E> void register(@NonNull Object plugin, @NonNull Class<E> eventClass, @NonNull EventHandler<E> handler) {
+    default <E> void register(Object plugin, Class<E> eventClass, EventHandler<E> handler) {
         register(plugin, eventClass, PostOrder.NORMAL, handler);
     }
 
@@ -34,42 +32,42 @@ default <E> void register(@NonNull Object plugin, @NonNull Class<E> eventClass,
      * @param handler the handler to register
      * @param <E> the event type to handle
      */
-    <E> void register(@NonNull Object plugin, @NonNull Class<E> eventClass, @NonNull PostOrder postOrder, @NonNull EventHandler<E> handler);
+    <E> void register(Object plugin, Class<E> eventClass, PostOrder postOrder, EventHandler<E> handler);
 
     /**
      * Fires the specified event to the event bus asynchronously. This allows Velocity to continue servicing connections
      * while a plugin handles a potentially long-running operation such as a database query.
      * @param event the event to fire
      * @return a {@link CompletableFuture} representing the posted event
      */
-    @NonNull <E> CompletableFuture<E> fire(@NonNull E event);
+    <E> CompletableFuture<E> fire(E event);
 
     /**
      * Posts the specified event to the event bus and discards the result.
      * @param event the event to fire
      */
-    default void fireAndForget(@NonNull Object event) {
+    default void fireAndForget(Object event) {
         fire(event);
     }
 
     /**
      * Unregisters all listeners for the specified {@code plugin}.
      * @param plugin the plugin to deregister listeners for
      */
-    void unregisterListeners(@NonNull Object plugin);
+    void unregisterListeners(Object plugin);
 
     /**
      * Unregisters a specific listener for a specific plugin.
      * @param plugin the plugin associated with the listener
      * @param listener the listener to deregister
      */
-    void unregisterListener(@NonNull Object plugin, @NonNull Object listener);
+    void unregisterListener(Object plugin, Object listener);
 
     /**
      * Unregisters a specific event handler for a specific plugin.
      * @param plugin the plugin to associate with the handler
      * @param handler the handler to register
      * @param <E> the event type to handle
      */
-    <E> void unregister(@NonNull Object plugin, @NonNull EventHandler<E> handler);
+    <E> void unregister(Object plugin, EventHandler<E> handler);
 }

api/src/main/java/com/velocitypowered/api/event/ResultedEvent.java

@@ -22,7 +22,7 @@ public interface ResultedEvent<R extends ResultedEvent.Result> {
      * Sets the result of this event. The result must be non-null.
      * @param result the new result
      */
-    void setResult(@NonNull R result);
+    void setResult(R result);
 
     /**
      * Represents a result for an event.
@@ -106,7 +106,7 @@ public static ComponentResult allowed() {
             return ALLOWED;
         }
 
-        public static ComponentResult denied(@NonNull Component reason) {
+        public static ComponentResult denied(Component reason) {
             Preconditions.checkNotNull(reason, "reason");
             return new ComponentResult(false, reason);
         }

api/src/main/java/com/velocitypowered/api/event/connection/ConnectionHandshakeEvent.java

@@ -2,15 +2,14 @@
 
 import com.google.common.base.Preconditions;
 import com.velocitypowered.api.proxy.InboundConnection;
-import org.checkerframework.checker.nullness.qual.NonNull;
 
 /**
  * This event is fired when a handshake is established between a client and Velocity.
  */
 public final class ConnectionHandshakeEvent {
-    private final @NonNull InboundConnection connection;
+    private final InboundConnection connection;
 
-    public ConnectionHandshakeEvent(@NonNull InboundConnection connection) {
+    public ConnectionHandshakeEvent(InboundConnection connection) {
         this.connection = Preconditions.checkNotNull(connection, "connection");
     }
 

api/src/main/java/com/velocitypowered/api/event/connection/DisconnectEvent.java

@@ -2,16 +2,15 @@
 
 import com.google.common.base.Preconditions;
 import com.velocitypowered.api.proxy.Player;
-import org.checkerframework.checker.nullness.qual.NonNull;
 
 /**
  * This event is fired when a player disconnects from the proxy. Operations on the provided player, aside from basic
  * data retrieval operations, may behave in undefined ways.
  */
 public final class DisconnectEvent {
-    private @NonNull final Player player;
+    private final Player player;
 
-    public DisconnectEvent(@NonNull Player player) {
+    public DisconnectEvent(Player player) {
         this.player = Preconditions.checkNotNull(player, "player");
     }
 

api/src/main/java/com/velocitypowered/api/event/connection/LoginEvent.java

@@ -3,7 +3,6 @@
 import com.google.common.base.Preconditions;
 import com.velocitypowered.api.event.ResultedEvent;
 import com.velocitypowered.api.proxy.Player;
-import org.checkerframework.checker.nullness.qual.NonNull;
 
 /**
  * This event is fired once the player has been authenticated but before they connect to a server on the proxy.
@@ -12,7 +11,7 @@ public final class LoginEvent implements ResultedEvent<ResultedEvent.ComponentRe
     private final Player player;
     private ComponentResult result;
 
-    public LoginEvent(@NonNull Player player) {
+    public LoginEvent(Player player) {
         this.player = Preconditions.checkNotNull(player, "player");
         this.result = ComponentResult.allowed();
     }
@@ -27,7 +26,7 @@ public ComponentResult getResult() {
     }
 
     @Override
-    public void setResult(@NonNull ComponentResult result) {
+    public void setResult(ComponentResult result) {
         this.result = Preconditions.checkNotNull(result, "result");
     }
 

api/src/main/java/com/velocitypowered/api/event/connection/PluginMessageEvent.java

@@ -36,7 +36,7 @@ public ForwardResult getResult() {
     }
 
     @Override
-    public void setResult(@NonNull ForwardResult result) {
+    public void setResult(ForwardResult result) {
         this.result = Preconditions.checkNotNull(result, "result");
     }