From 926dfd010b1224e9307ac78077d1dbb4656c038f Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Fri, 11 Apr 2025 12:36:39 +0200 Subject: [PATCH 1/7] Introduce `MappingCollection` interfaces as modifiable views of a tree's internal mapping store --- .../mappingio/tree/MappingCollection.java | 72 ++++ .../mappingio/tree/MappingCollectionImpl.java | 389 ++++++++++++++++++ .../mappingio/tree/MappingCollectionView.java | 68 +++ .../fabricmc/mappingio/tree/MappingTree.java | 33 +- .../mappingio/tree/MappingTreeView.java | 45 +- .../mappingio/tree/MemoryMappingTree.java | 184 ++++----- .../tests/tree/MappingCollectionTest.java | 215 ++++++++++ 7 files changed, 886 insertions(+), 120 deletions(-) create mode 100644 src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java create mode 100644 src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java create mode 100644 src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java create mode 100644 src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java new file mode 100644 index 00000000..3072cc37 --- /dev/null +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2025 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.mappingio.tree; + +import java.util.Collection; + +import org.jetbrains.annotations.ApiStatus; + +import net.fabricmc.mappingio.tree.MappingTreeView.ClassMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.ElementMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.FieldMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodArgMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; + +/** + * A {@link Collection}-based view of element mappings present in a mapping tree. + * + *

