ScriptModule: improve handling of return values

Whether the return value output is explicitly declared makes a big
difference. The behavior we want is:

                implicit output          explicit output
           /--------------------------------------------------\
           |   result = 2          |   // @output int result  |
 variable  |   1                   |   result = 2             |
 assigned  |                       |   1                      |
           |                       |                          |
           | BEHAVIOR: result is 1 | BEHAVIOR: result is 2    |
           |-----------------------+---------------------------
           |   1                   |   // @output int result  |
 variable  |                       |   1                      |
unassigned |                       |                          |
           | BEHAVIOR: result is 1 | BEHAVIOR: result is null |
           \--------------------------------------------------/

The thinking is that when the script writer declares an output called
"result", their intuition will be that the final assignment to the
variable called "result" should become the output value. But when they
_don't_ explicitly declare that output, the implicitly added output
called "result" refers to the return value of the script itself, not
the value of the variable called "result" in the bindings.

Author: ctrueden

src/main/java/org/scijava/script/ScriptInfo.java

@@ -87,6 +87,9 @@ public class ScriptInfo extends AbstractModuleInfo implements Contextual {
 	@Parameter
 	private ConvertService convertService;
 
+	/** True iff the return value is explicitly declared as an output. */
+	private boolean returnValueDeclared;
+
 	/**
 	 * Creates a script metadata object which describes the given script file.
 	 * 
@@ -207,6 +210,7 @@ public BufferedReader getReader() {
 	@Override
 	public void parseParameters() {
 		clearParameters();
+		returnValueDeclared = false;
 
 		try {
 			final BufferedReader in;
@@ -233,7 +237,7 @@ public void parseParameters() {
 			if (reader == null) in.close();
 			else in.reset();
 
-			addReturnValue();
+			if (!returnValueDeclared) addReturnValue();
 		}
 		catch (final IOException exc) {
 			log.error("Error reading script: " + path, exc);
@@ -243,6 +247,11 @@ public void parseParameters() {
 		}
 	}
 
+	/** Gets whether the return value is explicitly declared as an output. */
+	public boolean isReturnValueDeclared() {
+		return returnValueDeclared;
+	}
+
 	// -- ModuleInfo methods --
 
 	@Override
@@ -347,6 +356,7 @@ private void parseParam(final String param,
 		}
 		final Class<?> type = scriptService.lookupClass(typeName);
 		addItem(varName, type, attrs);
+		if (ScriptModule.RETURN_VALUE.equals(varName)) returnValueDeclared = true;
 	}
 
 	/** Parses a comma-delimited list of {@code key=value} pairs into a map. */

src/main/java/org/scijava/script/ScriptModule.java

@@ -189,17 +189,16 @@ public void run() {
 		for (final ModuleItem<?> item : getInfo().outputs()) {
 			final String name = item.getName();
 			if (isResolved(name)) continue;
-			final Object value = engine.get(name);
+			final Object value;
+			if (RETURN_VALUE.equals(name) && !getInfo().isReturnValueDeclared()) {
+				// NB: This is the special implicit return value output!
+				value = returnValue;
+			}
+			else value = engine.get(name);
 			final Object decoded = language.decode(value);
 			final Object typed = conversionService.convert(decoded, item.getType());
 			setOutput(name, typed);
-		}
-
-		// populate the return value as a special output
-		// NB: This only occurs if there was no declared output called "result".
-		if (!isResolved(RETURN_VALUE)) {
-			setOutput(RETURN_VALUE, language.decode(returnValue));
-			setResolved(RETURN_VALUE, true);
+			setResolved(name, true);
 		}
 
 		// flush output and error streams