Add some missing null annotations

api/src/main/java/net/luckperms/api/actionlog/Action.java

@@ -170,7 +170,7 @@ public interface Action extends Comparable<Action> {
          * @param type the type
          * @return the builder
          */
-        @NonNull Builder targetType(Action.Target.Type type);
+        @NonNull Builder targetType(Target.@NonNull Type type);
 
         /**
          * Sets the acted object for the entry.

api/src/main/java/net/luckperms/api/context/ContextCalculator.java

@@ -103,7 +103,7 @@ public interface ContextCalculator<T> {
      *
      * @return a set of potential contexts
      */
-    default ContextSet estimatePotentialContexts() {
+    default @NonNull ContextSet estimatePotentialContexts() {
         return ImmutableContextSet.empty();
     }
 

api/src/main/java/net/luckperms/api/messaging/MessagingService.java

@@ -40,7 +40,7 @@ public interface MessagingService {
      *
      * @return the name of this messaging service
      */
-    String getName();
+    @NonNull String getName();
 
     /**
      * Uses the messaging service to inform other servers about a general

api/src/main/java/net/luckperms/api/node/Node.java

@@ -201,11 +201,11 @@ public interface Node {
     /**
      * Gets the metadata corresponding to the given <code>key</code>, if present.
      *
-     * @param key the key
      * @param <T> the metadata type
+     * @param key the key
      * @return the data, if present
      */
-    <T> Optional<T> getMetadata(NodeMetadataKey<T> key);
+    <T> Optional<T> getMetadata(@NonNull NodeMetadataKey<T> key);
 
     /**
      * Gets the metadata corresponding to the given <code>key</code>, throwing an exception
@@ -216,7 +216,7 @@ public interface Node {
      * @return the data
      * @throws IllegalStateException if data isn't present
      */
-    default <T> T metadata(NodeMetadataKey<T> key) throws IllegalStateException {
+    default <T> T metadata(@NonNull NodeMetadataKey<T> key) throws IllegalStateException {
         return getMetadata(key).orElseThrow(() -> new IllegalStateException("Node '" + getKey() + "' does not have '" + key.name() + "' attached."));
     }
 

api/src/main/java/net/luckperms/api/node/NodeBuilder.java

@@ -109,7 +109,7 @@ public interface NodeBuilder<N extends ScopedNode<N, B>, B extends NodeBuilder<N
      * @param unit     the unit <code>duration</code> is measured in
      * @return the builder
      */
-    default @NonNull B expiry(long duration, TimeUnit unit) {
+    default @NonNull B expiry(long duration, @NonNull TimeUnit unit) {
         if (duration <= 0) {
             throw new IllegalArgumentException("duration must be positive");
         }

api/src/main/java/net/luckperms/api/node/NodeType.java

@@ -35,6 +35,8 @@ import net.luckperms.api.node.types.RegexPermissionNode;
 import net.luckperms.api.node.types.SuffixNode;
 import net.luckperms.api.node.types.WeightNode;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
@@ -48,59 +50,105 @@ public interface NodeType<T extends Node> {
     /**
      * Node type for {@link PermissionNode}.
      */
-    NodeType<PermissionNode> PERMISSION = new SimpleNodeType<>("PERMISSION", n ->  n instanceof PermissionNode, n -> (PermissionNode) n);
+    NodeType<PermissionNode> PERMISSION = new SimpleNodeType<>(
+            "PERMISSION",
+            n -> n instanceof PermissionNode,
+            n -> (PermissionNode) n
+    );
 
     /**
      * Node type for {@link RegexPermissionNode}.
      */
-    NodeType<RegexPermissionNode> REGEX_PERMISSION = new SimpleNodeType<>("REGEX_PERMISSION", n ->  n instanceof RegexPermissionNode, n -> (RegexPermissionNode) n);
+    NodeType<RegexPermissionNode> REGEX_PERMISSION = new SimpleNodeType<>(
+            "REGEX_PERMISSION",
+            n -> n instanceof RegexPermissionNode,
+            n -> (RegexPermissionNode) n
+    );
 
     /**
      * Node type for {@link InheritanceNode}.
      */
-    NodeType<InheritanceNode> INHERITANCE = new SimpleNodeType<>("INHERITANCE", n ->  n instanceof InheritanceNode, n -> (InheritanceNode) n);
+    NodeType<InheritanceNode> INHERITANCE = new SimpleNodeType<>(
+            "INHERITANCE",
+            n -> n instanceof InheritanceNode,
+            n -> (InheritanceNode) n
+    );
 
     /**
      * Node type for {@link PrefixNode}.
      */
-    NodeType<PrefixNode> PREFIX = new SimpleNodeType<>("PREFIX", n ->  n instanceof PrefixNode, n -> (PrefixNode) n);
+    NodeType<PrefixNode> PREFIX = new SimpleNodeType<>(
+            "PREFIX",
+            n -> n instanceof PrefixNode,
+            n -> (PrefixNode) n
+    );
 
     /**
      * Node type for {@link SuffixNode}.
      */
-    NodeType<SuffixNode> SUFFIX = new SimpleNodeType<>("SUFFIX", n ->  n instanceof SuffixNode, n -> (SuffixNode) n);
+    NodeType<SuffixNode> SUFFIX = new SimpleNodeType<>(
+            "SUFFIX",
+            n -> n instanceof SuffixNode,
+            n -> (SuffixNode) n
+    );
 
     /**
      * Node type for {@link MetaNode}.
      */
-    NodeType<MetaNode> META = new SimpleNodeType<>("META", n ->  n instanceof MetaNode, n -> (MetaNode) n);
+    NodeType<MetaNode> META = new SimpleNodeType<>(
+            "META",
+            n -> n instanceof MetaNode,
+            n -> (MetaNode) n
+    );
 
     /**
      * Node type for {@link WeightNode}.
      */
-    NodeType<WeightNode> WEIGHT = new SimpleNodeType<>("WEIGHT", n ->  n instanceof WeightNode, n -> (WeightNode) n);
+    NodeType<WeightNode> WEIGHT = new SimpleNodeType<>(
+            "WEIGHT",
+            n -> n instanceof WeightNode,
+            n -> (WeightNode) n
+    );
 
     /**
      * Node type for {@link DisplayNameNode}.
      */
-    NodeType<DisplayNameNode> DISPLAY_NAME = new SimpleNodeType<>("DISPLAY_NAME", n ->  n instanceof DisplayNameNode, n -> (DisplayNameNode) n);
+    NodeType<DisplayNameNode> DISPLAY_NAME = new SimpleNodeType<>(
+            "DISPLAY_NAME",
+            n -> n instanceof DisplayNameNode,
+            n -> (DisplayNameNode) n
+    );
 
     /**
      * Node type for {@link ChatMetaNode}.
+     *
+     * <p>This is an abstract type, and therefore will never
+     * be returned from {@link Node#getType()}.</p>
      */
-    NodeType<ChatMetaNode<?, ?>> CHAT_META = new SimpleNodeType<>("CHAT_META", n ->  n instanceof ChatMetaNode<?, ?>, n -> (ChatMetaNode<?, ?>) n);
+    NodeType<ChatMetaNode<?, ?>> CHAT_META = new SimpleNodeType<>(
+            "CHAT_META",
+            n -> n instanceof ChatMetaNode<?, ?>,
+            n -> (ChatMetaNode<?, ?>) n
+    );
 
     /**
      * Node type for {@link ChatMetaNode} or {@link MetaNode}.
+     *
+     * <p>This is an abstract type, and therefore will never
+     * be returned from {@link Node#getType()}.</p>
      */
-    NodeType<Node> META_OR_CHAT_META = new SimpleNodeType<>("META_OR_CHAT_META", n ->  META.matches(n) || CHAT_META.matches(n), Function.identity());
+    NodeType<Node> META_OR_CHAT_META = new SimpleNodeType<>(
+            "META_OR_CHAT_META",
+            n -> META.matches(n) || CHAT_META.matches(n),
+            Function.identity()
+    );
 
     /**
      * Gets a name for the node type.
      *
      * @return a name
      */
-    String name();
+    @NonNull String name();
 
     /**
      * Returns if the passed node matches the type
@@ -108,7 +156,7 @@ public interface NodeType<T extends Node> {
      * @param node the node to test
      * @return true if the node has the same type
      */
-    boolean matches(Node node);
+    boolean matches(@NonNull Node node);
 
     /**
      * Casts the given {@link Node} to the type defined by the {@link NodeType}.
@@ -120,7 +168,7 @@ public interface NodeType<T extends Node> {
      * @return the casted node
      * @throws IllegalArgumentException if the node to cast does not match the type
      */
-    T cast(Node node);
+    @NonNull T cast(@NonNull Node node);
 
     /**
      * Attempts to cast the given {@link Node} to the type defined by the
@@ -132,7 +180,7 @@ public interface NodeType<T extends Node> {
      * @param node the node to cast
      * @return an optional, possibly containing a casted node
      */
-    default Optional<T> tryCast(Node node) {
+    default @NonNull Optional<T> tryCast(@NonNull Node node) {
         Objects.requireNonNull(node, "node");
         if (!matches(node)) {
             return Optional.empty();
@@ -147,7 +195,7 @@ public interface NodeType<T extends Node> {
      *
      * @return a predicate for the {@link #matches(Node)} method.
      */
-    default Predicate<Node> predicate() {
+    default @NonNull Predicate<Node> predicate() {
         return this::matches;
     }
 
@@ -159,7 +207,7 @@ public interface NodeType<T extends Node> {
      * @param and a predicate to AND with the result of the type match check
      * @return a matching predicate, ANDed with the given predicate parameter
      */
-    default Predicate<Node> predicate(Predicate<? super T> and) {
+    default @NonNull Predicate<Node> predicate(@NonNull Predicate<? super T> and) {
         return node -> matches(node) && and.test(cast(node));
     }
 

api/src/main/java/net/luckperms/api/node/SimpleNodeType.java

@@ -25,6 +25,9 @@
 
 package net.luckperms.api.node;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.jetbrains.annotations.NotNull;
+
 import java.util.Objects;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -41,18 +44,18 @@ final class SimpleNodeType<T extends Node> implements NodeType<T> {
     }
 
     @Override
-    public String name() {
+    public @NotNull String name() {
         return this.name;
     }
 
     @Override
-    public boolean matches(Node node) {
+    public boolean matches(@NonNull Node node) {
         Objects.requireNonNull(node, "node");
         return this.matches.test(node);
     }
 
     @Override
-    public T cast(Node node) {
+    public @NotNull T cast(@NonNull Node node) {
         if (!matches(node)) {
             throw new IllegalArgumentException("Node " + node.getClass() + " does not match " + this.name);
         }

api/src/main/java/net/luckperms/api/node/matcher/NodeMatcher.java

@@ -134,7 +134,7 @@ public interface NodeMatcher<T extends Node> extends Predicate<Node> {
      * @param <T> the node type
      * @return the matcher
      */
-    static <T extends Node> @NonNull NodeMatcher<T> type(NodeType<? extends T> type) {
+    static <T extends Node> @NonNull NodeMatcher<T> type(@NonNull NodeType<? extends T> type) {
         return LuckPermsProvider.get().getNodeMatcherFactory().type(type);
     }
 

api/src/main/java/net/luckperms/api/node/matcher/NodeMatcherFactory.java

@@ -117,6 +117,6 @@ public interface NodeMatcherFactory {
      * @param <T> the node type
      * @return the matcher
      */
-    <T extends Node> @NonNull NodeMatcher<T> type(NodeType<? extends T> type);
+    <T extends Node> @NonNull NodeMatcher<T> type(@NonNull NodeType<? extends T> type);
 
 }

api/src/main/java/net/luckperms/api/query/dataorder/DataQueryOrder.java

@@ -93,7 +93,7 @@ public enum DataQueryOrder implements Comparator<DataType> {
      * @param comparator the comparator
      * @return the ordered data types
      */
-    public static List<DataType> order(@NonNull Comparator<? super DataType> comparator) {
+    public static @NonNull List<DataType> order(@NonNull Comparator<? super DataType> comparator) {
         int compare = comparator.compare(DataType.TRANSIENT, DataType.NORMAL);
         if (compare > 0) {
             // transient first

api/src/main/java/net/luckperms/api/query/dataorder/DataQueryOrderFunction.java

@@ -32,6 +32,7 @@ import net.luckperms.api.query.OptionKey;
 import org.checkerframework.checker.nullness.qual.NonNull;
 
 import java.util.Comparator;
+import java.util.Objects;
 
 /**
  * A function that generates a {@link DataQueryOrder} comparator for
@@ -52,7 +53,8 @@ public interface DataQueryOrderFunction {
      * @return the data query order function
      * @since 5.2
      */
-    static DataQueryOrderFunction always(Comparator<DataType> comparator) {
+    static @NonNull DataQueryOrderFunction always(@NonNull Comparator<DataType> comparator) {
+        Objects.requireNonNull(comparator, "comparator");
         return id -> comparator;
     }
 

api/src/main/java/net/luckperms/api/query/dataorder/DataTypeFilterFunction.java

@@ -31,6 +31,7 @@ import net.luckperms.api.query.OptionKey;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
 
+import java.util.Objects;
 import java.util.function.Predicate;
 
 /**
@@ -53,7 +54,8 @@ public interface DataTypeFilterFunction {
      * @param predicate the predicate
      * @return the data type filter function
      */
-    static DataTypeFilterFunction always(Predicate<DataType> predicate) {
+    static @NonNull DataTypeFilterFunction always(@NonNull Predicate<DataType> predicate) {
+        Objects.requireNonNull(predicate, "predicate");
         return id -> predicate;
     }
 

bukkit/src/main/java/me/lucko/luckperms/bukkit/context/BukkitPlayerCalculator.java

@@ -105,7 +105,7 @@ public class BukkitPlayerCalculator implements ContextCalculator<Player>, Listen
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         if (this.gamemode) {
             for (GameMode mode : GameMode.values()) {

bungee/src/main/java/me/lucko/luckperms/bungee/context/BungeePlayerCalculator.java

@@ -60,7 +60,7 @@ public class BungeePlayerCalculator implements ContextCalculator<ProxiedPlayer>,
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         for (ServerInfo server : this.plugin.getBootstrap().getProxy().getServers().values()) {
             builder.add(DefaultContextKeys.WORLD_KEY, server.getName());

bungee/src/main/java/me/lucko/luckperms/bungee/context/RedisBungeeCalculator.java

@@ -49,7 +49,7 @@ public class RedisBungeeCalculator implements StaticContextCalculator {
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NonNull ContextSet estimatePotentialContexts() {
         RedisBungeeAPI redisBungee = RedisBungee.getApi();
         if (redisBungee == null) {
             return ImmutableContextSetImpl.EMPTY;

common/src/main/java/me/lucko/luckperms/common/actionlog/LoggedAction.java

@@ -269,7 +269,7 @@ public class LoggedAction implements Action {
         }
 
         @Override
-        public @NonNull Builder targetType(Action.Target.Type type) {
+        public @NonNull Builder targetType(Target.@NonNull Type type) {
             this.targetType = Objects.requireNonNull(type, "type");
             return this;
         }

common/src/main/java/me/lucko/luckperms/common/api/implementation/ApiMessagingService.java

@@ -42,7 +42,7 @@ public class ApiMessagingService implements MessagingService {
     }
 
     @Override
-    public String getName() {
+    public @NonNull String getName() {
         return this.handle.getName();
     }
 

common/src/main/java/me/lucko/luckperms/common/context/ConfigurationContextCalculator.java

@@ -54,7 +54,7 @@ public class ConfigurationContextCalculator implements StaticContextCalculator {
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         calculate(builder::add);
         return builder.build();

common/src/main/java/me/lucko/luckperms/common/context/ContextSetComparator.java

@@ -53,8 +53,14 @@ public class ContextSetComparator implements Comparator<ImmutableContextSet> {
             return 0;
         }
 
+        // compare presence of any context (non-empty)
+        int result = Boolean.compare(!o1.isEmpty(), !o2.isEmpty());
+        if (result != 0) {
+            return result;
+        }
+
         // compare presence of a server context
-        int result = Boolean.compare(o1.containsKey(DefaultContextKeys.SERVER_KEY), o2.containsKey(DefaultContextKeys.SERVER_KEY));
+        result = Boolean.compare(o1.containsKey(DefaultContextKeys.SERVER_KEY), o2.containsKey(DefaultContextKeys.SERVER_KEY));
         if (result != 0) {
             return result;
         }
@@ -75,13 +81,10 @@ public class ContextSetComparator implements Comparator<ImmutableContextSet> {
         // However, we *have* to maintain transitivity in this comparator (despite how
         // expensive/complex it may be) as it is used in the PermissionHolder nodes treemap.
 
-        // in order to have consistent ordering, we have to compare the content of the context sets
+        // in order to have consistent ordering, we have to compare the content of the context sets.
-        // by sorting the contents and then comparing which set is greater.
+        // to do this, obtain sorted array representations of each set, then compare which is greater
-        Context[] o1Array = o1 instanceof ImmutableContextSetImpl ? ((ImmutableContextSetImpl) o1).toArray() : o1.toSet().toArray(new Context[0]);
+        Context[] o1Array = o1 instanceof ImmutableContextSetImpl ? ((ImmutableContextSetImpl) o1).toArray() : toArray(o1);
-        Context[] o2Array = o2 instanceof ImmutableContextSetImpl ? ((ImmutableContextSetImpl) o2).toArray() : o2.toSet().toArray(new Context[0]);
+        Context[] o2Array = o2 instanceof ImmutableContextSetImpl ? ((ImmutableContextSetImpl) o2).toArray() : toArray(o2);
-
-        Arrays.sort(o1Array, CONTEXT_COMPARATOR);
-        Arrays.sort(o2Array, CONTEXT_COMPARATOR);
 
         for (int i = 0; i < o1Array.length; i++) {
             Context ent1 = o1Array[i];
@@ -96,7 +99,13 @@ public class ContextSetComparator implements Comparator<ImmutableContextSet> {
         throw new AssertionError("sets are equal? " + o1 + " - " + o2);
     }
 
-    private static final Comparator<Context> CONTEXT_COMPARATOR = ContextSetComparator::compareContexts;
+    public static final Comparator<Context> CONTEXT_COMPARATOR = ContextSetComparator::compareContexts;
+
+    private static Context[] toArray(ImmutableContextSet set) {
+        Context[] array = set.toSet().toArray(new Context[0]);
+        Arrays.sort(array, CONTEXT_COMPARATOR);
+        return array;
+    }
 
     private static int compareContexts(Context o1, Context o2) {
         if (o1 == o2) {

common/src/main/java/me/lucko/luckperms/common/context/contextset/ImmutableContextSetImpl.java

@@ -32,6 +32,8 @@ import com.google.common.collect.Multimap;
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.SetMultimap;
 
+import me.lucko.luckperms.common.context.ContextSetComparator;
+
 import net.luckperms.api.context.Context;
 import net.luckperms.api.context.ContextSatisfyMode;
 import net.luckperms.api.context.ContextSet;
@@ -40,6 +42,7 @@ import net.luckperms.api.context.MutableContextSet;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Objects;
@@ -74,6 +77,8 @@ public final class ImmutableContextSetImpl extends AbstractContextSet implements
         for (Map.Entry<String, String> e : entries) {
             this.array[i++] = new ContextImpl(e.getKey(), e.getValue());
         }
+        // sort the array at construction so the comparator doesn't need to
+        Arrays.sort(this.array, ContextSetComparator.CONTEXT_COMPARATOR);
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/node/AbstractNode.java

@@ -97,7 +97,7 @@ public abstract class AbstractNode<N extends ScopedNode<N, B>, B extends NodeBui
     }
 
     @Override
-    public <T> Optional<T> getMetadata(NodeMetadataKey<T> key) {
+    public <T> Optional<T> getMetadata(@NonNull NodeMetadataKey<T> key) {
         //noinspection unchecked
         T value = (T) this.metadata.get(key);
         return Optional.ofNullable(value);

fabric/src/main/java/me/lucko/luckperms/fabric/context/FabricPlayerCalculator.java

@@ -44,6 +44,7 @@ import net.minecraft.util.Identifier;
 import net.minecraft.world.GameMode;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.Optional;
 import java.util.Set;
@@ -86,7 +87,7 @@ public class FabricPlayerCalculator implements ContextCalculator<ServerPlayerEnt
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NotNull @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
 
         if (this.gamemode) {

nukkit/src/main/java/me/lucko/luckperms/nukkit/context/NukkitPlayerCalculator.java

@@ -82,7 +82,7 @@ public class NukkitPlayerCalculator implements ContextCalculator<Player>, Listen
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         if (this.gamemode) {
             for (int mode : KNOWN_GAMEMODES) {

sponge/src/main/java/me/lucko/luckperms/sponge/context/SpongePlayerCalculator.java

@@ -95,7 +95,7 @@ public class SpongePlayerCalculator implements ContextCalculator<Subject> {
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         Game game = this.plugin.getBootstrap().getGame();
 

velocity/src/main/java/me/lucko/luckperms/velocity/context/VelocityPlayerCalculator.java

@@ -43,6 +43,7 @@ import net.luckperms.api.context.DefaultContextKeys;
 import net.luckperms.api.context.ImmutableContextSet;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.jetbrains.annotations.NotNull;
 
 public class VelocityPlayerCalculator implements ContextCalculator<Player> {
     private final LPVelocityPlugin plugin;
@@ -61,7 +62,7 @@ public class VelocityPlayerCalculator implements ContextCalculator<Player> {
     }
 
     @Override
-    public ContextSet estimatePotentialContexts() {
+    public @NotNull @NonNull ContextSet estimatePotentialContexts() {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         for (RegisteredServer server : this.plugin.getBootstrap().getProxy().getAllServers()) {
             builder.add(DefaultContextKeys.WORLD_KEY, server.getServerInfo().getName());