Skip to content

Commit

Permalink
validate lambda parameters for uniqueness and add more tests (#205)
Browse files Browse the repository at this point in the history
* start working on lambda validation

* checkstyle

* fix up tests

* start working on tests

* implement parent parameter name conflict with top level lambda and nested lambda

* make lambda index more incremental

* add parameter names test
  • Loading branch information
ix0rai authored May 2, 2024
1 parent 8f2d4af commit c796230
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public static JarIndex empty() {
BridgeMethodIndex bridgeMethodIndex = new BridgeMethodIndex(entryIndex, inheritanceIndex, referenceIndex);
PackageVisibilityIndex packageVisibilityIndex = new PackageVisibilityIndex();
EnclosingMethodIndex enclosingMethodIndex = new EnclosingMethodIndex();
return new JarIndex(entryIndex, inheritanceIndex, referenceIndex, bridgeMethodIndex, packageVisibilityIndex, enclosingMethodIndex);
LambdaIndex lambdaIndex = new LambdaIndex();
return new JarIndex(entryIndex, inheritanceIndex, referenceIndex, bridgeMethodIndex, packageVisibilityIndex, enclosingMethodIndex, lambdaIndex);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package org.quiltmc.enigma.api.analysis.index.jar;

import com.google.common.collect.ImmutableListMultimap;
import org.quiltmc.enigma.api.analysis.ReferenceTargetType;
import org.quiltmc.enigma.api.translation.representation.Lambda;
import org.quiltmc.enigma.api.translation.representation.entry.MethodDefEntry;
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;

import javax.annotation.Nullable;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class LambdaIndex implements JarIndexer {
private final Map<MethodEntry, MethodDefEntry> callers = new HashMap<>();
private ImmutableListMultimap<MethodEntry, MethodEntry> lambdas = null;
private final ImmutableListMultimap.Builder<MethodEntry, MethodEntry> lambdasBuilder = ImmutableListMultimap.builder();

@Override
public void indexLambda(MethodDefEntry callerEntry, Lambda lambda, ReferenceTargetType targetType) {
MethodEntry implMethod = (MethodEntry) lambda.implMethod();
this.callers.put(implMethod, callerEntry);

this.lambdasBuilder.put(callerEntry, implMethod);
}

@Override
public void processIndex(JarIndex index) {
var nestedLambdas = this.lambdasBuilder.build();

ImmutableListMultimap.Builder<MethodEntry, MethodEntry> multilevelLambdasBuilder = ImmutableListMultimap.builder();
for (var callerMethod : nestedLambdas.keySet()) {
// if caller method is a lambda itself, find the top level method
boolean isLambda = nestedLambdas.containsValue(callerMethod);
MethodEntry topLevel = callerMethod;

if (!isLambda) {
multilevelLambdasBuilder.put(topLevel, callerMethod);
} else {
// travel up the chain until we find the top level method, adding as we go
while (isLambda) {
topLevel = this.callers.get(topLevel);
isLambda = nestedLambdas.containsValue(topLevel);

multilevelLambdasBuilder.put(topLevel, callerMethod);
}
}
}

this.lambdas = multilevelLambdasBuilder.build();
}

/**
* {@return the top-level method that contains the given lambda}
* @param lambda the lambda to get the caller for
*/
public MethodDefEntry getCaller(MethodEntry lambda) {
return this.callers.get(lambda);
}

/**
* {@return all lambda methods nested inside the given method}
* @param method the method to get lambdas for
*/
@Nullable
public List<MethodEntry> getInternalLambdas(MethodEntry method) {
return this.lambdas.get(method);
}

@Override
public String getTranslationKey() {
return "progress.jar.indexing.process.lambdas";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.quiltmc.enigma.api.analysis.index.jar.EntryIndex;
import org.quiltmc.enigma.api.analysis.index.jar.InheritanceIndex;
import org.quiltmc.enigma.api.analysis.index.jar.JarIndex;
import org.quiltmc.enigma.api.analysis.index.jar.LambdaIndex;
import org.quiltmc.enigma.api.analysis.index.mapping.MappingsIndex;
import org.quiltmc.enigma.api.analysis.index.mapping.PackageIndex;
import org.quiltmc.enigma.api.translation.Translator;
Expand All @@ -19,9 +20,9 @@
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -128,9 +129,35 @@ private boolean validateUnique(ValidationContext context, Entry<?> entry, String
private boolean validateParameterUniqueness(ValidationContext context, String name, LocalVariableEntry parameter) {
MethodEntry parent = parameter.getParent();
if (parent != null) {
Iterator<LocalVariableEntry> iterator = parent.getParameterIterator(this.jarIndex.getIndex(EntryIndex.class), this.deobfuscator);
while (iterator.hasNext()) {
if (iterator.next().getName().equals(name)) {
var entryIndex = this.jarIndex.getIndex(EntryIndex.class);
var lambdaIndex = this.jarIndex.getIndex(LambdaIndex.class);
var parameters = parent.getParameters(entryIndex);

// add parent parameters for lambdas
Optional<MethodEntry> lambdaParentMethod = this.getLambdaParentMethod(parent);
List<MethodEntry> lambdaParents = new ArrayList<>();
while (lambdaParentMethod.isPresent()) {
lambdaParents.add(lambdaParentMethod.get());
lambdaParentMethod = this.getLambdaParentMethod(lambdaParentMethod.get());
}

if (!lambdaParents.isEmpty()) {
parameters.addAll(lambdaParents.stream().flatMap(m -> m.getParameters(entryIndex).stream()).toList());
}

// add nested lambda parents for normal methods
List<MethodEntry> internalLambdas = lambdaIndex.getInternalLambdas(parent);
if (internalLambdas != null) {
parameters.addAll(internalLambdas.stream().flatMap(m -> m.getParameters(entryIndex).stream()).toList());
}

for (LocalVariableEntry arg : parameters) {
LocalVariableEntry translatedArg = this.deobfuscator.translate(arg);
if (translatedArg != null) {
arg = translatedArg;
}

if (arg.getName().equals(name)) {
this.raiseConflict(context, parent, name, false);
return true;
}
Expand All @@ -140,6 +167,18 @@ private boolean validateParameterUniqueness(ValidationContext context, String na
return false;
}

private Optional<MethodEntry> getLambdaParentMethod(MethodEntry method) {
var lambdaIndex = this.jarIndex.getIndex(LambdaIndex.class);
var access = this.jarIndex.getIndex(EntryIndex.class).getMethodAccess(method);
var caller = lambdaIndex.getCaller(method);

if (access != null && access.isSynthetic() && access.isPrivate() && caller != null) {
return Optional.of(caller);
}

return Optional.empty();
}

private void raiseConflict(ValidationContext context, Entry<?> parent, String name, boolean shadow) {
if (parent != null) {
context.raise(shadow ? Message.SHADOWED_NAME_CLASS : Message.NON_UNIQUE_NAME_CLASS, name, parent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
public record Lambda(String invokedName, MethodDescriptor invokedType, MethodDescriptor samMethodType, ParentedEntry<?> implMethod, MethodDescriptor instantiatedMethodType) implements Translatable {
@Override
public TranslateResult<Lambda> extendedTranslate(Translator translator, EntryResolver resolver, EntryMap<EntryMapping> mappings) {
MethodEntry samMethod = new MethodEntry(this.getInterface(), this.invokedName, this.samMethodType);
MethodEntry samMethod = this.toSamMethod();
EntryMapping samMethodMapping = this.resolveMapping(resolver, mappings, samMethod);

return TranslateResult.of(
Expand Down Expand Up @@ -47,6 +47,10 @@ public ClassEntry getInterface() {
return this.invokedType.getReturnDesc().getTypeEntry();
}

public MethodEntry toSamMethod() {
return new MethodEntry(this.getInterface(), this.invokedName, this.samMethodType);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@
public class TestInnerClassParameterStats {
private static final Path JAR = TestUtil.obfJar("inner_classes");

/**
* Note: this test will break when parameters are added/removed from any input in the {@code inner_classes} package or the {@code Keep} class.
* This is ok! Simply change the assertions for the new values.
*/
@Test
public void testInnerClassParameterStats() {
EnigmaProject project = openProject();
ProjectStatsResult stats = new StatsGenerator(project).generate(ProgressListener.createEmpty(), EnumSet.of(StatType.PARAMETERS), null, false);
// 5/8 total parameters in our six classes are non-mappable, meaning that we should get 0/3 parameters mapped
// 8/13 total parameters in our six classes are non-mappable, meaning that we should get 0/3 parameters mapped
// these non-mappable parameters come from non-static inner classes taking their enclosing class as a parameter
// they are currently manually excluded by a check in the stats generator
assertThat(stats.getMappable(StatType.PARAMETERS), equalTo(3));
assertThat(stats.getMappable(StatType.PARAMETERS), equalTo(5));
assertThat(stats.getMapped(StatType.PARAMETERS), equalTo(0));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.quiltmc.enigma.input.inner_classes;

import java.util.function.Consumer;

public class NestedLambdas {
public static void main(String g) {
gaming((s) -> gaming((w) -> {
System.out.println(s);
gaming(System.out::println);
}));
}

public static void gaming(Consumer<String> runnable) {
runnable.accept("Gaming!");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,9 @@ private void methodC() {
public int methodF() {
return 1;
}

// a(II)I
public int methodG(int a, int b) {
return a + b;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.quiltmc.enigma.api.translation.mapping.EntryRemapper;
import org.quiltmc.enigma.api.translation.mapping.tree.EntryTree;
import org.quiltmc.enigma.api.translation.mapping.tree.HashEntryTree;
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;
import org.quiltmc.enigma.util.validation.Message;
import org.quiltmc.enigma.util.validation.ParameterizedMessage;
import org.quiltmc.enigma.util.validation.ValidationContext;
Expand Down Expand Up @@ -174,13 +175,30 @@ public void conflictingMethods() {
assertMessages(vc, Message.NON_UNIQUE_NAME_CLASS);
}

@RepeatedTest(value = 2, name = REPEATED_TEST_NAME)
public void testParameterNames() {
MethodEntry method = TestEntryFactory.newMethod("a", "a", "(II)I");

remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newParameter(method, 1), new EntryMapping("param01"));

ValidationContext vc = TestUtil.newVC();
remapper.validatePutMapping(vc, TestEntryFactory.newParameter(method, 2), new EntryMapping("param01"));
assertMessages(vc, Message.NON_UNIQUE_NAME_CLASS);

remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newParameter(method, 2), new EntryMapping("param02"));

vc = TestUtil.newVC();
remapper.validatePutMapping(vc, TestEntryFactory.newParameter(method, 1), new EntryMapping("param02"));
assertMessages(vc, Message.NON_UNIQUE_NAME_CLASS);
}

/**
* Assert that the validation context contains the messages.
*
* @param vc validation context
* @param messages the messages the validation context should contain
*/
private static void assertMessages(ValidationContext vc, Message... messages) {
public static void assertMessages(ValidationContext vc, Message... messages) {
assertThat(vc.getMessages().size(), is(messages.length));
for (int i = 0; i < messages.length; i++) {
ParameterizedMessage msg = vc.getMessages().get(i);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.quiltmc.enigma.translation.mapping;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.quiltmc.enigma.TestEntryFactory;
import org.quiltmc.enigma.TestUtil;
import org.quiltmc.enigma.api.Enigma;
import org.quiltmc.enigma.api.EnigmaProject;
import org.quiltmc.enigma.api.ProgressListener;
import org.quiltmc.enigma.api.class_provider.ClasspathClassProvider;
import org.quiltmc.enigma.api.translation.mapping.EntryMapping;
import org.quiltmc.enigma.api.translation.mapping.EntryRemapper;
import org.quiltmc.enigma.api.translation.representation.entry.LocalVariableEntry;
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;
import org.quiltmc.enigma.util.validation.Message;
import org.quiltmc.enigma.util.validation.ValidationContext;

import java.nio.file.Path;

public class TestMappingValidatorParameters {
public static final Path JAR = TestUtil.obfJar("inner_classes");
private static EntryRemapper remapper;

@BeforeAll
public static void beforeAll() throws Exception {
Enigma enigma = Enigma.create();
EnigmaProject project = enigma.openJar(JAR, new ClasspathClassProvider(), ProgressListener.createEmpty());
remapper = project.getRemapper();
}

@Test
public void testLambdaParameterNames() {
MethodEntry parent = TestEntryFactory.newMethod("g", "a", "(Ljava/lang/String;)V");
LocalVariableEntry parentParam = TestEntryFactory.newParameter(parent, 0);
MethodEntry firstLambda = TestEntryFactory.newMethod("g", "b", "(Ljava/lang/String;)V");
LocalVariableEntry firstLambdaParam = TestEntryFactory.newParameter(firstLambda, 0);
MethodEntry secondLambda = TestEntryFactory.newMethod("g", "a", "(Ljava/lang/String;Ljava/lang/String;)V");
LocalVariableEntry secondLambdaParam = TestEntryFactory.newParameter(secondLambda, 1);

// validate conflict with base method
remapper.putMapping(TestUtil.newVC(), parentParam, new EntryMapping("LEVEL_0"));

ValidationContext vc = TestUtil.newVC();
remapper.validatePutMapping(vc, firstLambdaParam, new EntryMapping("LEVEL_0"));
TestMappingValidator.assertMessages(vc, Message.NON_UNIQUE_NAME_CLASS);

ValidationContext vc2 = TestUtil.newVC();
remapper.validatePutMapping(vc2, secondLambdaParam, new EntryMapping("LEVEL_0"));
TestMappingValidator.assertMessages(vc2, Message.NON_UNIQUE_NAME_CLASS);

// validate nested lambda conflict with top level lambda
remapper.putMapping(TestUtil.newVC(), firstLambdaParam, new EntryMapping("LEVEL_1"));

ValidationContext vc3 = TestUtil.newVC();
remapper.validatePutMapping(vc3, secondLambdaParam, new EntryMapping("LEVEL_1"));
TestMappingValidator.assertMessages(vc3, Message.NON_UNIQUE_NAME_CLASS);

// validate parent parameter name conflict with top level lambda and nested lambda
remapper.putMapping(TestUtil.newVC(), secondLambdaParam, new EntryMapping("LEVEL_2"));

ValidationContext vc4 = TestUtil.newVC();
remapper.validatePutMapping(vc4, parentParam, new EntryMapping("LEVEL_2"));
TestMappingValidator.assertMessages(vc4, Message.NON_UNIQUE_NAME_CLASS);

ValidationContext vc5 = TestUtil.newVC();
remapper.validatePutMapping(vc5, parentParam, new EntryMapping("LEVEL_1"));
TestMappingValidator.assertMessages(vc5, Message.NON_UNIQUE_NAME_CLASS);

// validate nested lambda name conflict with nested-er lambda

ValidationContext vc6 = TestUtil.newVC();
remapper.validatePutMapping(vc6, firstLambdaParam, new EntryMapping("LEVEL_2"));
TestMappingValidator.assertMessages(vc6, Message.NON_UNIQUE_NAME_CLASS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.quiltmc.enigma.api.translation.representation.TypeDescriptor;
import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
import org.quiltmc.enigma.api.translation.representation.entry.FieldEntry;
import org.quiltmc.enigma.api.translation.representation.entry.LocalVariableEntry;
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;

public class TestEntryFactory {
Expand All @@ -28,6 +29,10 @@ public static MethodEntry newMethod(ClassEntry classEntry, String methodName, St
return new MethodEntry(classEntry, methodName, new MethodDescriptor(methodSignature));
}

public static LocalVariableEntry newParameter(MethodEntry parent, int index) {
return new LocalVariableEntry(parent, index);
}

public static EntryReference<FieldEntry, MethodEntry> newFieldReferenceByMethod(FieldEntry fieldEntry, String callerClassName, String callerName, String callerSignature) {
return new EntryReference<>(fieldEntry, "", newMethod(callerClassName, callerName, callerSignature));
}
Expand Down

0 comments on commit c796230

Please sign in to comment.