diff --git a/src/main/java/org/scijava/script/ScriptInfo.java b/src/main/java/org/scijava/script/ScriptInfo.java index 6d4657be6..efb1b26cf 100644 --- a/src/main/java/org/scijava/script/ScriptInfo.java +++ b/src/main/java/org/scijava/script/ScriptInfo.java @@ -94,8 +94,8 @@ 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; + /** True iff the return value should be appended as an output. */ + private boolean appendReturnValue; /** * Creates a script metadata object which describes the given script file. @@ -229,7 +229,7 @@ public BufferedReader getReader() { @Override public void parseParameters() { clearParameters(); - returnValueDeclared = false; + appendReturnValue = true; try { final BufferedReader in; @@ -254,7 +254,7 @@ public void parseParameters() { } in.close(); - if (!returnValueDeclared) addReturnValue(); + if (appendReturnValue) addReturnValue(); } catch (final IOException exc) { log.error("Error reading script: " + path, exc); @@ -264,9 +264,9 @@ public void parseParameters() { } } - /** Gets whether the return value is explicitly declared as an output. */ - public boolean isReturnValueDeclared() { - return returnValueDeclared; + /** Gets whether the return value is appended as an additional output. */ + public boolean isReturnValueAppended() { + return appendReturnValue; } // -- ModuleInfo methods -- @@ -372,7 +372,12 @@ private void parseParam(final String param, } final Class type = scriptService.lookupClass(typeName); addItem(varName, type, attrs); - if (ScriptModule.RETURN_VALUE.equals(varName)) returnValueDeclared = true; + + if (ScriptModule.RETURN_VALUE.equals(varName)) { + // NB: The return value variable is declared as an explicit OUTPUT. + // So we should not append the return value as an extra output. + appendReturnValue = false; + } } /** Parses a comma-delimited list of {@code key=value} pairs into a map. */ @@ -391,7 +396,7 @@ private void checkValid(final boolean valid, final String param) } /** Adds an output for the value returned by the script itself. */ - private void addReturnValue() throws ScriptException { + private void addReturnValue() { final HashMap attrs = new HashMap<>(); attrs.put("type", "OUTPUT"); addItem(ScriptModule.RETURN_VALUE, Object.class, attrs); @@ -407,7 +412,12 @@ private void addItem(final String name, final Class type, assignAttribute(item, key, value); } if (item.isInput()) registerInput(item); - if (item.isOutput()) registerOutput(item); + if (item.isOutput()) { + registerOutput(item); + // NB: Only append the return value as an extra + // output when no explicit outputs are declared. + appendReturnValue = false; + } } private void assignAttribute(final DefaultMutableModuleItem item, @@ -479,4 +489,12 @@ private static String getReaderContentsAsString(final Reader reader) return builder.toString(); } + // -- Deprecated methods -- + + /** @deprecated Use {@link #isReturnValueAppended()} instead. */ + @Deprecated + public boolean isReturnValueDeclared() { + return !isReturnValueAppended(); + } + } diff --git a/src/main/java/org/scijava/script/ScriptModule.java b/src/main/java/org/scijava/script/ScriptModule.java index 602545ebe..45d2028db 100644 --- a/src/main/java/org/scijava/script/ScriptModule.java +++ b/src/main/java/org/scijava/script/ScriptModule.java @@ -88,6 +88,8 @@ public class ScriptModule extends AbstractModule implements Contextual { /** Destination for standard error during script execution. */ private Writer error; + private Object returnValue; + public ScriptModule(final ScriptInfo info) { this.info = info; } @@ -130,7 +132,7 @@ public ScriptEngine getEngine() { /** Gets the return value of the script. */ public Object getReturnValue() { - return getOutput(RETURN_VALUE); + return returnValue; } // -- Module methods -- @@ -167,7 +169,7 @@ public void run() { } // execute script! - Object returnValue = null; + returnValue = null; try { final Reader reader = getInfo().getReader(); if (reader == null) returnValue = engine.eval(new FileReader(path)); @@ -190,7 +192,7 @@ public void run() { final String name = item.getName(); if (isResolved(name)) continue; final Object value; - if (RETURN_VALUE.equals(name) && !getInfo().isReturnValueDeclared()) { + if (RETURN_VALUE.equals(name) && getInfo().isReturnValueAppended()) { // NB: This is the special implicit return value output! value = returnValue; } diff --git a/src/test/java/org/scijava/script/ScriptInfoTest.java b/src/test/java/org/scijava/script/ScriptInfoTest.java index 53f9ed659..37b85214c 100644 --- a/src/test/java/org/scijava/script/ScriptInfoTest.java +++ b/src/test/java/org/scijava/script/ScriptInfoTest.java @@ -32,6 +32,7 @@ package org.scijava.script; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -45,6 +46,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.script.Bindings; import javax.script.ScriptContext; @@ -84,6 +86,42 @@ public static void tearDown() { // -- Tests -- + /** + * Tests that the return value is appended as an extra output when no + * explicit outputs were declared. + */ + @Test + public void testReturnValueAppended() throws Exception { + final String script = "" + // + "% @LogService log\n" + // + "% @int value\n"; + final ScriptModule scriptModule = + scriptService.run("include-return-value.bsizes", script, true).get(); + + final Map outputs = scriptModule.getOutputs(); + assertEquals(1, outputs.size()); + assertTrue(outputs.containsKey(ScriptModule.RETURN_VALUE)); + } + + /** + * Tests that the return value is not appended as an extra output + * when explicit outputs were declared. + */ + @Test + public void testReturnValueExcluded() throws Exception { + final String script = "" + // + "% @LogService log\n" + // + "% @OUTPUT int value\n"; + final ScriptModule scriptModule = + scriptService.run("exclude-return-value.bsizes", script, true).get(); + + final Map outputs = scriptModule.getOutputs(); + assertEquals(1, outputs.size()); + assertTrue(outputs.containsKey("value")); + assertFalse(outputs.containsKey(ScriptModule.RETURN_VALUE)); + } + + /** * Ensures parameters are parsed correctly from scripts, even in the presence * of noise like e-mail addresses. @@ -97,7 +135,7 @@ public void testNoisyParameters() throws Exception { final ScriptModule scriptModule = scriptService.run("hello.bsizes", script, true).get(); - final Object output = scriptModule.getOutput("result"); + final Object output = scriptModule.getReturnValue(); if (output == null) fail("null result"); else if (!(output instanceof Integer)) { @@ -169,10 +207,6 @@ public void testParameters() { assertItem("buffer", StringBuilder.class, null, ItemIO.BOTH, true, true, null, null, null, null, null, null, null, null, noChoices, buffer); - final ModuleItem result = info.getOutput("result"); - assertItem("result", Object.class, null, ItemIO.OUTPUT, true, true, null, - null, null, null, null, null, null, null, noChoices, result); - int inputCount = 0; final ModuleItem[] inputs = { log, sliderValue, animal, buffer }; for (final ModuleItem inItem : info.inputs()) { @@ -180,7 +214,7 @@ public void testParameters() { } int outputCount = 0; - final ModuleItem[] outputs = { buffer, result }; + final ModuleItem[] outputs = { buffer }; for (final ModuleItem outItem : info.outputs()) { assertSame(outputs[outputCount++], outItem); }