From 7cc0b56df02ec944dcc65913a1af1ceb63a83e86 Mon Sep 17 00:00:00 2001
From: Jan Eglinger <jan.eglinger@fmi.ch>
Date: Tue, 11 May 2021 21:28:20 +0200
Subject: [PATCH] CommandInfo: allow shadowed Service parameters

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.
---
 .../java/org/scijava/command/CommandInfo.java |  4 +-
 .../org/scijava/command/CommandInfoTest.java  | 38 ++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/scijava/command/CommandInfo.java b/src/main/java/org/scijava/command/CommandInfo.java
index 8d843d457..806c6f2ae 100644
--- a/src/main/java/org/scijava/command/CommandInfo.java
+++ b/src/main/java/org/scijava/command/CommandInfo.java
@@ -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;
@@ -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));
diff --git a/src/test/java/org/scijava/command/CommandInfoTest.java b/src/test/java/org/scijava/command/CommandInfoTest.java
index 63a698b23..822686f86 100644
--- a/src/test/java/org/scijava/command/CommandInfoTest.java
+++ b/src/test/java/org/scijava/command/CommandInfoTest.java
@@ -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;
 
@@ -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(
@@ -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. */
@@ -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
+		}
+	}
 }