Add missing visitElementContent calls (#103)

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Overhauled the internal `ColumnFileReader` to behave more consistently
 - Made handling of the `NEEDS_MULTIPLE_PASSES` flag more consistent, reducing memory usage in a few cases
 - Made some internal methods in Enigma and TSRG readers actually private
+- Added missing `visitElementContent` calls to CSRG and Recaf Simple readers
 
 ## [0.6.1] - 2024-04-15
 - Fixed CSRG and JAM writers sometimes skipping elements whose parents have incomplete destination names

src/main/java/net/fabricmc/mappingio/format/simple/RecafSimpleFileReader.java

@@ -132,8 +132,10 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
 					if (visitClass && memberSrcName != null) {
 						if (!isMethod && visitor.visitField(memberSrcName, memberSrcDesc)) {
 							visitor.visitDstName(MappedElementKind.FIELD, 0, memberDstName);
+							visitor.visitElementContent(MappedElementKind.FIELD);
 						} else if (isMethod && visitor.visitMethod(memberSrcName, memberSrcDesc)) {
 							visitor.visitDstName(MappedElementKind.METHOD, 0, memberDstName);
+							visitor.visitElementContent(MappedElementKind.METHOD);
 						}
 					}
 				} while (reader.nextLine(0));

src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java

@@ -143,6 +143,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
 
 							if (visitor.visitMethod(parts[2], parts[4])) {
 								visitor.visitDstName(MappedElementKind.METHOD, 0, dstName);
+								visitor.visitElementContent(MappedElementKind.METHOD);
 							}
 
 							continue;
@@ -155,6 +156,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
 
 							if (visitor.visitField(parts[2], null)) {
 								visitor.visitDstName(MappedElementKind.FIELD, 0, dstName);
+								visitor.visitElementContent(MappedElementKind.FIELD);
 							}
 
 							continue;

src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java

@@ -49,6 +49,7 @@ private void init() {
 		resetVisitedElementContentUpTo(0);
 		visitedEnd = false;
 		lastVisitedElement = null;
+		visitedLastElement = false;
 		lastSrcInfo.clear();
 	}
 
@@ -104,6 +105,7 @@ public boolean visitClass(String srcName) throws IOException {
 		SrcInfo srcInfo = new SrcInfo().srcName(srcName);
 
 		assertContentVisited();
+		assertLastElementContentVisited();
 		assertNewSrcInfo(elementKind, srcInfo);
 
 		visitedClass = true;
@@ -115,7 +117,7 @@ public boolean visitClass(String srcName) throws IOException {
 		lastSrcInfo.put(elementKind, srcInfo);
 		resetVisitedElementContentUpTo(elementKind.level);
 
-		return super.visitClass(srcName);
+		return visitedLastElement = super.visitClass(srcName);
 	}
 
 	@Override
@@ -126,6 +128,7 @@ public boolean visitField(String srcName, @Nullable String srcDesc) throws IOExc
 				.srcDesc(srcDesc);
 
 		assertClassVisited();
+		assertLastElementContentVisited();
 		assertElementContentVisited(elementKind.level - 1);
 		assertNewSrcInfo(elementKind, srcInfo);
 
@@ -137,7 +140,7 @@ public boolean visitField(String srcName, @Nullable String srcDesc) throws IOExc
 		lastSrcInfo.put(elementKind, srcInfo);
 		resetVisitedElementContentUpTo(elementKind.level);
 
-		return super.visitField(srcName, srcDesc);
+		return visitedLastElement = super.visitField(srcName, srcDesc);
 	}
 
 	@Override
@@ -148,6 +151,7 @@ public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOEx
 				.srcDesc(srcDesc);
 
 		assertClassVisited();
+		assertLastElementContentVisited();
 		assertElementContentVisited(elementKind.level - 1);
 		assertNewSrcInfo(elementKind, srcInfo);
 
@@ -159,7 +163,7 @@ public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOEx
 		lastSrcInfo.put(elementKind, srcInfo);
 		resetVisitedElementContentUpTo(elementKind.level);
 
