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.
This commit is contained in:
Magnus Ulf 2019-02-08 14:27:20 +01:00
parent 5302f808d7
commit b0086ba0f7
14 changed files with 55 additions and 150 deletions

View File

@ -184,7 +184,7 @@ public class Progressbar
public static String colorParse(String string, String colorTag, String color) 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 = string.replace(colorTag, color);
string = Txt.parse(string); string = Txt.parse(string);
return string; return string;

View File

@ -81,12 +81,13 @@ public final class SoundEffect implements Serializable
public void run(Location location) 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()); location.getWorld().playSound(location, this.getSound(), this.getVolume(), this.getPitch());
} }
public void run(HumanEntity human, Location location) public void run(HumanEntity human, Location location)
{ {
if (human == null) throw new NullPointerException("human");
if (MUtil.isntPlayer(human)) return; if (MUtil.isntPlayer(human)) return;
Player player = (Player)human; Player player = (Player)human;
player.playSound(location, this.getSound(), this.getVolume(), this.getPitch()); player.playSound(location, this.getSound(), this.getVolume(), this.getPitch());
@ -94,6 +95,7 @@ public final class SoundEffect implements Serializable
public void run(HumanEntity human) public void run(HumanEntity human)
{ {
if (human == null) throw new NullPointerException("human");
if (MUtil.isntPlayer(human)) return; if (MUtil.isntPlayer(human)) return;
this.run(human, human.getEyeLocation()); this.run(human, human.getEyeLocation());
} }
@ -144,8 +146,6 @@ public final class SoundEffect implements Serializable
@Override @Override
public boolean equals(Object obj) public boolean equals(Object obj)
{ {
if (this == obj) return true;
if (obj == null) return false;
if (!(obj instanceof SoundEffect)) return false; if (!(obj instanceof SoundEffect)) return false;
SoundEffect other = (SoundEffect) obj; SoundEffect other = (SoundEffect) obj;
if (Float.floatToIntBits(pitch) != Float.floatToIntBits(other.pitch)) return false; if (Float.floatToIntBits(pitch) != Float.floatToIntBits(other.pitch)) return false;

View File

@ -1651,27 +1651,4 @@ public class MassiveCommand implements Active, PluginIdentifiableCommand
return readArgAt(idx); return readArgAt(idx);
} }
// TODO: Some of these are still used by external plugins.
// TODO: Fix those plugins.
@Deprecated
public <T> T readArgFrom(Type<T> type) throws MassiveException
{
return this.readArgFrom(null, type);
}
@Deprecated
public <T> T readArgFrom(String str, Type<T> type) throws MassiveException
{
if (type == null) throw new IllegalArgumentException("type is null");
return type.read(str, this.sender);
}
@Deprecated
public <T> T readArgFrom(String str, Type<T> type, T defaultNotSet) throws MassiveException
{
if (str == null) return defaultNotSet;
return this.readArgFrom(str, type);
}
} }

View File

@ -35,7 +35,7 @@ public class EngineMassiveCorePlayerState extends Engine
if ( ! active) return; if ( ! active) return;
this.idToState.clear(); this.idToState.clear();
for (Player player : MUtil.getOnlinePlayers()) for (Player player : Bukkit.getOnlinePlayers())
{ {
this.idToState.put(player.getUniqueId(), PlayerState.JOINED); this.idToState.put(player.getUniqueId(), PlayerState.JOINED);
} }

View File

