From 9291132ab9d1eb32fceb3862c8a2301e9d5eed45 Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Fri, 10 Mar 2023 14:45:15 -0500 Subject: [PATCH] Converter and Comparator improvements (#5403) --- src/main/java/ch/njol/skript/Skript.java | 4 +- .../ch/njol/skript/registrations/Classes.java | 4 +- .../skript/registrations/Comparators.java | 2 +- .../njol/skript/registrations/Converters.java | 2 +- .../skript/lang/comparator/Comparators.java | 196 +++++++++---- .../lang/comparator/ConvertedComparator.java | 2 +- .../skript/lang/comparator/Relation.java | 6 +- .../lang/converter/ChainedConverter.java | 3 +- .../skript/lang/converter/Converters.java | 274 +++++++++++------- 9 files changed, 308 insertions(+), 185 deletions(-) diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index 5ade760813d..e78798f91a1 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -1263,9 +1263,9 @@ public static void checkAcceptRegistrations() { } private static void stopAcceptingRegistrations() { - acceptRegistrations = false; - Converters.createChainedConverters(); + + acceptRegistrations = false; Classes.onRegistrationsStop(); } diff --git a/src/main/java/ch/njol/skript/registrations/Classes.java b/src/main/java/ch/njol/skript/registrations/Classes.java index 8f48370db56..126b454efa5 100644 --- a/src/main/java/ch/njol/skript/registrations/Classes.java +++ b/src/main/java/ch/njol/skript/registrations/Classes.java @@ -499,7 +499,7 @@ public static T parse(final String s, final Class c, final ParseContext c log.printLog(); return t; } - for (final ConverterInfo conv : org.skriptlang.skript.lang.converter.Converters.getConverterInfo()) { + for (final ConverterInfo conv : Converters.getConverterInfos()) { if (context == ParseContext.COMMAND && (conv.getFlags() & Commands.CONVERTER_NO_COMMAND_ARGUMENTS) != 0) continue; if (c.isAssignableFrom(conv.getTo())) { @@ -539,7 +539,7 @@ public static Parser getParser(final Class to) { if (to.isAssignableFrom(ci.getC()) && ci.getParser() != null) return (Parser) ci.getParser(); } - for (final ConverterInfo conv : org.skriptlang.skript.lang.converter.Converters.getConverterInfo()) { + for (final ConverterInfo conv : Converters.getConverterInfos()) { if (to.isAssignableFrom(conv.getTo())) { for (int i = classInfos.length - 1; i >= 0; i--) { final ClassInfo ci = classInfos[i]; diff --git a/src/main/java/ch/njol/skript/registrations/Comparators.java b/src/main/java/ch/njol/skript/registrations/Comparators.java index 3e6a3a1f8b5..c027d7ad943 100644 --- a/src/main/java/ch/njol/skript/registrations/Comparators.java +++ b/src/main/java/ch/njol/skript/registrations/Comparators.java @@ -57,7 +57,7 @@ public static Relation compare(final @Nullable Object o1, final @Nullable Object } public static java.util.Comparator getJavaComparator() { - return org.skriptlang.skript.lang.comparator.Comparators.JAVA_COMPARATOR; + return (o1, o2) -> compare(o1, o2).getRelation(); } @Nullable diff --git a/src/main/java/ch/njol/skript/registrations/Converters.java b/src/main/java/ch/njol/skript/registrations/Converters.java index ae5282eeb5c..7a996faacab 100644 --- a/src/main/java/ch/njol/skript/registrations/Converters.java +++ b/src/main/java/ch/njol/skript/registrations/Converters.java @@ -36,7 +36,7 @@ private Converters() {} @SuppressWarnings("unchecked") public static List> getConverters() { - return org.skriptlang.skript.lang.converter.Converters.getConverterInfo().stream() + return org.skriptlang.skript.lang.converter.Converters.getConverterInfos().stream() .map(unknownInfo -> { org.skriptlang.skript.lang.converter.ConverterInfo info = (org.skriptlang.skript.lang.converter.ConverterInfo) unknownInfo; return new ConverterInfo<>(info.getFrom(), info.getTo(), info.getConverter()::convert, info.getFlags()); diff --git a/src/main/java/org/skriptlang/skript/lang/comparator/Comparators.java b/src/main/java/org/skriptlang/skript/lang/comparator/Comparators.java index 5540cb613d4..8e3aa23de72 100644 --- a/src/main/java/org/skriptlang/skript/lang/comparator/Comparators.java +++ b/src/main/java/org/skriptlang/skript/lang/comparator/Comparators.java @@ -20,10 +20,11 @@ import ch.njol.skript.Skript; import ch.njol.skript.SkriptAPIException; -import ch.njol.skript.classes.Converter; -import ch.njol.skript.registrations.Converters; import ch.njol.util.Pair; import org.eclipse.jdt.annotation.Nullable; +import org.jetbrains.annotations.Unmodifiable; +import org.skriptlang.skript.lang.converter.Converter; +import org.skriptlang.skript.lang.converter.Converters; import java.util.ArrayList; import java.util.Collections; @@ -44,35 +45,35 @@ private Comparators() {} /** * A default comparator to compare two objects using {@link Object#equals(Object)}. */ - public static final Comparator EQUALS_COMPARATOR = (o1, o2) -> Relation.get(o1.equals(o2)); - - /** - * A Java {@link java.util.Comparator} for comparing two objects using {@link #compare(Object, Object)}. - */ - public static final java.util.Comparator JAVA_COMPARATOR = (o1, o2) -> compare(o1, o2).getRelation(); + private static final ComparatorInfo EQUALS_COMPARATOR_INFO = new ComparatorInfo<>( + Object.class, + Object.class, + (o1, o2) -> Relation.get(o1.equals(o2)) + ); /** * A List containing information for all registered comparators. */ - private static final List> COMPARATORS = Collections.synchronizedList(new ArrayList<>()); + private static final List> COMPARATORS = new ArrayList<>(50); /** - * @return An unmodifiable, synchronized list containing all registered {@link ComparatorInfo}s. - * When traversing this list, please refer to {@link Collections#synchronizedList(List)} to ensure that - * the list is properly traversed due to its synchronized status. + * @return An unmodifiable list containing all registered {@link ComparatorInfo}s. * Please note that this does not include any special Comparators resolved by Skript during runtime. * This method ONLY returns Comparators explicitly registered during registration. * Thus, it is recommended to use {@link #getComparator(Class, Class)} if possible. */ + @Unmodifiable public static List> getComparatorInfos() { + assertIsDoneLoading(); return Collections.unmodifiableList(COMPARATORS); } /** * A map for quickly accessing comparators that have already been resolved. + * Some pairs may point to a null value, indicating that no comparator exists between the two types. * This is useful for skipping complex lookups that may require conversion and inversion. */ - private static final Map, Class>, Comparator> QUICK_ACCESS_COMPARATORS = Collections.synchronizedMap(new HashMap<>()); + private static final Map, Class>, ComparatorInfo> QUICK_ACCESS_COMPARATORS = new HashMap<>(50); /** * Registers a new Comparator with Skript's collection of Comparators. @@ -87,18 +88,20 @@ public static void registerComparator( ) { Skript.checkAcceptRegistrations(); - if (firstType == Object.class && secondType == Object.class) + if (firstType == Object.class && secondType == Object.class) { throw new IllegalArgumentException("It is not possible to add a comparator between objects"); + } - for (ComparatorInfo info : COMPARATORS) { - if (info.firstType == firstType && info.secondType == secondType) { - throw new SkriptAPIException( - "A Comparator comparing '" + firstType + "' and '" + secondType + " already exists!" - ); + synchronized (COMPARATORS) { + for (ComparatorInfo info : COMPARATORS) { + if (info.firstType == firstType && info.secondType == secondType) { + throw new SkriptAPIException( + "A Comparator comparing '" + firstType + "' and '" + secondType + "' already exists!" + ); + } } + COMPARATORS.add(new ComparatorInfo<>(firstType, secondType, comparator)); } - - COMPARATORS.add(new ComparatorInfo<>(firstType, secondType, comparator)); } /** @@ -106,41 +109,65 @@ public static void registerComparator( * @param first The first object for comparison. * @param second The second object for comparison. * @return The Relation between the two provided objects. + * Guaranteed to be {@link Relation#NOT_EQUAL} if either parameter is null. */ @SuppressWarnings("unchecked") public static Relation compare(@Nullable T1 first, @Nullable T2 second) { - if (first == null || second == null) + assertIsDoneLoading(); // this would be checked later on too, but we want this guaranteed to fail + + if (first == null || second == null) { return Relation.NOT_EQUAL; + } + + if (first == second) { // easiest check of them all! + return Relation.EQUAL; + } Comparator comparator = getComparator((Class) first.getClass(), (Class) second.getClass()); - if (comparator == null) + if (comparator == null) { return Relation.NOT_EQUAL; + } return comparator.compare(first, second); } /** - * A method for obtaining a comparator that can compare two objects of 'firstType' and 'secondType'. + * A method for obtaining a Comparator that can compare two objects of 'firstType' and 'secondType'. * Please note that comparators may convert objects if necessary for comparisons. * @param firstType The first type for comparison. * @param secondType The second type for comparison. - * @return A comparator capable of determine the {@link Relation} between two objects of 'firstType' and 'secondType'. + * @return A Comparator capable of determine the {@link Relation} between two objects of 'firstType' and 'secondType'. * Will be null if no comparator capable of comparing two objects of 'firstType' and 'secondType' was found. */ @Nullable - @SuppressWarnings("unchecked") public static Comparator getComparator(Class firstType, Class secondType) { - if (Skript.isAcceptRegistrations()) - throw new SkriptAPIException("Comparators cannot be retrieved until Skript has finished registrations."); + ComparatorInfo info = getComparatorInfo(firstType, secondType); + return info != null ? info.comparator : null; + } - Pair, Class> pair = new Pair<>(firstType, secondType); - Comparator comparator; + /** + * A method for obtaining the info of a Comparator that can compare two objects of 'firstType' and 'secondType'. + * Please note that comparators may convert objects if necessary for comparisons. + * @param firstType The first type for comparison. + * @param secondType The second type for comparison. + * @return The info of a Comparator capable of determine the {@link Relation} between two objects of 'firstType' and 'secondType'. + * Will be null if no info for comparing two objects of 'firstType' and 'secondType' was found. + */ + @Nullable + @SuppressWarnings("unchecked") + public static ComparatorInfo getComparatorInfo(Class firstType, Class secondType) { + assertIsDoneLoading(); - if (QUICK_ACCESS_COMPARATORS.containsKey(pair)) { - comparator = (Comparator) QUICK_ACCESS_COMPARATORS.get(pair); - } else { // Compute QUICK_ACCESS for provided types - comparator = getComparator_i(firstType, secondType); - QUICK_ACCESS_COMPARATORS.put(pair, comparator); + Pair, Class> pair = new Pair<>(firstType, secondType); + ComparatorInfo comparator; + + synchronized (QUICK_ACCESS_COMPARATORS) { + if (QUICK_ACCESS_COMPARATORS.containsKey(pair)) { + comparator = (ComparatorInfo) QUICK_ACCESS_COMPARATORS.get(pair); + } else { // Compute QUICK_ACCESS for provided types + comparator = getComparatorInfo_i(firstType, secondType); + QUICK_ACCESS_COMPARATORS.put(pair, comparator); + } } return comparator; @@ -151,7 +178,7 @@ public static Comparator getComparator(Class firstType, Cla * This method handles regular {@link Comparator}s, {@link ConvertedComparator}s, and {@link InverseComparator}s. * @param firstType The first type for comparison. * @param secondType The second type for comparison. - * @return A comparator capable of determine the {@link Relation} between two objects of 'firstType' and 'secondType'. + * @return The info of the comparator capable of determine the {@link Relation} between two objects of 'firstType' and 'secondType'. * Will be null if no comparator capable of comparing two objects of 'firstType' and 'secondType' was found. * @param The first type for comparison. * @param The second type for comparison. @@ -162,29 +189,32 @@ public static Comparator getComparator(Class firstType, Cla */ @Nullable @SuppressWarnings("unchecked") - private static Comparator getComparator_i( + private static ComparatorInfo getComparatorInfo_i( Class firstType, Class secondType ) { - // Look for an exact match for (ComparatorInfo info : COMPARATORS) { if (info.firstType == firstType && info.secondType == secondType) { - return (Comparator) info.comparator; + return (ComparatorInfo) info; } } // Look for a basically perfect match for (ComparatorInfo info : COMPARATORS) { if (info.firstType.isAssignableFrom(firstType) && info.secondType.isAssignableFrom(secondType)) { - return (Comparator) info.comparator; + return (ComparatorInfo) info; } } // Try to match and create an InverseComparator for (ComparatorInfo info : COMPARATORS) { if (info.comparator.supportsInversion() && info.firstType.isAssignableFrom(secondType) && info.secondType.isAssignableFrom(firstType)) { - return new InverseComparator<>((Comparator) info.comparator); + return new ComparatorInfo<>( + firstType, + secondType, + new InverseComparator<>((Comparator) info.comparator) + ); } } @@ -193,36 +223,57 @@ private static Comparator getComparator_i( ComparatorInfo info = (ComparatorInfo) unknownInfo; if (info.firstType.isAssignableFrom(firstType)) { // Attempt to convert the second argument to the second comparator type - Converter sc2 = (Converter) Converters.getConverter(secondType, info.secondType); - if (sc2 != null) - return new ConvertedComparator<>(null, info.comparator, sc2); + Converter sc2 = Converters.getConverter(secondType, info.secondType); + if (sc2 != null) { + return new ComparatorInfo<>( + firstType, + secondType, + new ConvertedComparator<>(null, info.comparator, sc2) + ); + } } if (info.secondType.isAssignableFrom(secondType)) { // Attempt to convert the first argument to the first comparator type - Converter fc1 = (Converter) Converters.getConverter(firstType, info.firstType); - if (fc1 != null) - return new ConvertedComparator<>(fc1, info.comparator, null); + Converter fc1 = Converters.getConverter(firstType, info.firstType); + if (fc1 != null) { + return new ComparatorInfo<>( + firstType, + secondType, + new ConvertedComparator<>(fc1, info.comparator, null) + ); + } } } // Attempt converting one parameter but with reversed types for (ComparatorInfo unknownInfo : COMPARATORS) { - if (!unknownInfo.comparator.supportsInversion()) // Unsupported for reversing types + if (!unknownInfo.comparator.supportsInversion()) { // Unsupported for reversing types continue; + } ComparatorInfo info = (ComparatorInfo) unknownInfo; if (info.secondType.isAssignableFrom(firstType)) { // Attempt to convert the second argument to the first comparator type - Converter sc1 = (Converter) Converters.getConverter(secondType, info.firstType); - if (sc1 != null) - return new InverseComparator<>(new ConvertedComparator<>(sc1, info.comparator, null)); + Converter sc1 = Converters.getConverter(secondType, info.firstType); + if (sc1 != null) { + return new ComparatorInfo<>( + firstType, + secondType, + new InverseComparator<>(new ConvertedComparator<>(sc1, info.comparator, null)) + ); + } } if (info.firstType.isAssignableFrom(secondType)) { // Attempt to convert the first argument to the second comparator type - Converter fc2 = (Converter) Converters.getConverter(firstType, info.secondType); - if (fc2 != null) - new InverseComparator<>(new ConvertedComparator<>(null, info.comparator, fc2)); + Converter fc2 = Converters.getConverter(firstType, info.secondType); + if (fc2 != null) { + return new ComparatorInfo<>( + firstType, + secondType, + new InverseComparator<>(new ConvertedComparator<>(null, info.comparator, fc2)) + ); + } } } @@ -231,34 +282,51 @@ private static Comparator getComparator_i( for (ComparatorInfo unknownInfo : COMPARATORS) { ComparatorInfo info = (ComparatorInfo) unknownInfo; - Converter c1 = (Converter) Converters.getConverter(firstType, info.firstType); - Converter c2 = (Converter) Converters.getConverter(secondType, info.secondType); - if (c1 != null && c2 != null) - return new ConvertedComparator<>(c1, info.comparator, c2); + Converter c1 = Converters.getConverter(firstType, info.firstType); + Converter c2 = Converters.getConverter(secondType, info.secondType); + if (c1 != null && c2 != null) { + return new ComparatorInfo<>( + firstType, + secondType, + new ConvertedComparator<>(c1, info.comparator, c2) + ); + } } // Attempt converting both parameters but with reversed types for (ComparatorInfo unknownInfo : COMPARATORS) { - if (!unknownInfo.comparator.supportsInversion()) // Unsupported for reversing types + if (!unknownInfo.comparator.supportsInversion()) { // Unsupported for reversing types continue; + } ComparatorInfo info = (ComparatorInfo) unknownInfo; - Converter c1 = (Converter) Converters.getConverter(firstType, info.secondType); - Converter c2 = (Converter) Converters.getConverter(secondType, info.firstType); - if (c1 != null && c2 != null) - return new InverseComparator<>(new ConvertedComparator<>(c2, info.comparator, c1)); + Converter c1 = Converters.getConverter(firstType, info.secondType); + Converter c2 = Converters.getConverter(secondType, info.firstType); + if (c1 != null && c2 != null) { + return new ComparatorInfo<>( + firstType, + secondType, + new InverseComparator<>(new ConvertedComparator<>(c2, info.comparator, c1)) + ); + } } // Same class but no comparator if (firstType != Object.class && secondType == firstType) { - return (Comparator) EQUALS_COMPARATOR; + return (ComparatorInfo) EQUALS_COMPARATOR_INFO; } // Well, we tried! return null; } + private static void assertIsDoneLoading() { + if (Skript.isAcceptRegistrations()) { + throw new SkriptAPIException("Comparators cannot be retrieved until Skript has finished registrations."); + } + } + } diff --git a/src/main/java/org/skriptlang/skript/lang/comparator/ConvertedComparator.java b/src/main/java/org/skriptlang/skript/lang/comparator/ConvertedComparator.java index d38a84521ba..eced387a77c 100644 --- a/src/main/java/org/skriptlang/skript/lang/comparator/ConvertedComparator.java +++ b/src/main/java/org/skriptlang/skript/lang/comparator/ConvertedComparator.java @@ -18,8 +18,8 @@ */ package org.skriptlang.skript.lang.comparator; -import ch.njol.skript.classes.Converter; import org.eclipse.jdt.annotation.Nullable; +import org.skriptlang.skript.lang.converter.Converter; /** * A ConvertedComparator is a comparator that converts its parameters so that they may be used diff --git a/src/main/java/org/skriptlang/skript/lang/comparator/Relation.java b/src/main/java/org/skriptlang/skript/lang/comparator/Relation.java index fcba1245c6c..1f52ed46106 100644 --- a/src/main/java/org/skriptlang/skript/lang/comparator/Relation.java +++ b/src/main/java/org/skriptlang/skript/lang/comparator/Relation.java @@ -72,8 +72,9 @@ public static Relation get(double d) { * @return Whether this Relation is part of the given Relation, e.g. GREATER_OR_EQUAL.isImpliedBy(EQUAL) returns true. */ public boolean isImpliedBy(Relation other) { - if (other == this) + if (other == this) { return true; + } switch (this) { case EQUAL: case GREATER: @@ -96,8 +97,9 @@ public boolean isImpliedBy(Relation other) { */ public boolean isImpliedBy(Relation... others) { for (Relation other : others) { - if (isImpliedBy(other)) + if (isImpliedBy(other)) { return true; + } } return false; } diff --git a/src/main/java/org/skriptlang/skript/lang/converter/ChainedConverter.java b/src/main/java/org/skriptlang/skript/lang/converter/ChainedConverter.java index ee6abc15c9e..6be4fb80ce0 100644 --- a/src/main/java/org/skriptlang/skript/lang/converter/ChainedConverter.java +++ b/src/main/java/org/skriptlang/skript/lang/converter/ChainedConverter.java @@ -52,8 +52,9 @@ final class ChainedConverter implements Converter { @Nullable public T convert(F from) { M middle = first.convert(from); - if (middle == null) + if (middle == null) { return null; + } return second.convert(middle); } diff --git a/src/main/java/org/skriptlang/skript/lang/converter/Converters.java b/src/main/java/org/skriptlang/skript/lang/converter/Converters.java index 23a4ddf5051..e8d6a98f8e9 100644 --- a/src/main/java/org/skriptlang/skript/lang/converter/Converters.java +++ b/src/main/java/org/skriptlang/skript/lang/converter/Converters.java @@ -20,7 +20,9 @@ import ch.njol.skript.Skript; import ch.njol.skript.SkriptAPIException; +import ch.njol.util.Pair; import org.eclipse.jdt.annotation.Nullable; +import org.jetbrains.annotations.Unmodifiable; import java.lang.reflect.Array; import java.util.ArrayList; @@ -29,7 +31,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; /** * Converters are used to provide Skript with specific instructions for converting an object to a different type. @@ -42,25 +43,26 @@ private Converters() {} /** * A List containing information for all registered converters. */ - private static final List> CONVERTERS = Collections.synchronizedList(new ArrayList<>(50)); + private static final List> CONVERTERS = new ArrayList<>(50); /** - * @return An unmodifiable, synchronized list containing all registered {@link ConverterInfo}s. - * When traversing this list, please refer to {@link Collections#synchronizedList(List)} to ensure that - * the list is properly traversed due to its synchronized status. + * @return An unmodifiable list containing all registered {@link ConverterInfo}s. * Please note that this does not include any special Converters resolved by Skript during runtime. * This method ONLY returns converters explicitly registered during registration. * Thus, it is recommended to use {@link #getConverter(Class, Class)}. */ - public static List> getConverterInfo() { + @Unmodifiable + public static List> getConverterInfos() { + assertIsDoneLoading(); return Collections.unmodifiableList(CONVERTERS); } /** * A map for quickly access converters that have already been resolved. + * Some pairs may point to a null value, indicating that no converter exists between the two types. * This is useful for skipping complex lookups that may require chaining. */ - private static final Map> QUICK_ACCESS_CONVERTERS = Collections.synchronizedMap(new HashMap<>(50)); + private static final Map, Class>, ConverterInfo> QUICK_ACCESS_CONVERTERS = new HashMap<>(50); /** * Registers a new Converter with Skript's collection of Converters. @@ -84,13 +86,14 @@ public static void registerConverter(Class from, Class to, Converte ConverterInfo info = new ConverterInfo<>(from, to, converter, flags); - if (exactConverterExists(from, to)) { - throw new SkriptAPIException( - "A Converter from '" + from + "' to '" + to + " already exists!" - ); + synchronized (CONVERTERS) { + if (exactConverterExists(from, to)) { + throw new SkriptAPIException( + "A Converter from '" + from + "' to '" + to + "' already exists!" + ); + } + CONVERTERS.add(info); } - - CONVERTERS.add(info); } /** @@ -102,64 +105,68 @@ public static void registerConverter(Class from, Class to, Converte // REMIND how to manage overriding of converters? - shouldn't actually matter @SuppressWarnings("unchecked") public static void createChainedConverters() { - for (int i = 0; i < CONVERTERS.size(); i++) { - - ConverterInfo unknownInfo1 = CONVERTERS.get(i); - for (int j = 0; j < CONVERTERS.size(); j++) { // Not from j = i+1 since new converters get added during the loops - - ConverterInfo unknownInfo2 = CONVERTERS.get(j); - - // chain info -> info2 - if ( - unknownInfo2.getFrom() != Object.class // Object can only exist at the beginning of a chain - && (unknownInfo1.getFlags() & Converter.NO_RIGHT_CHAINING) == 0 - && (unknownInfo2.getFlags() & Converter.NO_LEFT_CHAINING) == 0 - && unknownInfo2.getFrom().isAssignableFrom(unknownInfo1.getTo()) - && !exactConverterExists(unknownInfo1.getFrom(), unknownInfo2.getTo()) - ) { - ConverterInfo info1 = (ConverterInfo) unknownInfo1; - ConverterInfo info2 = (ConverterInfo) unknownInfo2; - - CONVERTERS.add(new ConverterInfo<>( - info1.getFrom(), - info2.getTo(), - new ChainedConverter<>(info1.getConverter(), info2.getConverter()), - info1.getFlags() | info2.getFlags() - )); - } + Skript.checkAcceptRegistrations(); + synchronized (CONVERTERS) { + for (int i = 0; i < CONVERTERS.size(); i++) { + + ConverterInfo unknownInfo1 = CONVERTERS.get(i); + for (int j = 0; j < CONVERTERS.size(); j++) { // Not from j = i+1 since new converters get added during the loops + + ConverterInfo unknownInfo2 = CONVERTERS.get(j); + + // chain info -> info2 + if ( + unknownInfo2.getFrom() != Object.class // Object can only exist at the beginning of a chain + && (unknownInfo1.getFlags() & Converter.NO_RIGHT_CHAINING) == 0 + && (unknownInfo2.getFlags() & Converter.NO_LEFT_CHAINING) == 0 + && unknownInfo2.getFrom().isAssignableFrom(unknownInfo1.getTo()) + && !exactConverterExists(unknownInfo1.getFrom(), unknownInfo2.getTo()) + ) { + ConverterInfo info1 = (ConverterInfo) unknownInfo1; + ConverterInfo info2 = (ConverterInfo) unknownInfo2; + + CONVERTERS.add(new ConverterInfo<>( + info1.getFrom(), + info2.getTo(), + new ChainedConverter<>(info1.getConverter(), info2.getConverter()), + info1.getFlags() | info2.getFlags() + )); + } + + // chain info2 -> info + else if ( + unknownInfo1.getFrom() != Object.class // Object can only exist at the beginning of a chain + && (unknownInfo1.getFlags() & Converter.NO_LEFT_CHAINING) == 0 + && (unknownInfo2.getFlags() & Converter.NO_RIGHT_CHAINING) == 0 + && unknownInfo1.getFrom().isAssignableFrom(unknownInfo2.getTo()) + && !exactConverterExists(unknownInfo2.getFrom(), unknownInfo1.getTo()) + ) { + ConverterInfo info1 = (ConverterInfo) unknownInfo1; + ConverterInfo info2 = (ConverterInfo) unknownInfo2; + + CONVERTERS.add(new ConverterInfo<>( + info2.getFrom(), + info1.getTo(), + new ChainedConverter<>(info2.getConverter(), info1.getConverter()), + info2.getFlags() | info1.getFlags() + )); + } - // chain info2 -> info - else if ( - unknownInfo1.getFrom() != Object.class // Object can only exist at the beginning of a chain - && (unknownInfo1.getFlags() & Converter.NO_LEFT_CHAINING) == 0 - && (unknownInfo2.getFlags() & Converter.NO_RIGHT_CHAINING) == 0 - && unknownInfo1.getFrom().isAssignableFrom(unknownInfo2.getTo()) - && !exactConverterExists(unknownInfo2.getFrom(), unknownInfo1.getTo()) - ) { - ConverterInfo info1 = (ConverterInfo) unknownInfo1; - ConverterInfo info2 = (ConverterInfo) unknownInfo2; - - CONVERTERS.add(new ConverterInfo<>( - info2.getFrom(), - info1.getTo(), - new ChainedConverter<>(info2.getConverter(), info1.getConverter()), - info2.getFlags() | info1.getFlags() - )); } } - } } /** - * Internal method. + * Internal method. All calling locations are expected to manually synchronize this method if necessary. * @return Whether a Converter exists that EXACTLY matches the provided types. */ private static boolean exactConverterExists(Class from, Class to) { for (ConverterInfo info : CONVERTERS) { - if (from == info.getFrom() && to == info.getTo()) + if (from == info.getFrom() && to == info.getTo()) { return true; + } } return false; } @@ -168,8 +175,10 @@ private static boolean exactConverterExists(Class from, Class to) { * @return Whether a Converter capable of converting 'fromType' to 'toType' exists. */ public static boolean converterExists(Class fromType, Class toType) { - if (toType.isAssignableFrom(fromType) || fromType.isAssignableFrom(toType)) + assertIsDoneLoading(); + if (toType.isAssignableFrom(fromType) || fromType.isAssignableFrom(toType)) { return true; + } return getConverter(fromType, toType) != null; } @@ -177,13 +186,28 @@ public static boolean converterExists(Class fromType, Class toType) { * @return Whether a Converter capable of converting 'fromType' to one of the provided 'toTypes' exists. */ public static boolean converterExists(Class fromType, Class... toTypes) { + assertIsDoneLoading(); for (Class toType : toTypes) { - if (converterExists(fromType, toType)) + if (converterExists(fromType, toType)) { return true; + } } return false; } + /** + * A method for obtaining a Converter that can convert an object of 'fromType' into an object of 'toType'. + * @param fromType The type to convert from. + * @param toType The type to convert to. + * @return A Converter capable of converting an object of 'fromType' into an object of 'toType'. + * Will return null if no such Converter exists. + */ + @Nullable + public static Converter getConverter(Class fromType, Class toType) { + ConverterInfo info = getConverterInfo(fromType, toType); + return info != null ? info.getConverter() : null; + } + /** * A method for obtaining the ConverterInfo of a Converter that can convert * an object of 'fromType' into an object of 'toType'. @@ -195,29 +219,21 @@ public static boolean converterExists(Class fromType, Class... toTypes) { @Nullable @SuppressWarnings("unchecked") public static ConverterInfo getConverterInfo(Class fromType, Class toType) { - int hash = Objects.hash(fromType, toType); + assertIsDoneLoading(); - ConverterInfo info = (ConverterInfo) QUICK_ACCESS_CONVERTERS.get(hash); + Pair, Class> pair = new Pair<>(fromType, toType); + ConverterInfo converter; - if (info == null) { // Manual lookup - info = getConverter_i(fromType, toType); - QUICK_ACCESS_CONVERTERS.put(hash, info); + synchronized (QUICK_ACCESS_CONVERTERS) { + if (QUICK_ACCESS_CONVERTERS.containsKey(pair)) { + converter = (ConverterInfo) QUICK_ACCESS_CONVERTERS.get(pair); + } else { // Compute QUICK_ACCESS for provided types + converter = getConverterInfo_i(fromType, toType); + QUICK_ACCESS_CONVERTERS.put(pair, converter); + } } - return info; - } - - /** - * A method for obtaining a Converter that can convert an object of 'fromType' into an object of 'toType'. - * @param fromType The type to convert from. - * @param toType The type to convert to. - * @return A Converter capable of converting an object of 'fromType' into an object of 'toType'. - * Will return null if no such Converter exists. - */ - @Nullable - public static Converter getConverter(Class fromType, Class toType) { - ConverterInfo info = getConverterInfo(fromType, toType); - return info != null ? info.getConverter() : null; + return converter; } /** @@ -237,26 +253,31 @@ public static Converter getConverter(Class fromType, Class to */ @Nullable @SuppressWarnings("unchecked") - private static ConverterInfo getConverter_i( + private static ConverterInfo getConverterInfo_i( Class fromType, Class toType ) { // Check for an exact match for (ConverterInfo info : CONVERTERS) { - if (fromType == info.getFrom() && toType == info.getTo()) + if (fromType == info.getFrom() && toType == info.getTo()) { return (ConverterInfo) info; + } } // Check for an almost perfect match for (ConverterInfo info : CONVERTERS) { - if (info.getFrom().isAssignableFrom(fromType) && toType.isAssignableFrom(info.getTo())) + if (info.getFrom().isAssignableFrom(fromType) && toType.isAssignableFrom(info.getTo())) { return (ConverterInfo) info; + } } // We don't want to create "maybe" converters for 'Object -> X' conversions // Instead, we should just try and convert during runtime when we have a better idea of the fromType - if (fromType == Object.class) - return new ConverterInfo<>(fromType, toType, fromObject -> Converters.convert(fromObject, toType), 0); + if (fromType == Object.class) { + return new ConverterInfo<>( + fromType, toType, fromObject -> Converters.convert(fromObject, toType), Converter.NO_LEFT_CHAINING + ); + } // Attempt to find converters that have either 'from' OR 'to' not exactly matching for (ConverterInfo unknownInfo : CONVERTERS) { @@ -267,10 +288,11 @@ private static Converte // Basically, this converter might convert 'F' into something that's shares a parent with 'T' return new ConverterInfo<>(fromType, toType, fromObject -> { Object converted = info.getConverter().convert(fromObject); - if (toType.isInstance(converted)) + if (toType.isInstance(converted)) { return (T) converted; + } return null; - }, 0); + }, Converter.ALL_CHAINING); } else if (fromType.isAssignableFrom(unknownInfo.getFrom()) && toType.isAssignableFrom(unknownInfo.getTo())) { ConverterInfo info = (ConverterInfo) unknownInfo; @@ -278,10 +300,11 @@ private static Converte // 'from' doesn't exactly match and needs to be filtered // Basically, this converter will only convert certain 'F' objects return new ConverterInfo<>(fromType, toType, fromObject -> { - if (!info.getFrom().isInstance(fromType)) + if (!info.getFrom().isInstance(fromType)) { return null; + } return info.getConverter().convert((SubType) fromObject); - }, 0); + }, Converter.ALL_CHAINING); } } @@ -295,17 +318,20 @@ private static Converte // Basically, this converter will only convert certain 'F' objects // and some conversion results will only share a parent with 'T' return new ConverterInfo<>(fromType, toType, fromObject -> { - if (!info.getFrom().isInstance(fromObject)) + if (!info.getFrom().isInstance(fromObject)) { return null; + } Object converted = info.getConverter().convert((SubType) fromObject); - if (toType.isInstance(converted)) + if (toType.isInstance(converted)) { return (T) converted; + } return null; - }, 0); + }, Converter.ALL_CHAINING); } } + // No converter available return null; } @@ -319,15 +345,19 @@ private static Converte @Nullable @SuppressWarnings("unchecked") public static To convert(@Nullable From from, Class toType) { - if (from == null) + assertIsDoneLoading(); + if (from == null) { return null; + } - if (toType.isInstance(from)) + if (toType.isInstance(from)) { return (To) from; + } Converter converter = getConverter((Class) from.getClass(), toType); - if (converter == null) + if (converter == null) { return null; + } return converter.convert(from); } @@ -341,19 +371,23 @@ public static To convert(@Nullable From from, Class toType) { @Nullable @SuppressWarnings("unchecked") public static To convert(@Nullable From from, Class[] toTypes) { - if (from == null) + assertIsDoneLoading(); + if (from == null) { return null; + } for (Class toType : toTypes) { - if (toType.isInstance(from)) + if (toType.isInstance(from)) { return (To) from; + } } Class fromType = (Class) from.getClass(); for (Class toType : toTypes) { Converter converter = getConverter(fromType, toType); - if (converter != null) + if (converter != null) { return converter.convert(from); + } } return null; @@ -368,19 +402,22 @@ public static To convert(@Nullable From from, Class[] t * This can happen if an object contained within 'from' is not successfully converted. */ @SuppressWarnings("unchecked") - public static To[] convert(@Nullable Object[] from, Class toType) { - //noinspection ConstantConditions - if (from == null) + public static To[] convert(Object @Nullable [] from, Class toType) { + assertIsDoneLoading(); + if (from == null) { return (To[]) Array.newInstance(toType, 0); + } - if (toType.isAssignableFrom(from.getClass().getComponentType())) + if (toType.isAssignableFrom(from.getClass().getComponentType())) { return (To[]) from; + } List converted = new ArrayList<>(from.length); for (Object fromSingle : from) { To convertedSingle = convert(fromSingle, toType); - if (convertedSingle != null) + if (convertedSingle != null) { converted.add(convertedSingle); + } } return converted.toArray((To[]) Array.newInstance(toType, converted.size())); @@ -397,23 +434,26 @@ public static To[] convert(@Nullable Object[] from, Class toType) { * And, of course, the returned array may contain objects of a different type. */ @SuppressWarnings("unchecked") - public static To[] convert(@Nullable Object[] from, Class[] toTypes, Class superType) { - //noinspection ConstantConditions - if (from == null) + public static To[] convert(Object @Nullable [] from, Class[] toTypes, Class superType) { + assertIsDoneLoading(); + if (from == null) { return (To[]) Array.newInstance(superType, 0); + } Class fromType = from.getClass().getComponentType(); for (Class toType : toTypes) { - if (toType.isAssignableFrom(fromType)) + if (toType.isAssignableFrom(fromType)) { return (To[]) from; + } } List converted = new ArrayList<>(from.length); for (Object fromSingle : from) { To convertedSingle = convert(fromSingle, toTypes); - if (convertedSingle != null) + if (convertedSingle != null) { converted.add(convertedSingle); + } } return converted.toArray((To[]) Array.newInstance(superType, converted.size())); @@ -430,17 +470,20 @@ public static To[] convert(@Nullable Object[] from, Class[] t */ @SuppressWarnings("unchecked") public static To[] convert(From[] from, Class toType, Converter converter) { + assertIsDoneLoading(); To[] converted = (To[]) Array.newInstance(toType, from.length); int j = 0; for (From fromSingle : from) { To convertedSingle = fromSingle == null ? null : converter.convert(fromSingle); - if (convertedSingle != null) + if (convertedSingle != null) { converted[j++] = convertedSingle; + } } - if (j != converted.length) + if (j != converted.length) { converted = Arrays.copyOf(converted, j); + } return converted; } @@ -454,8 +497,9 @@ public static To[] convert(From[] from, Class toType, Converter To convertStrictly(Object from, Class toType) { To converted = convert(from, toType); - if (converted == null) + if (converted == null) { throw new ClassCastException("Cannot convert '" + from + "' to an object of type '" + toType + "'"); + } return converted; } @@ -468,12 +512,14 @@ public static To convertStrictly(Object from, Class toType) { */ @SuppressWarnings("unchecked") public static To[] convertStrictly(Object[] from, Class toType) { + assertIsDoneLoading(); To[] converted = (To[]) Array.newInstance(toType, from.length); for (int i = 0; i < from.length; i++) { To convertedSingle = convert(from[i], toType); - if (convertedSingle == null) + if (convertedSingle == null) { throw new ClassCastException("Cannot convert '" + from[i] + "' to an object of type '" + toType + "'"); + } converted[i] = convertedSingle; } @@ -496,4 +542,10 @@ public static To[] convertUnsafe(From[] from, Class toType, Conver return convert(from, (Class) toType, converter); } + private static void assertIsDoneLoading() { + if (Skript.isAcceptRegistrations()) { + throw new SkriptAPIException("Converters cannot be retrieved until Skript has finished registrations."); + } + } + }