Contrary to what's defined in {@link Collection}'s Javadocs, the {@code add} + * methods here do not guarantee adding a passed element into the collection, + * instead its data may be merged into a compatible existing entry in case and such that + * {@link #containsCompatible(Object)} returns {@code true} for the passed element. + * + *

The following methods also have alternative versions that operate on + * compatible elements rather than equal ones: + *

+ * + *

Additionally, the {@link Collection#add(Object)} and {@link Collection#addAll(Collection)} + * methods have overloaded variants that accept read-only views of the held mapping element type, + * which are converted to the tree's internal representation if necessary and then added to the tree. + * + * @param The stored Elements' type. + * @param The View type correlating to the stored mapping type. + */ +@ApiStatus.NonExtendable +public interface MappingCollection extends MappingCollectionView { + boolean add(V e); + boolean addAllViews(Collection c); + boolean removeCompatible(V e); + boolean removeAllCompatible(Collection c); + boolean retainAllCompatible(Collection c); + + interface ClassMappingCollection extends ClassMappingCollectionView { } + + interface FieldMappingCollection extends FieldMappingCollectionView { } + + interface MethodMappingCollection extends MethodMappingCollectionView { } + + interface MethodArgMappingCollection extends MethodArgMappingCollectionView { } + + interface MethodVarMappingCollection extends MethodVarMappingCollectionView { } +} diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java new file mode 100644 index 00000000..dd7e18ee --- /dev/null +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java @@ -0,0 +1,389 @@ +/* + * Copyright (c) 2025 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.mappingio.tree; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; + +import org.jetbrains.annotations.Nullable; + +import net.fabricmc.mappingio.MappedElementKind; +import net.fabricmc.mappingio.tree.MappingTree.ClassMapping; +import net.fabricmc.mappingio.tree.MappingTree.ElementMapping; +import net.fabricmc.mappingio.tree.MappingTree.MethodMapping; +import net.fabricmc.mappingio.tree.MappingTreeView.ClassMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.ElementMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.FieldMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodArgMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; + +/** + * @param The stored Elements' type. + * @param The View type correlating to the stored mapping type. + * @param The stored elements' Owner type, or any if this is a root collection. + */ +abstract class MappingCollectionImpl implements MappingCollection { + private MappingCollectionImpl(MemoryMappingTree tree, @Nullable O owner, Collection backing, MappedElementKind elementKind) { + this.tree = tree; + this.owner = owner; + this.backing = backing; + this.elementKind = elementKind; + } + + @Override + public int size() { + return backing.size(); + } + + @Override + public boolean isEmpty() { + return backing.isEmpty(); + } + + @Nullable + protected abstract E getCompatible(V o); + + @Override + public boolean contains(Object o) { + if (o instanceof ElementMappingView) { + ElementMappingView oElem = (ElementMappingView) o; + + if (oElem.getKind() == elementKind) { + return backing.contains(o); + } + } + + return false; + } + + @Override + public boolean containsCompatible(V o) { + return getCompatible(o) != null; + } + + @Override + public boolean containsAll(Collection c) { + for (Object o : c) { + if (!contains(o)) { + return false; + } + } + + return true; + } + + @Override + public boolean containsAllCompatible(Collection c) { + for (V o : c) { + if (!containsCompatible(o)) { + return false; + } + } + + return true; + } + + @Override + public Iterator iterator() { + return new IteratorWrapper(new ArrayList<>(backing).iterator(), this); + } + + @Override + public Object[] toArray() { + return backing.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return backing.toArray(a); + } + + // Mutating methods + + @Override + public abstract boolean add(V e); + + @Override + public boolean addAllViews(Collection c) { + tree.assertNotInVisitPass(); + boolean addedAny = false; + + for (V e : c) { + addedAny |= add(e); + } + + return addedAny; + } + + @Override + @SuppressWarnings("unchecked") + public boolean addAll(Collection c) { + return addAllViews((Collection) c); + } + + @Override + public boolean remove(Object o) { + tree.assertNotInVisitPass(); + return backing.remove(o); + } + + @Override + @SuppressWarnings("unchecked") + public boolean removeCompatible(V o) { + tree.assertNotInVisitPass(); + + if (o instanceof ElementMappingView) { + ElementMappingView oElem = (ElementMappingView) o; + + if (oElem.getKind() == elementKind) { + return removeCompatibleInternal((V) oElem); + } + } + + return false; + } + + protected abstract boolean removeCompatibleInternal(V e); + + @Override + public boolean removeAll(Collection c) { + tree.assertNotInVisitPass(); + boolean removedAny = false; + + for (Object o : c) { + removedAny |= remove(o); + } + + return removedAny; + } + + @Override + public boolean removeAllCompatible(Collection c) { + tree.assertNotInVisitPass(); + boolean removedAny = false; + + for (V o : c) { + removedAny |= removeCompatible(o); + } + + return removedAny; + } + + @Override + public boolean retainAll(Collection c) { + tree.assertNotInVisitPass(); + boolean removedAny = false; + + for (Iterator it = iterator(); it.hasNext();) { + E e = it.next(); + + if (!c.contains(e)) { + it.remove(); + removedAny = true; + } + } + + return removedAny; + } + + @Override + public boolean retainAllCompatible(Collection c) { + tree.assertNotInVisitPass(); + Set toRemove = new HashSet<>(this); + boolean removedAny = false; + + for (V o : c) { + E e = getCompatible(o); + + if (e != null) { + toRemove.remove(e); + + if (toRemove.isEmpty()) { + break; + } + } + } + + for (E e : toRemove) { + assert remove(e); + removedAny = true; + } + + return removedAny; + } + + @Override + public void clear() { + tree.assertNotInVisitPass(); + + for (Iterator it = iterator(); it.hasNext();) { + it.next(); + it.remove(); + } + } + + private final class IteratorWrapper implements Iterator { + IteratorWrapper(Iterator backing, MappingCollectionImpl owner) { + this.backing = backing; + this.owner = owner; + } + + @Override + public boolean hasNext() { + return backing.hasNext(); + } + + @Override + public E next() { + lastReturned = backing.next(); + return lastReturned; + } + + @Override + public void remove() { + owner.remove(lastReturned); + } + + private final Iterator backing; + private final MappingCollectionImpl owner; + private E lastReturned; + } + + static class ClassMappingCollectionImpl extends MappingCollectionImpl implements ClassMappingCollection { + ClassMappingCollectionImpl(MemoryMappingTree tree, Collection backing) { + super(tree, null, backing, MappedElementKind.CLASS); + } + + @Override + public boolean add(ClassMappingView e) { + tree.addClass(e); + return true; + } + + @Override + @SuppressWarnings("unchecked") + protected E getCompatible(ClassMappingView o) { + return (E) tree.getClass(o.getSrcName()); + } + + @Override + protected boolean removeCompatibleInternal(ClassMappingView o) { + return tree.removeClass(o.getSrcName()) != null; + } + } + + static class FieldMappingCollectionImpl extends MappingCollectionImpl implements FieldMappingCollection { + FieldMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { + super(tree, owner, backing, MappedElementKind.FIELD); + } + + @Override + public boolean add(FieldMappingView e) { + owner.addField(e); + return true; + } + + @Override + @SuppressWarnings("unchecked") + protected E getCompatible(FieldMappingView o) { + return (E) owner.getField(o.getSrcName(), o.getSrcDesc()); + } + + @Override + protected boolean removeCompatibleInternal(FieldMappingView o) { + return owner.removeField(o.getSrcName(), o.getSrcDesc()) != null; + } + } + + static class MethodMappingCollectionImpl extends MappingCollectionImpl implements MethodMappingCollection { + MethodMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { + super(tree, owner, backing, MappedElementKind.METHOD); + } + + @Override + public boolean add(MethodMappingView e) { + owner.addMethod(e); + return true; + } + + @Override + @SuppressWarnings("unchecked") + protected E getCompatible(MethodMappingView o) { + return (E) owner.getMethod(o.getSrcName(), o.getSrcDesc()); + } + + @Override + protected boolean removeCompatibleInternal(MethodMappingView o) { + return owner.removeMethod(o.getSrcName(), o.getSrcDesc()) != null; + } + } + + static class MethodArgMappingCollectionImpl extends MappingCollectionImpl implements MethodArgMappingCollection { + MethodArgMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { + super(tree, owner, backing, MappedElementKind.METHOD_ARG); + } + + @Override + public boolean add(MethodArgMappingView e) { + owner.addArg(e); + return true; + } + + @Override + @SuppressWarnings("unchecked") + protected E getCompatible(MethodArgMappingView o) { + return (E) owner.getArg(o.getArgPosition(), o.getLvIndex(), o.getSrcName()); + } + + @Override + protected boolean removeCompatibleInternal(MethodArgMappingView o) { + return owner.removeArg(o.getArgPosition(), o.getLvIndex(), o.getSrcName()) != null; + } + } + + static class MethodVarMappingCollectionImpl extends MappingCollectionImpl implements MethodVarMappingCollection { + MethodVarMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { + super(tree, owner, backing, MappedElementKind.METHOD_VAR); + } + + @Override + public boolean add(MethodVarMappingView e) { + owner.addVar(e); + return true; + } + + @Override + @SuppressWarnings("unchecked") + protected E getCompatible(MethodVarMappingView o) { + return (E) owner.getVar(o.getLvtRowIndex(), o.getLvIndex(), o.getStartOpIdx(), o.getEndOpIdx(), o.getSrcName()); + } + + @Override + protected boolean removeCompatibleInternal(MethodVarMappingView o) { + return owner.removeVar(o.getLvtRowIndex(), o.getLvIndex(), o.getStartOpIdx(), o.getEndOpIdx(), o.getSrcName()) != null; + } + } + + protected final MemoryMappingTree tree; + protected final @Nullable O owner; + protected final MappedElementKind elementKind; + protected final Collection backing; +} diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java new file mode 100644 index 00000000..e4479d16 --- /dev/null +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2025 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.mappingio.tree; + +import java.util.Collection; + +import org.jetbrains.annotations.ApiStatus; + +import net.fabricmc.mappingio.tree.MappingTreeView.ClassMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.ElementMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.FieldMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodArgMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; + +/** + * A {@link Collection}-based view of element mappings present in a mapping tree. + * + *

Contrary to what's defined in {@link Collection}'s Javadocs, the {@code add} + * methods here do not guarantee adding a passed element into the collection, + * instead its data may be merged into a compatible existing entry in case and such that + * {@link #containsCompatible(Object)} returns {@code true} for the passed element. + * + *

The following methods also have alternative versions that operate on + * compatible elements rather than equal ones: + *

+ * + *

Additionally, the {@link Collection#add(Object)} and {@link Collection#addAll(Collection)} + * methods have overloaded variants that accept read-only views of the held mapping element type, + * which are converted to the tree's internal representation if necessary and then added to the tree. + * + * @param The type of element mapping. + */ +@ApiStatus.NonExtendable +public interface MappingCollectionView extends Collection { + boolean containsCompatible(V o); + boolean containsAllCompatible(Collection c); + + interface ClassMappingCollectionView extends MappingCollection { } + + interface FieldMappingCollectionView extends MappingCollection { } + + interface MethodMappingCollectionView extends MappingCollection { } + + interface MethodArgMappingCollectionView extends MappingCollection { } + + interface MethodVarMappingCollectionView extends MappingCollection { } +} diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingTree.java b/src/main/java/net/fabricmc/mappingio/tree/MappingTree.java index a24de62d..df25471d 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingTree.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingTree.java @@ -25,11 +25,17 @@ import net.fabricmc.mappingio.adapter.MappingDstNsReorder; import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch; +import net.fabricmc.mappingio.tree.MappingCollection.ClassMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.FieldMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.MethodArgMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.MethodMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.MethodVarMappingCollection; /** * Mutable mapping tree. * *

All returned collections are to be assumed unmodifiable, unless explicitly stated otherwise. + * {@linkplain MappingCollection}s are an exception. */ public interface MappingTree extends MappingTreeView { /** @@ -85,7 +91,7 @@ public interface MappingTree extends MappingTreeView { boolean removeMetadata(String key); @Override - Collection getClasses(); + ClassMappingCollection getClasses(); @Override @Nullable ClassMapping getClass(String srcName); @@ -102,7 +108,7 @@ default ClassMapping getClass(String name, int namespace) { * @return The {@link ClassMapping} instance present in the tree after the merge has occurred. * May or may not be the passed instance. */ - ClassMapping addClass(ClassMapping cls); + ClassMapping addClass(ClassMappingView cls); /** * Removes a class mapping from the tree. @@ -229,7 +235,7 @@ interface ElementMapping extends ElementMappingView { interface ClassMapping extends ElementMapping, ClassMappingView { @Override - Collection getFields(); + FieldMappingCollection getFields(); @Override @Nullable FieldMapping getField(String srcName, @Nullable String srcDesc); @@ -246,7 +252,7 @@ default FieldMapping getField(String name, @Nullable String desc, int namespace) * @return The {@link FieldMapping} instance present in the parent {@link ClassMapping} after the merge has occurred. * May or may not be the passed instance. */ - FieldMapping addField(FieldMapping field); + FieldMapping addField(FieldMappingView field); /** * Removes a field mapping from the class. @@ -257,7 +263,7 @@ default FieldMapping getField(String name, @Nullable String desc, int namespace) FieldMapping removeField(String srcName, @Nullable String srcDesc); @Override - Collection getMethods(); + MethodMappingCollection getMethods(); @Override @Nullable MethodMapping getMethod(String srcName, @Nullable String srcDesc); @@ -274,7 +280,7 @@ default MethodMapping getMethod(String name, @Nullable String desc, int namespac * @return The {@link MethodMapping} instance present in the parent {@link ClassMapping} after the merge has occurred. * May or may not be the passed instance. */ - MethodMapping addMethod(MethodMapping method); + MethodMapping addMethod(MethodMappingView method); /** * Removes a method mapping from the class. @@ -295,11 +301,18 @@ interface FieldMapping extends MemberMapping, FieldMappingView { } interface MethodMapping extends MemberMapping, MethodMappingView { @Override - Collection getArgs(); + MethodArgMappingCollection getArgs(); @Override @Nullable MethodArgMapping getArg(int argPosition, int lvIndex, @Nullable String srcName); - MethodArgMapping addArg(MethodArgMapping arg); + + /** + * Merges an argument mapping into the method. + * + * @return The {@link MethodArgMapping} instance present in the parent {@link MethodMapping} after the merge has occurred. + * May or may not be the passed instance. + */ + MethodArgMapping addArg(MethodArgMappingView arg); /** * Removes an argument mapping from the method. @@ -310,7 +323,7 @@ interface MethodMapping extends MemberMapping, MethodMappingView { MethodArgMapping removeArg(int argPosition, int lvIndex, @Nullable String srcName); @Override - Collection getVars(); + MethodVarMappingCollection getVars(); @Override @Nullable MethodVarMapping getVar(int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName); @@ -321,7 +334,7 @@ interface MethodMapping extends MemberMapping, MethodMappingView { * @return The {@link MethodVarMapping} instance present in the parent {@link MethodMapping} after the merge has occurred. * May or may not be the passed instance. */ - MethodVarMapping addVar(MethodVarMapping var); + MethodVarMapping addVar(MethodVarMappingView var); /** * Removes a variable mapping from the method. diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingTreeView.java b/src/main/java/net/fabricmc/mappingio/tree/MappingTreeView.java index 3dacb7b1..961d5365 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingTreeView.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingTreeView.java @@ -17,12 +17,17 @@ package net.fabricmc.mappingio.tree; import java.io.IOException; -import java.util.Collection; import java.util.List; import org.jetbrains.annotations.Nullable; +import net.fabricmc.mappingio.MappedElementKind; import net.fabricmc.mappingio.MappingVisitor; +import net.fabricmc.mappingio.tree.MappingCollectionView.ClassMappingCollectionView; +import net.fabricmc.mappingio.tree.MappingCollectionView.FieldMappingCollectionView; +import net.fabricmc.mappingio.tree.MappingCollectionView.MethodArgMappingCollectionView; +import net.fabricmc.mappingio.tree.MappingCollectionView.MethodMappingCollectionView; +import net.fabricmc.mappingio.tree.MappingCollectionView.MethodVarMappingCollectionView; /** * Read-only mapping tree. @@ -85,7 +90,7 @@ default String getNamespaceName(int id) { */ List getMetadata(String key); - Collection getClasses(); + ClassMappingCollectionView getClasses(); @Nullable ClassMappingView getClass(String srcName); @Nullable @@ -217,6 +222,7 @@ interface MetadataEntryView { } interface ElementMappingView { + MappedElementKind getKind(); MappingTreeView getTree(); String getSrcName(); @@ -248,7 +254,12 @@ default String getName(String namespace) { } interface ClassMappingView extends ElementMappingView { - Collection getFields(); + @Override + default MappedElementKind getKind() { + return MappedElementKind.CLASS; + } + + FieldMappingCollectionView getFields(); /** * @see MappingTreeView#getField(String, String, String, int) @@ -274,7 +285,7 @@ default FieldMappingView getField(String name, @Nullable String desc, int namesp return null; } - Collection getMethods(); + MethodMappingCollectionView getMethods(); /** * @see MappingTreeView#getMethod(String, String, String, int) @@ -337,25 +348,45 @@ default String getDesc(String namespace) { } } - interface FieldMappingView extends MemberMappingView { } + interface FieldMappingView extends MemberMappingView { + @Override + default MappedElementKind getKind() { + return MappedElementKind.FIELD; + } + } interface MethodMappingView extends MemberMappingView { - Collection getArgs(); + @Override + default MappedElementKind getKind() { + return MappedElementKind.METHOD; + } + + MethodArgMappingCollectionView getArgs(); @Nullable MethodArgMappingView getArg(int argPosition, int lvIndex, @Nullable String srcName); - Collection getVars(); + MethodVarMappingCollectionView getVars(); @Nullable MethodVarMappingView getVar(int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName); } interface MethodArgMappingView extends ElementMappingView { + @Override + default MappedElementKind getKind() { + return MappedElementKind.METHOD_ARG; + } + MethodMappingView getMethod(); int getArgPosition(); int getLvIndex(); } interface MethodVarMappingView extends ElementMappingView { + @Override + default MappedElementKind getKind() { + return MappedElementKind.METHOD_VAR; + } + MethodMappingView getMethod(); int getLvtRowIndex(); int getLvIndex(); diff --git a/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java b/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java index 67f5c03a..5df1f997 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java @@ -40,6 +40,16 @@ import net.fabricmc.mappingio.MappingFlag; import net.fabricmc.mappingio.MappingVisitor; import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch; +import net.fabricmc.mappingio.tree.MappingCollection.ClassMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.FieldMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.MethodArgMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.MethodMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollection.MethodVarMappingCollection; +import net.fabricmc.mappingio.tree.MappingCollectionImpl.ClassMappingCollectionImpl; +import net.fabricmc.mappingio.tree.MappingCollectionImpl.FieldMappingCollectionImpl; +import net.fabricmc.mappingio.tree.MappingCollectionImpl.MethodArgMappingCollectionImpl; +import net.fabricmc.mappingio.tree.MappingCollectionImpl.MethodMappingCollectionImpl; +import net.fabricmc.mappingio.tree.MappingCollectionImpl.MethodVarMappingCollectionImpl; /** * {@link VisitableMappingTree} implementation that stores all data in memory. @@ -284,7 +294,7 @@ public boolean removeMetadata(String key) { } @Override - public Collection getClasses() { + public ClassMappingCollection getClasses() { return classesView; } @@ -305,7 +315,7 @@ public ClassMapping getClass(String name, int namespace) { } @Override - public ClassMapping addClass(ClassMapping cls) { + public ClassMapping addClass(ClassMappingView cls) { assertNotInVisitPass(); ClassEntry entry = cls instanceof ClassEntry && cls.getTree() == this ? (ClassEntry) cls : new ClassEntry(this, cls, getSrcNsEquivalent(cls)); ClassEntry ret = classesBySrcName.putIfAbsent(cls.getSrcName(), entry); @@ -325,7 +335,7 @@ public ClassMapping addClass(ClassMapping cls) { return entry; } - private int getSrcNsEquivalent(ElementMapping mapping) { + private int getSrcNsEquivalent(ElementMappingView mapping) { int ret = mapping.getTree().getNamespaceId(srcNamespace); if (ret == NULL_NAMESPACE_ID) throw new UnsupportedOperationException("can't find source namespace in referenced mapping tree"); @@ -848,7 +858,7 @@ protected Entry(MemoryMappingTree tree, String srcName) { this.dstNames = new String[tree.dstNamespaces.size()]; } - protected Entry(MemoryMappingTree tree, ElementMapping src, int srcNsEquivalent) { + protected Entry(MemoryMappingTree tree, ElementMappingView src, int srcNsEquivalent) { this(tree, src.getName(srcNsEquivalent)); for (int i = 0; i < dstNames.length; i++) { @@ -862,7 +872,10 @@ protected Entry(MemoryMappingTree tree, ElementMapping src, int srcNsEquivalent) setCommentInternal(src.getComment()); } - public abstract MappedElementKind getKind(); + @Override + public MemoryMappingTree getTree() { + return tree; + } final boolean isSrcNameMissing() { return srcName == null; @@ -995,28 +1008,18 @@ static final class ClassEntry extends Entry implements ClassMapping super(tree, srcName); } - ClassEntry(MemoryMappingTree tree, ClassMapping src, int srcNsEquivalent) { + ClassEntry(MemoryMappingTree tree, ClassMappingView src, int srcNsEquivalent) { super(tree, src, srcNsEquivalent); - for (FieldMapping field : src.getFields()) { + for (FieldMappingView field : src.getFields()) { addFieldInternal(field); } - for (MethodMapping method : src.getMethods()) { + for (MethodMappingView method : src.getMethods()) { addMethodInternal(method); } } - @Override - public MappedElementKind getKind() { - return MappedElementKind.CLASS; - } - - @Override - public MemoryMappingTree getTree() { - return tree; - } - @Override void setDstNameInternal(String name, int namespace) { if (tree.indexByDstNames) { @@ -1038,12 +1041,18 @@ void setDstNameInternal(String name, int namespace) { } @Override - public Collection getFields() { - if (fields == null) return Collections.emptyList(); - + public FieldMappingCollection getFields() { + initFieldCollectionsIfRequired(); return fieldsView; } + private void initFieldCollectionsIfRequired() { + if (fields == null) { + fields = new LinkedHashMap<>(); + fieldsView = new FieldMappingCollectionImpl<>(tree, this, fields.values()); + } + } + @Override @Nullable public FieldEntry getField(String srcName, @Nullable String srcDesc) { @@ -1057,18 +1066,15 @@ public FieldEntry getField(String name, @Nullable String desc, int namespace) { } @Override - public FieldEntry addField(FieldMapping field) { + public FieldEntry addField(FieldMappingView field) { tree.assertNotInVisitPass(); return addFieldInternal(field); } - FieldEntry addFieldInternal(FieldMapping field) { + FieldEntry addFieldInternal(FieldMappingView field) { FieldEntry entry = field instanceof FieldEntry && field.getOwner() == this ? (FieldEntry) field : new FieldEntry(this, field, tree.getSrcNsEquivalent(field)); - if (fields == null) { - fields = new LinkedHashMap<>(); - fieldsView = Collections.unmodifiableCollection(fields.values()); - } + initFieldCollectionsIfRequired(); return addMember(entry, fields, FLAG_HAS_ANY_FIELD_DESC, FLAG_MISSES_ANY_FIELD_DESC); } @@ -1085,12 +1091,18 @@ public FieldEntry removeField(String srcName, @Nullable String srcDesc) { } @Override - public Collection getMethods() { - if (methods == null) return Collections.emptyList(); - + public MethodMappingCollection getMethods() { + initMethodCollectionsIfRequired(); return methodsView; } + private void initMethodCollectionsIfRequired() { + if (methods == null) { + methods = new LinkedHashMap<>(); + methodsView = new MethodMappingCollectionImpl<>(tree, this, methods.values()); + } + } + @Override @Nullable public MethodEntry getMethod(String srcName, @Nullable String srcDesc) { @@ -1104,18 +1116,15 @@ public MethodEntry getMethod(String name, @Nullable String desc, int namespace) } @Override - public MethodEntry addMethod(MethodMapping method) { + public MethodEntry addMethod(MethodMappingView method) { tree.assertNotInVisitPass(); return addMethodInternal(method); } - MethodEntry addMethodInternal(MethodMapping method) { + MethodEntry addMethodInternal(MethodMappingView method) { MethodEntry entry = method instanceof MethodEntry && method.getOwner() == this ? (MethodEntry) method : new MethodEntry(this, method, tree.getSrcNsEquivalent(method)); - if (methods == null) { - methods = new LinkedHashMap<>(); - methodsView = Collections.unmodifiableCollection(methods.values()); - } + initMethodCollectionsIfRequired(); return addMember(entry, methods, FLAG_HAS_ANY_METHOD_DESC, FLAG_MISSES_ANY_METHOD_DESC); } @@ -1314,8 +1323,8 @@ public String toString() { private Map fields = null; private Map methods = null; - private Collection fieldsView = null; - private Collection methodsView = null; + private FieldMappingCollection fieldsView = null; + private MethodMappingCollection methodsView = null; private byte flags; } @@ -1328,7 +1337,7 @@ protected MemberEntry(ClassEntry owner, String srcName, @Nullable String srcDesc this.key = new MemberKey(srcName, srcDesc); } - protected MemberEntry(ClassEntry owner, MemberMapping src, int srcNsEquivalent) { + protected MemberEntry(ClassEntry owner, MemberMappingView src, int srcNsEquivalent) { super(owner.tree, src, srcNsEquivalent); this.owner = owner; @@ -1336,11 +1345,6 @@ protected MemberEntry(ClassEntry owner, MemberMapping src, int srcNsEquivalent) this.key = new MemberKey(getSrcName(), srcDesc); } - @Override - public MappingTree getTree() { - return owner.tree; - } - @Override public final ClassEntry getOwner() { return owner; @@ -1403,15 +1407,10 @@ static final class FieldEntry extends MemberEntry implements FieldMa super(owner, srcName, srcDesc); } - FieldEntry(ClassEntry owner, FieldMapping src, int srcNsEquivalent) { + FieldEntry(ClassEntry owner, FieldMappingView src, int srcNsEquivalent) { super(owner, src, srcNsEquivalent); } - @Override - public MappedElementKind getKind() { - return MappedElementKind.FIELD; - } - @Override public void setSrcDesc(@Nullable String desc) { tree.assertNotInVisitPass(); @@ -1460,23 +1459,18 @@ static final class MethodEntry extends MemberEntry implements Metho super(owner, srcName, srcDesc); } - MethodEntry(ClassEntry owner, MethodMapping src, int srcNsEquivalent) { + MethodEntry(ClassEntry owner, MethodMappingView src, int srcNsEquivalent) { super(owner, src, srcNsEquivalent); - for (MethodArgMapping arg : src.getArgs()) { + for (MethodArgMappingView arg : src.getArgs()) { addArgInternal(arg); } - for (MethodVarMapping var : src.getVars()) { + for (MethodVarMappingView var : src.getVars()) { addVarInternal(var); } } - @Override - public MappedElementKind getKind() { - return MappedElementKind.METHOD; - } - @Override public void setSrcDesc(@Nullable String desc) { tree.assertNotInVisitPass(); @@ -1509,12 +1503,18 @@ void setSrcDescInternal(@Nullable String desc) { } @Override - public Collection getArgs() { - if (args == null) return Collections.emptyList(); - + public MethodArgMappingCollection getArgs() { + initArgCollectionsIfRequired(); return argsView; } + private void initArgCollectionsIfRequired() { + if (args == null) { + args = new ArrayList<>(); + argsView = new MethodArgMappingCollectionImpl<>(tree, this, args); + } + } + @Override @Nullable public MethodArgEntry getArg(int argPosition, int lvIndex, @Nullable String srcName) { @@ -1544,21 +1544,17 @@ public MethodArgEntry getArg(int argPosition, int lvIndex, @Nullable String srcN } @Override - public MethodArgEntry addArg(MethodArgMapping arg) { + public MethodArgEntry addArg(MethodArgMappingView arg) { tree.assertNotInVisitPass(); return addArgInternal(arg); } - MethodArgEntry addArgInternal(MethodArgMapping arg) { + MethodArgEntry addArgInternal(MethodArgMappingView arg) { MethodArgEntry entry = arg instanceof MethodArgEntry && arg.getMethod() == this ? (MethodArgEntry) arg : new MethodArgEntry(this, arg, owner.tree.getSrcNsEquivalent(arg)); MethodArgEntry prev = getArg(arg.getArgPosition(), arg.getLvIndex(), arg.getSrcName()); if (prev == null) { - if (args == null) { - args = new ArrayList<>(); - argsView = Collections.unmodifiableList(args); - } - + initArgCollectionsIfRequired(); args.add(entry); } else { prev.copyFrom(entry, true); @@ -1579,12 +1575,18 @@ public MethodArgEntry removeArg(int argPosition, int lvIndex, @Nullable String s } @Override - public Collection getVars() { - if (vars == null) return Collections.emptyList(); - + public MethodVarMappingCollection getVars() { + initVarCollectionsIfRequired(); return varsView; } + private void initVarCollectionsIfRequired() { + if (vars == null) { + vars = new ArrayList<>(); + varsView = new MethodVarMappingCollectionImpl<>(tree, this, vars); + } + } + @Override @Nullable public MethodVarEntry getVar(int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName) { @@ -1661,21 +1663,17 @@ public MethodVarEntry getVar(int lvtRowIndex, int lvIndex, int startOpIdx, int e } @Override - public MethodVarEntry addVar(MethodVarMapping var) { + public MethodVarEntry addVar(MethodVarMappingView var) { tree.assertNotInVisitPass(); return addVarInternal(var); } - MethodVarEntry addVarInternal(MethodVarMapping var) { + MethodVarEntry addVarInternal(MethodVarMappingView var) { MethodVarEntry entry = var instanceof MethodVarEntry && var.getMethod() == this ? (MethodVarEntry) var : new MethodVarEntry(this, var, owner.tree.getSrcNsEquivalent(var)); MethodVarEntry prev = getVar(var.getLvtRowIndex(), var.getLvIndex(), var.getStartOpIdx(), var.getEndOpIdx(), var.getSrcName()); if (prev == null) { - if (vars == null) { - vars = new ArrayList<>(); - varsView = Collections.unmodifiableList(vars); - } - + initVarCollectionsIfRequired(); vars.add(entry); } else { prev.copyFrom(entry, true); @@ -1755,8 +1753,8 @@ public String toString() { private List args = null; private List vars = null; - private List argsView = null; - private List varsView = null; + private MethodArgMappingCollection argsView = null; + private MethodVarMappingCollection varsView = null; } static final class MethodArgEntry extends Entry implements MethodArgMapping { @@ -1768,7 +1766,7 @@ static final class MethodArgEntry extends Entry implements Metho this.lvIndex = lvIndex; } - MethodArgEntry(MethodEntry method, MethodArgMapping src, int srcNsEquivalent) { + MethodArgEntry(MethodEntry method, MethodArgMappingView src, int srcNsEquivalent) { super(method.owner.tree, src, srcNsEquivalent); this.method = method; @@ -1776,16 +1774,6 @@ static final class MethodArgEntry extends Entry implements Metho this.lvIndex = src.getLvIndex(); } - @Override - public MappingTree getTree() { - return method.owner.tree; - } - - @Override - public MappedElementKind getKind() { - return MappedElementKind.METHOD_ARG; - } - @Override public MethodEntry getMethod() { return method; @@ -1865,7 +1853,7 @@ static final class MethodVarEntry extends Entry implements Metho this.endOpIdx = endOpIdx; } - MethodVarEntry(MethodEntry method, MethodVarMapping src, int srcNs) { + MethodVarEntry(MethodEntry method, MethodVarMappingView src, int srcNs) { super(method.owner.tree, src, srcNs); this.method = method; @@ -1875,16 +1863,6 @@ static final class MethodVarEntry extends Entry implements Metho this.endOpIdx = src.getEndOpIdx(); } - @Override - public MappingTree getTree() { - return method.owner.tree; - } - - @Override - public MappedElementKind getKind() { - return MappedElementKind.METHOD_VAR; - } - @Override public MethodEntry getMethod() { return method; @@ -2104,7 +2082,7 @@ public String toString() { private List dstNamespaces = Collections.emptyList(); private final List metadata = new ArrayList<>(); private final Map classesBySrcName = new LinkedHashMap<>(); - private final Collection classesView = Collections.unmodifiableCollection(classesBySrcName.values()); + private final ClassMappingCollection classesView = new ClassMappingCollectionImpl<>(this, classesBySrcName.values()); private Map[] classesByDstNames; private HierarchyInfoProvider hierarchyInfo; diff --git a/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java b/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java new file mode 100644 index 00000000..3424b675 --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java @@ -0,0 +1,215 @@ +/* + * Copyright (c) 2025 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.mappingio.test.tests.tree; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collections; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import net.fabricmc.mappingio.MappedElementKind; +import net.fabricmc.mappingio.tree.MappingTree.ClassMapping; +import net.fabricmc.mappingio.tree.MappingTree.FieldMapping; +import net.fabricmc.mappingio.tree.MappingTreeView.ClassMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.FieldMappingView; +import net.fabricmc.mappingio.tree.MemoryMappingTree; +import net.fabricmc.mappingio.tree.VisitableMappingTree; + +public class MappingCollectionTest { + private static final String nsA = "nsA"; + private static final String nsB = "nsB"; + private static final String cls1NsAName = "cls1NsAName"; + private static final String cls1NsBName = "cls1NsBName"; + private static final String cls2NsAName = "cls2NsAName"; + private static final String fld1NsAName = "fld1NsAName"; + private static final String fld1NsBName = "fld1NsBName"; + private static final String fld2NsAName = "fld2NsAName"; + private static final String fld1NsADesc = "L" + cls1NsAName + ";"; + private static final String fld2NsADesc = "L" + cls2NsAName + ";"; + private static final String fld1Comment = "fld1Comment"; + private MemoryMappingTree tree1, tree2, emptyTree; + + @BeforeEach + public void setup() throws Exception { + tree1 = new MemoryMappingTree(); + tree2 = new MemoryMappingTree(); + emptyTree = new MemoryMappingTree(); + + for (int i = 1; i <= 3; i++) { + VisitableMappingTree tree = i == 1 ? tree1 : i == 2 ? tree2 : emptyTree; + + if (tree.visitHeader()) { + tree.visitNamespaces(nsA, Collections.singletonList(nsB)); + } + + if (tree.visitContent()) { + if (tree != emptyTree && tree.visitClass(cls1NsAName)) { + tree.visitDstName(MappedElementKind.CLASS, 0, cls1NsBName); + + if (tree.visitElementContent(MappedElementKind.CLASS)) { + if (tree.visitField(fld1NsAName, fld1NsADesc)) { + if (tree == tree2) { + tree.visitDstName(MappedElementKind.FIELD, 0, fld1NsBName); + } + + if (tree.visitElementContent(MappedElementKind.FIELD) && tree == tree2) { + tree.visitComment(MappedElementKind.FIELD, fld1Comment); + } + } + + if (tree == tree2 && tree.visitField(fld2NsAName, fld2NsADesc)) { + tree.visitElementContent(MappedElementKind.FIELD); + } + } + } + } + + tree.visitEnd(); + } + } + + @Test + public void emptyCollections() { + assertTrue(emptyTree.getClasses().isEmpty()); + + tree2Cls1().getFields().clear(); + tree2Cls1().getMethods().clear(); + assertTrue(emptyTree.getClasses().add(tree2Cls1())); + + ClassMapping cls = emptyTree.getClass(cls1NsAName); + assertTrue(cls.getFields().isEmpty()); + assertTrue(cls.getMethods().isEmpty()); + } + + @Test + public void test() { + assertTrue(tree1.getClasses().contains(tree1Cls1())); + assertTrue(tree2.getClasses().contains(tree2Cls1())); + assertFalse(tree1.getClasses().contains(tree2Cls1())); + assertTrue(tree1.getClasses().containsCompatible(tree2Cls1())); + assertFalse(tree2.getClasses().contains(tree1Cls1())); + assertTrue(tree2.getClasses().containsCompatible(tree1Cls1())); + assertTrue(tree1Cls1().getFields().contains(tree1Fld1())); + assertFalse(tree1Cls1().getFields().contains(tree2Fld2())); + assertFalse(tree2Cls1().getFields().contains(tree1Fld1())); + assertTrue(tree2Cls1().getFields().containsCompatible(tree1Fld1())); + assertTrue(tree2Cls1().getFields().contains(tree2Fld2())); + assertTrue(tree2Cls1().getFields().containsCompatible(tree2Fld2())); + + assertEquals(1, tree1.getClasses().size()); + assertEquals(1, tree2.getClasses().size()); + assertEquals(1, tree1Cls1().getFields().size()); + assertEquals(2, tree2Cls1().getFields().size()); + assertEquals(0, tree1Cls1().getMethods().size()); + assertEquals(0, tree2Cls1().getMethods().size()); + + assertNull(tree1Fld1().getComment()); + ((ClassMapping) tree1Cls1()).getFields().add(tree2Fld1()); + assertEquals(fld1Comment, tree1Fld1().getComment()); + + assertTrue(tree2Cls1().getFields().contains(tree2Fld1())); + tree2Cls1().getFields().remove(tree2Fld1()); + assertFalse(tree2Cls1().getFields().contains(tree2Fld1())); + assertEquals(1, tree2Cls1().getFields().size()); + + for (int i = 0; i <= 4; i++) { + switch (i) { + case 0: + ((ClassMapping) tree1Cls1()).getFields().add(tree2Fld2()); + break; + case 1: + ((ClassMapping) tree1Cls1()).getFields().addAllViews(tree2Cls1().getFields()); + break; + case 2: + ((ClassMapping) tree1Cls1()).getFields().addAllViews(Collections.singleton((FieldMapping) tree2Fld2())); + break; + case 3: + case 4: + ((ClassMapping) tree1Cls1()).getFields().addAllViews(Collections.singleton(tree2Fld2())); + break; + } + + assertFalse(tree1Cls1().getFields().contains(tree2Fld2())); + assertTrue(tree1Cls1().getFields().containsCompatible(tree2Fld2())); + assertEquals(2, tree1Cls1().getFields().size()); + + switch (i) { + case 0: + assertFalse(tree1Cls1().getFields().remove(tree2Fld2())); + assertTrue(tree1Cls1().getFields().removeCompatible(tree2Fld2())); + break; + case 1: + assertFalse(tree1Cls1().getFields().removeAll(tree2Cls1().getFields())); + assertTrue(tree1Cls1().getFields().removeAllCompatible(tree2Cls1().getFields())); + break; + case 2: + assertFalse(tree1Cls1().getFields().removeAll(Collections.singleton(tree2Fld2()))); + assertTrue(tree1Cls1().getFields().removeAllCompatible(Collections.singleton(tree2Fld2()))); + break; + case 3: + assertTrue(tree1Cls1().getFields().retainAll(Collections.singleton(tree1Fld1()))); + assertFalse(tree1Cls1().getFields().retainAll(Collections.singleton(tree1Fld1()))); + assertFalse(tree1Cls1().getFields().retainAllCompatible(Collections.singleton(tree1Fld1()))); + break; + case 4: + assertTrue(tree1Cls1().getFields().retainAllCompatible(Collections.singleton(tree1Fld1()))); + assertFalse(tree1Cls1().getFields().retainAllCompatible(Collections.singleton(tree1Fld1()))); + assertFalse(tree1Cls1().getFields().retainAll(Collections.singleton(tree1Fld1()))); + break; + } + + assertFalse(tree1Cls1().getFields().contains(tree2Fld2())); + assertFalse(tree1Cls1().getFields().containsCompatible(tree2Fld2())); + assertEquals(1, tree1Cls1().getFields().size()); + } + + tree1Cls1().getFields().clear(); + assertEquals(0, tree1Cls1().getFields().size()); + + tree1.getClasses().clear(); + assertEquals(0, tree1.getClasses().size()); + } + + private ClassMappingView tree1Cls1() { + return tree1.getClass(cls1NsAName); + } + + private ClassMappingView tree2Cls1() { + return tree2.getClass(cls1NsAName); + } + + private ClassMappingView tree1Cls2() { + return tree1.getClass(cls2NsAName); + } + + private FieldMappingView tree1Fld1() { + return tree1Cls1().getField(fld1NsAName, fld1NsADesc); + } + + private FieldMappingView tree2Fld1() { + return tree2Cls1().getField(fld1NsAName, fld1NsADesc); + } + + private FieldMappingView tree2Fld2() { + return tree2Cls1().getField(fld2NsAName, fld2NsADesc); + } +} From ab0ee4159934f6a24992172c2750484464c05985 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 12 Apr 2025 15:05:16 +0200 Subject: [PATCH 2/7] Add tests for methods, args and vars --- .../tests/tree/MappingCollectionTest.java | 291 ++++++++++++------ 1 file changed, 202 insertions(+), 89 deletions(-) diff --git a/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java b/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java index 3424b675..d56d9a58 100644 --- a/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java +++ b/src/test/java/net/fabricmc/mappingio/test/tests/tree/MappingCollectionTest.java @@ -27,9 +27,14 @@ import org.junit.jupiter.api.Test; import net.fabricmc.mappingio.MappedElementKind; +import net.fabricmc.mappingio.MappingVisitor; +import net.fabricmc.mappingio.test.visitors.VisitOrderVerifyingVisitor; +import net.fabricmc.mappingio.tree.MappingCollectionView.FieldMappingCollectionView; import net.fabricmc.mappingio.tree.MappingTree.ClassMapping; import net.fabricmc.mappingio.tree.MappingTree.FieldMapping; -import net.fabricmc.mappingio.tree.MappingTreeView.ClassMappingView; +import net.fabricmc.mappingio.tree.MappingTree.MethodArgMapping; +import net.fabricmc.mappingio.tree.MappingTree.MethodMapping; +import net.fabricmc.mappingio.tree.MappingTree.MethodVarMapping; import net.fabricmc.mappingio.tree.MappingTreeView.FieldMappingView; import net.fabricmc.mappingio.tree.MemoryMappingTree; import net.fabricmc.mappingio.tree.VisitableMappingTree; @@ -39,13 +44,33 @@ public class MappingCollectionTest { private static final String nsB = "nsB"; private static final String cls1NsAName = "cls1NsAName"; private static final String cls1NsBName = "cls1NsBName"; - private static final String cls2NsAName = "cls2NsAName"; private static final String fld1NsAName = "fld1NsAName"; private static final String fld1NsBName = "fld1NsBName"; private static final String fld2NsAName = "fld2NsAName"; - private static final String fld1NsADesc = "L" + cls1NsAName + ";"; - private static final String fld2NsADesc = "L" + cls2NsAName + ";"; + private static final String fld1NsADesc = "I"; + private static final String fld2NsADesc = "L" + cls1NsAName + ";"; private static final String fld1Comment = "fld1Comment"; + private static final String mth1NsAName = "mth1NsAName"; + private static final String mth1NsBName = "mth1NsBName"; + private static final String mth2NsAName = "mth2NsAName"; + private static final String mth1NsADesc = "()I"; + private static final String mth2NsADesc = "()L" + cls1NsAName + ";"; + private static final String arg1NsAName = "arg1NsAName"; + private static final String arg2NsAName = "arg2NsAName"; + private static final int arg1ArgPos = 0; + private static final int arg1LvIdx = 0; + private static final int arg2ArgPos = 1; + private static final int arg2LvIdx = 1; + private static final String var1NsAName = "var1NsAName"; + private static final String var2NsAName = "var2NsAName"; + private static final int var1LvtIdx = 0; + private static final int var1LvIdx = 0; + private static final int var1StartOpIdx = 0; + private static final int var1EndOpIdx = 1; + private static final int var2LvtIdx = 1; + private static final int var2LvIdx = 1; + private static final int var2StartOpIdx = 2; + private static final int var2EndOpIdx = 3; private MemoryMappingTree tree1, tree2, emptyTree; @BeforeEach @@ -56,160 +81,248 @@ public void setup() throws Exception { for (int i = 1; i <= 3; i++) { VisitableMappingTree tree = i == 1 ? tree1 : i == 2 ? tree2 : emptyTree; + MappingVisitor delegate = new VisitOrderVerifyingVisitor(tree); - if (tree.visitHeader()) { - tree.visitNamespaces(nsA, Collections.singletonList(nsB)); + if (delegate.visitHeader()) { + delegate.visitNamespaces(nsA, Collections.singletonList(nsB)); } - if (tree.visitContent()) { - if (tree != emptyTree && tree.visitClass(cls1NsAName)) { - tree.visitDstName(MappedElementKind.CLASS, 0, cls1NsBName); + if (delegate.visitContent()) { + if (tree != emptyTree && delegate.visitClass(cls1NsAName)) { + delegate.visitDstName(MappedElementKind.CLASS, 0, cls1NsBName); - if (tree.visitElementContent(MappedElementKind.CLASS)) { - if (tree.visitField(fld1NsAName, fld1NsADesc)) { + if (delegate.visitElementContent(MappedElementKind.CLASS)) { + if (delegate.visitField(fld1NsAName, fld1NsADesc)) { if (tree == tree2) { - tree.visitDstName(MappedElementKind.FIELD, 0, fld1NsBName); + delegate.visitDstName(MappedElementKind.FIELD, 0, fld1NsBName); } - if (tree.visitElementContent(MappedElementKind.FIELD) && tree == tree2) { - tree.visitComment(MappedElementKind.FIELD, fld1Comment); + if (delegate.visitElementContent(MappedElementKind.FIELD) && tree == tree2) { + delegate.visitComment(MappedElementKind.FIELD, fld1Comment); } } - if (tree == tree2 && tree.visitField(fld2NsAName, fld2NsADesc)) { - tree.visitElementContent(MappedElementKind.FIELD); + if (tree == tree2 && delegate.visitField(fld2NsAName, fld2NsADesc)) { + delegate.visitElementContent(MappedElementKind.FIELD); + } + + if (delegate.visitMethod(mth1NsAName, mth1NsADesc)) { + if (tree == tree2) { + delegate.visitDstName(MappedElementKind.METHOD, 0, mth1NsBName); + } + + if (delegate.visitElementContent(MappedElementKind.METHOD)) { + if (delegate.visitMethodArg(arg1ArgPos, arg1LvIdx, arg1NsAName)) { + delegate.visitElementContent(MappedElementKind.METHOD_ARG); + } + + if (tree == tree2 && delegate.visitMethodArg(arg2ArgPos, arg2LvIdx, arg2NsAName)) { + delegate.visitElementContent(MappedElementKind.METHOD_ARG); + } + + if (delegate.visitMethodVar(var1LvtIdx, var1LvIdx, var1StartOpIdx, var1EndOpIdx, var1NsAName)) { + delegate.visitElementContent(MappedElementKind.METHOD_VAR); + } + + if (tree == tree2 && delegate.visitMethodVar(var2LvtIdx, var2LvIdx, var2StartOpIdx, var2EndOpIdx, var2NsAName)) { + delegate.visitElementContent(MappedElementKind.METHOD_VAR); + } + } + } + + if (tree == tree2 && delegate.visitMethod(mth2NsAName, mth2NsADesc)) { + delegate.visitElementContent(MappedElementKind.METHOD); } } } } - tree.visitEnd(); + delegate.visitEnd(); } } @Test - public void emptyCollections() { + public void isEmptyClear() { assertTrue(emptyTree.getClasses().isEmpty()); - - tree2Cls1().getFields().clear(); - tree2Cls1().getMethods().clear(); - assertTrue(emptyTree.getClasses().add(tree2Cls1())); - - ClassMapping cls = emptyTree.getClass(cls1NsAName); - assertTrue(cls.getFields().isEmpty()); - assertTrue(cls.getMethods().isEmpty()); + emptyTree.getClasses().clear(); + + cls1(tree1).getFields().clear(); + cls1(tree1).getMethods().clear(); + assertTrue(emptyTree.getClasses().add(cls1(tree1))); + assertTrue(cls1(emptyTree).getFields().isEmpty()); + assertTrue(cls1(emptyTree).getMethods().isEmpty()); + + mth1(cls1(tree2)).getArgs().clear(); + mth1(cls1(tree2)).getVars().clear(); + assertTrue(cls1(emptyTree).getMethods().add(mth1(cls1(tree2)))); + assertTrue(mth1(cls1(emptyTree)).getArgs().isEmpty()); + assertTrue(mth1(cls1(emptyTree)).getVars().isEmpty()); } @Test - public void test() { - assertTrue(tree1.getClasses().contains(tree1Cls1())); - assertTrue(tree2.getClasses().contains(tree2Cls1())); - assertFalse(tree1.getClasses().contains(tree2Cls1())); - assertTrue(tree1.getClasses().containsCompatible(tree2Cls1())); - assertFalse(tree2.getClasses().contains(tree1Cls1())); - assertTrue(tree2.getClasses().containsCompatible(tree1Cls1())); - assertTrue(tree1Cls1().getFields().contains(tree1Fld1())); - assertFalse(tree1Cls1().getFields().contains(tree2Fld2())); - assertFalse(tree2Cls1().getFields().contains(tree1Fld1())); - assertTrue(tree2Cls1().getFields().containsCompatible(tree1Fld1())); - assertTrue(tree2Cls1().getFields().contains(tree2Fld2())); - assertTrue(tree2Cls1().getFields().containsCompatible(tree2Fld2())); - + public void size() { + // Classes assertEquals(1, tree1.getClasses().size()); assertEquals(1, tree2.getClasses().size()); - assertEquals(1, tree1Cls1().getFields().size()); - assertEquals(2, tree2Cls1().getFields().size()); - assertEquals(0, tree1Cls1().getMethods().size()); - assertEquals(0, tree2Cls1().getMethods().size()); - assertNull(tree1Fld1().getComment()); - ((ClassMapping) tree1Cls1()).getFields().add(tree2Fld1()); - assertEquals(fld1Comment, tree1Fld1().getComment()); + // Members + assertEquals(1, cls1(tree1).getFields().size()); + assertEquals(2, cls1(tree2).getFields().size()); + assertEquals(1, cls1(tree1).getMethods().size()); + assertEquals(2, cls1(tree2).getMethods().size()); + + // Locals + assertEquals(1, mth1(cls1(tree1)).getArgs().size()); + assertEquals(2, mth1(cls1(tree2)).getArgs().size()); + assertEquals(1, mth1(cls1(tree1)).getVars().size()); + assertEquals(2, mth1(cls1(tree2)).getVars().size()); + assertEquals(0, mth2(cls1(tree2)).getArgs().size()); + assertEquals(0, mth2(cls1(tree2)).getVars().size()); + } - assertTrue(tree2Cls1().getFields().contains(tree2Fld1())); - tree2Cls1().getFields().remove(tree2Fld1()); - assertFalse(tree2Cls1().getFields().contains(tree2Fld1())); - assertEquals(1, tree2Cls1().getFields().size()); + @Test + public void contains() { + // Classes + assertTrue(tree1.getClasses().contains(cls1(tree1))); + assertTrue(tree2.getClasses().contains(cls1(tree2))); + assertFalse(tree1.getClasses().contains(cls1(tree2))); + assertTrue(tree1.getClasses().containsCompatible(cls1(tree2))); + assertFalse(tree2.getClasses().contains(cls1(tree1))); + assertTrue(tree2.getClasses().containsCompatible(cls1(tree1))); + + // Fields + assertTrue(cls1(tree1).getFields().contains(fld1(cls1(tree1)))); + assertFalse(cls1(tree1).getFields().contains(fld2(cls1(tree2)))); + assertFalse(cls1(tree2).getFields().contains(fld1(cls1(tree1)))); + assertTrue(cls1(tree2).getFields().containsCompatible(fld1(cls1(tree1)))); + assertTrue(cls1(tree2).getFields().contains(fld2(cls1(tree2)))); + assertTrue(cls1(tree2).getFields().containsCompatible(fld2(cls1(tree2)))); + + // Methods + assertTrue(cls1(tree1).getMethods().contains(mth1(cls1(tree1)))); + assertFalse(cls1(tree1).getMethods().contains(mth2(cls1(tree2)))); + assertFalse(cls1(tree2).getMethods().contains(mth1(cls1(tree1)))); + assertTrue(cls1(tree2).getMethods().containsCompatible(mth1(cls1(tree1)))); + assertTrue(cls1(tree2).getMethods().contains(mth2(cls1(tree2)))); + assertTrue(cls1(tree2).getMethods().containsCompatible(mth2(cls1(tree2)))); + + // Args + assertTrue(mth1(cls1(tree1)).getArgs().contains(arg1(mth1(cls1(tree1))))); + assertFalse(mth1(cls1(tree1)).getArgs().contains(arg2(mth1(cls1(tree2))))); + assertFalse(mth1(cls1(tree2)).getArgs().contains(arg1(mth1(cls1(tree1))))); + assertTrue(mth1(cls1(tree2)).getArgs().containsCompatible(arg1(mth1(cls1(tree1))))); + assertTrue(mth1(cls1(tree2)).getArgs().contains(arg2(mth1(cls1(tree2))))); + assertTrue(mth1(cls1(tree2)).getArgs().containsCompatible(arg2(mth1(cls1(tree2))))); + + // Vars + assertTrue(mth1(cls1(tree1)).getVars().contains(var1(mth1(cls1(tree1))))); + assertFalse(mth1(cls1(tree1)).getVars().contains(var2(mth1(cls1(tree2))))); + assertFalse(mth1(cls1(tree2)).getVars().contains(var1(mth1(cls1(tree1))))); + assertTrue(mth1(cls1(tree2)).getVars().containsCompatible(var1(mth1(cls1(tree1))))); + assertTrue(mth1(cls1(tree2)).getVars().contains(var2(mth1(cls1(tree2))))); + assertTrue(mth1(cls1(tree2)).getVars().containsCompatible(var2(mth1(cls1(tree2))))); + } + + @Test + public void merge() { + assertNull(fld1(cls1(tree1)).getComment()); + cls1(tree1).getFields().add(fld1(cls1(tree2))); + assertEquals(fld1Comment, fld1(cls1(tree1)).getComment()); + } + + @Test + public void addRemoveRetain() { + cls1(tree2).getFields().remove(fld1(cls1(tree2))); + assertFalse(cls1(tree2).getFields().contains(fld1(cls1(tree2)))); + assertEquals(1, cls1(tree2).getFields().size()); for (int i = 0; i <= 4; i++) { switch (i) { case 0: - ((ClassMapping) tree1Cls1()).getFields().add(tree2Fld2()); + cls1(tree1).getFields().add(fld2(cls1(tree2))); break; case 1: - ((ClassMapping) tree1Cls1()).getFields().addAllViews(tree2Cls1().getFields()); + cls1(tree1).getFields().addAllViews(cls1(tree2).getFields()); break; case 2: - ((ClassMapping) tree1Cls1()).getFields().addAllViews(Collections.singleton((FieldMapping) tree2Fld2())); + cls1(tree1).getFields().addAllViews((FieldMappingCollectionView) cls1(tree2).getFields()); break; case 3: + cls1(tree1).getFields().addAllViews(Collections.singleton((FieldMapping) fld2(cls1(tree2)))); + break; case 4: - ((ClassMapping) tree1Cls1()).getFields().addAllViews(Collections.singleton(tree2Fld2())); + cls1(tree1).getFields().addAllViews(Collections.singleton(fld2(cls1(tree2)))); break; } - assertFalse(tree1Cls1().getFields().contains(tree2Fld2())); - assertTrue(tree1Cls1().getFields().containsCompatible(tree2Fld2())); - assertEquals(2, tree1Cls1().getFields().size()); + assertFalse(cls1(tree1).getFields().contains(fld2(cls1(tree2)))); + assertTrue(cls1(tree1).getFields().containsCompatible(fld2(cls1(tree2)))); + assertEquals(2, cls1(tree1).getFields().size()); switch (i) { case 0: - assertFalse(tree1Cls1().getFields().remove(tree2Fld2())); - assertTrue(tree1Cls1().getFields().removeCompatible(tree2Fld2())); + assertFalse(cls1(tree1).getFields().remove(fld2(cls1(tree2)))); + assertTrue(cls1(tree1).getFields().removeCompatible(fld2(cls1(tree2)))); break; case 1: - assertFalse(tree1Cls1().getFields().removeAll(tree2Cls1().getFields())); - assertTrue(tree1Cls1().getFields().removeAllCompatible(tree2Cls1().getFields())); + assertFalse(cls1(tree1).getFields().removeAll(cls1(tree2).getFields())); + assertTrue(cls1(tree1).getFields().removeAllCompatible(cls1(tree2).getFields())); break; case 2: - assertFalse(tree1Cls1().getFields().removeAll(Collections.singleton(tree2Fld2()))); - assertTrue(tree1Cls1().getFields().removeAllCompatible(Collections.singleton(tree2Fld2()))); + assertFalse(cls1(tree1).getFields().removeAll(Collections.singleton(fld2(cls1(tree2))))); + assertTrue(cls1(tree1).getFields().removeAllCompatible(Collections.singleton(fld2(cls1(tree2))))); break; case 3: - assertTrue(tree1Cls1().getFields().retainAll(Collections.singleton(tree1Fld1()))); - assertFalse(tree1Cls1().getFields().retainAll(Collections.singleton(tree1Fld1()))); - assertFalse(tree1Cls1().getFields().retainAllCompatible(Collections.singleton(tree1Fld1()))); + assertTrue(cls1(tree1).getFields().retainAll(Collections.singleton(fld1(cls1(tree1))))); + assertFalse(cls1(tree1).getFields().retainAll(Collections.singleton(fld1(cls1(tree1))))); + assertFalse(cls1(tree1).getFields().retainAllCompatible(Collections.singleton(fld1(cls1(tree1))))); break; case 4: - assertTrue(tree1Cls1().getFields().retainAllCompatible(Collections.singleton(tree1Fld1()))); - assertFalse(tree1Cls1().getFields().retainAllCompatible(Collections.singleton(tree1Fld1()))); - assertFalse(tree1Cls1().getFields().retainAll(Collections.singleton(tree1Fld1()))); + assertTrue(cls1(tree1).getFields().retainAllCompatible(Collections.singleton(fld1(cls1(tree1))))); + assertFalse(cls1(tree1).getFields().retainAllCompatible(Collections.singleton(fld1(cls1(tree1))))); + assertFalse(cls1(tree1).getFields().retainAll(Collections.singleton(fld1(cls1(tree1))))); break; } - assertFalse(tree1Cls1().getFields().contains(tree2Fld2())); - assertFalse(tree1Cls1().getFields().containsCompatible(tree2Fld2())); - assertEquals(1, tree1Cls1().getFields().size()); + assertFalse(cls1(tree1).getFields().contains(fld2(cls1(tree2)))); + assertFalse(cls1(tree1).getFields().containsCompatible(fld2(cls1(tree2)))); + assertEquals(1, cls1(tree1).getFields().size()); } + } - tree1Cls1().getFields().clear(); - assertEquals(0, tree1Cls1().getFields().size()); + private ClassMapping cls1(MemoryMappingTree tree) { + return tree.getClass(cls1NsAName); + } + + private FieldMapping fld1(ClassMapping cls) { + return cls.getField(fld1NsAName, fld1NsADesc); + } - tree1.getClasses().clear(); - assertEquals(0, tree1.getClasses().size()); + private FieldMapping fld2(ClassMapping cls) { + return cls.getField(fld2NsAName, fld2NsADesc); } - private ClassMappingView tree1Cls1() { - return tree1.getClass(cls1NsAName); + private MethodMapping mth1(ClassMapping cls) { + return cls.getMethod(mth1NsAName, mth1NsADesc); } - private ClassMappingView tree2Cls1() { - return tree2.getClass(cls1NsAName); + private MethodMapping mth2(ClassMapping cls) { + return cls.getMethod(mth2NsAName, mth2NsADesc); } - private ClassMappingView tree1Cls2() { - return tree1.getClass(cls2NsAName); + private MethodArgMapping arg1(MethodMapping mth) { + return mth.getArg(arg1ArgPos, arg1LvIdx, arg1NsAName); } - private FieldMappingView tree1Fld1() { - return tree1Cls1().getField(fld1NsAName, fld1NsADesc); + private MethodArgMapping arg2(MethodMapping mth) { + return mth.getArg(arg2ArgPos, arg2LvIdx, arg2NsAName); } - private FieldMappingView tree2Fld1() { - return tree2Cls1().getField(fld1NsAName, fld1NsADesc); + private MethodVarMapping var1(MethodMapping mth) { + return mth.getVar(var1LvtIdx, var1LvIdx, var1StartOpIdx, var1EndOpIdx, var1NsAName); } - private FieldMappingView tree2Fld2() { - return tree2Cls1().getField(fld2NsAName, fld2NsADesc); + private MethodVarMapping var2(MethodMapping mth) { + return mth.getVar(var2LvtIdx, var2LvIdx, var2StartOpIdx, var2EndOpIdx, var2NsAName); } } From b1ab030ed970cd25bbb8b5bfffc04d62748b9c2a Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 12 Apr 2025 15:52:52 +0200 Subject: [PATCH 3/7] Improve Javadocs --- .../mappingio/tree/MappingCollection.java | 3 ++- .../mappingio/tree/MappingCollectionView.java | 26 +++++-------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java index 3072cc37..3e282a9c 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java @@ -28,7 +28,7 @@ import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; /** - * A {@link Collection}-based view of element mappings present in a mapping tree. + * A {@link Collection}-based modifiable view of element mappings present in a mapping tree. * *

Contrary to what's defined in {@link Collection}'s Javadocs, the {@code add} * methods here do not guarantee adding a passed element into the collection, @@ -44,6 +44,7 @@ *

  • {@link Collection#removeAll(Collection)} and *
  • {@link Collection#retainAll(Collection)}. * + * Compatibility is determined via the backing mapping tree's mapping element getters. * *

    Additionally, the {@link Collection#add(Object)} and {@link Collection#addAll(Collection)} * methods have overloaded variants that accept read-only views of the held mapping element type, diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java index e4479d16..6e4de275 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java @@ -28,28 +28,14 @@ import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; /** - * A {@link Collection}-based view of element mappings present in a mapping tree. + * A {@link Collection}-based read-only view of element mappings present in a mapping tree. * - *

    Contrary to what's defined in {@link Collection}'s Javadocs, the {@code add} - * methods here do not guarantee adding a passed element into the collection, - * instead its data may be merged into a compatible existing entry in case and such that - * {@link #containsCompatible(Object)} returns {@code true} for the passed element. + *

    The meaning of "compatibility" as used in {@link #containsCompatible(Object)} + * and {@link #containsAllCompatible(Collection)} is determined by the backing + * mapping tree's mapping element getters. * - *

    The following methods also have alternative versions that operate on - * compatible elements rather than equal ones: - *

    - * - *

    Additionally, the {@link Collection#add(Object)} and {@link Collection#addAll(Collection)} - * methods have overloaded variants that accept read-only views of the held mapping element type, - * which are converted to the tree's internal representation if necessary and then added to the tree. - * - * @param The type of element mapping. + * @param The stored Elements' type. + * @param The View type correlating to the stored mapping type. */ @ApiStatus.NonExtendable public interface MappingCollectionView extends Collection { From 5a9d7eabee9cd8ed703fae93dee0c7986e25f39c Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 12 Apr 2025 16:42:45 +0200 Subject: [PATCH 4/7] Add `MappingCollection#toUnmodifiableView` --- .../mappingio/tree/MappingCollection.java | 1 + .../mappingio/tree/MappingCollectionImpl.java | 150 ++++++++++++++---- 2 files changed, 124 insertions(+), 27 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java index 3e282a9c..726ccd03 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java @@ -60,6 +60,7 @@ public interface MappingCollection c); boolean retainAllCompatible(Collection c); + MappingCollectionView toUnmodifiableView(); interface ClassMappingCollection extends ClassMappingCollectionView { } diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java index dd7e18ee..db7d854f 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.Set; @@ -41,11 +42,17 @@ * @param The stored elements' Owner type, or any if this is a root collection. */ abstract class MappingCollectionImpl implements MappingCollection { - private MappingCollectionImpl(MemoryMappingTree tree, @Nullable O owner, Collection backing, MappedElementKind elementKind) { + private MappingCollectionImpl(MemoryMappingTree tree, @Nullable O owner, Collection backing, MappedElementKind elementKind, boolean readOnly) { this.tree = tree; this.owner = owner; - this.backing = backing; this.elementKind = elementKind; + this.readOnly = readOnly; + + if (readOnly) { + this.backing = Collections.unmodifiableCollection(backing); + } else { + this.backing = backing; + } } @Override @@ -103,7 +110,10 @@ public boolean containsAllCompatible(Collection c) { @Override public Iterator iterator() { - return new IteratorWrapper(new ArrayList<>(backing).iterator(), this); + Collection backing = readOnly + ? this.backing + : new ArrayList<>(this.backing); + return new IteratorWrapper(backing.iterator(), this); } @Override @@ -119,11 +129,17 @@ public T[] toArray(T[] a) { // Mutating methods @Override - public abstract boolean add(V e); + public boolean add(V e) { + assertModifiable(); + addInternal(e); + return true; + } + + public abstract void addInternal(V e); @Override public boolean addAllViews(Collection c) { - tree.assertNotInVisitPass(); + assertModifiable(); boolean addedAny = false; for (V e : c) { @@ -141,14 +157,14 @@ public boolean addAll(Collection c) { @Override public boolean remove(Object o) { - tree.assertNotInVisitPass(); + assertModifiable(); return backing.remove(o); } @Override @SuppressWarnings("unchecked") public boolean removeCompatible(V o) { - tree.assertNotInVisitPass(); + assertModifiable(); if (o instanceof ElementMappingView) { ElementMappingView oElem = (ElementMappingView) o; @@ -165,7 +181,7 @@ public boolean removeCompatible(V o) { @Override public boolean removeAll(Collection c) { - tree.assertNotInVisitPass(); + assertModifiable(); boolean removedAny = false; for (Object o : c) { @@ -177,7 +193,7 @@ public boolean removeAll(Collection c) { @Override public boolean removeAllCompatible(Collection c) { - tree.assertNotInVisitPass(); + assertModifiable(); boolean removedAny = false; for (V o : c) { @@ -189,7 +205,7 @@ public boolean removeAllCompatible(Collection c) { @Override public boolean retainAll(Collection c) { - tree.assertNotInVisitPass(); + assertModifiable(); boolean removedAny = false; for (Iterator it = iterator(); it.hasNext();) { @@ -206,7 +222,7 @@ public boolean retainAll(Collection c) { @Override public boolean retainAllCompatible(Collection c) { - tree.assertNotInVisitPass(); + assertModifiable(); Set toRemove = new HashSet<>(this); boolean removedAny = false; @@ -232,7 +248,7 @@ public boolean retainAllCompatible(Collection c) { @Override public void clear() { - tree.assertNotInVisitPass(); + assertModifiable(); for (Iterator it = iterator(); it.hasNext();) { it.next(); @@ -240,6 +256,14 @@ public void clear() { } } + protected void assertModifiable() { + if (readOnly) { + throw new UnsupportedOperationException("Attempted modification of read-only collection"); + } + + tree.assertNotInVisitPass(); + } + private final class IteratorWrapper implements Iterator { IteratorWrapper(Iterator backing, MappingCollectionImpl owner) { this.backing = backing; @@ -259,6 +283,7 @@ public E next() { @Override public void remove() { + assertModifiable(); owner.remove(lastReturned); } @@ -269,13 +294,16 @@ public void remove() { static class ClassMappingCollectionImpl extends MappingCollectionImpl implements ClassMappingCollection { ClassMappingCollectionImpl(MemoryMappingTree tree, Collection backing) { - super(tree, null, backing, MappedElementKind.CLASS); + this(tree, backing, false); + } + + private ClassMappingCollectionImpl(MemoryMappingTree tree, Collection backing, boolean readOnly) { + super(tree, null, backing, MappedElementKind.CLASS, readOnly); } @Override - public boolean add(ClassMappingView e) { + public void addInternal(ClassMappingView e) { tree.addClass(e); - return true; } @Override @@ -288,17 +316,31 @@ protected E getCompatible(ClassMappingView o) { protected boolean removeCompatibleInternal(ClassMappingView o) { return tree.removeClass(o.getSrcName()) != null; } + + @Override + public MappingCollectionView toUnmodifiableView() { + if (view == null) { + view = new ClassMappingCollectionImpl<>(tree, backing, true); + } + + return view; + } + + protected ClassMappingCollectionView view; } static class FieldMappingCollectionImpl extends MappingCollectionImpl implements FieldMappingCollection { FieldMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { - super(tree, owner, backing, MappedElementKind.FIELD); + this(tree, owner, backing, false); + } + + private FieldMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing, boolean readOnly) { + super(tree, owner, backing, MappedElementKind.FIELD, readOnly); } @Override - public boolean add(FieldMappingView e) { + public void addInternal(FieldMappingView e) { owner.addField(e); - return true; } @Override @@ -311,17 +353,31 @@ protected E getCompatible(FieldMappingView o) { protected boolean removeCompatibleInternal(FieldMappingView o) { return owner.removeField(o.getSrcName(), o.getSrcDesc()) != null; } + + @Override + public MappingCollectionView toUnmodifiableView() { + if (view == null) { + view = new FieldMappingCollectionImpl<>(tree, owner, backing, true); + } + + return view; + } + + protected FieldMappingCollectionView view; } static class MethodMappingCollectionImpl extends MappingCollectionImpl implements MethodMappingCollection { MethodMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { - super(tree, owner, backing, MappedElementKind.METHOD); + this(tree, owner, backing, false); + } + + private MethodMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing, boolean readOnly) { + super(tree, owner, backing, MappedElementKind.METHOD, readOnly); } @Override - public boolean add(MethodMappingView e) { + public void addInternal(MethodMappingView e) { owner.addMethod(e); - return true; } @Override @@ -334,17 +390,31 @@ protected E getCompatible(MethodMappingView o) { protected boolean removeCompatibleInternal(MethodMappingView o) { return owner.removeMethod(o.getSrcName(), o.getSrcDesc()) != null; } + + @Override + public MappingCollectionView toUnmodifiableView() { + if (view == null) { + view = new MethodMappingCollectionImpl<>(tree, owner, backing, true); + } + + return view; + } + + protected MethodMappingCollectionView view; } static class MethodArgMappingCollectionImpl extends MappingCollectionImpl implements MethodArgMappingCollection { MethodArgMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { - super(tree, owner, backing, MappedElementKind.METHOD_ARG); + this(tree, owner, backing, false); + } + + private MethodArgMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing, boolean readOnly) { + super(tree, owner, backing, MappedElementKind.METHOD_ARG, readOnly); } @Override - public boolean add(MethodArgMappingView e) { + public void addInternal(MethodArgMappingView e) { owner.addArg(e); - return true; } @Override @@ -357,17 +427,31 @@ protected E getCompatible(MethodArgMappingView o) { protected boolean removeCompatibleInternal(MethodArgMappingView o) { return owner.removeArg(o.getArgPosition(), o.getLvIndex(), o.getSrcName()) != null; } + + @Override + public MappingCollectionView toUnmodifiableView() { + if (view == null) { + view = new MethodArgMappingCollectionImpl<>(tree, owner, backing, true); + } + + return view; + } + + protected MethodArgMappingCollectionView view; } static class MethodVarMappingCollectionImpl extends MappingCollectionImpl implements MethodVarMappingCollection { MethodVarMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing) { - super(tree, owner, backing, MappedElementKind.METHOD_VAR); + this(tree, owner, backing, false); + } + + private MethodVarMappingCollectionImpl(MemoryMappingTree tree, O owner, Collection backing, boolean readOnly) { + super(tree, owner, backing, MappedElementKind.METHOD_VAR, readOnly); } @Override - public boolean add(MethodVarMappingView e) { + public void addInternal(MethodVarMappingView e) { owner.addVar(e); - return true; } @Override @@ -380,10 +464,22 @@ protected E getCompatible(MethodVarMappingView o) { protected boolean removeCompatibleInternal(MethodVarMappingView o) { return owner.removeVar(o.getLvtRowIndex(), o.getLvIndex(), o.getStartOpIdx(), o.getEndOpIdx(), o.getSrcName()) != null; } + + @Override + public MappingCollectionView toUnmodifiableView() { + if (view == null) { + view = new MethodVarMappingCollectionImpl<>(tree, owner, Collections.unmodifiableCollection(backing)); + } + + return view; + } + + protected MethodVarMappingCollectionView view; } protected final MemoryMappingTree tree; protected final @Nullable O owner; protected final MappedElementKind elementKind; protected final Collection backing; + protected final boolean readOnly; } From 27ad70ff1237404e96688c3f8bbe7d252edab0c0 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 12 Apr 2025 16:48:50 +0200 Subject: [PATCH 5/7] Fix return types --- .../mappingio/tree/MappingCollection.java | 25 +++++++++++++++---- .../mappingio/tree/MappingCollectionImpl.java | 10 ++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java index 726ccd03..5be682f7 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java @@ -62,13 +62,28 @@ public interface MappingCollection c); MappingCollectionView toUnmodifiableView(); - interface ClassMappingCollection extends ClassMappingCollectionView { } + interface ClassMappingCollection extends ClassMappingCollectionView { + @Override + ClassMappingCollectionView toUnmodifiableView(); + } - interface FieldMappingCollection extends FieldMappingCollectionView { } + interface FieldMappingCollection extends FieldMappingCollectionView { + @Override + FieldMappingCollectionView toUnmodifiableView(); + } - interface MethodMappingCollection extends MethodMappingCollectionView { } + interface MethodMappingCollection extends MethodMappingCollectionView { + @Override + MethodMappingCollectionView toUnmodifiableView(); + } - interface MethodArgMappingCollection extends MethodArgMappingCollectionView { } + interface MethodArgMappingCollection extends MethodArgMappingCollectionView { + @Override + MethodArgMappingCollectionView toUnmodifiableView(); + } - interface MethodVarMappingCollection extends MethodVarMappingCollectionView { } + interface MethodVarMappingCollection extends MethodVarMappingCollectionView { + @Override + MethodVarMappingCollectionView toUnmodifiableView(); + } } diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java index db7d854f..93af3ee4 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionImpl.java @@ -318,7 +318,7 @@ protected boolean removeCompatibleInternal(ClassMappingView o) { } @Override - public MappingCollectionView toUnmodifiableView() { + public ClassMappingCollectionView toUnmodifiableView() { if (view == null) { view = new ClassMappingCollectionImpl<>(tree, backing, true); } @@ -355,7 +355,7 @@ protected boolean removeCompatibleInternal(FieldMappingView o) { } @Override - public MappingCollectionView toUnmodifiableView() { + public FieldMappingCollectionView toUnmodifiableView() { if (view == null) { view = new FieldMappingCollectionImpl<>(tree, owner, backing, true); } @@ -392,7 +392,7 @@ protected boolean removeCompatibleInternal(MethodMappingView o) { } @Override - public MappingCollectionView toUnmodifiableView() { + public MethodMappingCollectionView toUnmodifiableView() { if (view == null) { view = new MethodMappingCollectionImpl<>(tree, owner, backing, true); } @@ -429,7 +429,7 @@ protected boolean removeCompatibleInternal(MethodArgMappingView o) { } @Override - public MappingCollectionView toUnmodifiableView() { + public MethodArgMappingCollectionView toUnmodifiableView() { if (view == null) { view = new MethodArgMappingCollectionImpl<>(tree, owner, backing, true); } @@ -466,7 +466,7 @@ protected boolean removeCompatibleInternal(MethodVarMappingView o) { } @Override - public MappingCollectionView toUnmodifiableView() { + public MethodVarMappingCollectionView toUnmodifiableView() { if (view == null) { view = new MethodVarMappingCollectionImpl<>(tree, owner, Collections.unmodifiableCollection(backing)); } From e3104c458f51434c63478fcb1c6fb8986ee2a55b Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sun, 13 Apr 2025 09:46:46 +0200 Subject: [PATCH 6/7] Add changes to changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9870a97b..88702c55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - Added a simplified `MappingNsCompleter` constructor for completing all destination names with the source names - Added `MappingTree#propagateOuterClassNames` as a more efficient tree-API alternative to `OuterClassNamePropagator` +- Added `MappingCollection` interfaces as modifiable views of a tree's internal mapping store +- Added `getKind` method to `ElementMappingView` +- Made `MappingTree`'s `add` methods accept read-only mapping views - Made `OuterClassNamePropagator` configurable - Made Enigma writer always output destination names if visited explicitly, establishing consistency across all writers - Adjusted format detection to only return ENIGMA_DIR for non-empty directories with at least one `.mapping` file From 17d20e97884f3f6d29088fc835bdf543a481863c Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sun, 13 Apr 2025 16:28:00 +0200 Subject: [PATCH 7/7] `mapping element` -> `element mapping` --- .../java/net/fabricmc/mappingio/tree/MappingCollection.java | 4 ++-- .../net/fabricmc/mappingio/tree/MappingCollectionView.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java index 5be682f7..e154a5cd 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollection.java @@ -44,10 +44,10 @@ *

  • {@link Collection#removeAll(Collection)} and *
  • {@link Collection#retainAll(Collection)}. * - * Compatibility is determined via the backing mapping tree's mapping element getters. + * Compatibility is determined via the backing mapping tree's element mapping getters. * *

    Additionally, the {@link Collection#add(Object)} and {@link Collection#addAll(Collection)} - * methods have overloaded variants that accept read-only views of the held mapping element type, + * methods have overloaded variants that accept read-only views of the held element mapping type, * which are converted to the tree's internal representation if necessary and then added to the tree. * * @param The stored Elements' type. diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java index 6e4de275..e5bbbfbe 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MappingCollectionView.java @@ -32,7 +32,7 @@ * *

    The meaning of "compatibility" as used in {@link #containsCompatible(Object)} * and {@link #containsAllCompatible(Collection)} is determined by the backing - * mapping tree's mapping element getters. + * mapping tree's element mapping getters. * * @param The stored Elements' type. * @param The View type correlating to the stored mapping type.