Skip to content

Commit

Permalink
Ensure element visits aren't called multiple times with the same data
Browse files Browse the repository at this point in the history
  • Loading branch information
@NebelNidas
NebelNidas committed Apr 25, 2024
1 parent 9014fd2 commit 948ca3f
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 17 deletions.
36 changes: 24 additions & 12 deletions src/main/java/net/fabricmc/mappingio/format/srg/JamFileReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
if (visitor.visitContent()) {
String lastClass = null;
boolean visitLastClass = false;
String lastMethod = null;
boolean visitLastMethod = false;
boolean visitLastMethodContent = false;

do {
boolean isMethod;
Expand Down Expand Up @@ -136,25 +139,34 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
}

if (!visitLastClass) continue;
boolean visitMethod = false;

if (isMethod || isArg) {
visitMethod = visitor.visitMethod(memberSrcName, memberSrcDesc);
boolean isNewMethod = false;
boolean isField = !isMethod && !isArg;

if (!isField && !memberSrcName.equals(lastMethod)) {
isNewMethod = true;
lastMethod = memberSrcName;
visitLastMethod = visitor.visitMethod(memberSrcName, memberSrcDesc);
visitLastMethodContent = false;
}

if (visitMethod) {
if (isField) {
if (visitor.visitField(memberSrcName, memberSrcDesc)) {
visitor.visitDstName(MappedElementKind.FIELD, 0, dstName);
visitor.visitElementContent(MappedElementKind.FIELD);
}
} else if (visitLastMethod) {
if (isMethod) {
visitor.visitDstName(MappedElementKind.METHOD, 0, dstName);
visitor.visitElementContent(MappedElementKind.METHOD);
} else {
visitor.visitElementContent(MappedElementKind.METHOD);
visitor.visitMethodArg(argSrcPos, -1, null);
}

if (isMethod || isNewMethod) {
visitLastMethodContent = visitor.visitElementContent(MappedElementKind.METHOD);
}

if (isArg && visitLastMethodContent && visitor.visitMethodArg(argSrcPos, -1, null)) {
visitor.visitDstName(MappedElementKind.METHOD_ARG, 0, dstName);
visitor.visitElementContent(MappedElementKind.METHOD_ARG);
}
} else if (!isMethod && !isArg && visitor.visitField(memberSrcName, memberSrcDesc)) {
visitor.visitDstName(MappedElementKind.FIELD, 0, dstName);
visitor.visitElementContent(MappedElementKind.FIELD);
}
}
} while (reader.nextLine(0));
Expand Down
113 changes: 108 additions & 5 deletions src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
package net.fabricmc.mappingio;

import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -46,6 +49,7 @@ private void init() {
resetVisitedElementContentUpTo(0);
visitedEnd = false;
lastVisitedElement = null;
lastSrcInfo.clear();
}

private void resetVisitedElementContentUpTo(int inclusiveLevel) {
Expand Down Expand Up @@ -97,15 +101,18 @@ public boolean visitContent() throws IOException {
@Override
public boolean visitClass(String srcName) throws IOException {
MappedElementKind elementKind = MappedElementKind.CLASS;
SrcInfo srcInfo = new SrcInfo().srcName(srcName);

assertContentVisited();
assertNewSrcInfo(elementKind, srcInfo);

visitedClass = true;
visitedField = false;
visitedMethod = false;
visitedMethodArg = false;
visitedMethodVar = false;
lastVisitedElement = elementKind;
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitClass(srcName);
Expand All @@ -114,15 +121,20 @@ public boolean visitClass(String srcName) throws IOException {
@Override
public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException {
MappedElementKind elementKind = MappedElementKind.FIELD;
SrcInfo srcInfo = new SrcInfo()
.srcName(srcName)
.srcDesc(srcDesc);

assertClassVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

visitedField = true;
visitedMethod = false;
visitedMethodArg = false;
visitedMethodVar = false;
lastVisitedElement = elementKind;
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitField(srcName, srcDesc);
Expand All @@ -131,20 +143,20 @@ public boolean visitField(String srcName, @Nullable String srcDesc) throws IOExc
@Override
public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException {
MappedElementKind elementKind = MappedElementKind.METHOD;
SrcInfo srcInfo = new SrcInfo()
.srcName(srcName)
.srcDesc(srcDesc);

assertClassVisited();
assertElementContentVisited(elementKind.level - 1);

if (!visitedMethod) {
assertMethodArgNotVisited();
assertMethodVarNotVisited();
}
assertNewSrcInfo(elementKind, srcInfo);

visitedField = false;
visitedMethod = true;
visitedMethodArg = false;
visitedMethodVar = false;
lastVisitedElement = elementKind;
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitMethod(srcName, srcDesc);
Expand All @@ -153,14 +165,20 @@ public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOEx
@Override
public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String srcName) throws IOException {
MappedElementKind elementKind = MappedElementKind.METHOD_ARG;
SrcInfo srcInfo = new SrcInfo()
.argPosition(argPosition)
.lvIndex(lvIndex)
.srcName(srcName);

assertFieldNotVisited();
assertMethodVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

visitedMethodArg = true;
visitedMethodVar = false;
lastVisitedElement = elementKind;
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitMethodArg(argPosition, lvIndex, srcName);
Expand All @@ -169,14 +187,22 @@ public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String src
@Override
public boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName) throws IOException {
MappedElementKind elementKind = MappedElementKind.METHOD_VAR;
SrcInfo srcInfo = new SrcInfo()
.lvtRowIndex(lvtRowIndex)
.lvIndex(lvIndex)
.startOpIdx(startOpIdx)
.endOpIdx(endOpIdx)
.srcName(srcName);

assertFieldNotVisited();
assertMethodVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

visitedMethodArg = false;
visitedMethodVar = true;
lastVisitedElement = elementKind;
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitMethodVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName);
Expand Down Expand Up @@ -369,6 +395,12 @@ private void assertEndNotVisited() {
}
}

private void assertNewSrcInfo(MappedElementKind kind, SrcInfo srcInfo) {
if (srcInfo.equals(lastSrcInfo.get(kind))) {
throw new IllegalStateException("Same source name visited twice in a row");
}
}

boolean visitedHeader;
boolean visitedNamespaces;
boolean visitedMetadata;
Expand All @@ -381,4 +413,75 @@ private void assertEndNotVisited() {
boolean[] visitedElementContent = new boolean[3];
boolean visitedEnd;
MappedElementKind lastVisitedElement;
Map<MappedElementKind, SrcInfo> lastSrcInfo = new HashMap<>();

private static class SrcInfo {
SrcInfo srcName(String srcName) {
this.srcName = srcName;
return this;
}

SrcInfo srcDesc(String srcDesc) {
this.srcDesc = srcDesc;
return this;
}

SrcInfo argPosition(int argPosition) {
this.argPosition = argPosition;
return this;
}

SrcInfo lvIndex(int lvIndex) {
this.lvIndex = lvIndex;
return this;
}

SrcInfo lvtRowIndex(int lvtRowIndex) {
this.lvtRowIndex = lvtRowIndex;
return this;
}

SrcInfo startOpIdx(int startOpIdx) {
this.startOpIdx = startOpIdx;
return this;
}

SrcInfo endOpIdx(int endOpIdx) {
this.endOpIdx = endOpIdx;
return this;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}

if (o == null) {
return false;
}

if (o instanceof SrcInfo) {
SrcInfo other = (SrcInfo) o;

return Objects.equals(srcName, other.srcName)
&& Objects.equals(srcDesc, other.srcDesc)
&& argPosition == other.argPosition
&& lvIndex == other.lvIndex
&& lvtRowIndex == other.lvtRowIndex
&& startOpIdx == other.startOpIdx
&& endOpIdx == other.endOpIdx;
}

return false;
}

private String srcName;
private String srcDesc;
private int argPosition;
private int lvIndex;
private int lvtRowIndex;
private int startOpIdx;
private int endOpIdx;
}
}

0 comments on commit 948ca3f

Please sign in to comment.