1
0
mirror of https://github.com/SquidDev-CC/CC-Tweaked synced 2024-06-26 07:03:22 +00:00

Support arguments being coerced from strings

In this case, we use Lua's tostring(x) semantics (well, modulo
metamethods), instead of Java's Object.toString(x) call. This ensures
that values are formatted (mostly) consistently between Lua and Java
methods.

 - Add IArguments.getStringCoerced, which uses Lua's tostring semantics.

 - Add a Coerced<T> wrapper type, which says to use the .getXCoerced
   methods. I'm not thrilled about this interface - there's definitely
   an argument for using annotations - but this is probably more
   consistent for now.

 - Convert existing methods to use this call.

Closes #1445
This commit is contained in:
Jonathan Coates 2023-05-20 18:52:21 +01:00
parent e0216f8792
commit 3112f455ae
No known key found for this signature in database
GPG Key ID: B9E431FF07C98D06
18 changed files with 118 additions and 46 deletions

View File

@ -48,7 +48,7 @@ jqwik = "1.7.2"
junit = "5.9.2"
# Build tools
cctJavadoc = "1.6.1"
cctJavadoc = "1.7.0"
checkstyle = "10.3.4"
curseForgeGradle = "1.0.11"
errorProne-core = "2.18.0"

View File

@ -4,7 +4,7 @@
package dan200.computercraft.shared.peripheral.printer;
import dan200.computercraft.api.lua.IArguments;
import dan200.computercraft.api.lua.Coerced;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.LuaFunction;
import dan200.computercraft.api.peripheral.IPeripheral;
@ -45,13 +45,12 @@ public String getType() {
/**
* Writes text to the current page.
*
* @param arguments The values to write to the page.
* @param textA The value to write to the page.
* @throws LuaException If any values couldn't be converted to a string, or if no page is started.
* @cc.tparam string|number ... The values to write to the page.
*/
@LuaFunction
public final void write(IArguments arguments) throws LuaException {
var text = StringUtil.toString(arguments.get(0));
public final void write(Coerced<String> textA) throws LuaException {
var text = textA.value();
var page = getCurrentPage();
page.write(text);
page.setCursorPos(page.getCursorX() + text.length(), page.getCursorY());

View File

@ -4,7 +4,7 @@
package dan200.computercraft.gametest
import dan200.computercraft.api.lua.ObjectArguments
import dan200.computercraft.api.lua.Coerced
import dan200.computercraft.client.gui.AbstractComputerScreen
import dan200.computercraft.core.apis.RedstoneAPI
import dan200.computercraft.core.apis.TermAPI
@ -88,7 +88,7 @@ fun Computer_peripheral(context: GameTestHelper) = context.sequence {
@ClientGameTest
fun Open_on_client(context: GameTestHelper) = context.sequence {
// Write "Hello, world!" and then print each event to the terminal.
thenOnComputer { getApi<TermAPI>().write(ObjectArguments("Hello, world!")) }
thenOnComputer { getApi<TermAPI>().write(Coerced("Hello, world!")) }
thenStartComputer {
val term = getApi<TermAPI>().terminal
while (true) {

View File

@ -4,7 +4,7 @@
package dan200.computercraft.gametest
import dan200.computercraft.api.lua.ObjectArguments
import dan200.computercraft.api.lua.Coerced
import dan200.computercraft.client.pocket.ClientPocketComputers
import dan200.computercraft.core.apis.TermAPI
import dan200.computercraft.gametest.api.*
@ -34,7 +34,7 @@ fun Sync_state(context: GameTestHelper) = context.sequence {
context.givePocketComputer(unique)
}
// Write some text to the computer.
thenOnComputer(unique) { getApi<TermAPI>().write(ObjectArguments("Hello, world!")) }
thenOnComputer(unique) { getApi<TermAPI>().write(Coerced("Hello, world!")) }
// And ensure its synced to the client.
thenIdle(4)
thenOnClient {
@ -49,7 +49,7 @@ fun Sync_state(context: GameTestHelper) = context.sequence {
val term = getApi<TermAPI>()
term.setCursorPos(1, 1)
term.setCursorBlink(true)
term.write(ObjectArguments("Updated text :)"))
term.write(Coerced("Updated text :)"))
}
// And ensure the new computer state and terminal are sent.
thenIdle(4)

View File

@ -0,0 +1,26 @@
// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.api.lua;
/**
* A wrapper type for "coerced" values.
* <p>
* This is designed to be used with {@link LuaFunction} annotated functions, to mark an argument as being coerced to
* the given type, rather than requiring an exact type.
*
* <h2>Example:</h2>
* <pre>{@code
* @LuaFunction
* public final void doSomething(Coerced<String> myString) {
* var value = myString.value();
* }
* }</pre>
*
* @param value The argument value.
* @param <T> The type of the underlying value.
* @see IArguments#getStringCoerced(int)
*/
public record Coerced<T>(T value) {
}

View File

@ -154,6 +154,36 @@ default String getString(int index) throws LuaException {
return string;
}
/**
* Get the argument, converting it to a string by following Lua conventions.
* <p>
* Unlike {@code Objects.toString(arguments.get(i))}, this may follow Lua's string formatting, so {@code nil} will be
* converted to {@code "nil"} and tables/functions will use their original hash.
*
* @param index The argument number.
* @return The argument's representation as a string.
* @throws LuaException If the argument cannot be converted to Java. This should be thrown in extraneous
* circumstances (if the conversion would allocate too much memory) and should
* <em>not</em> be thrown if the original argument is not present or is an unsupported
* data type (such as a function or userdata).
* @throws IllegalStateException If accessing these arguments outside the scope of the original function. See
* {@link #escapes()}.
* @see Coerced
*/
default String getStringCoerced(int index) throws LuaException {
var value = get(index);
if (value == null) return "nil";
if (value instanceof Boolean || value instanceof String) return value.toString();
if (value instanceof Number number) {
var asDouble = number.doubleValue();
var asInt = (int) asDouble;
return asInt == asDouble ? Integer.toString(asInt) : Double.toString(asDouble);
}
// This is somewhat bogus - the hash codes don't match up - but it's a good approximation.
return String.format("%s: %08x", getType(index), value.hashCode());
}
/**
* Get a string argument as a byte array.
*

View File

@ -4,12 +4,12 @@
package dan200.computercraft.core.apis;
import dan200.computercraft.api.lua.Coerced;
import dan200.computercraft.api.lua.IArguments;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.LuaFunction;
import dan200.computercraft.core.terminal.Palette;
import dan200.computercraft.core.terminal.Terminal;
import dan200.computercraft.core.util.StringUtil;
import java.nio.ByteBuffer;
@ -36,13 +36,12 @@ private static int getHighestBit(int group) {
* Unlike functions like {@code write} and {@code print}, this does not wrap the text - it simply copies the
* text to the current terminal line.
*
* @param arguments The text to write.
* @param textA The text to write.
* @throws LuaException (hidden) If the terminal cannot be found.
* @cc.param text The text to write.
*/
@LuaFunction
public final void write(IArguments arguments) throws LuaException {
var text = StringUtil.toString(arguments.get(0));
public final void write(Coerced<String> textA) throws LuaException {
var text = textA.value();
var terminal = getTerminal();
synchronized (terminal) {
terminal.write(text);
@ -79,7 +78,7 @@ public final Object[] getCursorPos() throws LuaException {
}
/**
* Set the position of the cursor. {@link #write(IArguments) terminal writes} will begin from this position.
* Set the position of the cursor. {@link #write(Coerced) terminal writes} will begin from this position.
*
* @param x The new x position of the cursor.
* @param y The new y position of the cursor.
@ -236,7 +235,7 @@ public final boolean getIsColour() throws LuaException {
/**
* Writes {@code text} to the terminal with the specific foreground and background colours.
* <p>
* As with {@link #write(IArguments)}, the text will be written at the current cursor location, with the cursor
* As with {@link #write(Coerced)}, the text will be written at the current cursor location, with the cursor
* moving to the end of the text.
* <p>
* {@code textColour} and {@code backgroundColour} must both be strings the same length as {@code text}. All

View File

@ -4,11 +4,10 @@
package dan200.computercraft.core.apis.handles;
import dan200.computercraft.api.lua.IArguments;
import dan200.computercraft.api.lua.Coerced;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.LuaFunction;
import dan200.computercraft.core.filesystem.TrackingCloseable;
import dan200.computercraft.core.util.StringUtil;
import java.io.BufferedWriter;
import java.io.IOException;
@ -34,14 +33,13 @@ public EncodedWritableHandle(BufferedWriter writer, TrackingCloseable closable)
/**
* Write a string of characters to the file.
*
* @param args The value to write.
* @param textA The text to write to the file.
* @throws LuaException If the file has been closed.
* @cc.param value The value to write to the file.
*/
@LuaFunction
public final void write(IArguments args) throws LuaException {
public final void write(Coerced<String> textA) throws LuaException {
checkOpen();
var text = StringUtil.toString(args.get(0));
var text = textA.value();
try {
writer.write(text, 0, text.length());
} catch (IOException e) {
@ -52,14 +50,13 @@ public final void write(IArguments args) throws LuaException {
/**
* Write a string of characters to the file, following them with a new line character.
*
* @param args The value to write.
* @param textA The text to write to the file.
* @throws LuaException If the file has been closed.
* @cc.param value The value to write to the file.
*/
@LuaFunction
public final void writeLine(IArguments args) throws LuaException {
public final void writeLine(Coerced<String> textA) throws LuaException {
checkOpen();
var text = StringUtil.toString(args.get(0));
var text = textA.value();
try {
writer.write(text, 0, text.length());
writer.newLine();

View File

@ -8,7 +8,6 @@
import dan200.computercraft.api.lua.*;
import dan200.computercraft.core.apis.http.options.Options;
import dan200.computercraft.core.metrics.Metrics;
import dan200.computercraft.core.util.StringUtil;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.handler.codec.http.websocketx.BinaryWebSocketFrame;
@ -75,10 +74,10 @@ public final MethodResult receive(Optional<Double> timeout) throws LuaException
* @cc.changed 1.81.0 Added argument for binary mode.
*/
@LuaFunction
public final void send(Object message, Optional<Boolean> binary) throws LuaException {
public final void send(Coerced<String> message, Optional<Boolean> binary) throws LuaException {
checkOpen();
var text = StringUtil.toString(message);
var text = message.value();
if (options.websocketMessage != 0 && text.length() > options.websocketMessage) {
throw new LuaException("Message is too large");
}

View File

@ -9,10 +9,7 @@
import com.google.common.cache.LoadingCache;
import com.google.common.primitives.Primitives;
import com.google.common.reflect.TypeToken;
import dan200.computercraft.api.lua.IArguments;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.LuaFunction;
import dan200.computercraft.api.lua.MethodResult;
import dan200.computercraft.api.lua.*;
import dan200.computercraft.api.peripheral.PeripheralType;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.MethodVisitor;
@ -47,6 +44,8 @@ public final class Generator<T> {
private static final String INTERNAL_ARGUMENTS = Type.getInternalName(IArguments.class);
private static final String DESC_ARGUMENTS = Type.getDescriptor(IArguments.class);
private static final String INTERNAL_COERCED = Type.getInternalName(Coerced.class);
private final Class<T> base;
private final List<Class<?>> context;
@ -276,6 +275,21 @@ private Boolean loadArg(MethodVisitor mw, Class<?> target, Method method, boolea
return false;
}
if (arg == Coerced.class) {
var klass = Reflect.getRawType(method, TypeToken.of(genericArg).resolveType(Reflect.COERCED_IN).getType(), false);
if (klass == null) return null;
if (klass == String.class) {
mw.visitTypeInsn(NEW, INTERNAL_COERCED);
mw.visitInsn(DUP);
mw.visitVarInsn(ALOAD, 2 + context.size());
Reflect.loadInt(mw, argIndex);
mw.visitMethodInsn(INVOKEINTERFACE, INTERNAL_ARGUMENTS, "getStringCoerced", "(I)Ljava/lang/String;", true);
mw.visitMethodInsn(INVOKESPECIAL, INTERNAL_COERCED, "<init>", "(Ljava/lang/Object;)V", false);
return true;
}
}
if (arg == Optional.class) {
var klass = Reflect.getRawType(method, TypeToken.of(genericArg).resolveType(Reflect.OPTIONAL_IN).getType(), false);
if (klass == null) return null;

View File

@ -4,6 +4,7 @@
package dan200.computercraft.core.asm;
import dan200.computercraft.api.lua.Coerced;
import dan200.computercraft.api.lua.LuaTable;
import org.objectweb.asm.MethodVisitor;
import org.slf4j.Logger;
@ -20,6 +21,7 @@
final class Reflect {
private static final Logger LOG = LoggerFactory.getLogger(Reflect.class);
static final java.lang.reflect.Type OPTIONAL_IN = Optional.class.getTypeParameters()[0];
static final java.lang.reflect.Type COERCED_IN = Coerced.class.getTypeParameters()[0];
private Reflect() {
}

View File

@ -106,6 +106,13 @@ public Object get(int index) {
return converted;
}
@Override
public String getStringCoerced(int index) {
checkAccessible();
// This doesn't run __tostring, which is _technically_ wrong, but avoids a lot of complexity.
return varargs.arg(index + 1).toString();
}
@Override
public String getType(int index) {
checkAccessible();

View File

@ -4,8 +4,6 @@
package dan200.computercraft.core.util;
import javax.annotation.Nullable;
public final class StringUtil {
private StringUtil() {
}
@ -24,8 +22,4 @@ public static String normaliseLabel(String label) {
return builder.toString();
}
public static String toString(@Nullable Object value) {
return value == null ? "" : value.toString();
}
}

View File

@ -39,7 +39,7 @@ public Object[] call(String name, Object... args) throws LuaException {
return method.apply(object, this, new ObjectArguments(args)).getResult();
}
@SuppressWarnings("unchecked")
@SuppressWarnings({ "unchecked", "TypeParameterUnusedInFormals" })
public <T> T callOf(String name, Object... args) throws LuaException {
return (T) call(name, args)[0];
}

View File

@ -137,6 +137,7 @@ public static void go() {
public static class IllegalThrows {
@LuaFunction
@SuppressWarnings("DoNotCallSuggester")
public final void go() throws IOException {
throw new IOException();
}

View File

@ -173,11 +173,13 @@ public Iterable<Object> getExtra() {
public static class PeripheralThrow implements IPeripheral {
@LuaFunction
@SuppressWarnings("DoNotCallSuggester")
public final void thisThread() throws LuaException {
throw new LuaException("!");
}
@LuaFunction(mainThread = true)
@SuppressWarnings("DoNotCallSuggester")
public final void mainThread() throws LuaException {
throw new LuaException("!");
}

View File

@ -6,8 +6,8 @@
import com.google.common.io.Files;
import dan200.computercraft.api.filesystem.WritableMount;
import dan200.computercraft.api.lua.Coerced;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.ObjectArguments;
import dan200.computercraft.core.TestFiles;
import dan200.computercraft.core.apis.handles.EncodedWritableHandle;
import org.junit.jupiter.api.Test;
@ -45,7 +45,7 @@ public void testWriteTruncates() throws FileSystemException, LuaException, IOExc
{
var writer = fs.openForWrite("out.txt", false, EncodedWritableHandle::openUtf8);
var handle = new EncodedWritableHandle(writer.get(), writer);
handle.write(new ObjectArguments("This is a long line"));
handle.write(new Coerced<>("This is a long line"));
handle.doClose();
}
@ -54,7 +54,7 @@ public void testWriteTruncates() throws FileSystemException, LuaException, IOExc
{
var writer = fs.openForWrite("out.txt", false, EncodedWritableHandle::openUtf8);
var handle = new EncodedWritableHandle(writer.get(), writer);
handle.write(new ObjectArguments("Tiny line"));
handle.write(new Coerced<>("Tiny line"));
handle.doClose();
}
@ -72,7 +72,7 @@ public void testUnmountCloses() throws FileSystemException {
fs.unmount("disk");
var err = assertThrows(LuaException.class, () -> handle.write(new ObjectArguments("Tiny line")));
var err = assertThrows(LuaException.class, () -> handle.write(new Coerced<>("Tiny line")));
assertEquals("attempt to use a closed file", err.getMessage());
}

View File

@ -221,10 +221,12 @@ private static class Entry {
this.comment = comment;
}
@SuppressWarnings("UnusedMethod")
public final String translationKey() {
return TRANSLATION_PREFIX + path;
}
@SuppressWarnings("UnusedMethod")
public final String comment() {
return comment;
}