-		return super.visitMethod(srcName, srcDesc);
+		return visitedLastElement = super.visitMethod(srcName, srcDesc);
 	}
 
 	@Override
@@ -172,6 +176,7 @@ public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String src
 
 		assertFieldNotVisited();
 		assertMethodVisited();
+		assertLastElementContentVisited();
 		assertElementContentVisited(elementKind.level - 1);
 		assertNewSrcInfo(elementKind, srcInfo);
 
@@ -181,7 +186,7 @@ public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String src
 		lastSrcInfo.put(elementKind, srcInfo);
 		resetVisitedElementContentUpTo(elementKind.level);
 
-		return super.visitMethodArg(argPosition, lvIndex, srcName);
+		return visitedLastElement = super.visitMethodArg(argPosition, lvIndex, srcName);
 	}
 
 	@Override
@@ -196,6 +201,7 @@ public boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int
 
 		assertFieldNotVisited();
 		assertMethodVisited();
+		assertLastElementContentVisited();
 		assertElementContentVisited(elementKind.level - 1);
 		assertNewSrcInfo(elementKind, srcInfo);
 
@@ -205,7 +211,7 @@ public boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int
 		lastSrcInfo.put(elementKind, srcInfo);
 		resetVisitedElementContentUpTo(elementKind.level);
 
-		return super.visitMethodVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName);
+		return visitedLastElement = super.visitMethodVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName);
 	}
 
 	@Override
@@ -219,7 +225,7 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam
 	@Override
 	public void visitDstDesc(MappedElementKind targetKind, int namespace, String desc) throws IOException {
 		assertElementVisited(targetKind);
-		assertElementContentNotVisitedUpTo(targetKind.level + 1);
+		assertElementContentNotVisitedUpTo(targetKind.level + 1); // prevent visitation after visitElementContent of same level
 
 		super.visitDstDesc(targetKind, namespace, desc);
 	}
@@ -245,15 +251,15 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept
 	@Override
 	public void visitComment(MappedElementKind targetKind, String comment) throws IOException {
 		assertElementVisited(targetKind);
-		assertElementContentVisited(targetKind.level);
-		assertElementContentNotVisitedUpTo(targetKind.level + 1);
+		assertLastElementContentVisited();
 
 		super.visitComment(targetKind, comment);
 	}
 
 	@Override
 	public boolean visitEnd() throws IOException {
 		assertContentVisited();
+		assertLastElementContentVisited();
 		assertEndNotVisited();
 
 		init();
@@ -375,17 +381,24 @@ private void assertMethodVarVisited() {
 		}
 	}
 
+	private void assertLastElementContentVisited() {
+		if (visitedLastElement) {
+			assertElementContentVisited(lastVisitedElement.level);
+			assertElementContentNotVisitedUpTo(lastVisitedElement.level + 1);
+		}
+	}
+
 	private void assertElementContentNotVisitedUpTo(int inclusiveLevel) {
 		for (int i = visitedElementContent.length - 1; i >= inclusiveLevel; i--) {
 			if (visitedElementContent[i]) {
-				throw new IllegalStateException("Element content already visited");
+				throw new IllegalStateException(lastVisitedElement + " element content already visited");
 			}
 		}
 	}
 
 	private void assertElementContentVisited(int depth) {
 		if (!visitedElementContent[depth]) {
-			throw new IllegalStateException("Element content not visited");
+			throw new IllegalStateException(lastVisitedElement + " element content not visited");
 		}
 	}
 
@@ -413,6 +426,7 @@ private void assertNewSrcInfo(MappedElementKind kind, SrcInfo srcInfo) {
 	boolean[] visitedElementContent = new boolean[3];
 	boolean visitedEnd;
 	MappedElementKind lastVisitedElement;
+	boolean visitedLastElement;
 	Map<MappedElementKind, SrcInfo> lastSrcInfo = new HashMap<>();
 
 	private static class SrcInfo {