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.

Sign up for GitHub

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

Jump to bottom

Rework the handling of inner classes. #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rework the handling of inner classes. #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 42 additions & 17 deletions src/main/java/net/fabricmc/stitch/commands/GenState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ public void generate(File file, JarRootEntry jarEntry, JarRootEntry jarOld) thro
}

public static boolean isMappedClass(ClassStorage storage, JarClassEntry c) {
return !c.isAnonymous();
// if an anonymous class' name does not follow the $ convention, we give it a new name anyway
return !(c.isAnonymous() && c.getName().startsWith(c.getEnclosingClassName() + "$"));
}

public static boolean isMappedField(ClassStorage storage, JarClassEntry c, JarFieldEntry f) {
Expand All @@ -143,7 +144,7 @@ private String getFieldName(ClassStorage storage, JarClassEntry c, JarFieldEntry
}

if (newToIntermediary != null) {
EntryTriple findEntry = newToIntermediary.getField(c.getFullyQualifiedName(), f.getName(), f.getDescriptor());
EntryTriple findEntry = newToIntermediary.getField(c.getName(), f.getName(), f.getDescriptor());
if (findEntry != null) {
if (findEntry.getName().contains("field_")) {
return findEntry.getName();
Expand All @@ -156,7 +157,7 @@ private String getFieldName(ClassStorage storage, JarClassEntry c, JarFieldEntry
}

if (newToOld != null) {
EntryTriple findEntry = newToOld.getField(c.getFullyQualifiedName(), f.getName(), f.getDescriptor());
EntryTriple findEntry = newToOld.getField(c.getName(), f.getName(), f.getDescriptor());
if (findEntry != null) {
findEntry = oldToIntermediary.getField(findEntry);
if (findEntry != null) {
Expand All @@ -181,7 +182,7 @@ private String getPropagation(ClassStorage storage, JarClassEntry classEntry) {
return "";
}

StringBuilder builder = new StringBuilder(classEntry.getFullyQualifiedName());
StringBuilder builder = new StringBuilder(classEntry.getName());
List<String> strings = new ArrayList<>();
String scs = getPropagation(storage, classEntry.getSuperClass(storage));
if (!scs.isEmpty()) {
Expand Down Expand Up @@ -240,28 +241,28 @@ private void findNames(ClassStorage storageOld, ClassStorage storageNew, JarClas
for (JarClassEntry cc : ccList) {
EntryTriple findEntry = null;
if (newToIntermediary != null) {
findEntry = newToIntermediary.getMethod(cc.getFullyQualifiedName(), m.getName(), m.getDescriptor());
findEntry = newToIntermediary.getMethod(cc.getName(), m.getName(), m.getDescriptor());
if (findEntry != null) {
names.computeIfAbsent(findEntry.getName(), (s) -> new TreeSet<>()).add(getNamesListEntry(storageNew, cc) + suffix);
}
}

if (findEntry == null && newToOld != null) {
findEntry = newToOld.getMethod(cc.getFullyQualifiedName(), m.getName(), m.getDescriptor());
findEntry = newToOld.getMethod(cc.getName(), m.getName(), m.getDescriptor());
if (findEntry != null) {
EntryTriple newToOldEntry = findEntry;
findEntry = oldToIntermediary.getMethod(newToOldEntry);
if (findEntry != null) {
names.computeIfAbsent(findEntry.getName(), (s) -> new TreeSet<>()).add(getNamesListEntry(storageNew, cc) + suffix);
} else {
// more involved...
JarClassEntry oldBase = storageOld.getClass(newToOldEntry.getOwner(), false);
JarClassEntry oldBase = storageOld.getClass(newToOldEntry.getOwner(), null);
if (oldBase != null) {
JarMethodEntry oldM = oldBase.getMethod(newToOldEntry.getName() + newToOldEntry.getDesc());
List<JarClassEntry> cccList = oldM.getMatchingEntries(storageOld, oldBase);

for (JarClassEntry ccc : cccList) {
findEntry = oldToIntermediary.getMethod(ccc.getFullyQualifiedName(), oldM.getName(), oldM.getDescriptor());
findEntry = oldToIntermediary.getMethod(ccc.getName(), oldM.getName(), oldM.getDescriptor());
if (findEntry != null) {
names.computeIfAbsent(findEntry.getName(), (s) -> new TreeSet<>()).add(getNamesListEntry(storageOld, ccc) + suffix);
}
Expand Down Expand Up @@ -349,21 +350,44 @@ private String getMethodName(ClassStorage storageOld, ClassStorage storageNew, J
}

private void addClass(BufferedWriter writer, JarClassEntry c, ClassStorage storageOld, ClassStorage storage, String translatedPrefix) throws IOException {
String className = c.getName();
String fullName = c.getName();
// Typically inner class names are of the form com/example/Example$InnerName
// but this is not required by the JVM spec! However, for the names we generate
// we do follow this convention.
if (c.isLocal()) {
String enclName = c.getEnclosingClassName();
String innerName = c.getInnerName();
// typically, local classes' full names have a number prefix
// before the inner name part - this is useful for allowing
// multiple local classes to have the same inner name, so we add it
if (fullName.startsWith(enclName + "$") && fullName.endsWith(innerName)) {
translatedPrefix += fullName.substring(enclName.length() + 1, fullName.length() - innerName.length());
}
}
String cname = "";
String prefixSaved = translatedPrefix;

if(!this.obfuscatedPatterns.stream().anyMatch(p -> p.matcher(className).matches())) {
translatedPrefix = c.getFullyQualifiedName();
if(!this.obfuscatedPatterns.stream().anyMatch(p -> p.matcher(fullName).matches())) {
translatedPrefix = fullName;
} else {
if (!isMappedClass(storage, c)) {
cname = c.getName();
if (c.isAnonymous()) {
// anonymous classes are only unmapped if their name
// follows the standard $ convention
cname = fullName.substring(c.getEnclosingClassName().length() + 1);
} else {
// throw exception in case the impl of isMappedClass
// changes but we forget to deal with it here
throw new IllegalStateException("don't know what to do with class " + fullName);
}
} else {
cname = null;

if (newToIntermediary != null) {
String findName = newToIntermediary.getClass(c.getFullyQualifiedName());
String findName = newToIntermediary.getClass(fullName);
if (findName != null) {
// the names we generate follow the $ convention for inner class names,
// so we can safely find the inner name like this
String[] r = findName.split("\\$");
cname = r[r.length - 1];
if (r.length == 1) {
Expand All @@ -373,10 +397,11 @@ private void addClass(BufferedWriter writer, JarClassEntry c, ClassStorage stora
}

if (cname == null && newToOld != null) {
String findName = newToOld.getClass(c.getFullyQualifiedName());
String findName = newToOld.getClass(fullName);
if (findName != null) {
findName = oldToIntermediary.getClass(findName);
if (findName != null) {
// for the same reason as above, we can safely find the inner name like this
String[] r = findName.split("\\$");
cname = r[r.length - 1];
if (r.length == 1) {
Expand All @@ -400,7 +425,7 @@ private void addClass(BufferedWriter writer, JarClassEntry c, ClassStorage stora
}
}

writer.write("CLASS\t" + c.getFullyQualifiedName() + "\t" + translatedPrefix + cname + "\n");
writer.write("CLASS\t" + fullName + "\t" + translatedPrefix + cname + "\n");

for (JarFieldEntry f : c.getFields()) {
String fName = getFieldName(storage, c, f);
Expand All @@ -409,7 +434,7 @@ private void addClass(BufferedWriter writer, JarClassEntry c, ClassStorage stora
}

if (fName != null) {
writer.write("FIELD\t" + c.getFullyQualifiedName()
writer.write("FIELD\t" + fullName
+ "\t" + f.getDescriptor()
+ "\t" + f.getName()
+ "\t" + fName + "\n");
Expand All @@ -425,7 +450,7 @@ private void addClass(BufferedWriter writer, JarClassEntry c, ClassStorage stora
}

if (mName != null) {
writer.write("METHOD\t" + c.getFullyQualifiedName()
writer.write("METHOD\t" + fullName
+ "\t" + m.getDescriptor()
+ "\t" + m.getName()
+ "\t" + mName + "\n");
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/fabricmc/stitch/representation/ClassStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@
package net.fabricmc.stitch.representation;

public interface ClassStorage {
JarClassEntry getClass(String name, boolean create);
JarClassEntry getClass(String name, JarClassEntry.Populator populator);
}
133 changes: 107 additions & 26 deletions src/main/java/net/fabricmc/stitch/representation/JarClassEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.stream.Collectors;

public class JarClassEntry extends AbstractJarEntry {
String fullyQualifiedName;
final Map<String, JarClassEntry> innerClasses;
final Map<String, JarFieldEntry> fields;
final Map<String, JarMethodEntry> methods;
Expand All @@ -34,11 +33,17 @@ public class JarClassEntry extends AbstractJarEntry {
List<String> interfaces;
List<String> subclasses;
List<String> implementers;

protected JarClassEntry(String name, String fullyQualifiedName) {
/** outer class for inner classes */
String declaringClass;
/** outer class for anonymous and local classes */
String enclosingClass;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you should track the declaring and enclosing class both with the same variable. Consider:

class TopLevel {
  Comparator<Integer> comparator = new Comparator() {...};
  class Inner {}
}

your model cannot distinguish between the two classes here; Inner has a non-null "outer class" (declaring class), while the anonymous comparator implementation class has an EnclosingMethod with a class reference but no method name and type reference.

Copy link
Author

Choose a reason for hiding this comment

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

The distinction is that Inner will have a non-null "inner name", while the anonymous class will have its inner name set to null.

I do realize that I only look at the outer class attribute for the enclosing class' name, while for inner classes that attribute isn't present so I need to get it from the inner classes attribute.

Copy link
Contributor

@liach liach May 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No:

class TopLevel {
  static {
    class Local {}
  }
  class Inner {}
}

Should've given this example first and foremost

Copy link
Author

Choose a reason for hiding this comment

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

Huh, I assumed that would give <clinit> as the enclosing method, but that is not the case. You're right, I should track the enclosing/declaring class separately to properly be able to distinguish between inner and local classes.

Copy link
Author

Choose a reason for hiding this comment

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

I think that's fixed now

String enclosingMethod;
String enclosingMethodDesc;
String innerName;

protected JarClassEntry(String name) {
super(name);

this.fullyQualifiedName = fullyQualifiedName;
this.innerClasses = new TreeMap<>(Comparator.naturalOrder());
this.fields = new TreeMap<>(Comparator.naturalOrder());
this.methods = new TreeMap<>(Comparator.naturalOrder());
Expand All @@ -48,24 +53,40 @@ protected JarClassEntry(String name, String fullyQualifiedName) {
this.implementers = new ArrayList<>();
}

protected void populate(int access, String signature, String superclass, String[] interfaces) {
this.setAccess(access);
this.signature = signature;
this.superclass = superclass;
this.interfaces = Arrays.asList(interfaces);
protected void populate(Populator populator) {
this.setAccess(populator.access);
this.signature = populator.signature;
this.superclass = populator.superclass;
this.interfaces = Arrays.asList(populator.interfaces);
if (populator.nested) {
this.declaringClass = populator.declaringClass;
this.enclosingClass = populator.enclosingClass;
this.enclosingMethod = populator.enclosingMethod;
this.enclosingMethodDesc = populator.enclosingMethodDesc;
this.innerName = populator.innerName;
}
}

protected void populateParents(ClassStorage storage) {
protected void populateRelations(ClassStorage storage) {
JarClassEntry superEntry = getSuperClass(storage);
if (superEntry != null) {
superEntry.subclasses.add(fullyQualifiedName);
superEntry.subclasses.add(name);
}

for (JarClassEntry itf : getInterfaces(storage)) {
if (itf != null) {
itf.implementers.add(fullyQualifiedName);
itf.implementers.add(name);
}
}

JarClassEntry declaringEntry = getDeclaringClass(storage);
if (declaringEntry != null) {
declaringEntry.innerClasses.put(name, this);
}
JarClassEntry enclosingEntry = getEnclosingClass(storage);
if (enclosingEntry != null) {
enclosingEntry.innerClasses.put(name, this);
}
}

// unstable
Expand All @@ -74,10 +95,6 @@ public Collection<Pair<JarClassEntry, String>> getRelatedMethods(JarMethodEntry
return relatedMethods.getOrDefault(m.getKey(), Collections.EMPTY_SET);
}

public String getFullyQualifiedName() {
return fullyQualifiedName;
}

public String getSignature() {
return signature;
}
Expand All @@ -87,7 +104,7 @@ public String getSuperClassName() {
}

public JarClassEntry getSuperClass(ClassStorage storage) {
return storage.getClass(superclass, false);
return storage.getClass(superclass, null);
}

public List<String> getInterfaceNames() {
Expand Down Expand Up @@ -120,11 +137,43 @@ private List<JarClassEntry> toClassEntryList(ClassStorage storage, List<String>
}

return stringList.stream()
.map((s) -> storage.getClass(s, false))
.map((s) -> storage.getClass(s, null))
.filter(Objects::nonNull)
.collect(Collectors.toList());
}

public String getDeclaringClassName() {
return declaringClass;
}

public JarClassEntry getDeclaringClass(ClassStorage storage) {
return hasDeclaringClass() ? storage.getClass(declaringClass, null) : null;
}

public String getEnclosingClassName() {
return enclosingClass;
}

public JarClassEntry getEnclosingClass(ClassStorage storage) {
return hasEnclosingClass() ? storage.getClass(enclosingClass, null) : null;
}

public String getEnclosingMethodName() {
return enclosingMethod;
}

public String getEnclosingMethodDescriptor() {
return enclosingMethodDesc;
}

public JarMethodEntry getEnclosingMethod(JarRootEntry storage) {
return hasEnclosingClass() && hasEnclosingMethod() ? getEnclosingClass(storage).getMethod(enclosingMethod + enclosingMethodDesc) : null;
}

public String getInnerName() {
return innerName;
}

public JarClassEntry getInnerClass(String name) {
return innerClasses.get(name);
}
Expand Down Expand Up @@ -153,20 +202,29 @@ public boolean isInterface() {
return Access.isInterface(getAccess());
}

public boolean hasDeclaringClass() {
return declaringClass != null;
}

public boolean hasEnclosingClass() {
return enclosingClass != null;
}

public boolean hasEnclosingMethod() {
return enclosingMethod != null;
}

public boolean isAnonymous() {
return getName().matches("[0-9]+");
return hasEnclosingClass() && innerName == null;
}

@Override
public String getKey() {
return getFullyQualifiedName();
public boolean isLocal() {
return hasEnclosingClass() && innerName != null;
}

public void remap(Remapper remapper) {
String oldName = fullyQualifiedName;
fullyQualifiedName = remapper.map(fullyQualifiedName);
String[] s = fullyQualifiedName.split("\\$");
name = s[s.length - 1];
String oldName = name;
name = remapper.map(name);

if (superclass != null) {
superclass = remapper.map(superclass);
Expand Down Expand Up @@ -209,4 +267,27 @@ public void remap(Remapper remapper) {
relatedMethods.put(methodKeyRemaps.getOrDefault(entry.getKey(), entry.getKey()), entry.getValue());
}
}

public static class Populator {
public int access;
public String name;
public String signature;
public String superclass;
public String[] interfaces;
public String declaringClass;
public String enclosingClass;
public String enclosingMethod;
public String enclosingMethodDesc;
public String innerName;

boolean nested;

public Populator(int access, String name, String signature, String superclass, String[] interfaces) {
this.access = access;
this.name = name;
this.signature = signature;
this.superclass = superclass;
this.interfaces = interfaces;
}
}
}
Loading