From f0729a895b034ca1f8260060dc2b9a311968adc3 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Sun, 27 Oct 2024 18:28:38 +0300 Subject: [PATCH 1/9] Fix expression conversion --- .../java/ch/njol/skript/lang/SkriptParser.java | 18 ++++++++++++++---- .../7164-expressions sometimes dont convert | 13 +++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 src/test/skript/tests/regressions/7164-expressions sometimes dont convert diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java index cefa92b2549..85cefa3f116 100644 --- a/src/main/java/ch/njol/skript/lang/SkriptParser.java +++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java @@ -371,8 +371,13 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral for (Class type : types) { // Check return type against everything that expression accepts if (parsedExpression.canReturn(type)) { - log.printLog(); - return (Expression) parsedExpression; + Expression convertedExpression = parsedExpression.getConvertedExpression(type); + if (convertedExpression != null) { + log.printLog(); + return convertedExpression; + } + log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(type), ErrorQuality.NOT_AN_EXPRESSION); + return null; } } @@ -556,8 +561,13 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo return null; } - log.printLog(); - return parsedExpression; + Expression convertedExpression = parsedExpression.getConvertedExpression(type); + if (convertedExpression != null) { + log.printLog(); + return convertedExpression; + } + log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(type), ErrorQuality.NOT_AN_EXPRESSION); + return null; } } diff --git a/src/test/skript/tests/regressions/7164-expressions sometimes dont convert b/src/test/skript/tests/regressions/7164-expressions sometimes dont convert new file mode 100644 index 00000000000..f7954b3a642 --- /dev/null +++ b/src/test/skript/tests/regressions/7164-expressions sometimes dont convert @@ -0,0 +1,13 @@ +test "expressions sometimes dont convert": + assert formatted ({_foo} + {_bar}) is not set with "formatting nothing shouldn't throw an error" + + set {_foo} to "test" + assert formatted ({_foo} + {_bar}) is not set with "formatting string + none shouldn't throw an error" + + set {_foo} to 1 + set {_bar} to 2 + assert formatted ({_foo} + {_bar}) is not set with "formatting number + number shouldn't throw an error" + + set {_foo} to "foo" + set {_bar} to "bar" + assert formatted ({_foo} + {_bar}) is "foobar" with "formatting strings doesn't work" From 8595e2ba41359868e23c76d69edb7b1c34238971 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:58:28 +0300 Subject: [PATCH 2/9] Extract duplicate code into a separate helper method --- .../ch/njol/skript/lang/SkriptParser.java | 53 ++++++------------- 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java index 85cefa3f116..90e7f14aefe 100644 --- a/src/main/java/ch/njol/skript/lang/SkriptParser.java +++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java @@ -326,7 +326,6 @@ private static Variable parseVariable(String expr, Class[] r } } - @Nullable @SuppressWarnings({"unchecked", "rawtypes"}) private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable LogEntry error, Class... types) { @@ -370,27 +369,12 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral if (parsedExpression != null) { // Expression/VariableString parsing success for (Class type : types) { // Check return type against everything that expression accepts - if (parsedExpression.canReturn(type)) { - Expression convertedExpression = parsedExpression.getConvertedExpression(type); - if (convertedExpression != null) { - log.printLog(); - return convertedExpression; - } - log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(type), ErrorQuality.NOT_AN_EXPRESSION); - return null; - } + if (parsedExpression.canReturn(type)) + return convertExpression(parsedExpression, log, type); } // No directly same type found - Class[] objTypes = (Class[]) types; // Java generics... ? - Expression convertedExpression = parsedExpression.getConvertedExpression(objTypes); - if (convertedExpression != null) { - log.printLog(); - return convertedExpression; - } - // Print errors, if we couldn't get the correct type - log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); - return null; + return convertExpression(parsedExpression, log, (Class[]) types); } log.clear(); } @@ -561,13 +545,7 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo return null; } - Expression convertedExpression = parsedExpression.getConvertedExpression(type); - if (convertedExpression != null) { - log.printLog(); - return convertedExpression; - } - log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(type), ErrorQuality.NOT_AN_EXPRESSION); - return null; + return convertExpression(parsedExpression, log, type); } } @@ -576,16 +554,8 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo return null; } - // No directly same type found - Expression convertedExpression = parsedExpression.getConvertedExpression((Class[]) types); - if (convertedExpression != null) { - log.printLog(); - return convertedExpression; - } - - // Print errors, if we couldn't get the correct type - log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); - return null; + //noinspection unchecked + return convertExpression(parsedExpression, log, (Class[]) types); } log.clear(); } @@ -629,6 +599,17 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo } } + @SafeVarargs + private static Expression convertExpression(Expression expression, ParseLogHandler log, Class... types) { + Expression convertedExpression = expression.getConvertedExpression(types); + if (convertedExpression != null) { + log.printLog(); + return convertedExpression; + } + log.printError(expression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); + return null; + } + /** * Matches ',', 'and', 'or', etc. as well as surrounding whitespace. *

From 983e864e358f298d1e4ff5613fc49c25bf59c80d Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:19:21 +0300 Subject: [PATCH 3/9] improve conversion strategy --- .../ch/njol/skript/lang/SkriptParser.java | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java index 90e7f14aefe..10f8a79d3d5 100644 --- a/src/main/java/ch/njol/skript/lang/SkriptParser.java +++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java @@ -34,10 +34,7 @@ import ch.njol.skript.lang.util.SimpleLiteral; import ch.njol.skript.localization.Language; import ch.njol.skript.localization.Message; -import ch.njol.skript.log.ErrorQuality; -import ch.njol.skript.log.LogEntry; -import ch.njol.skript.log.ParseLogHandler; -import ch.njol.skript.log.SkriptLogger; +import ch.njol.skript.log.*; import ch.njol.skript.patterns.MalformedPatternException; import ch.njol.skript.patterns.PatternCompiler; import ch.njol.skript.patterns.SkriptPattern; @@ -369,12 +366,15 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral if (parsedExpression != null) { // Expression/VariableString parsing success for (Class type : types) { // Check return type against everything that expression accepts - if (parsedExpression.canReturn(type)) - return convertExpression(parsedExpression, log, type); + if (!parsedExpression.canReturn(type)) + continue; + Expression converted = convertExpression(parsedExpression, log, type); + if (converted != null) + return converted; } // No directly same type found - return convertExpression(parsedExpression, log, (Class[]) types); + return convertExpressionOrError(parsedExpression, log, (Class[]) types); } log.clear(); } @@ -529,33 +529,25 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo if ((flags & PARSE_EXPRESSIONS) != 0) { Expression parsedExpression = parseExpression(types, expr); if (parsedExpression != null) { // Expression/VariableString parsing success - for (int i = 0; i < types.length; i++) { - Class type = types[i]; - if (type == null) // Ignore invalid (null) types - continue; - - // Check return type against everything that expression accepts - if (parsedExpression.canReturn(type)) { - if (!exprInfo.isPlural[i] && !parsedExpression.isSingle()) { // Wrong number of arguments - if (context == ParseContext.COMMAND) { - Skript.error(Commands.m_too_many_arguments.toString(exprInfo.classes[i].getName().getIndefiniteArticle(), exprInfo.classes[i].getName().toString()), ErrorQuality.SEMANTIC_ERROR); - } else { - Skript.error("'" + expr + "' can only accept a single " + exprInfo.classes[i].getName() + ", not more", ErrorQuality.SEMANTIC_ERROR); - } - return null; - } - - return convertExpression(parsedExpression, log, type); - } - } - if (onlySingular && !parsedExpression.isSingle()) { Skript.error("'" + expr + "' can only accept singular expressions, not plural", ErrorQuality.SEMANTIC_ERROR); return null; } + for (Class type : types) { + if (type == null) // Ignore invalid (null) types + continue; + + // Check return type against everything that expression accepts + if (!parsedExpression.canReturn(type)) + continue; + Expression converted = convertExpression(parsedExpression, log, type); + if (converted != null) + return converted; + } + //noinspection unchecked - return convertExpression(parsedExpression, log, (Class[]) types); + return convertExpressionOrError(parsedExpression, log, (Class[]) types); } log.clear(); } @@ -600,12 +592,19 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo } @SafeVarargs - private static Expression convertExpression(Expression expression, ParseLogHandler log, Class... types) { + private static @Nullable Expression convertExpression(Expression expression, ParseLogHandler log, Class... types) { Expression convertedExpression = expression.getConvertedExpression(types); - if (convertedExpression != null) { - log.printLog(); + if (convertedExpression == null) + return null; + log.printLog(); + return convertedExpression; + } + + @SafeVarargs + private static @Nullable Expression convertExpressionOrError(Expression expression, ParseLogHandler log, Class... types) { + Expression convertedExpression = convertExpression(expression, log, types); + if (convertedExpression != null) return convertedExpression; - } log.printError(expression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); return null; } From e4ebe924aaf65452af9fe1465a0b230d6564885b Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Sat, 9 Nov 2024 23:54:57 +0300 Subject: [PATCH 4/9] Add .sk to test file --- ...es dont convert => 7164-expressions sometimes dont convert.sk} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/test/skript/tests/regressions/{7164-expressions sometimes dont convert => 7164-expressions sometimes dont convert.sk} (100%) diff --git a/src/test/skript/tests/regressions/7164-expressions sometimes dont convert b/src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk similarity index 100% rename from src/test/skript/tests/regressions/7164-expressions sometimes dont convert rename to src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk From c6773bae714a038e73e17938bc0f8388c9e44845 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sat, 9 Nov 2024 17:27:39 -0500 Subject: [PATCH 5/9] Simplify conversion usage We need to use conversion whenever there are multiple return types. If the expression does not accept our supertype, then we can attempt to convert it, which will already handle safety checks for multiple return types --- .../ch/njol/skript/lang/SkriptParser.java | 88 +++++++++++-------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java index 10f8a79d3d5..6cffb2a30fd 100644 --- a/src/main/java/ch/njol/skript/lang/SkriptParser.java +++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java @@ -34,7 +34,10 @@ import ch.njol.skript.lang.util.SimpleLiteral; import ch.njol.skript.localization.Language; import ch.njol.skript.localization.Message; -import ch.njol.skript.log.*; +import ch.njol.skript.log.ErrorQuality; +import ch.njol.skript.log.LogEntry; +import ch.njol.skript.log.ParseLogHandler; +import ch.njol.skript.log.SkriptLogger; import ch.njol.skript.patterns.MalformedPatternException; import ch.njol.skript.patterns.PatternCompiler; import ch.njol.skript.patterns.SkriptPattern; @@ -364,17 +367,24 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral if ((flags & PARSE_EXPRESSIONS) != 0) { Expression parsedExpression = parseExpression(types, expr); if (parsedExpression != null) { // Expression/VariableString parsing success + Class parsedReturnType = parsedExpression.getReturnType(); for (Class type : types) { - // Check return type against everything that expression accepts - if (!parsedExpression.canReturn(type)) - continue; - Expression converted = convertExpression(parsedExpression, log, type); - if (converted != null) - return converted; + if (type.isAssignableFrom(parsedReturnType)) { + log.printLog(); + return (Expression) parsedExpression; + } } // No directly same type found - return convertExpressionOrError(parsedExpression, log, (Class[]) types); + Class[] objTypes = (Class[]) types; // Java generics... ? + Expression convertedExpression = parsedExpression.getConvertedExpression(objTypes); + if (convertedExpression != null) { + log.printLog(); + return convertedExpression; + } + // Print errors, if we couldn't get the correct type + log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); + return null; } log.clear(); } @@ -529,25 +539,43 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo if ((flags & PARSE_EXPRESSIONS) != 0) { Expression parsedExpression = parseExpression(types, expr); if (parsedExpression != null) { // Expression/VariableString parsing success + Class parsedReturnType = parsedExpression.getReturnType(); + for (int i = 0; i < types.length; i++) { + Class type = types[i]; + if (type == null) // Ignore invalid (null) types + continue; + + // Check return type against the expression's return type + if (type.isAssignableFrom(parsedReturnType)) { + if (!exprInfo.isPlural[i] && !parsedExpression.isSingle()) { // Wrong number of arguments + if (context == ParseContext.COMMAND) { + Skript.error(Commands.m_too_many_arguments.toString(exprInfo.classes[i].getName().getIndefiniteArticle(), exprInfo.classes[i].getName().toString()), ErrorQuality.SEMANTIC_ERROR); + } else { + Skript.error("'" + expr + "' can only accept a single " + exprInfo.classes[i].getName() + ", not more", ErrorQuality.SEMANTIC_ERROR); + } + return null; + } + + log.printLog(); + return parsedExpression; + } + } + if (onlySingular && !parsedExpression.isSingle()) { Skript.error("'" + expr + "' can only accept singular expressions, not plural", ErrorQuality.SEMANTIC_ERROR); return null; } - for (Class type : types) { - if (type == null) // Ignore invalid (null) types - continue; - - // Check return type against everything that expression accepts - if (!parsedExpression.canReturn(type)) - continue; - Expression converted = convertExpression(parsedExpression, log, type); - if (converted != null) - return converted; - } + // No directly same type found + Expression convertedExpression = parsedExpression.getConvertedExpression((Class[]) types); + if (convertedExpression != null) { + log.printLog(); + return convertedExpression; + } - //noinspection unchecked - return convertExpressionOrError(parsedExpression, log, (Class[]) types); + // Print errors, if we couldn't get the correct type + log.printError(parsedExpression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); + return null; } log.clear(); } @@ -591,24 +619,6 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo } } - @SafeVarargs - private static @Nullable Expression convertExpression(Expression expression, ParseLogHandler log, Class... types) { - Expression convertedExpression = expression.getConvertedExpression(types); - if (convertedExpression == null) - return null; - log.printLog(); - return convertedExpression; - } - - @SafeVarargs - private static @Nullable Expression convertExpressionOrError(Expression expression, ParseLogHandler log, Class... types) { - Expression convertedExpression = convertExpression(expression, log, types); - if (convertedExpression != null) - return convertedExpression; - log.printError(expression.toString(null, false) + " " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); - return null; - } - /** * Matches ',', 'and', 'or', etc. as well as surrounding whitespace. *

From aad133903ab2dcaef20931da2983224f9aa96403 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sat, 9 Nov 2024 17:40:42 -0500 Subject: [PATCH 6/9] SimpleExpression: fix possible return type conversion This fixes SimpleExpression not converting possible return types that are not contained in the desired types array. For example, if an expression can return a Number or a String, and we want an Expression that is a Number or an World, it will now include converters for String->Number and String->World --- .../ch/njol/skript/lang/util/SimpleExpression.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java index c576cd8d774..1f8861e436d 100644 --- a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java @@ -36,6 +36,7 @@ import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.converter.Converter; import org.skriptlang.skript.lang.converter.ConverterInfo; +import org.skriptlang.skript.lang.converter.Converters; import java.lang.reflect.Array; import java.util.ArrayList; @@ -203,8 +204,9 @@ public static boolean check(@Nullable T[] values, Checker checker @Nullable @SuppressWarnings("unchecked") public Expression getConvertedExpression(Class... to) { + Class returnType = getReturnType(); // check whether this expression is already of type R - if (CollectionUtils.containsSuperclass(to, getReturnType())) + if (CollectionUtils.containsSuperclass(to, returnType)) return (Expression) this; // we might be to cast some of the possible return types to R @@ -214,11 +216,18 @@ public Expression getConvertedExpression(Class... to) { // build a converter that for casting to R // safety check is present in the event that we do not get this type at runtime final Class toType = (Class) type; - infos.add(new ConverterInfo<>(getReturnType(), toType, fromObject -> { + infos.add(new ConverterInfo<>(returnType, toType, fromObject -> { if (toType.isInstance(fromObject)) return (R) fromObject; return null; }, 0)); + } else { // this possible return type is not included in 'to' + // build all converters for converting the possible return type into any of the types of 'to' + for (Class toType : to) { + ConverterInfo converter = Converters.getConverterInfo(type, toType); + if (converter != null) + infos.add(converter); + } } } int size = infos.size(); From bfe9b493710b34e8e18db3d7a946a15d98ec89c5 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Tue, 12 Nov 2024 15:25:22 -0500 Subject: [PATCH 7/9] Use safety checks of ConvertedExpression --- .../skript/lang/util/SimpleExpression.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java index 1f8861e436d..70f815d170d 100644 --- a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java @@ -204,23 +204,18 @@ public static boolean check(@Nullable T[] values, Checker checker @Nullable @SuppressWarnings("unchecked") public Expression getConvertedExpression(Class... to) { - Class returnType = getReturnType(); // check whether this expression is already of type R - if (CollectionUtils.containsSuperclass(to, returnType)) + if (CollectionUtils.containsSuperclass(to, getReturnType())) return (Expression) this; - // we might be to cast some of the possible return types to R + // we might be able to cast some (or all) of the possible return types to R + // for possible return types that can't be directly cast, regular converters will be used List> infos = new ArrayList<>(); for (Class type : this.possibleReturnTypes()) { if (CollectionUtils.containsSuperclass(to, type)) { // this type is of R - // build a converter that for casting to R + // build a converter for casting to R // safety check is present in the event that we do not get this type at runtime - final Class toType = (Class) type; - infos.add(new ConverterInfo<>(returnType, toType, fromObject -> { - if (toType.isInstance(fromObject)) - return (R) fromObject; - return null; - }, 0)); + infos.add(new ConverterInfo<>(type, (Class) type, fromObject -> (R) fromObject, 0)); } else { // this possible return type is not included in 'to' // build all converters for converting the possible return type into any of the types of 'to' for (Class toType : to) { @@ -233,12 +228,18 @@ public Expression getConvertedExpression(Class... to) { int size = infos.size(); if (size == 1) { // if there is only one info, there is no need to wrap it in a list ConverterInfo info = infos.get(0); + // we need to remake this converter info with a runtime safety check + // this is handled by ConvertedExpression //noinspection rawtypes - return new ConvertedExpression(this, info.getTo(), info); + return new ConvertedExpression(this, info.getTo(), new ConverterInfo<>(info.getFrom(), info.getTo(), fromObject -> { + if (info.getFrom().isInstance(fromObject)) + return (R) fromObject; + return null; + }, info.getFlags())); } if (size > 1) { //noinspection rawtypes - return new ConvertedExpression(this, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, false); + return new ConvertedExpression(this, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, true); } // attempt traditional conversion with proper converters From 6d337be333e41b5a74db9ff5715d10b1788e1fcc Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sun, 29 Dec 2024 19:01:21 -0500 Subject: [PATCH 8/9] Remove incorrect converter remake --- .../ch/njol/skript/lang/util/SimpleExpression.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java index 70f815d170d..4c139af7ca1 100644 --- a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java @@ -225,19 +225,7 @@ public Expression getConvertedExpression(Class... to) { } } } - int size = infos.size(); - if (size == 1) { // if there is only one info, there is no need to wrap it in a list - ConverterInfo info = infos.get(0); - // we need to remake this converter info with a runtime safety check - // this is handled by ConvertedExpression - //noinspection rawtypes - return new ConvertedExpression(this, info.getTo(), new ConverterInfo<>(info.getFrom(), info.getTo(), fromObject -> { - if (info.getFrom().isInstance(fromObject)) - return (R) fromObject; - return null; - }, info.getFlags())); - } - if (size > 1) { + if (!infos.isEmpty()) { // there are converters for (at least some of) the return types //noinspection rawtypes return new ConvertedExpression(this, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, true); } From 344404272fd4da1807fdb93573bfa9ea2369a97b Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sun, 29 Dec 2024 20:07:55 -0500 Subject: [PATCH 9/9] Move logic from SimpleExpression to ConvertedExpression --- .../skript/lang/util/ConvertedExpression.java | 40 +++++++++++-------- .../skript/lang/util/SimpleExpression.java | 28 ------------- 2 files changed, 24 insertions(+), 44 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java b/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java index 4d6228112f0..e6150fda79c 100644 --- a/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java @@ -24,6 +24,7 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.registrations.Classes; +import ch.njol.skript.util.Utils; import ch.njol.util.Checker; import ch.njol.util.Kleenean; import ch.njol.util.coll.CollectionUtils; @@ -96,24 +97,31 @@ public ConvertedExpression(Expression source, Class to, Collecti @Nullable public static ConvertedExpression newInstance(Expression from, Class... to) { assert !CollectionUtils.containsSuperclass(to, from.getReturnType()); - // we track a list of converters that may work - List> converters = new ArrayList<>(); - for (Class type : to) { // REMIND try more converters? -> also change WrapperExpression (and maybe ExprLoopValue) - assert type != null; - // casting to is wrong, but since the converter is only used for values returned by the expression - // (which are instances of "") this won't result in any ClassCastExceptions. - for (Class checking : from.possibleReturnTypes()) { - //noinspection unchecked - ConverterInfo converter = (ConverterInfo) Converters.getConverterInfo(checking, type); - if (converter != null) - converters.add(converter); + + // we might be able to cast some (or all) of the possible return types to T + // for possible return types that can't be directly cast, regular converters will be used + List> infos = new ArrayList<>(); + for (Class type : from.possibleReturnTypes()) { + if (CollectionUtils.containsSuperclass(to, type)) { // this type is of T, build a converter simply casting + // noinspection unchecked - 'type' is a desired type in 'to' + Class toType = (Class) type; + infos.add(new ConverterInfo<>(type, toType, toType::cast, 0)); + } else { // this possible return type is not included in 'to' + // build all converters for converting the possible return type into any of the types of 'to' + for (Class toType : to) { + ConverterInfo converter = Converters.getConverterInfo(type, toType); + if (converter != null) + infos.add(converter); + } } - int size = converters.size(); - if (size == 1) // if there is only one info, there is no need to wrap it in a list - return new ConvertedExpression<>(from, type, converters.get(0)); - if (size > 1) - return new ConvertedExpression<>(from, type, converters, true); } + if (!infos.isEmpty()) { // there are converters for (at least some of) the return types + // a note: casting to is wrong, but since the converter is used only for values + // returned by the expression (which are instances of ), this won't result in any CCEs + // noinspection rawtypes, unchecked + return new ConvertedExpression(from, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, true); + } + return null; } diff --git a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java index 4c139af7ca1..eccb3c73ba5 100644 --- a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java @@ -35,14 +35,10 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.converter.Converter; -import org.skriptlang.skript.lang.converter.ConverterInfo; -import org.skriptlang.skript.lang.converter.Converters; import java.lang.reflect.Array; -import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; -import java.util.List; /** * An implementation of the {@link Expression} interface. You should usually extend this class to make a new expression. @@ -207,30 +203,6 @@ public Expression getConvertedExpression(Class... to) { // check whether this expression is already of type R if (CollectionUtils.containsSuperclass(to, getReturnType())) return (Expression) this; - - // we might be able to cast some (or all) of the possible return types to R - // for possible return types that can't be directly cast, regular converters will be used - List> infos = new ArrayList<>(); - for (Class type : this.possibleReturnTypes()) { - if (CollectionUtils.containsSuperclass(to, type)) { // this type is of R - // build a converter for casting to R - // safety check is present in the event that we do not get this type at runtime - infos.add(new ConverterInfo<>(type, (Class) type, fromObject -> (R) fromObject, 0)); - } else { // this possible return type is not included in 'to' - // build all converters for converting the possible return type into any of the types of 'to' - for (Class toType : to) { - ConverterInfo converter = Converters.getConverterInfo(type, toType); - if (converter != null) - infos.add(converter); - } - } - } - if (!infos.isEmpty()) { // there are converters for (at least some of) the return types - //noinspection rawtypes - return new ConvertedExpression(this, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, true); - } - - // attempt traditional conversion with proper converters return this.getConvertedExpr(to); }