Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public <E extends Entry<?>> Collection<E> resolveEntry(E entry, ResolutionStrate
}

Entry<ClassEntry> entryAsClassChild = this.getClassChild(entry);
if (entryAsClassChild != null && !(entryAsClassChild instanceof ClassEntry)) {
if (entryAsClassChild != null) {
AccessFlags access = this.entryIndex.getEntryAccess(entryAsClassChild);

// If we're looking for the closest and this entry exists, we're done looking
Expand All @@ -73,8 +73,12 @@ public <E extends Entry<?>> Collection<E> resolveEntry(E entry, ResolutionStrate
}

// Don't search existing private and/or static entries up the hierarchy
// Fields and classes can't be redefined, don't search them up the hierarchy
if (access != null && (access.isPrivate() || access.isStatic() || entry instanceof FieldEntry || entry instanceof ClassEntry)) {
// Fields can't be redefined, don't search them up the hierarchy
// Constructors can't be redefined, don't search them up the hierarchy
if (
(access != null && (access.isPrivate() || access.isStatic() || entry instanceof FieldEntry))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only checks for FieldEntrys when access is non-null, as before
that doesn't seem correct, but changing it breaks tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @IotaBread
reordering the check breaks shadow behaviour
i don't know exactly when the access will be null -- if we can predict which fields will have their definitions indexed, we need to document our reliance on that behaviour, and if we can't, this needs to be changed

|| (entry instanceof MethodEntry method && method.isConstructor())
) {
return Collections.singleton(entry);
} else {
// Search the entry up the hierarchy; if the entry exists we can skip static entries, since this one isn't static
Expand All @@ -97,7 +101,10 @@ public <E extends Entry<?>> Collection<E> resolveEntry(E entry, ResolutionStrate
* Get a direct child of any class that is an ancestor of the given entry.
*
* @param entry the descendant of a class
* @return the direct child of a class, which is an ancestor of the given entry or the entry itself
*
* @return the direct child of a class, which is an ancestor of the given entry or the entry itself;
* returns {@code null} if the passed {@code entry} is a {@link ClassEntry} or if all of its ancestors are
* {@link ClassEntry}s; never returns a {@link ClassEntry}
*/
@Nullable
private Entry<ClassEntry> getClassChild(Entry<?> entry) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.quiltmc.enigma.api.class_provider.ProjectClassProvider;
import org.quiltmc.enigma.api.translation.mapping.EntryResolver;
import org.quiltmc.enigma.api.translation.mapping.IndexEntryResolver;
import org.quiltmc.enigma.api.translation.mapping.ResolutionStrategy;
import org.quiltmc.enigma.api.translation.representation.AccessFlags;
import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
import org.quiltmc.enigma.api.translation.representation.entry.FieldEntry;
Expand All @@ -34,13 +35,21 @@ public class TestJarIndexInheritanceTree {
public static final Path JAR = TestUtil.obfJar("inheritance_tree");

private static final ClassEntry OBJECT_CLASS = TestEntryFactory.newClass("java/lang/Object");

private static final ClassEntry BASE_CLASS = TestEntryFactory.newClass("a");
private static final ClassEntry SUB_CLASS_A = TestEntryFactory.newClass("b");
private static final ClassEntry SUB_CLASS_AA = TestEntryFactory.newClass("d");
private static final ClassEntry SUB_CLASS_B = TestEntryFactory.newClass("c");
private static final ClassEntry SUB_CLASS_A = TestEntryFactory.newClass("d");
private static final ClassEntry SUB_CLASS_AA = TestEntryFactory.newClass("f");
private static final ClassEntry SUB_CLASS_B = TestEntryFactory.newClass("e");
private static final FieldEntry NAME_FIELD = TestEntryFactory.newField(BASE_CLASS, "a", "Ljava/lang/String;");
private static final FieldEntry NUM_THINGS_FIELD = TestEntryFactory.newField(SUB_CLASS_B, "a", "I");

private static final ClassEntry CONSTRUCTOR_PARENT = TestEntryFactory.newClass("b");
private static final ClassEntry CONSTRUCTOR_SUB_CLASS = TestEntryFactory.newClass("c");
private static final MethodEntry PARENT_NO_ARGS_CONSTRUCTOR = TestEntryFactory.newMethod(CONSTRUCTOR_PARENT.getFullName(), "<init>", "()V");
private static final MethodEntry PARENT_INT_CONSTRUCTOR = TestEntryFactory.newMethod(CONSTRUCTOR_PARENT.getFullName(), "<init>", "(I)V");
private static final MethodEntry SUB_CLASS_NO_ARGS_CONSTRUCTOR = TestEntryFactory.newMethod(CONSTRUCTOR_SUB_CLASS.getFullName(), "<init>", "()V");
private static final MethodEntry SUB_CLASS_INT_CONSTRUCTOR = TestEntryFactory.newMethod(CONSTRUCTOR_SUB_CLASS.getFullName(), "<init>", "(I)V");

private final JarIndex index;

public TestJarIndexInheritanceTree() throws Exception {
Expand All @@ -52,7 +61,9 @@ public TestJarIndexInheritanceTree() throws Exception {
@Test
public void obfEntries() {
assertThat(this.index.getIndex(EntryIndex.class).getClasses(), Matchers.containsInAnyOrder(
TestEntryFactory.newClass("org/quiltmc/enigma/input/Keep"), BASE_CLASS, SUB_CLASS_A, SUB_CLASS_AA, SUB_CLASS_B
TestEntryFactory.newClass("org/quiltmc/enigma/input/Keep"),
BASE_CLASS, SUB_CLASS_A, SUB_CLASS_AA, SUB_CLASS_B,
CONSTRUCTOR_PARENT, CONSTRUCTOR_SUB_CLASS
));
}

Expand Down Expand Up @@ -216,4 +227,19 @@ public void containsEntries() {
assertThat(entryIndex.hasMethod(TestEntryFactory.newMethod(SUB_CLASS_AA, "b", "()V")), is(false));
assertThat(entryIndex.hasMethod(TestEntryFactory.newMethod(SUB_CLASS_B, "b", "()V")), is(true));
}

@Test
public void noConstructorInheritance() {
assertThat(
this.index.getEntryResolver()
.resolveEntry(SUB_CLASS_NO_ARGS_CONSTRUCTOR, ResolutionStrategy.RESOLVE_ROOT),
contains(SUB_CLASS_NO_ARGS_CONSTRUCTOR)
);

assertThat(
this.index.getEntryResolver()
.resolveEntry(SUB_CLASS_INT_CONSTRUCTOR, ResolutionStrategy.RESOLVE_ROOT),
contains(SUB_CLASS_INT_CONSTRUCTOR)
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.quiltmc.enigma.input.inheritance_tree;

public class ConstructorParent {
ConstructorParent() { }

ConstructorParent(int i) { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.quiltmc.enigma.input.inheritance_tree;

class ConstructorSubClass extends ConstructorParent {
ConstructorSubClass() { }

ConstructorSubClass(int i) { }
}