@ -1,7 +1,7 @@
package com.massivecraft.massivecore.mixin; package com.massivecraft.massivecore.mixin;
import com.massivecraft.massivecore.util.IdUtil; import com.massivecraft.massivecore.util.IdUtil;
import com.massivecraft.massivecore.util.MUtil; import org.bukkit.Bukkit;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
public class MixinVisibility extends Mixin public class MixinVisibility extends Mixin
@ -29,7 +29,7 @@ public class MixinVisibility extends Mixin
Player pwatchee = IdUtil.getPlayer(watcheeObject); Player pwatchee = IdUtil.getPlayer(watcheeObject);
if (pwatchee == null) return true; if (pwatchee == null) return true;
for (Player pwatcher : MUtil.getOnlinePlayers()) for (Player pwatcher : Bukkit.getOnlinePlayers())
{ {
if (pwatchee.equals(pwatcher)) continue; if (pwatchee.equals(pwatcher)) continue;
if (pwatcher.canSee(pwatchee)) continue; if (pwatcher.canSee(pwatchee)) continue;

View File

@ -1,8 +1,8 @@
package com.massivecraft.massivecore.particleeffect; package com.massivecraft.massivecore.particleeffect;
import com.massivecraft.massivecore.particleeffect.ReflectionUtils.PackageType; import com.massivecraft.massivecore.particleeffect.ReflectionUtils.PackageType;
import com.massivecraft.massivecore.util.MUtil;
import com.massivecraft.massivecore.util.ReflectionUtil; import com.massivecraft.massivecore.util.ReflectionUtil;
import org.bukkit.Bukkit;
import org.bukkit.Location; import org.bukkit.Location;
import org.bukkit.Material; import org.bukkit.Material;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
@ -1512,7 +1512,7 @@ public enum ParticleEffect {
} }
String worldName = center.getWorld().getName(); String worldName = center.getWorld().getName();
double squared = range * range; 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) { if (!player.getWorld().getName().equals(worldName) || player.getLocation().distanceSquared(center) > squared) {
continue; continue;
} }

View File

@ -145,7 +145,7 @@ public class BoardUtil extends Engine
Map<String, Player> players = new MassiveMap<>(); Map<String, Player> players = new MassiveMap<>();
// Fill // Fill
for (Player player : MUtil.getOnlinePlayers()) for (Player player : Bukkit.getOnlinePlayers())
{ {
players.put(player.getName(), player); players.put(player.getName(), player);
} }

View File

