From b0086ba0f725d96578d5f237502bd9232d5f2e17 Mon Sep 17 00:00:00 2001 From: Magnus Ulf Date: Fri, 8 Feb 2019 14:27:20 +0100 Subject: [PATCH] Make code less null-happy A lot of the code is filled with acceptance of null, even when null could mean something went wrong. We no longer want to fail-safe but want to fail fast, so this approach to null is changing. This will surely cause some bugs in the beginning, but will also make them less frequent in the long run. --- .../massivecraft/massivecore/Progressbar.java | 2 +- .../massivecraft/massivecore/SoundEffect.java | 6 +- .../massivecore/command/MassiveCommand.java | 23 ---- .../engine/EngineMassiveCorePlayerState.java | 2 +- .../massivecore/mixin/MixinVisibility.java | 4 +- .../particleeffect/ParticleEffect.java | 4 +- .../massivecore/util/BoardUtil.java | 2 +- .../massivecraft/massivecore/util/IdUtil.java | 16 +-- .../massivecraft/massivecore/util/MUtil.java | 113 +++--------------- .../massivecore/util/PermissionUtil.java | 6 +- .../massivecore/util/PlayerUtil.java | 10 +- .../massivecore/util/SmokeUtil.java | 2 +- .../massivecore/util/TimeUnit.java | 2 +- .../massivecraft/massivecore/util/Txt.java | 13 +- 14 files changed, 55 insertions(+), 150 deletions(-) diff --git a/src/com/massivecraft/massivecore/Progressbar.java b/src/com/massivecraft/massivecore/Progressbar.java index 32d0b2f1..4f89146f 100644 --- a/src/com/massivecraft/massivecore/Progressbar.java +++ b/src/com/massivecraft/massivecore/Progressbar.java @@ -184,7 +184,7 @@ public class Progressbar public static String colorParse(String string, String colorTag, String color) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); string = string.replace(colorTag, color); string = Txt.parse(string); return string; diff --git a/src/com/massivecraft/massivecore/SoundEffect.java b/src/com/massivecraft/massivecore/SoundEffect.java index 38bbbe5c..855fab16 100644 --- a/src/com/massivecraft/massivecore/SoundEffect.java +++ b/src/com/massivecraft/massivecore/SoundEffect.java @@ -81,12 +81,13 @@ public final class SoundEffect implements Serializable public void run(Location location) { - if (location == null) return; + if (location == null) throw new NullPointerException("location"); location.getWorld().playSound(location, this.getSound(), this.getVolume(), this.getPitch()); } public void run(HumanEntity human, Location location) { + if (human == null) throw new NullPointerException("human"); if (MUtil.isntPlayer(human)) return; Player player = (Player)human; player.playSound(location, this.getSound(), this.getVolume(), this.getPitch()); @@ -94,6 +95,7 @@ public final class SoundEffect implements Serializable public void run(HumanEntity human) { + if (human == null) throw new NullPointerException("human"); if (MUtil.isntPlayer(human)) return; this.run(human, human.getEyeLocation()); } @@ -144,8 +146,6 @@ public final class SoundEffect implements Serializable @Override public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null) return false; if (!(obj instanceof SoundEffect)) return false; SoundEffect other = (SoundEffect) obj; if (Float.floatToIntBits(pitch) != Float.floatToIntBits(other.pitch)) return false; diff --git a/src/com/massivecraft/massivecore/command/MassiveCommand.java b/src/com/massivecraft/massivecore/command/MassiveCommand.java index a0120314..26931945 100644 --- a/src/com/massivecraft/massivecore/command/MassiveCommand.java +++ b/src/com/massivecraft/massivecore/command/MassiveCommand.java @@ -1650,28 +1650,5 @@ public class MassiveCommand implements Active, PluginIdentifiableCommand // Increment is done in this method return readArgAt(idx); } - - // TODO: Some of these are still used by external plugins. - // TODO: Fix those plugins. - - @Deprecated - public T readArgFrom(Type type) throws MassiveException - { - return this.readArgFrom(null, type); - } - - @Deprecated - public T readArgFrom(String str, Type type) throws MassiveException - { - if (type == null) throw new IllegalArgumentException("type is null"); - return type.read(str, this.sender); - } - - @Deprecated - public T readArgFrom(String str, Type type, T defaultNotSet) throws MassiveException - { - if (str == null) return defaultNotSet; - return this.readArgFrom(str, type); - } } diff --git a/src/com/massivecraft/massivecore/engine/EngineMassiveCorePlayerState.java b/src/com/massivecraft/massivecore/engine/EngineMassiveCorePlayerState.java index 5593e49f..e1d185b5 100644 --- a/src/com/massivecraft/massivecore/engine/EngineMassiveCorePlayerState.java +++ b/src/com/massivecraft/massivecore/engine/EngineMassiveCorePlayerState.java @@ -35,7 +35,7 @@ public class EngineMassiveCorePlayerState extends Engine if ( ! active) return; this.idToState.clear(); - for (Player player : MUtil.getOnlinePlayers()) + for (Player player : Bukkit.getOnlinePlayers()) { this.idToState.put(player.getUniqueId(), PlayerState.JOINED); } diff --git a/src/com/massivecraft/massivecore/mixin/MixinVisibility.java b/src/com/massivecraft/massivecore/mixin/MixinVisibility.java index 4649272c..916ece6c 100644 --- a/src/com/massivecraft/massivecore/mixin/MixinVisibility.java +++ b/src/com/massivecraft/massivecore/mixin/MixinVisibility.java @@ -1,7 +1,7 @@ package com.massivecraft.massivecore.mixin; import com.massivecraft.massivecore.util.IdUtil; -import com.massivecraft.massivecore.util.MUtil; +import org.bukkit.Bukkit; import org.bukkit.entity.Player; public class MixinVisibility extends Mixin @@ -29,7 +29,7 @@ public class MixinVisibility extends Mixin Player pwatchee = IdUtil.getPlayer(watcheeObject); if (pwatchee == null) return true; - for (Player pwatcher : MUtil.getOnlinePlayers()) + for (Player pwatcher : Bukkit.getOnlinePlayers()) { if (pwatchee.equals(pwatcher)) continue; if (pwatcher.canSee(pwatchee)) continue; diff --git a/src/com/massivecraft/massivecore/particleeffect/ParticleEffect.java b/src/com/massivecraft/massivecore/particleeffect/ParticleEffect.java index 2693c19f..bdd08994 100644 --- a/src/com/massivecraft/massivecore/particleeffect/ParticleEffect.java +++ b/src/com/massivecraft/massivecore/particleeffect/ParticleEffect.java @@ -1,8 +1,8 @@ package com.massivecraft.massivecore.particleeffect; import com.massivecraft.massivecore.particleeffect.ReflectionUtils.PackageType; -import com.massivecraft.massivecore.util.MUtil; import com.massivecraft.massivecore.util.ReflectionUtil; +import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.entity.Player; @@ -1512,7 +1512,7 @@ public enum ParticleEffect { } String worldName = center.getWorld().getName(); double squared = range * range; - for (Player player : MUtil.getOnlinePlayers()) { + for (Player player : Bukkit.getOnlinePlayers()) { if (!player.getWorld().getName().equals(worldName) || player.getLocation().distanceSquared(center) > squared) { continue; } diff --git a/src/com/massivecraft/massivecore/util/BoardUtil.java b/src/com/massivecraft/massivecore/util/BoardUtil.java index d7ef61f5..3509c891 100644 --- a/src/com/massivecraft/massivecore/util/BoardUtil.java +++ b/src/com/massivecraft/massivecore/util/BoardUtil.java @@ -145,7 +145,7 @@ public class BoardUtil extends Engine Map players = new MassiveMap<>(); // Fill - for (Player player : MUtil.getOnlinePlayers()) + for (Player player : Bukkit.getOnlinePlayers()) { players.put(player.getName(), player); } diff --git a/src/com/massivecraft/massivecore/util/IdUtil.java b/src/com/massivecraft/massivecore/util/IdUtil.java index a8aa6611..a2beb2ac 100644 --- a/src/com/massivecraft/massivecore/util/IdUtil.java +++ b/src/com/massivecraft/massivecore/util/IdUtil.java @@ -181,7 +181,7 @@ public class IdUtil implements Listener, Runnable Set ret = new MassiveSet<>(); // Add Online Players - ret.addAll(MUtil.getOnlinePlayers()); + ret.addAll(Bukkit.getOnlinePlayers()); // Add Console ret.add(getConsole()); @@ -461,7 +461,7 @@ public class IdUtil implements Listener, Runnable public static IdData getData(Object senderObject) { // Null Return - if (senderObject == null) return null; + if (senderObject == null) throw new NullPointerException("senderObject"); // Already Done if (senderObject instanceof IdData) return (IdData) senderObject; @@ -525,7 +525,7 @@ public class IdUtil implements Listener, Runnable public static CommandSender getSender(Object senderObject) { // Null Return - if (senderObject == null) return null; + if (senderObject == null) throw new NullPointerException("senderObject"); // Already Done if (senderObject instanceof CommandSender) return (CommandSender) senderObject; @@ -590,7 +590,7 @@ public class IdUtil implements Listener, Runnable public static UUID getUuid(Object senderObject) { // Null Return - if (senderObject == null) return null; + if (senderObject == null) throw new NullPointerException("senderObject"); // Already Done if (senderObject instanceof UUID) return (UUID)senderObject; @@ -655,7 +655,7 @@ public class IdUtil implements Listener, Runnable public static String getId(Object senderObject) { // Null Return - if (senderObject == null) return null; + if (senderObject == null) throw new NullPointerException("senderObject"); // Already Done if (senderObject instanceof String && MUtil.isUuid((String)senderObject)) return (String)senderObject; @@ -731,7 +731,7 @@ public class IdUtil implements Listener, Runnable public static String getName(Object senderObject) { // Null Return - if (senderObject == null) return null; + if (senderObject == null) throw new NullPointerException("senderObject"); // Already Done // Handled at "Data" (not applicable - names can look differently) @@ -780,7 +780,7 @@ public class IdUtil implements Listener, Runnable public static OfflinePlayer getOfflinePlayer(Object senderObject) { // Null Return - if (senderObject == null) return null; + if (senderObject == null) throw new NullPointerException("senderObject"); // Already done if (senderObject instanceof OfflinePlayer) return (OfflinePlayer) senderObject; @@ -965,7 +965,7 @@ public class IdUtil implements Listener, Runnable long millis = System.currentTimeMillis(); - for (Player player : MUtil.getOnlinePlayers()) + for (Player player : Bukkit.getOnlinePlayers()) { String id = getId(player); if (id == null) throw new NullPointerException("id"); diff --git a/src/com/massivecraft/massivecore/util/MUtil.java b/src/com/massivecraft/massivecore/util/MUtil.java index 24c7eda4..61d3a3f6 100644 --- a/src/com/massivecraft/massivecore/util/MUtil.java +++ b/src/com/massivecraft/massivecore/util/MUtil.java @@ -56,7 +56,6 @@ import org.bukkit.potion.PotionEffect; import org.bukkit.potion.PotionEffectType; import org.bukkit.projectiles.ProjectileSource; -import java.lang.reflect.Method; import java.math.BigDecimal; import java.math.RoundingMode; import java.net.InetAddress; @@ -81,99 +80,17 @@ import java.util.regex.Pattern; public class MUtil { - // -------------------------------------------- // - // CONSTANTS - // -------------------------------------------- // - - private static Method methodGetOnlinePlayers; - - static - { - methodGetOnlinePlayers = getMethodGetOnlinePlayers(); - } - // -------------------------------------------- // // GET ONLINE PLAYERS // -------------------------------------------- // - // It seems we can not always trust the Bukkit.getOnlinePlayers() method. - // Due to compilation issue this method might not exist in the form we compiled against. - // Spigot 1.8 and the 1.7 Bukkit might have been compiled slightly differently resulting in this issue. - // Issue Example: https://github.com/MassiveCraft/MassiveCore/issues/192 - - public static Method getMethodGetOnlinePlayers() - { - Method ret = null; - - try - { - for (Method method : Bukkit.class.getDeclaredMethods()) - { - // The method name must be getOnlinePlayers ... - if ( ! method.getName().equals("getOnlinePlayers")) continue; - - // ... if we find such a method it's better than nothing ... - if (ret == null) ret = method; - - // ... but if the method additionally returns a collection ... - if ( ! method.getReturnType().isAssignableFrom(Collection.class)) continue; - - // ... that is preferable ... - ret = method; - - // ... and we need not look any further. - break; - } - - ret.setAccessible(true); - } - catch (Exception e) - { - // If we fail we do so silently. - // This method is probably almost never going to be used anyways. - } - - return ret; - } - + // Old thing that had to do with the converfrion from 1.17 to 1.8 + + @Deprecated public static Collection getOnlinePlayers() { - // Fetch some kind of playersObject. - Object playersObject = null; - try - { - playersObject = Bukkit.getOnlinePlayers(); - } - catch (Throwable t) - { - // That didn't work! - // We probably just caught a NoSuchMethodError. - // So let's try with reflection instead. - try - { - playersObject = methodGetOnlinePlayers.invoke(null); - } - catch (Exception e) - { - e.printStackTrace(); - } - } - - // Now return the playersObject. - if (playersObject instanceof Collection) - { - @SuppressWarnings("unchecked") - Collection playersCollection = (Collection)playersObject; - return playersCollection; - } - else if (playersObject instanceof Player[]) - { - Player[] playersArray = (Player[])playersObject; - return Arrays.asList(playersArray); - } - else - { - throw new RuntimeException("Failed retrieving online players."); - } + Collectioncoll = Bukkit.getOnlinePlayers(); + List ret = new MassiveList<>(coll.size()); + return ret; } // -------------------------------------------- // @@ -195,7 +112,7 @@ public class MUtil // Fill final World world = location.getWorld(); final double radiusSquared = radius * radius; - for (Player player : MUtil.getOnlinePlayers()) + for (Player player : Bukkit.getOnlinePlayers()) { Location playerLocation = player.getLocation(); World playerWorld = playerLocation.getWorld(); @@ -251,7 +168,7 @@ public class MUtil public static UUID asUuid(String string) { // Null - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); // Avoid Exception if (string.length() != 36) return null; @@ -285,7 +202,7 @@ public class MUtil public static boolean containsCommand(String needle, Iterable haystack) { - if (needle == null) return false; + if (needle == null) throw new NullPointerException("needle"); needle = prepareCommand(needle); for (String straw : haystack) @@ -314,7 +231,7 @@ public class MUtil private static String prepareCommand(String string) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); string = Txt.removeLeadingCommandDust(string); string = string.toLowerCase(); string = string.trim(); @@ -384,6 +301,8 @@ public class MUtil public static boolean isNpc(Object object) { + if (object == null) throw new NullPointerException("object"); + if ( ! (object instanceof Metadatable)) return false; Metadatable metadatable = (Metadatable)object; try @@ -405,6 +324,8 @@ public class MUtil public static boolean isSender(Object object) { + if (object == null) throw new NullPointerException("object"); + if (!(object instanceof CommandSender)) return false; if (isNpc(object)) return false; return true; @@ -416,6 +337,8 @@ public class MUtil public static boolean isPlayer(Object object) { + if (object == null) throw new NullPointerException("object"); + if (!(object instanceof Player)) return false; if (isNpc(object)) return false; return true; @@ -953,7 +876,7 @@ public class MUtil public static T regexPickFirstVal(String input, Map regex2val) { - if (regex2val == null) return null; + if (regex2val == null) throw new NullPointerException("regex2val"); T ret = null; for (Entry entry : regex2val.entrySet()) @@ -969,7 +892,7 @@ public class MUtil public static T equalsPickFirstVal(E input, Map thing2val) { - if (thing2val == null) return null; + if (thing2val == null) throw new NullPointerException("thing2val"); T ret = null; for (Entry entry : thing2val.entrySet()) diff --git a/src/com/massivecraft/massivecore/util/PermissionUtil.java b/src/com/massivecraft/massivecore/util/PermissionUtil.java index 494490f2..61521af5 100644 --- a/src/com/massivecraft/massivecore/util/PermissionUtil.java +++ b/src/com/massivecraft/massivecore/util/PermissionUtil.java @@ -83,7 +83,7 @@ public class PermissionUtil public static String asPermissionId(Object object) { - if (object == null) return null; + if (object == null) throw new NullPointerException("object"); if (object instanceof String) return (String)object; if (object instanceof Identified) return ((Identified)object).getId(); @@ -94,7 +94,7 @@ public class PermissionUtil public static Permission asPermission(Object object) { - if (object == null) return null; + if (object == null) throw new NullPointerException("object"); if (object instanceof Permission) return (Permission)object; @@ -228,6 +228,8 @@ public class PermissionUtil public static boolean setPermissionStandardChildren(Permission permission, PermissionDefault standard, Map children) { + if (permission == null) throw new NullPointerException("permission"); + boolean childrenChanged = false; boolean standardChanged = false; diff --git a/src/com/massivecraft/massivecore/util/PlayerUtil.java b/src/com/massivecraft/massivecore/util/PlayerUtil.java index a51abb63..0423f96e 100644 --- a/src/com/massivecraft/massivecore/util/PlayerUtil.java +++ b/src/com/massivecraft/massivecore/util/PlayerUtil.java @@ -64,7 +64,7 @@ public class PlayerUtil extends Engine public static void setLastMoveMillis(Player player, long millis) { - if (player == null) return; + if (player == null) throw new NullPointerException("player"); idToLastMoveMillis.put(player.getUniqueId(), millis); } @@ -94,7 +94,7 @@ public class PlayerUtil extends Engine public static long getLastMoveMillis(Player player) { - if (player == null) return 0; + if (player == null) throw new NullPointerException("player"); Long ret = idToLastMoveMillis.get(player.getUniqueId()); if (ret == null) return 0; return ret; @@ -102,7 +102,7 @@ public class PlayerUtil extends Engine public static long getStandStillMillis(Player player) { - if (player == null) return 0; + if (player == null) throw new NullPointerException("player"); if (player.isDead()) return 0; if (!player.isOnline()) return 0; @@ -122,8 +122,8 @@ public class PlayerUtil extends Engine public static void setLastDamageMillis(Player player, long millis) { + if (player == null) throw new NullPointerException("player"); if (MUtil.isntPlayer(player)) return; - if (player == null) return; idToLastDamageMillis.put(player.getUniqueId(), millis); } @@ -157,6 +157,7 @@ public class PlayerUtil extends Engine public static long getLastDamageMillis(Player player) { + if (player == null) throw new NullPointerException("player"); if (MUtil.isntPlayer(player)) return 0; Long ret = idToLastDamageMillis.get(player.getUniqueId()); if (ret == null) return 0; @@ -165,6 +166,7 @@ public class PlayerUtil extends Engine public static long getNoDamageMillis(Player player) { + if (player == null) throw new NullPointerException("player"); if (MUtil.isntPlayer(player)) return 0; if (player.isDead()) return 0; if (!player.isOnline()) return 0; diff --git a/src/com/massivecraft/massivecore/util/SmokeUtil.java b/src/com/massivecraft/massivecore/util/SmokeUtil.java index 59707278..6730abcd 100644 --- a/src/com/massivecraft/massivecore/util/SmokeUtil.java +++ b/src/com/massivecraft/massivecore/util/SmokeUtil.java @@ -34,7 +34,7 @@ public class SmokeUtil // Single ======== public static void spawnSingle(Location location, int direction) { - if (location == null) return; + if (location == null) throw new NullPointerException("location"); location.getWorld().playEffect(location, Effect.SMOKE, direction); } diff --git a/src/com/massivecraft/massivecore/util/TimeUnit.java b/src/com/massivecraft/massivecore/util/TimeUnit.java index 2a5ca36d..48c271d0 100644 --- a/src/com/massivecraft/massivecore/util/TimeUnit.java +++ b/src/com/massivecraft/massivecore/util/TimeUnit.java @@ -70,7 +70,7 @@ public class TimeUnit implements Comparable public static TimeUnit get(String timeUnitString) { - if (timeUnitString == null) return null; + if (timeUnitString == null) throw new NullPointerException("timeUnitString"); String timeUnitStringLowerCase = timeUnitString.toLowerCase(); for (TimeUnit timeUnit : all) { diff --git a/src/com/massivecraft/massivecore/util/Txt.java b/src/com/massivecraft/massivecore/util/Txt.java index 488e624f..3f818b96 100644 --- a/src/com/massivecraft/massivecore/util/Txt.java +++ b/src/com/massivecraft/massivecore/util/Txt.java @@ -170,7 +170,7 @@ public class Txt public static String parse(String string) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); StringBuffer ret = new StringBuffer(); Matcher matcher = parsePattern.matcher(string); while (matcher.find()) @@ -202,7 +202,7 @@ public class Txt public static ArrayList wrap(final String string) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); return new ArrayList<>(Arrays.asList(PATTERN_NEWLINE.split(string))); } @@ -245,13 +245,13 @@ public class Txt public static String upperCaseFirst(String string) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); if (string.length() == 0) return string; return string.substring(0, 1).toUpperCase() + string.substring(1); } public static String lowerCaseFirst(String string) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); if (string.length() == 0) return string; return string.substring(0, 1).toLowerCase() + string.substring(1); } @@ -392,7 +392,8 @@ public class Txt public static boolean isVowel(String str) { - if (str == null || str.length() == 0) return false; + if (str == null) throw new NullPointerException("str"); + if (str.length() == 0) return false; return vowel.contains(str.substring(0, 1)); } @@ -782,7 +783,7 @@ public class Txt public static String removeSmartQuotes(String string) { - if (string == null) return null; + if (string == null) throw new NullPointerException("string"); // LEFT SINGLE QUOTATION MARK string = string.replace("\u2018", "'");