Remove commons-collection4 from the project (#7561)

* Replace commons-collections4 CollectionUtils with JDK equivalents

Two call sites used CollectionUtils.emptyIfNull() to guard against null
collections before streaming. Both are replaced with explicit null checks:

- VariableChannel.checkForVeto(): a stream-then-findAny-isPresent chain
  collapses to a single anyMatch() with a null guard.
- VisionDisplay.getVision(): early return on null preserves the same
  empty-string output without constructing an empty collection.

Step 1 of removing the commons-collections4 dependency.

* Replace commons-collections4 IteratorUtils with JDK StreamSupport

The single call site converted an Iterator into a List via
IteratorUtils.toList(). Replaced with StreamSupport.stream() over a
Spliterator wrapper of the Iterator and Stream.toList(), which is the
idiomatic JDK pattern for the same conversion.

Step 2 of removing the commons-collections4 dependency.

* Replace commons-collections4 ListUtils and remove the dependency

Four call sites in CDOMObjectUtilities used ListUtils.emptyIfNull() to
guard a forEach against null. Replaced with explicit null checks: skip
the iteration if the list is null, otherwise call forEach directly.

This was the last remaining commons-collections4 import after the prior
two commits removed CollectionUtils and IteratorUtils, so this change
also drops the dependency from build.gradle and the requires clause
from module-info.java.

Step 3 of removing the commons-collections4 dependency. Saves ~880 KB
from the runtime classpath.

* Refactor veto mechanism: replace BiFunction with BiPredicate

BiPredicate<T, T> is the specialized functional interface for boolean-returning two-argument functions, so call sites use .test() rather than .apply() and Sonar's "use the most specialized functional interface" warning goes away. Also tighten getVariableID() to return VariableID<T> instead of VariableID<?>.

The veto API itself (VetoableReferenceFacade, addVetoToChannel) has no in-tree callers and PR #5577 (2019) proposed removing it. The original author (thpr) pushed back, asking that cdom/formula code not be changed without discussion since "it is rarely unused completely" — third-party plugins may rely on it. Keeping the API; only modernizing the signature.

Signed-off-by: Vest <Vest@users.noreply.github.com>

* Refactor ExportDialogController: simplify context creation and streamline UI logic
Fix SonarLint errors/warnings.

Signed-off-by: Vest <Vest@users.noreply.github.com>

* Refactor CDOMObjectUtilities: streamline list handling with getSafeListFor

Signed-off-by: Vest <Vest@users.noreply.github.com>

---------

Signed-off-by: Vest <Vest@users.noreply.github.com>

Author: Vest

build.gradle

@@ -231,7 +231,6 @@ dependencies {
         exclude group: 'xalan', module: 'serializer'
     }
     implementation 'net.sf.saxon:Saxon-HE:12.9'
-    implementation 'org.apache.commons:commons-collections4:4.5.0'
     implementation 'org.scijava:jep:2.4.2'
     implementation 'org.freemarker:freemarker:2.3.34'
     implementation 'org.jdom:jdom2:2.0.6.1'
@@ -305,7 +304,7 @@ jar {
 jlink {
     options = ['--strip-debug', '--no-header-files', '--no-man-pages']
     forceMerge('PCGen-base', 'PCGen-Formula', 'jep', 'fop', 'Saxon-HE',
-               'commons-lang3', 'commons-io', 'commons-collections4', 'spring',
+               'commons-lang3', 'commons-io', 'spring',
                'freemarker', 'jdom2', 'argparse4j', 'xmlunit', 'controlsfx',
                'annotations', 'spotbugs', 'xmlresolver')
 

code/src/java/module-info.java

@@ -24,7 +24,6 @@
 
     requires org.apache.commons.lang3;
     requires org.apache.commons.io;
-    requires org.apache.commons.collections4;
     requires freemarker;
     requires org.jdom2;
     requires net.sourceforge.argparse4j;

code/src/java/pcgen/cdom/base/CDOMObjectUtilities.java

@@ -17,13 +17,9 @@
  */
 package pcgen.cdom.base;
 
-import java.util.List;
-
 import pcgen.cdom.enumeration.ListKey;
 import pcgen.core.PlayerCharacter;
 
-import org.apache.commons.collections4.ListUtils;
-
 /**
  * CDOMObjectUtilities is a utility class designed to provide utility methods
  * when working with pcgen.cdom.base.CDOMObject Objects
@@ -71,8 +67,7 @@ public static void addAdds(CDOMObject cdo, PlayerCharacter pc)
 		{
 			return;
 		}
-		List<PersistentTransitionChoice<?>> addList = ListUtils.emptyIfNull(cdo.getListFor(ListKey.ADD));
-		addList.forEach(tc -> driveChoice(cdo, tc, pc));
+		cdo.getSafeListFor(ListKey.ADD).forEach(tc -> driveChoice(cdo, tc, pc));
 	}
 
 	public static void removeAdds(CDOMObject cdo, PlayerCharacter pc)
@@ -81,8 +76,7 @@ public static void removeAdds(CDOMObject cdo, PlayerCharacter pc)
 		{
 			return;
 		}
-		List<PersistentTransitionChoice<?>> addList = ListUtils.emptyIfNull(cdo.getListFor(ListKey.ADD));
-		addList.forEach(tc -> tc.remove(cdo, pc));
+		cdo.getSafeListFor(ListKey.ADD).forEach(tc -> tc.remove(cdo, pc));
 	}
 
 	public static void checkRemovals(CDOMObject cdo, PlayerCharacter pc)
@@ -91,8 +85,7 @@ public static void checkRemovals(CDOMObject cdo, PlayerCharacter pc)
 		{
 			return;
 		}
-		List<PersistentTransitionChoice<?>> removeList = ListUtils.emptyIfNull(cdo.getListFor(ListKey.REMOVE));
-		removeList.forEach(tc -> driveChoice(cdo, tc, pc));
+		cdo.getSafeListFor(ListKey.REMOVE).forEach(tc -> driveChoice(cdo, tc, pc));
 	}
 
 	public static void restoreRemovals(CDOMObject cdo, PlayerCharacter pc)
@@ -101,8 +94,7 @@ public static void restoreRemovals(CDOMObject cdo, PlayerCharacter pc)
 		{
 			return;
 		}
-		List<PersistentTransitionChoice<?>> removeList = ListUtils.emptyIfNull(cdo.getListFor(ListKey.REMOVE));
-		removeList.forEach(tc -> tc.remove(cdo, pc));
+		cdo.getSafeListFor(ListKey.REMOVE).forEach(tc -> tc.remove(cdo, pc));
 	}
 
 	private static <T> void driveChoice(CDOMObject cdo, TransitionChoice<T> tc, final PlayerCharacter pc)

code/src/java/pcgen/cdom/formula/VariableChannel.java

@@ -20,15 +20,13 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
 
 import pcgen.base.formula.base.VariableID;
 import pcgen.base.solver.SolverManager;
 import pcgen.facade.util.AbstractReferenceFacade;
 import pcgen.facade.util.VetoableReferenceFacade;
 
-import org.apache.commons.collections4.CollectionUtils;
-
 /**
  * A VariableChannel provides a common mechanism for reading and writing to a
  * variable from a system external to the PCGen core.
@@ -66,7 +64,7 @@ public final class VariableChannel<T> extends AbstractReferenceFacade<T>
 	/**
 	 * The list of functions allowed to veto changes to this variable channel.
 	 */
-	private List<BiFunction<T, T, Boolean>> vetoList = null;
+	private final List<BiPredicate<T, T>> vetoList = new ArrayList<>(2);
 
 	/**
 	 * Constructs a new VariableChannel with the given SolverManager,
@@ -115,11 +113,7 @@ public void set(T object)
 	private boolean checkForVeto(T proposedValue)
 	{
 		T oldValue = varStore.get(varID);
-		return CollectionUtils.emptyIfNull(vetoList)
-			.stream()
-			.filter(f -> f.apply(oldValue, proposedValue))
-			.findAny()
-			.isPresent();
+		return vetoList.stream().anyMatch(f -> f.test(oldValue, proposedValue));
 	}
 
 	/**
@@ -172,18 +166,14 @@ public static <T> VariableChannel<T> construct(SolverManager manager,
 	 * 
 	 * @return The VariableID for this VariableChannel
 	 */
-	public VariableID<?> getVariableID()
+	public VariableID<T> getVariableID()
 	{
 		return varID;
 	}
 
 	@Override
-	public void addVetoToChannel(BiFunction<T, T, Boolean> function)
+	public void addVetoToChannel(BiPredicate<T, T> function)
 	{
-		if (vetoList == null)
-		{
-			vetoList = new ArrayList<>(2);
-		}
 		vetoList.add(Objects.requireNonNull(function));
 	}
 

code/src/java/pcgen/core/display/VisionDisplay.java

@@ -26,7 +26,6 @@
 import pcgen.core.PlayerCharacter;
 import pcgen.core.Vision;
 
-import org.apache.commons.collections4.CollectionUtils;
 import org.jetbrains.annotations.NotNull;
 
 public final class VisionDisplay
@@ -42,7 +41,11 @@ public static String getVision(final PlayerCharacter aPC, CDOMObject cdo)
 		{
 			return "";
 		}
-		Collection<CDOMReference<Vision>> mods = CollectionUtils.emptyIfNull(cdo.getListMods(Vision.VISIONLIST));
+		Collection<CDOMReference<Vision>> mods = cdo.getListMods(Vision.VISIONLIST);
+		if (mods == null)
+		{
+			return "";
+		}
 
 		StringJoiner visionString = new StringJoiner(";");
 		mods.stream().flatMap(ref -> ref.getContainedObjects().stream()).map(v -> v.toString(aPC))

code/src/java/pcgen/facade/util/VetoableReferenceFacade.java

@@ -15,25 +15,25 @@
  */
 package pcgen.facade.util;
 
-import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
 
 /**
  * A VetoableReferenceFacade is a WriteableReferenceFacade that allows functions to veto
  * changes to the underlying value
- * 
+ *
  * @param <T>
  *            The format of object stored in this VetoableReferenceFacade
  */
 public interface VetoableReferenceFacade<T> extends WriteableReferenceFacade<T>
 {
 
 	/**
-	 * Adds a BiFunction to determine if a change should be vetoed. Returning TRUE results
+	 * Adds a BiPredicate to determine if a change should be vetoed. Returning TRUE results
 	 * in an item being vetoed.
-	 * 
+	 *
 	 * @param function
 	 *            The function to be added to determine if a change will be vetoed
 	 */
-	void addVetoToChannel(BiFunction<T, T, Boolean> function);
+	void addVetoToChannel(BiPredicate<T, T> function);
 
 }

code/src/java/pcgen/gui3/dialog/ExportDialogController.java

@@ -27,7 +27,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
-import java.util.stream.Collectors;
 
 import javax.swing.JCheckBox;
 
@@ -58,7 +57,6 @@
 import javafx.scene.control.ListView;
 import javafx.scene.control.ProgressBar;
 import javafx.stage.FileChooser;
-import org.apache.commons.collections4.IteratorUtils;
 import org.apache.commons.lang3.StringUtils;
 
 /**
@@ -67,6 +65,8 @@
  */
 public class ExportDialogController
 {
+	private static final String EXPORT_DIALOG_CONTEXT = "ExportDialog";
+
 	@FXML
 	private ComboBox<CharacterFacade> selectCharacterBox;
 	@FXML
@@ -144,7 +144,7 @@ private static void doExportEntireParty(String sheetFilterPath, URI template, bo
 		}
 		else
 		{
-			PropertyContext context = UIPropertyContext.createContext("ExportDialog");
+			PropertyContext context = UIPropertyContext.createContext(EXPORT_DIALOG_CONTEXT);
 			context.setProperty(HTML_EXPORT_DIR_PROP, outFile.getParent());
 			SettingsHandler.setSelectedPartyHTMLOutputSheet(templateAsFile.getAbsolutePath());
 			BatchExporter.exportPartyToNonPDF(party, outFile, templateAsFile);
@@ -283,14 +283,7 @@ public void changed(final ObservableValue<? extends Boolean> observable,
 		                    final Boolean newValue)
 		{
 			templateSelect.getSelectionModel().clearSelection();
-			if (newValue == true)
-			{
-				selectCharacterBox.setDisable(true);
-			}
-			else
-			{
-				selectCharacterBox.setDisable(false);
-			}
+			selectCharacterBox.setDisable(newValue);
 			refreshFiles(exportSheetType.getSelectionModel().getSelectedItem(), entireParty.isSelected());
 		}
 	}
@@ -302,20 +295,14 @@ public void changed(final ObservableValue<? extends URI> observable,
 		                    final URI oldValue,
 		                    final URI newValue)
 		{
-			if (newValue != null)
-			{
-				doExport.setDisable(false);
-			} else
-			{
-				doExport.setDisable(true);
-			}
+			doExport.setDisable(newValue == null);
 		}
 	}
 
 	private void refreshFiles(ExportUtilities.SheetFilter sheetFilter, boolean exportParty)
 	{
 		URI[] validFiles = ExportUtilities.getValidFiles(allTemplates, sheetFilter, exportParty);
-		List<URI> validFilesAsList = Arrays.stream(validFiles).collect(Collectors.toList());
+		List<URI> validFilesAsList = Arrays.stream(validFiles).toList();
 		ObservableList<URI> observableTemplates = FXCollections.observableList(validFilesAsList);
 		templateSelect.itemsProperty().setValue(observableTemplates);
 	}
@@ -327,7 +314,7 @@ public void changed(final ObservableValue<? extends ExportUtilities.SheetFilter>
 		                    final ExportUtilities.SheetFilter oldValue,
 		                    final ExportUtilities.SheetFilter newValue)
 		{
-			PropertyContext context = UIPropertyContext.createContext("ExportDialog");
+			PropertyContext context = UIPropertyContext.createContext(EXPORT_DIALOG_CONTEXT);
 			context.setProperty(UIPropertyContext.DEFAULT_OS_TYPE,
 				exportSheetType.getSelectionModel().getSelectedItem().toString());
 			refreshFiles(newValue, entireParty.isSelected());
@@ -345,19 +332,19 @@ private void doOnClose(final ActionEvent actionEvent)
 	void initialize()
 	{
 		PartyFacade characters = CharacterManager.getCharacters();
-		ObservableList<CharacterFacade> observableList =
-				FXCollections.observableList(IteratorUtils.toList(characters.iterator()));
+		ObservableList<CharacterFacade> observableList = FXCollections.observableArrayList();
+		characters.forEach(observableList::add);
 		selectCharacterBox.setItems(observableList);
 		// todo: maybe select "current" character pcgenFrame.getSelectedCharacterRef() ???
 		selectCharacterBox.getSelectionModel().select(0);
 		entireParty.selectedProperty().addListener(new PartyCheckboxChangeListener());
 		templateSelect.getSelectionModel().selectedItemProperty().addListener(new TemplateSelectionListener());
 		List<ExportUtilities.SheetFilter> sheetFilterValues = Arrays.stream(
-				ExportUtilities.SheetFilter.values()).collect(Collectors.toList());
+				ExportUtilities.SheetFilter.values()).toList();
 		exportSheetType.setItems(FXCollections.observableList(sheetFilterValues));
 		exportSheetType.getSelectionModel().selectedItemProperty().addListener(new ExportSheetTypeSelectionListener());
 		exportSheetType.getSelectionModel().select(0);
-		PropertyContext context = UIPropertyContext.createContext("ExportDialog");
+		PropertyContext context = UIPropertyContext.createContext(EXPORT_DIALOG_CONTEXT);
 		String defaultOSType = context.getProperty(UIPropertyContext.DEFAULT_OS_TYPE);
 		if (defaultOSType != null)
 		{