Improve documentation (#84)

* Improve documentation

* `MappingTreeView` isn't immutable

Co-authored-by: Juuz <[email protected]>

* Fix typo

Co-authored-by: Juuz <[email protected]>

* Remove redundant `ForwardingMappingVisitor` comment

* Fix inconsistencies

* `TsrgFileReader` also reads CSRG

* Fix typo

* `@link` -> `@linkplain`

* `endOpIdx` is exclusive

---------

Co-authored-by: Juuz <[email protected]>
Co-authored-by: modmuss <[email protected]>

CHANGELOG.md

@@ -4,6 +4,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Improved documentation
 - Fixed NPE in `MemoryMappingTree`
 - Fixed TSRG2 reader not handling multiple passes correctly
 

README.md

@@ -3,18 +3,18 @@ Mapping-IO is a small and efficient library for working with deobfuscation mappi
 
 The API is inspired by ObjectWeb's [ASM](https://asm.ow2.io/): At its core, Mapping-IO is [visitor-based](./src/main/java/net/fabricmc/mappingio/MappingVisitor.java), but it also provides a [tree API](./src/main/java/net/fabricmc/mappingio/tree/) for in-memory storage and easier data manipulation.
 
-Utilities for more sophisticated use cases can be found in the [mapping-io-extras](./mapping-io-extras/) module; they've been moved out from the core publication due to their additional dependency requirements.
+Utilities for more sophisticated use cases can be found in the [mapping-io-extras](./mapping-io-extras/) module; they've been moved out from the core publication due to their additional dependencies.
 
 
 ## Usage
-Reading and writing can be easily be achieved via the [`MappingReader`](./src/main/java/net/fabricmc/mappingio/MappingReader.java) and [`MappingWriter`](./src/main/java/net/fabricmc/mappingio/MappingWriter.java) interfaces:
+Reading and writing can be easily achieved via the [`MappingReader`](./src/main/java/net/fabricmc/mappingio/MappingReader.java) and [`MappingWriter`](./src/main/java/net/fabricmc/mappingio/MappingWriter.java) interfaces:
 ```java
 MappingReader.read(inputPath, /* optional */ inputFormat,
 		MappingWriter.create(outputPath, outputFormat));
 ```
 
-The above example reads mappings from the input path directly into a mapping writer, writing all contents to disk in the specified mapping format.
-Keep in mind that the conversion process might be lossy if the two formats' feature sets differ; see the comparison table [here](./src/main/java/net/fabricmc/mappingio/format/MappingFormat.java) for more information.
+The above example reads mappings from the input path directly into a `MappingWriter`, writing all contents to disk in the specified format.
+Keep in mind that the conversion process might be lossy if the two formats' feature sets differ; see the comparison table [here](./src/main/java/net/fabricmc/mappingio/format/MappingFormat.java) for more details.
 
 You can also read into a tree first:
 ```java
@@ -25,8 +25,23 @@ MappingReader.read(inputPath, inputFormat, tree);
 tree.accept(MappingWriter.create(outputPath, outputFormat));
 ```
 
-If the input format is known and more direct control over specific reading parameters is desired, the formats' readers (or writers) may also be invoked directly.
+If the input format is known beforehand and more direct control over specific reading parameters is desired, the formats' readers (or writers) may also be invoked directly.
 
-Mapping manipulation is achieved either via the tree API and/or specialized `MappingVisitor`s, hand-crafted or using pre-made ones found in the [adapter](./src/main/java/net/fabricmc/mappingio/adapter/) package.
+Mapping manipulation is achieved either via the tree API or specialized `MappingVisitor`s, hand-crafted or utilizing first-party ones found in the [adapter](./src/main/java/net/fabricmc/mappingio/adapter/) package.
 
-For more information, please consult the project's Javadoc comments.
+For further information, please consult the project's Javadocs.
+
+
+### Maven
+Mapping-IO is available from the [FabricMC Maven](https://maven.fabricmc.net/net/fabricmc/mapping-io), version 0.4.2 and onwards can also be found on Maven Central.
+
+Gradle snippet:
+```gradle
+repositories {
+	mavenCentral()
+}
+
+dependencies {
+	api 'net.fabricmc:mapping-io:${mappingio_version}'
+}
+```

mapping-io-extras/src/main/java/net/fabricmc/mappingio/extras/MappingTreeRemapper.java

@@ -23,7 +23,7 @@
 import net.fabricmc.mappingio.tree.MappingTreeView;
 
 /**
- * An ASM remapper that remaps between two namespaces in a {@link MappingTreeView}.
+ * An ASM {@link Remapper} that remaps between two namespaces in a {@link MappingTreeView}.
  */
 public final class MappingTreeRemapper extends Remapper {
 	private final MappingTreeView tree;
@@ -33,9 +33,9 @@ public final class MappingTreeRemapper extends Remapper {
 	/**
 	 * Constructs a {@code MappingTreeRemapper}.
 	 *
-	 * @param tree the mapping tree view
-	 * @param from the input namespace, must be in the tree
-	 * @param to   the output namespace, must be in the tree
+	 * @param tree The mapping tree view.
+	 * @param from The input namespace, must be in the tree.
+	 * @param to The output namespace, must be in the tree.
 	 */
 	public MappingTreeRemapper(MappingTreeView tree, String from, String to) {
 		Objects.requireNonNull(tree, "Mapping tree cannot be null");

src/main/java/net/fabricmc/mappingio/FlatMappingVisitor.java

@@ -28,6 +28,9 @@
 import net.fabricmc.mappingio.adapter.FlatAsRegularMappingVisitor;
 import net.fabricmc.mappingio.adapter.RegularAsFlatMappingVisitor;
 
+/**
+ * A mapping visitor that provides the entire data for a given element within a single visit call.
+ */
 public interface FlatMappingVisitor {
 	default Set<MappingFlag> getFlags() {
 		return MappingFlag.NONE;
@@ -40,7 +43,7 @@ default void reset() {
 	/**
 	 * Determine whether the header (namespaces, metadata if part of the header) should be visited.
 	 *
-	 * @return true if the header is to be visited, false otherwise
+	 * @return {@code true} if the header is to be visited, {@code false} otherwise.
 	 */
 	default boolean visitHeader() throws IOException {
 		return true;
@@ -53,7 +56,7 @@ default void visitMetadata(String key, @Nullable String value) throws IOExceptio
 	/**
 	 * Determine whether the mapping content (classes and anything below, metadata if not part of the header) should be visited.
 	 *
-	 * @return true if content is to be visited, false otherwise
+	 * @return {@code true} if content is to be visited, {@code false} otherwise.
 	 */
 	default boolean visitContent() throws IOException {
 		return true;
@@ -96,7 +99,8 @@ void visitMethodVarComment(String srcClsName, String srcMethodName, @Nullable St
 
 	/**
 	 * Finish the visitation pass.
-	 * @return true if the visitation pass is final, false if it should be started over
+	 *
+	 * @return {@code true} if the visitation pass is final, {@code false} if it should be started over.
 	 */
 	default boolean visitEnd() throws IOException {
 		return true;

src/main/java/net/fabricmc/mappingio/MappedElementKind.java

@@ -16,6 +16,9 @@
 
 package net.fabricmc.mappingio;
 
+/**
+ * A kind of element that can be mapped.
+ */
 public enum MappedElementKind {
 	CLASS(0),
 	FIELD(1),

src/main/java/net/fabricmc/mappingio/MappingFlag.java

@@ -20,6 +20,9 @@
 import java.util.EnumSet;
 import java.util.Set;
 
+/**
+ * Flags a {@link MappingVisitor} may provide to inform the caller about certain requirements.
+ */
 public enum MappingFlag {
 	/**
 	 * Indication that the visitor may require multiple passes.

src/main/java/net/fabricmc/mappingio/MappingReader.java

@@ -58,7 +58,7 @@ public static MappingFormat detectFormat(Reader reader) throws IOException {
 		int pos = 0;
 		int len;
 
-		// Be careful not to close the reader, thats upto the caller.
+		// Be careful not to close the reader, that's up to the caller.
 		BufferedReader br = reader instanceof BufferedReader ? (BufferedReader) reader : new BufferedReader(reader);
 
 		br.mark(DETECT_HEADER_LEN);
@@ -177,35 +177,65 @@ public static List<String> getNamespaces(Reader reader, MappingFormat format) th
 		}
 	}
 
-	public static void read(Path file, MappingVisitor visitor) throws IOException {
-		read(file, null, visitor);
+	/**
+	 * Tries to detect the format of the given path and read it.
+	 *
+	 * @param path The path to read from. Can be a file or a directory.
+	 * @param visitor The receiving visitor.
+	 * @throws IOException If the format can't be detected or reading fails.
+	 */
+	public static void read(Path path, MappingVisitor visitor) throws IOException {
+		read(path, null, visitor);
 	}
 
-	public static void read(Path file, MappingFormat format, MappingVisitor visitor) throws IOException {
+	/**
+	 * Tries to read the given path using the passed format's reader.
+	 *
+	 * @param path The path to read from. Can be a file or a directory.
+	 * @param format The format to use. Has to match the path's format.
+	 * @param visitor The receiving visitor.
+	 * @throws IOException If reading fails.
+	 */
+	public static void read(Path path, MappingFormat format, MappingVisitor visitor) throws IOException {
 		if (format == null) {
-			format = detectFormat(file);
+			format = detectFormat(path);
 			if (format == null) throw new IOException("invalid/unsupported mapping format");
 		}
 
 		if (format.hasSingleFile()) {
-			try (Reader reader = Files.newBufferedReader(file)) {
+			try (Reader reader = Files.newBufferedReader(path)) {
 				read(reader, format, visitor);
 			}
 		} else {
 			switch (format) {
 			case ENIGMA_DIR:
-				EnigmaDirReader.read(file, visitor);
+				EnigmaDirReader.read(path, visitor);
 				break;
 			default:
 				throw new IllegalStateException();
 			}
 		}
 	}
 
+	/**
+	 * Tries to detect the reader's content's format and read it.
+	 *
+	 * @param reader The reader to read from.
+	 * @param visitor The receiving visitor.
+	 * @throws IOException If the format can't be detected or reading fails.
+	 */
 	public static void read(Reader reader, MappingVisitor visitor) throws IOException {
 		read(reader, null, visitor);
 	}
 
+	/**
+	 * Tries to read the reader's content using the passed format's mapping reader.
+	 *
+	 * @param reader The reader to read from.
+	 * @param format The format to use. Has to match the reader's content's format.
+	 * @param visitor The receiving visitor.
+	 * @throws IOException If reading fails.
+	 */
 	public static void read(Reader reader, MappingFormat format, MappingVisitor visitor) throws IOException {
 		if (format == null) {
 			if (!reader.markSupported()) reader = new BufferedReader(reader);

src/main/java/net/fabricmc/mappingio/MappingVisitor.java

@@ -54,7 +54,7 @@ default Set<MappingFlag> getFlags() {
 	}
 
 	/**
-	 * Reset the visitor including any chained visitors to allow for another independent visit (excluding visitEnd=false).
+	 * Reset the visitor, including any chained visitors, to allow for another independent visit (excluding visitEnd=false).
 	 */
 	default void reset() {
 		throw new UnsupportedOperationException();
@@ -63,7 +63,7 @@ default void reset() {
 	/**
 	 * Determine whether the header (namespaces, metadata if part of the header) should be visited.
 	 *
-	 * @return true if the header is to be visited, false otherwise
+	 * @return {@code true} if the header is to be visited, {@code false} otherwise.
 	 */
 	default boolean visitHeader() throws IOException {
 		return true;
@@ -76,21 +76,61 @@ default void visitMetadata(String key, @Nullable String value) throws IOExceptio
 	/**
 	 * Determine whether the mapping content (classes and anything below, metadata if not part of the header) should be visited.
 	 *
-	 * @return true if content is to be visited, false otherwise
+	 * @return {@code true} if content is to be visited, {@code false} otherwise.
 	 */
 	default boolean visitContent() throws IOException {
 		return true;
 	}
 
+	/**
+	 * Visit a class.
+	 *
+	 * @param srcName The fully qualified source name of the class, in internal form
+	 * (slashes instead of dots, dollar signs for delimiting inner classes).
+	 * @return Whether or not the class's content should be visited too.
+	 */
 	boolean visitClass(String srcName) throws IOException;
 	boolean visitField(String srcName, @Nullable String srcDesc) throws IOException;
 	boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException;
+
+	/**
+	 * Visit a parameter.
+	 *
+	 * @param argPosition Always starts at 0 and gets incremented by 1 for each additional parameter.
+	 * @param lvIndex The parameter's local variable index in the current method.
+	 * Starts at 0 for static methods, 1 otherwise. For each additional parameter,
+	 * it gets incremented by 1, or by 2 if it's a primitive {@code long} or {@code double}.
+	 * @param srcName The optional source name of the parameter.
+	 * @return Whether or not the arg's content should be visited too.
+	 */
 	boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String srcName) throws IOException;
+
+	/**
+	 * Visit a variable.
+	 *
+	 * @param lvtRowIndex The variable's index in the method's LVT
+	 * (local variable table). It is optional, so -1 can be passed instead.
+	 * This is the case since LVTs themselves are optional debug information, see
+	 * <a href="https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.13">JVMS 4.7.13</a>.
+	 * @param lvIndex The var's local variable index in the current method. For each additional variable,
+	 * it gets incremented by 1, or by 2 if it's a primitive {@code long} or {@code double}.
+	 * The first variable starts where the last parameter left off (plus the offset).
+	 * @param startOpIdx Required for cases when the lvIndex alone doesn't uniquely identify a local variable.
+	 * This is the case when variables get re-defined later on, in which case most decompilers opt to
+	 * not re-define the existing var, but instead generate a new one (with both sharing the same lvIndex).
+	 * @param endOpIdx Counterpart to startOpIdx. Exclusive.
+	 * @param srcName The optional source name of the variable.
+	 * @return Whether or not the var's content should be visited too.
+	 */
 	boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName) throws IOException;
 
 	/**
 	 * Finish the visitation pass.
-	 * @return true if the visitation pass is final, false if it should be started over
+	 *
+	 * <p>Implementors may throw an exception if a second pass is requested without the {@code NEEDS_MULTIPLE_PASSES}
+	 * flag having been passed beforehand, but only if that behavior is documented.
+	 *
+	 * @return {@code true} if the visitation pass is final, {@code false} if it should be started over.
 	 */
 	default boolean visitEnd() throws IOException {
 		return true;
@@ -99,8 +139,8 @@ default boolean visitEnd() throws IOException {
 	/**
 	 * Destination name for the current element.
 	 *
-	 * @param namespace namespace index, index into the dstNamespaces List in {@link #visitNamespaces}
-	 * @param name destination name
+	 * @param namespace Namespace index (index into the dstNamespaces list in {@link #visitNamespaces}).
+	 * @param name Destination name.
 	 */
 	void visitDstName(MappedElementKind targetKind, int namespace, String name) throws IOException;
 
@@ -114,7 +154,7 @@ default void visitDstDesc(MappedElementKind targetKind, int namespace, String de
 	 *
 	 * <p>This is also a notification about all available dst names having been passed on.
 	 *
-	 * @return true if the contents are to be visited, false otherwise
+	 * @return {@code true} if the contents are to be visited, {@code false} otherwise
 	 */
 	default boolean visitElementContent(MappedElementKind targetKind) throws IOException {
 		return true;
@@ -123,7 +163,7 @@ default boolean visitElementContent(MappedElementKind targetKind) throws IOExcep
 	/**
 	 * Comment for the specified element (last content-visited or any parent).
 	 *
-	 * @param comment comment as a potentially multi-line string
+	 * @param comment Comment as a potentially multi-line string.
 	 */
 	void visitComment(MappedElementKind targetKind, String comment) throws IOException;
 }

src/main/java/net/fabricmc/mappingio/adapter/FlatAsRegularMappingVisitor.java

@@ -28,6 +28,13 @@
 import net.fabricmc.mappingio.MappingFlag;
 import net.fabricmc.mappingio.MappingVisitor;
 
+/**
+ * A mapping visitor that forwards all relevant data to a {@link FlatMappingVisitor}.
+ *
+ * <p>Element data is relayed upon {@link #visitElementContent(MappedElementKind)}
+ * or {@link #visitComment(MappedElementKind, String)} invocation.
+ * If no data was collected for the current element, the corresponding {@link FlatMappingVisitor}'s visit method is not called.
+ */
 public final class FlatAsRegularMappingVisitor implements MappingVisitor {
 	public FlatAsRegularMappingVisitor(FlatMappingVisitor out) {
 		this.next = out;

src/main/java/net/fabricmc/mappingio/adapter/ForwardingMappingVisitor.java

@@ -27,6 +27,11 @@
 import net.fabricmc.mappingio.MappingFlag;
 import net.fabricmc.mappingio.MappingVisitor;
 
+/**
+ * A mapping visitor that forwards all visit calls to another {@link MappingVisitor}.
+ *
+ * <p>Subclasses should override the visit methods they want to intercept.
+ */
 public abstract class ForwardingMappingVisitor implements MappingVisitor {
 	protected ForwardingMappingVisitor(MappingVisitor next) {
 		Objects.requireNonNull(next, "null next");

src/main/java/net/fabricmc/mappingio/adapter/MappingDstNsReorder.java

@@ -24,7 +24,15 @@
 import net.fabricmc.mappingio.MappedElementKind;
 import net.fabricmc.mappingio.MappingVisitor;
 
+/**
+ * A mapping visitor that reorders and/or drops destination namespaces.
+ */
 public final class MappingDstNsReorder extends ForwardingMappingVisitor {
+	/**
+	 * @param next The next visitor to forward the data to.
+	 * @param newDstNs The destination namespaces, in the desired order.
+	 * Omitting entries from the list is going to drop them.
+	 */
 	public MappingDstNsReorder(MappingVisitor next, List<String> newDstNs) {
 		super(next);
 
@@ -33,6 +41,11 @@ public MappingDstNsReorder(MappingVisitor next, List<String> newDstNs) {
 		this.newDstNs = newDstNs;
 	}
 
+	/**
+	 * @param next The next visitor to forward the data to
+	 * @param newDstNs The destination namespaces, in the desired order.
+	 * Omitting entries from the list is going to drop them.
+	 */
 	public MappingDstNsReorder(MappingVisitor next, String... newDstNs) {
 		this(next, Arrays.asList(newDstNs));
 	}