CommandInfo: disallow immutable BOTH parameters

The purpose of BOTH is to indicate than an input reference will be
changed in-place. It is a programming error to label an immutable
parameter as BOTH; rather, the immutable parameter should be an INPUT
and the (presumably affiliated) OUTPUT should be a separate variable.

Arguably, this logic should live somewhere deeper than CommandInfo;
we can cross that bridge later when we come to it.

Author: ctrueden

src/main/java/org/scijava/command/CommandInfo.java

@@ -43,6 +43,7 @@
 import org.scijava.Cancelable;
 import org.scijava.Context;
 import org.scijava.InstantiableException;
+import org.scijava.ItemIO;
 import org.scijava.ItemVisibility;
 import org.scijava.ValidityProblem;
 import org.scijava.event.EventService;
@@ -470,6 +471,15 @@ private void checkFields(final Class<?> type) {
 				valid = false;
 			}
 
+			if (param.type() == ItemIO.BOTH && isImmutable(f.getType())) {
+				// NB: The BOTH type signifies that the parameter will be changed
+				// in-place somehow. But immutable parameters cannot be changed in
+				// such a manner, so it makes no sense to label them as BOTH.
+				final String error = "Immutable BOTH parameter: " + f;
+				problems.add(new ValidityProblem(error));
+				valid = false;
+			}
+
 			if (!valid) {
 				// NB: Skip invalid parameters.
 				continue;
@@ -491,6 +501,13 @@ private void checkFields(final Class<?> type) {
 		}
 	}
 
+	private boolean isImmutable(final Class<?> type) {
+		// NB: All eight primitive types, as well as the boxed primitive
+		// wrapper classes, as well as strings, are immutable objects.
+		return ClassUtils.isNumber(type) || ClassUtils.isText(type) ||
+			ClassUtils.isBoolean(type);
+	}
+
 	private Class<?> loadCommandClass() {
 		try {
 			return loadClass();