Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle local vars for single word boolean methods #5

Merged
merged 5 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 93 additions & 4 deletions src/main/java/io/papermc/codebook/lvt/LvtAssignmentSuggester.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,32 @@

package io.papermc.codebook.lvt;

import static dev.denwav.hypo.asm.HypoAsmUtil.toJvmType;
import static dev.denwav.hypo.model.HypoModelUtil.wrapFunction;
import static org.objectweb.asm.Type.getType;

import dev.denwav.hypo.core.HypoContext;
import dev.denwav.hypo.model.data.MemberData;
import dev.denwav.hypo.model.data.MethodDescriptor;
import dev.denwav.hypo.model.data.types.JvmType;
import java.io.IOException;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
import org.objectweb.asm.tree.AbstractInsnNode;
import org.objectweb.asm.tree.FieldInsnNode;
import org.objectweb.asm.tree.MethodInsnNode;

public final class LvtAssignmentSuggester {

private LvtAssignmentSuggester() {}

public static @Nullable String suggestNameFromAssignment(final String methodName, final MethodInsnNode insn) {
public static @Nullable String suggestNameFromAssignment(
final @Nullable HypoContext context, final String methodName, final MethodInsnNode insn)
throws IOException {
@Nullable String suggested = suggestGeneric(methodName);
if (suggested != null) {
return suggested;
Expand All @@ -47,6 +63,11 @@ private LvtAssignmentSuggester() {}
return suggested;
}

suggested = suggestNameFromSingleWorldVerbBoolean(context, methodName, insn);
if (suggested != null) {
return suggested;
}

suggested = suggestNameFromAs(methodName);
if (suggested != null) {
return suggested;
Expand Down Expand Up @@ -118,7 +139,7 @@ public static String suggestNameFromRecord(final String methodName) {
return null;
}

String prefix = null;
@Nullable String prefix = null;
for (final String possiblePrefix : BOOL_METHOD_PREFIXES) {
if (!possiblePrefix.equals(methodName) && methodName.startsWith(possiblePrefix)) {
prefix = possiblePrefix;
Expand All @@ -136,6 +157,74 @@ public static String suggestNameFromRecord(final String methodName) {
}
}

private static @Nullable String suggestNameFromSingleWorldVerbBoolean(
final @Nullable HypoContext context, final String methodName, final MethodInsnNode insn)
throws IOException {
if (insn.desc == null || !insn.desc.endsWith("Z")) {
return null;
}

final String prefix;
if (methodName.equals("is")) {
prefix = "is";
} else if (methodName.equals("has")) {
prefix = "has";
} else {
return null;
}

final MethodDescriptor descriptor = MethodDescriptor.parseDescriptor(insn.desc);
final List<JvmType> paramTypes = descriptor.getParams();
if (paramTypes.size() != 1) {
return null;
}
final String paramTypeDesc = paramTypes.get(0).asInternalName();

final AbstractInsnNode prev = insn.getPrevious();
if (prev instanceof final FieldInsnNode fieldInsnNode
&& fieldInsnNode.getOpcode() == Opcodes.GETSTATIC
&& fieldInsnNode.name != null
&& isStringAllUppercase(fieldInsnNode.name)) {

final boolean isFinal = Optional.ofNullable(context)
.map(wrapFunction(ctx -> ctx.getContextProvider().findClass(fieldInsnNode.owner)))
.map(owner -> owner.field(fieldInsnNode.name, toJvmType(getType(fieldInsnNode.desc))))
.map(MemberData::isFinal)
.orElse(false);
if (!isFinal) {
return null;
}

return prefix + convertStaticFieldNameToLocalVarName(fieldInsnNode);
} else {
if ("Lnet/minecraft/tags/TagKey;".equals(paramTypeDesc)) { // isTag is better than isTagKey
return "isTag";
}
final String typeName = LvtTypeSuggester.suggestNameFromType(context, toJvmType(getType(paramTypeDesc)));
return prefix + Character.toUpperCase(typeName.charAt(0)) + typeName.substring(1);
}
}

private static String convertStaticFieldNameToLocalVarName(final FieldInsnNode fieldInsnNode) {
final StringBuilder output = new StringBuilder();
for (final String s : fieldInsnNode.name.split("_")) {
output.append(s.charAt(0)).append(s.substring(1).toLowerCase(Locale.ENGLISH));
}
return output.toString();
}

private static boolean isStringAllUppercase(final String input) {
for (int i = 0; i < input.length(); i++) {
final char ch = input.charAt(i);
if (Character.isAlphabetic(ch)) {
if (!Character.isUpperCase(ch)) {
return false;
}
}
}
return true;
}

private static @Nullable String suggestNameFromAs(final String methodName) {
if (!methodName.startsWith("as") || methodName.equals("as")) {
return null;
Expand All @@ -152,7 +241,7 @@ public static String suggestNameFromRecord(final String methodName) {
}
}

private static @Nullable String suggestNameFromNew(final String methodName, MethodInsnNode insn) {
private static @Nullable String suggestNameFromNew(final String methodName, final MethodInsnNode insn) {
if (!methodName.startsWith("new") || methodName.equals("new")) {
return null;
}
Expand Down Expand Up @@ -202,7 +291,7 @@ public static String suggestNameFromRecord(final String methodName) {
return null;
}

private static final Type stringType = Type.getType("Ljava/lang/String;");
private static final Type stringType = getType("Ljava/lang/String;");

private static @Nullable String suggestNameFromStrings(final String methodName, final MethodInsnNode insn) {
if (methodName.startsWith("split")) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/papermc/codebook/lvt/LvtSuggester.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ private static String determineFinalName(final String suggestedName, final Set<S
if (ownerClass != null && ownerClass.kind() == ClassKind.RECORD) {
return LvtAssignmentSuggester.suggestNameFromRecord(name);
} else {
return LvtAssignmentSuggester.suggestNameFromAssignment(name, methodInsnNode);
return LvtAssignmentSuggester.suggestNameFromAssignment(context, name, methodInsnNode);
}
}

Expand Down
35 changes: 21 additions & 14 deletions src/main/java/io/papermc/codebook/lvt/LvtTypeSuggester.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public final class LvtTypeSuggester {

private LvtTypeSuggester() {}

public static String suggestNameFromType(final HypoContext context, final JvmType type) throws IOException {
public static String suggestNameFromType(final @Nullable HypoContext context, final JvmType type)
throws IOException {
if (type instanceof PrimitiveType) {
return switch ((PrimitiveType) type) {
case CHAR -> "c";
Expand Down Expand Up @@ -74,7 +75,8 @@ public static String suggestNameFromType(final HypoContext context, final JvmTyp
}
}

private static String suggestNameFromClassType(final HypoContext context, final ClassType type) throws IOException {
private static String suggestNameFromClassType(final @Nullable HypoContext context, final ClassType type)
throws IOException {
final String name = type.asInternalName();
if (name.equals("Ljava/lang/String;")) {
return "string";
Expand All @@ -85,18 +87,23 @@ private static String suggestNameFromClassType(final HypoContext context, final
}

// TODO Try to determine name from signature, rather than just descriptor
final @Nullable ClassData typeClass = context.getContextProvider().findClass(type);
if (typeClass != null) {
@Nullable final ClassData listClass = context.getContextProvider().findClass("java/util/List");
@Nullable final ClassData setClass = context.getContextProvider().findClass("java/util/Set");
@Nullable final ClassData mapClass = context.getContextProvider().findClass("java/util/Map");

if (listClass != null && typeClass.doesImplement(listClass)) {
return "list";
} else if (setClass != null && typeClass.doesImplement(setClass)) {
return "set";
} else if (mapClass != null && typeClass.doesImplement(mapClass)) {
return "map";
if (context != null) {
final @Nullable ClassData typeClass = context.getContextProvider().findClass(type);
if (typeClass != null) {
@Nullable
final ClassData listClass = context.getContextProvider().findClass("java/util/List");
@Nullable
final ClassData setClass = context.getContextProvider().findClass("java/util/Set");
@Nullable
final ClassData mapClass = context.getContextProvider().findClass("java/util/Map");

if (listClass != null && typeClass.doesImplement(listClass)) {
return "list";
} else if (setClass != null && typeClass.doesImplement(setClass)) {
return "set";
} else if (mapClass != null && typeClass.doesImplement(mapClass)) {
return "map";
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.io.IOException;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvFileSource;
Expand All @@ -34,12 +35,10 @@ class LvtAssignmentSuggesterTest {
@ParameterizedTest
@CsvFileSource(resources = "/io/papermc/codebook/lvt/LvtAssignmentSuggesterTest.csv", numLinesToSkip = 1)
public void testSuggester(
final String methodName,
final String methodOwner,
final String methodDescriptor,
final String expectedName) {
final String methodName, final String methodOwner, final String methodDescriptor, final String expectedName)
throws IOException {
final @Nullable String result = LvtAssignmentSuggester.suggestNameFromAssignment(
methodName, new MethodInsnNode(-1, methodOwner, methodName, methodDescriptor));
null, methodName, new MethodInsnNode(-1, methodOwner, methodName, methodDescriptor));
assertEquals(expectedName, result);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,10 @@ hasSomeProperty,io/paper/Paper,()Z,hasSomeProperty
should,io/paper/Paper,()Z,
shouldDoSomething,io/paper/Paper,()Ljava/lang/String;,
shouldDoSomething,io/paper/Paper,()Z,shouldDoSomething
# single-word bool methods is;has,,,
is,io/paper/Paper,(Lcom/Block;)Z,isBlock
is,io/paper/Paper,(Lcom/Block;I)Z,
is,io/paper/Paper,()Z,
has,io/paper/Paper,(Lcom/OptionSpec;)Z,hasOptionSpec
has,io/paper/Paper,(Lcom/OptionSpec;)I,
has,io/paper/Paper,()Z,