Skip to content

Commit

Permalink
CommandInfo: allow shadowed Service parameters
Browse files Browse the repository at this point in the history
When a command extends another command, we often have both classes use specific services.
It's better to be able to name these Service parameters consistently, than to avoid shadowing parameters at all costs. So let's exclude all Service parameters from the validity check.
  • Loading branch information
imagejan committed May 11, 2021
1 parent c4cb3a7 commit 7cc0b56
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
4 changes: 3 additions & 1 deletion src/main/java/org/scijava/command/CommandInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.scijava.plugin.Parameter;
import org.scijava.plugin.Plugin;
import org.scijava.plugin.PluginInfo;
import org.scijava.service.Service;
import org.scijava.util.ClassUtils;
import org.scijava.util.StringMaker;
import org.scijava.util.Types;
Expand Down Expand Up @@ -460,7 +461,8 @@ private void checkFields(final Class<?> type) {
}

final String name = f.getName();
if (inputMap.containsKey(name) || outputMap.containsKey(name)) {
if ((inputMap.containsKey(name) || outputMap.containsKey(name))
&& !Service.class.isAssignableFrom(f.getType())) {
// NB: Shadowed parameters are bad because they are ambiguous.
final String error = "Invalid duplicate parameter: " + f;
problems.add(new ValidityProblem(error));
Expand Down
38 changes: 37 additions & 1 deletion src/test/java/org/scijava/command/CommandInfoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@
import java.util.Arrays;
import java.util.Iterator;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.scijava.Context;
import org.scijava.command.CommandInfoTest.CommandWithEnumParam.Choice;
import org.scijava.log.LogService;
import org.scijava.module.ModuleItem;
import org.scijava.plugin.Parameter;

Expand All @@ -53,14 +55,20 @@
*/
public class CommandInfoTest {

private Context ctx;
private CommandService commandService;

@Before
public void setUp() {
final Context ctx = new Context(CommandService.class);
ctx = new Context(CommandService.class);
commandService = ctx.getService(CommandService.class);
}

@After
public void tearDown() {
ctx.dispose();
}

@Test
public void testEnumParam() {
final CommandInfo info = commandService.getCommand(
Expand Down Expand Up @@ -88,6 +96,12 @@ public void testEnumParam() {
choice.getChoices());
}

@Test
public void testDuplicateServiceParameters() {
CommandInfo commandInfo = new CommandInfo(ExtendedServiceCommand.class);
assertTrue(commandInfo.isValid());
}

// -- Helper classes --

/** A command with an enum parameter. */
Expand All @@ -112,4 +126,26 @@ public void run() {
// NB: No implementation needed.
}
}

private static class ServiceCommand implements Command {

@Parameter
private LogService logService;

@Override
public void run() {
// do nothing
}
}

private static class ExtendedServiceCommand extends ServiceCommand {

@Parameter
private LogService logService;

@Override
public void run() {
// do nothing
}
}
}

0 comments on commit 7cc0b56

Please sign in to comment.