@ -181,7 +181,7 @@ public class IdUtil implements Listener, Runnable
Set<CommandSender> ret = new MassiveSet<>(); Set<CommandSender> ret = new MassiveSet<>();
// Add Online Players // Add Online Players
ret.addAll(MUtil.getOnlinePlayers()); ret.addAll(Bukkit.getOnlinePlayers());
// Add Console // Add Console
ret.add(getConsole()); ret.add(getConsole());
@ -461,7 +461,7 @@ public class IdUtil implements Listener, Runnable
public static IdData getData(Object senderObject) public static IdData getData(Object senderObject)
{ {
// Null Return // Null Return
if (senderObject == null) return null; if (senderObject == null) throw new NullPointerException("senderObject");
// Already Done // Already Done
if (senderObject instanceof IdData) return (IdData) senderObject; if (senderObject instanceof IdData) return (IdData) senderObject;
@ -525,7 +525,7 @@ public class IdUtil implements Listener, Runnable
public static CommandSender getSender(Object senderObject) public static CommandSender getSender(Object senderObject)
{ {
// Null Return // Null Return
if (senderObject == null) return null; if (senderObject == null) throw new NullPointerException("senderObject");
// Already Done // Already Done
if (senderObject instanceof CommandSender) return (CommandSender) senderObject; if (senderObject instanceof CommandSender) return (CommandSender) senderObject;
@ -590,7 +590,7 @@ public class IdUtil implements Listener, Runnable
public static UUID getUuid(Object senderObject) public static UUID getUuid(Object senderObject)
{ {
// Null Return // Null Return
if (senderObject == null) return null; if (senderObject == null) throw new NullPointerException("senderObject");
// Already Done // Already Done
if (senderObject instanceof UUID) return (UUID)senderObject; if (senderObject instanceof UUID) return (UUID)senderObject;
@ -655,7 +655,7 @@ public class IdUtil implements Listener, Runnable
public static String getId(Object senderObject) public static String getId(Object senderObject)
{ {
// Null Return // Null Return
if (senderObject == null) return null; if (senderObject == null) throw new NullPointerException("senderObject");
// Already Done // Already Done
if (senderObject instanceof String && MUtil.isUuid((String)senderObject)) return (String)senderObject; 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) public static String getName(Object senderObject)
{ {
// Null Return // Null Return
if (senderObject == null) return null; if (senderObject == null) throw new NullPointerException("senderObject");
// Already Done // Already Done
// Handled at "Data" (not applicable - names can look differently) // 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) public static OfflinePlayer getOfflinePlayer(Object senderObject)
{ {
// Null Return // Null Return
if (senderObject == null) return null; if (senderObject == null) throw new NullPointerException("senderObject");
// Already done // Already done
if (senderObject instanceof OfflinePlayer) return (OfflinePlayer) senderObject; if (senderObject instanceof OfflinePlayer) return (OfflinePlayer) senderObject;
@ -965,7 +965,7 @@ public class IdUtil implements Listener, Runnable
long millis = System.currentTimeMillis(); long millis = System.currentTimeMillis();
for (Player player : MUtil.getOnlinePlayers()) for (Player player : Bukkit.getOnlinePlayers())
{ {
String id = getId(player); String id = getId(player);
if (id == null) throw new NullPointerException("id"); if (id == null) throw new NullPointerException("id");

View File

@ -56,7 +56,6 @@ import org.bukkit.potion.PotionEffect;
import org.bukkit.potion.PotionEffectType; import org.bukkit.potion.PotionEffectType;
import org.bukkit.projectiles.ProjectileSource; import org.bukkit.projectiles.ProjectileSource;
import java.lang.reflect.Method;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.math.RoundingMode; import java.math.RoundingMode;
import java.net.InetAddress; import java.net.InetAddress;
@ -81,99 +80,17 @@ import java.util.regex.Pattern;
public class MUtil public class MUtil
{ {
// -------------------------------------------- //
// CONSTANTS
// -------------------------------------------- //
private static Method methodGetOnlinePlayers;
static
{
methodGetOnlinePlayers = getMethodGetOnlinePlayers();
}
// -------------------------------------------- // // -------------------------------------------- //
// GET ONLINE PLAYERS // GET ONLINE PLAYERS
// -------------------------------------------- // // -------------------------------------------- //
// It seems we can not always trust the Bukkit.getOnlinePlayers() method. // Old thing that had to do with the converfrion from 1.17 to 1.8
// 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;
}
@Deprecated
public static Collection<Player> getOnlinePlayers() public static Collection<Player> getOnlinePlayers()
{ {
// Fetch some kind of playersObject. Collection<? extends Player>coll = Bukkit.getOnlinePlayers();
Object playersObject = null; List<Player> ret = new MassiveList<>(coll.size());
try return ret;
{
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<Player> playersCollection = (Collection<Player>)playersObject;
return playersCollection;
}
else if (playersObject instanceof Player[])
{
Player[] playersArray = (Player[])playersObject;
return Arrays.asList(playersArray);
}
else
{
throw new RuntimeException("Failed retrieving online players.");
}
} }
// -------------------------------------------- // // -------------------------------------------- //
@ -195,7 +112,7 @@ public class MUtil
// Fill // Fill
final World world = location.getWorld(); final World world = location.getWorld();
final double radiusSquared = radius * radius; final double radiusSquared = radius * radius;
for (Player player : MUtil.getOnlinePlayers()) for (Player player : Bukkit.getOnlinePlayers())
{ {
Location playerLocation = player.getLocation(); Location playerLocation = player.getLocation();
World playerWorld = playerLocation.getWorld(); World playerWorld = playerLocation.getWorld();
@ -251,7 +168,7 @@ public class MUtil
public static UUID asUuid(String string) public static UUID asUuid(String string)
{ {
// Null // Null
if (string == null) return null; if (string == null) throw new NullPointerException("string");
// Avoid Exception // Avoid Exception
if (string.length() != 36) return null; if (string.length() != 36) return null;
@ -285,7 +202,7 @@ public class MUtil
public static boolean containsCommand(String needle, Iterable<String> haystack) public static boolean containsCommand(String needle, Iterable<String> haystack)
{ {
if (needle == null) return false; if (needle == null) throw new NullPointerException("needle");
needle = prepareCommand(needle); needle = prepareCommand(needle);
for (String straw : haystack) for (String straw : haystack)
@ -314,7 +231,7 @@ public class MUtil
private static String prepareCommand(String string) private static String prepareCommand(String string)
{ {
if (string == null) return null; if (string == null) throw new NullPointerException("string");
string = Txt.removeLeadingCommandDust(string); string = Txt.removeLeadingCommandDust(string);
string = string.toLowerCase(); string = string.toLowerCase();
string = string.trim(); string = string.trim();
@ -384,6 +301,8 @@ public class MUtil
public static boolean isNpc(Object object) public static boolean isNpc(Object object)
{ {
if (object == null) throw new NullPointerException("object");
if ( ! (object instanceof Metadatable)) return false; if ( ! (object instanceof Metadatable)) return false;
Metadatable metadatable = (Metadatable)object; Metadatable metadatable = (Metadatable)object;
try try
@ -405,6 +324,8 @@ public class MUtil
public static boolean isSender(Object object) public static boolean isSender(Object object)
{ {
if (object == null) throw new NullPointerException("object");
if (!(object instanceof CommandSender)) return false; if (!(object instanceof CommandSender)) return false;
if (isNpc(object)) return false; if (isNpc(object)) return false;
return true; return true;
@ -416,6 +337,8 @@ public class MUtil
public static boolean isPlayer(Object object) public static boolean isPlayer(Object object)
{ {
if (object == null) throw new NullPointerException("object");
if (!(object instanceof Player)) return false; if (!(object instanceof Player)) return false;
if (isNpc(object)) return false; if (isNpc(object)) return false;
return true; return true;
@ -953,7 +876,7 @@ public class MUtil
public static <T> T regexPickFirstVal(String input, Map<String, T> regex2val) public static <T> T regexPickFirstVal(String input, Map<String, T> regex2val)
{ {
if (regex2val == null) return null; if (regex2val == null) throw new NullPointerException("regex2val");
T ret = null; T ret = null;
for (Entry<String, T> entry : regex2val.entrySet()) for (Entry<String, T> entry : regex2val.entrySet())
@ -969,7 +892,7 @@ public class MUtil
public static <E, T> T equalsPickFirstVal(E input, Map<E, T> thing2val) public static <E, T> T equalsPickFirstVal(E input, Map<E, T> thing2val)
{ {
if (thing2val == null) return null; if (thing2val == null) throw new NullPointerException("thing2val");
T ret = null; T ret = null;
for (Entry<E, T> entry : thing2val.entrySet()) for (Entry<E, T> entry : thing2val.entrySet())

View File

@ -83,7 +83,7 @@ public class PermissionUtil
public static String asPermissionId(Object object) 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 String) return (String)object;
if (object instanceof Identified) return ((Identified)object).getId(); if (object instanceof Identified) return ((Identified)object).getId();
@ -94,7 +94,7 @@ public class PermissionUtil
public static Permission asPermission(Object object) 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; if (object instanceof Permission) return (Permission)object;
@ -228,6 +228,8 @@ public class PermissionUtil
public static boolean setPermissionStandardChildren(Permission permission, PermissionDefault standard, Map<String, Boolean> children) public static boolean setPermissionStandardChildren(Permission permission, PermissionDefault standard, Map<String, Boolean> children)
{ {
if (permission == null) throw new NullPointerException("permission");
boolean childrenChanged = false; boolean childrenChanged = false;
boolean standardChanged = false; boolean standardChanged = false;

View File

@ -64,7 +64,7 @@ public class PlayerUtil extends Engine
public static void setLastMoveMillis(Player player, long millis) public static void setLastMoveMillis(Player player, long millis)
{ {
if (player == null) return; if (player == null) throw new NullPointerException("player");
idToLastMoveMillis.put(player.getUniqueId(), millis); idToLastMoveMillis.put(player.getUniqueId(), millis);
} }
@ -94,7 +94,7 @@ public class PlayerUtil extends Engine
public static long getLastMoveMillis(Player player) public static long getLastMoveMillis(Player player)
{ {
if (player == null) return 0; if (player == null) throw new NullPointerException("player");
Long ret = idToLastMoveMillis.get(player.getUniqueId()); Long ret = idToLastMoveMillis.get(player.getUniqueId());
if (ret == null) return 0; if (ret == null) return 0;
return ret; return ret;
@ -102,7 +102,7 @@ public class PlayerUtil extends Engine
public static long getStandStillMillis(Player player) 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.isDead()) return 0;
if (!player.isOnline()) return 0; if (!player.isOnline()) return 0;
@ -122,8 +122,8 @@ public class PlayerUtil extends Engine
public static void setLastDamageMillis(Player player, long millis) public static void setLastDamageMillis(Player player, long millis)
{ {
if (player == null) throw new NullPointerException("player");
if (MUtil.isntPlayer(player)) return; if (MUtil.isntPlayer(player)) return;
if (player == null) return;
idToLastDamageMillis.put(player.getUniqueId(), millis); idToLastDamageMillis.put(player.getUniqueId(), millis);
} }
@ -157,6 +157,7 @@ public class PlayerUtil extends Engine
public static long getLastDamageMillis(Player player) public static long getLastDamageMillis(Player player)
{ {
if (player == null) throw new NullPointerException("player");
if (MUtil.isntPlayer(player)) return 0; if (MUtil.isntPlayer(player)) return 0;
Long ret = idToLastDamageMillis.get(player.getUniqueId()); Long ret = idToLastDamageMillis.get(player.getUniqueId());
if (ret == null) return 0; if (ret == null) return 0;
@ -165,6 +166,7 @@ public class PlayerUtil extends Engine
public static long getNoDamageMillis(Player player) public static long getNoDamageMillis(Player player)
{ {
if (player == null) throw new NullPointerException("player");
if (MUtil.isntPlayer(player)) return 0; if (MUtil.isntPlayer(player)) return 0;
if (player.isDead()) return 0; if (player.isDead()) return 0;
if (!player.isOnline()) return 0; if (!player.isOnline()) return 0;

View File

@ -34,7 +34,7 @@ public class SmokeUtil
// Single ======== // Single ========
public static void spawnSingle(Location location, int direction) 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); location.getWorld().playEffect(location, Effect.SMOKE, direction);
} }

View File

@ -70,7 +70,7 @@ public class TimeUnit implements Comparable<TimeUnit>
public static TimeUnit get(String timeUnitString) public static TimeUnit get(String timeUnitString)
{ {
if (timeUnitString == null) return null; if (timeUnitString == null) throw new NullPointerException("timeUnitString");
String timeUnitStringLowerCase = timeUnitString.toLowerCase(); String timeUnitStringLowerCase = timeUnitString.toLowerCase();
for (TimeUnit timeUnit : all) for (TimeUnit timeUnit : all)
{ {

View File

@ -170,7 +170,7 @@ public class Txt
public static String parse(String string) public static String parse(String string)
{ {
if (string == null) return null; if (string == null) throw new NullPointerException("string");
StringBuffer ret = new StringBuffer(); StringBuffer ret = new StringBuffer();
Matcher matcher = parsePattern.matcher(string); Matcher matcher = parsePattern.matcher(string);
while (matcher.find()) while (matcher.find())
@ -202,7 +202,7 @@ public class Txt
public static ArrayList<String> wrap(final String string) public static ArrayList<String> 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))); return new ArrayList<>(Arrays.asList(PATTERN_NEWLINE.split(string)));
} }
@ -245,13 +245,13 @@ public class Txt
public static String upperCaseFirst(String string) public static String upperCaseFirst(String string)
{ {
if (string == null) return null; if (string == null) throw new NullPointerException("string");
if (string.length() == 0) return string; if (string.length() == 0) return string;
return string.substring(0, 1).toUpperCase() + string.substring(1); return string.substring(0, 1).toUpperCase() + string.substring(1);
} }
public static String lowerCaseFirst(String string) public static String lowerCaseFirst(String string)
{ {
if (string == null) return null; if (string == null) throw new NullPointerException("string");
if (string.length() == 0) return string; if (string.length() == 0) return string;
return string.substring(0, 1).toLowerCase() + string.substring(1); return string.substring(0, 1).toLowerCase() + string.substring(1);
} }
@ -392,7 +392,8 @@ public class Txt
public static boolean isVowel(String str) 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)); return vowel.contains(str.substring(0, 1));
} }
@ -782,7 +783,7 @@ public class Txt
public static String removeSmartQuotes(String string) public static String removeSmartQuotes(String string)
{ {
if (string == null) return null; if (string == null) throw new NullPointerException("string");
// LEFT SINGLE QUOTATION MARK // LEFT SINGLE QUOTATION MARK
string = string.replace("\u2018", "'"); string = string.replace("\u2018", "'");