From 78082e25c7cd697f8cd8df230096448d4af4622b Mon Sep 17 00:00:00 2001
From: Eric Bottard <eric.bottard+github@gmail.com>
Date: Wed, 2 Apr 2014 16:33:09 +0200
Subject: [PATCH] SHL-113: Don't report ambiguity on an exact match

---
 .../shell/core/SimpleParser.java              | 24 ++++++++++---
 .../shell/core/SimpleParserTests.java         | 36 +++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/springframework/shell/core/SimpleParser.java b/src/main/java/org/springframework/shell/core/SimpleParser.java
index 7bc9ef47..c7905944 100644
--- a/src/main/java/org/springframework/shell/core/SimpleParser.java
+++ b/src/main/java/org/springframework/shell/core/SimpleParser.java
@@ -103,6 +103,7 @@ public class SimpleParser implements Parser {
 		return optionsKeys;
 	}
 
+	@Override
 	public ParseResult parse(final String rawInput) {
 		synchronized (mutex) {
 			Assert.notNull(rawInput, "Raw input required");
@@ -124,12 +125,25 @@ public class SimpleParser implements Parser {
 				}
 				return null;
 			}
+			MethodTarget methodTarget = null;
 			if (matchingTargets.size() > 1) {
-				LOGGER.warning("Ambigious command '" + input + "' (for assistance press "
-						+ AbstractShell.completionKeys + " or type \"hint\" then hit ENTER)");
-				return null;
+				// Any prefix of a valid command will do. Don't fail if the user used
+				// the exact key of a command though.
+				for (MethodTarget candidate : matchingTargets) {
+					if (candidate.getKey().equals(input) || input.startsWith(candidate.getKey() + " ")) {
+						methodTarget = candidate;
+						break;
+					}
+				}
+				if (methodTarget == null) {
+					LOGGER.warning("Ambigious command '" + input + "' (for assistance press "
+							+ AbstractShell.completionKeys + " or type \"hint\" then hit ENTER)");
+					return null;
+				}
+			}
+			else {
+				methodTarget = matchingTargets.iterator().next();
 			}
-			MethodTarget methodTarget = matchingTargets.iterator().next();
 
 			// Argument conversion time
 			Annotation[][] parameterAnnotations = methodTarget.getMethod().getParameterAnnotations();
@@ -514,6 +528,7 @@ public class SimpleParser implements Parser {
 		return null; // Not a match
 	}
 
+	@Override
 	public int complete(String buffer, int cursor, final List<String> candidates) {
 		final List<Completion> completions = new ArrayList<Completion>();
 		int result = completeAdvanced(buffer, cursor, completions);
@@ -523,6 +538,7 @@ public class SimpleParser implements Parser {
 		return result;
 	}
 
+	@Override
 	public int completeAdvanced(String buffer, int cursor, final List<Completion> candidates) {
 		synchronized (mutex) {
 			Assert.notNull(buffer, "Buffer required");
diff --git a/src/test/java/org/springframework/shell/core/SimpleParserTests.java b/src/test/java/org/springframework/shell/core/SimpleParserTests.java
index 7049a119..42e754a0 100644
--- a/src/test/java/org/springframework/shell/core/SimpleParserTests.java
+++ b/src/test/java/org/springframework/shell/core/SimpleParserTests.java
@@ -37,6 +37,7 @@ import org.hamcrest.Matcher;
 import org.junit.Test;
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
+import org.springframework.shell.event.ParseResult;
 
 /**
  * Tests for parsing and completion logic.
@@ -368,6 +369,23 @@ public class SimpleParserTests {
 		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 \"def\" ")))));
 	}
 
+	/**
+	 * @see https://jira.spring.io/browse/SHL-113
+	 */
+	@Test
+	public void testFalseAmbiguity() {
+		parser.add(new SamePrefixCommands());
+		ParseResult result = parser.parse("foo");
+		assertThat(result.getMethod().getName(), equalTo("foo"));
+	}
+
+	@Test
+	public void testRealAmbiguity() {
+		parser.add(new SamePrefixCommands());
+		ParseResult result = parser.parse("fo");
+		assertThat(result, nullValue(ParseResult.class));
+	}
+
 	/**
 	 * Return a matcher that asserts that a completion, when added to {@link #buffer} at the given {@link #offset},
 	 * indeed matches the provided matcher.
@@ -375,6 +393,7 @@ public class SimpleParserTests {
 	private Matcher<Completion> completionThat(final Matcher<String> matcher) {
 		return new DiagnosingMatcher<Completion>() {
 
+			@Override
 			public void describeTo(Description description) {
 				description.appendText("a completion that ").appendDescriptionOf(matcher);
 			}
@@ -448,6 +467,20 @@ public class SimpleParserTests {
 		}
 	}
 
+	public static class SamePrefixCommands implements CommandMarker {
+		@CliCommand(value = "foo")
+		public String foo(@CliOption(key = "")
+		String arg) {
+			return "foo " + arg;
+		}
+
+		@CliCommand(value = "fooBar")
+		public String fooBar(@CliOption(key = "")
+		String arg) {
+			return "fooBar " + arg;
+		}
+	}
+
 	public static class StringCompletions implements Converter<String> {
 
 		private final List<String> completions;
@@ -463,14 +496,17 @@ public class SimpleParserTests {
 			this.canContinue = canContinue;
 		}
 
+		@Override
 		public boolean supports(Class<?> type, String optionContext) {
 			return type == String.class;
 		}
 
+		@Override
 		public String convertFromText(String value, Class<?> targetType, String optionContext) {
 			return value;
 		}
 
+		@Override
 		public boolean getAllPossibleValues(List<Completion> completions, Class<?> targetType, String existingData,
 				String optionContext, MethodTarget target) {
 			for (String s : this.completions) {