Add tests and fix completion corner cases

gradle.properties

@@ -11,7 +11,7 @@ jlineVersion=2.11
 
 
 # Testing
-junitVersion = 4.10
+junitVersion = 4.11
 mockitoVersion = 1.8.5
 hamcrestDateVersion = 0.9.3
 

src/main/java/org/springframework/shell/core/SimpleParser.java

@@ -545,7 +545,7 @@ public class SimpleParser implements Parser {
 
 			if (targets.isEmpty()) {
 				// Nothing matches the buffer they've presented
-				return cursor;
+				return -1;
 			}
 			if (targets.size() > 1) {
 				// Assist them locate a particular target
@@ -658,7 +658,7 @@ public class SimpleParser implements Parser {
 
 			// Handle if they are trying to find out the available option keys; always present option keys in order
 			// of their declaration on the method signature, thus we can stop when mandatory options are filled in
-			if (methodTarget.getRemainingBuffer().endsWith("--") && !tokenizer.lastValueHadQuote()) {
+			if (methodTarget.getRemainingBuffer().endsWith("--") && !tokenizer.lastValueIsStillBeingTyped()) {
 				boolean showAllRemaining = true;
 				for (CliOption include : unspecified) {
 					if (include.mandatory()) {
@@ -685,7 +685,7 @@ public class SimpleParser implements Parser {
 			// option key/value pair)
 			if (lastOptionKey == null
 					|| (!"".equals(lastOptionKey) && !"".equals(lastOptionValue) && translated.endsWith(" ") && !tokenizer
-							.lastValueHadQuote())) {
+							.lastValueIsStillBeingTyped())) {
 				// We have either NEVER specified an option key/value pair
 				// OR we have specified a full option key/value pair
 
@@ -749,11 +749,9 @@ public class SimpleParser implements Parser {
 			}
 
 			// Handle completing the option key they're presently typing
-			if ((lastOptionValue == null || "".equals(lastOptionValue))
-					&& !(translated.endsWith(" ") || translated.endsWith(" \""))) {
-				// Given we haven't got an option value of any form, and there's no space at the buffer end, we must
-				// still be typing an option key
-				// System.out.println("completing an option");
+			if ("".equals(lastOptionValue)) {
+				// Given we haven't got an option value of any form, we must
+				// still be typing an option key.
 				for (CliOption option : cliOptions) {
 					for (String value : option.key()) {
 						if (value != null && lastOptionKey != null
@@ -821,21 +819,12 @@ public class SimpleParser implements Parser {
 								}
 							}
 
-							String prefix = "";
-							if (!tokenizer.lastValueHadQuote() && !translated.endsWith(" ")) {
-								prefix = " ";
-							}
-							else if (tokenizer.lastValueHadQuote()) {
-								// Re-install opening quote if there was one
-								prefix = " \"";
-							}
-
 							// Only include in the candidates those results which are compatible with the present buffer
 							for (Completion currentValue : allValues) {
 								// We only provide a suggestion if the lastOptionValue == ""
 								if (!StringUtils.hasText(lastOptionValue)) {
 									// We should add the result, as they haven't typed anything yet
-									results.add(new Completion(prefix + currentValue.getValue() + suffix, currentValue
+									results.add(new Completion(currentValue.getValue() + suffix, currentValue
 											.getFormattedValue(), currentValue.getHeading(), currentValue.getOrder()));
 								}
 								else {
@@ -843,10 +832,12 @@ public class SimpleParser implements Parser {
 									// already typed it in full
 									if (currentValue.getValue().toLowerCase().startsWith(lastOptionValue.toLowerCase())
 											&& !lastOptionValue.equalsIgnoreCase(currentValue.getValue())
-											&& lastOptionValue.length() < currentValue.getValue().length()) {
-										results.add(new Completion(prefix + currentValue.getValue() + suffix,
-												currentValue.getFormattedValue(), currentValue.getHeading(),
-												currentValue.getOrder()));
+											&& lastOptionValue.length() < currentValue.getValue().length()
+											&& (tokenizer.getLastValueDelimiter() == ' ' || tokenizer
+													.lastValueIsStillBeingTyped())) {
+										results.add(new Completion(currentValue.getValue() + suffix, currentValue
+												.getFormattedValue(), currentValue.getHeading(), currentValue
+												.getOrder()));
 									}
 								}
 							}
@@ -889,18 +880,14 @@ public class SimpleParser implements Parser {
 								if (suggestion.equals(lastOptionValue)) {
 									// They have pressed TAB in the default value, and the default value has already
 									// been provided as an explicit option
-									return 0;
+									return -1;
 								}
 							}
 
 							if (results.size() > 0) {
 								candidates.addAll(results);
-								if (tokenizer.lastValueHadQuote()) {
-									return translated.lastIndexOf(" \"");
-								}
-								else {
-									return translated.lastIndexOf(" ") + 1;
-								}
+								return methodTarget.getKey().length() + " ".length()
+										+ tokenizer.getLastValueStartOffset();
 							}
 							return 0;
 						}
@@ -908,7 +895,7 @@ public class SimpleParser implements Parser {
 				}
 			}
 
-			return 0;
+			return -1;
 		}
 	}
 

src/main/java/org/springframework/shell/core/Tokenizer.java

@@ -54,7 +54,11 @@ public class Tokenizer {
 	/**
 	 * Used to indicate that the last value was indeed half enclosed in quotes. Useful so that parser can re-add it.
 	 */
-	private boolean lastValueHadQuote;
+	private boolean lastValueIsStillBeingTyped;
+
+	private char lastValueDelimiter;
+
+	private int lastValueStartOffset;
 
 	public Tokenizer(String text) {
 		this(text, false);
@@ -114,6 +118,9 @@ public class Tokenizer {
 			endDelimiter = '"';
 			pos++;
 		}
+		// So that it can be retrieved later (if this is actually the last value)
+		lastValueDelimiter = endDelimiter;
+		lastValueStartOffset = pos;
 		while (pos < buffer.length && buffer[pos] != endDelimiter) {
 			if (buffer[pos] == ESCAPE_CHAR && pos + 1 < buffer.length && buffer[pos + 1] == endDelimiter) {
 				sb.append(endDelimiter);
@@ -130,7 +137,7 @@ public class Tokenizer {
 				(buffer[pos - 1] != '"' || // quotes are not properly closed
 				sb.length() == 0)) { // BUT it's ok if consumed nothing (pos-1 is *opening* quote then)
 			if (allowUnbalancedLastQuotedValue) {
-				lastValueHadQuote = true;
+				lastValueIsStillBeingTyped = true;
 				return sb.toString();
 			}
 			else {
@@ -142,8 +149,25 @@ public class Tokenizer {
 		return sb.toString();
 	}
 
-	public boolean lastValueHadQuote() {
-		return lastValueHadQuote;
+	/**
+	 * Return the offset at which the last value seen started (NOT including any delimiter).
+	 */
+	public int getLastValueStartOffset() {
+		return lastValueStartOffset;
+	}
+
+	/**
+	 * Return the delimiter (space or quotes) that was (or is being) used for the last value.
+	 */
+	public char getLastValueDelimiter() {
+		return lastValueDelimiter;
+	}
+
+	/**
+	 * Return whether the last value was meant to be enclosed in quotes, but the closing quote has not been typed yet.
+	 */
+	public boolean lastValueIsStillBeingTyped() {
+		return lastValueIsStillBeingTyped;
 	}
 
 	/**
@@ -173,4 +197,14 @@ public class Tokenizer {
 		}
 		return new String(buffer, start, pos - start);
 	}
+
+	@Override
+	public String toString() {
+		StringBuilder result = new StringBuilder().append(buffer).append('\n');
+		for (int i = 0; i < lastValueStartOffset; i++) {
+			result.append(' ');
+		}
+		result.append('^');
+		return result.toString();
+	}
 }

src/test/java/org/springframework/shell/core/SimpleParserTests.java

@@ -0,0 +1,204 @@
+/*
+ * Copyright 2013 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.shell.core;
+
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.startsWith;
+import static org.junit.Assert.assertThat;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.hamcrest.Description;
+import org.hamcrest.DiagnosingMatcher;
+import org.hamcrest.Matcher;
+import org.junit.Test;
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+/**
+ * Tests for parsing and completion logic.
+ * 
+ * @author Eric Bottard
+ */
+public class SimpleParserTests {
+
+	private SimpleParser parser = new SimpleParser();
+
+	private int offset;
+
+	private String buffer;
+
+	private ArrayList<Completion> candidates = new ArrayList<Completion>();
+
+	@Test
+	public void testSimpleCommandNameCompletion() {
+		parser.add(new MyCommands());
+
+		buffer = "f";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(is(equalTo("foo")))));
+	}
+
+	@Test
+	public void testSimpleArgumentNameCompletion() {
+		parser.add(new MyCommands());
+
+		buffer = "bar --op";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 ")))));
+	}
+
+	@Test
+	public void testSimpleArgumentValueCompletion() {
+		parser.add(new MyCommands());
+		parser.add(new StringCompletions(Arrays.asList("abc", "def", "ghi")));
+
+		buffer = "bar --option1 a";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 abc")))));
+	}
+
+	@Test
+	public void testArgumentValueCompletionWhenQuoted() {
+		parser.add(new MyCommands());
+		parser.add(new StringCompletions(Arrays.asList("abc", "def", "ghi")));
+
+		buffer = "bar --option1 \"a";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 \"abc")))));
+	}
+
+	@Test
+	public void testCompletionInMiddleOfBuffer() {
+		parser.add(new MyCommands());
+
+		buffer = "bar --optimum";
+		offset = parser.completeAdvanced(buffer, "bar --opti".length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 ")))));
+
+	}
+
+	@Test
+	public void testArgumentValueCompletionWhenAmbiguity() {
+		parser.add(new MyCommands());
+		parser.add(new StringCompletions(Arrays.asList("abc", "def", "abd")));
+
+		buffer = "bar --option1 a";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(startsWith("bar --option1 ab"))));
+	}
+
+	@Test
+	public void testArgumentValueCompletionWhenAmbiguityUsingQuotes() {
+		parser.add(new MyCommands());
+		parser.add(new StringCompletions(Arrays.asList("abc", "def", "abd")));
+
+		buffer = "bar --option1 \"a";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 \"abc")))));
+		assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 \"abd")))));
+	}
+
+	@Test
+	public void testArgumentValueCompletionUnderstandsEndQuote() {
+		parser.add(new MyCommands());
+		parser.add(new StringCompletions(Arrays.asList("abc", "def", "abd")));
+
+		buffer = "bar --option1 \"ab\"";
+		offset = parser.completeAdvanced(buffer, buffer.length(), candidates);
+
+		assertThat(candidates, is(empty()));
+	}
+
+	/**
+	 * Return a matcher that asserts that a completion, when added to {@link #buffer} at the given {@link #offset},
+	 * indeed matches the provided matcher.
+	 */
+	private Matcher<Completion> completionThat(final Matcher<String> matcher) {
+		return new DiagnosingMatcher<Completion>() {
+
+			public void describeTo(Description description) {
+				description.appendText("a completion that ").appendDescriptionOf(matcher);
+			}
+
+			@Override
+			protected boolean matches(Object item, Description mismatchDescription) {
+				Completion completion = (Completion) item;
+				StringBuilder sb = new StringBuilder(buffer);
+				sb.setLength(offset);
+				sb.append(completion.getValue());
+				boolean match = matcher.matches(sb.toString());
+				mismatchDescription.appendText("result was ")
+						.appendValue(sb.insert(offset, '[').append(']').toString());
+				return match;
+			}
+		};
+	}
+
+	public static class MyCommands implements CommandMarker {
+
+		@CliCommand("foo")
+		public void foo() {
+
+		}
+
+		@CliCommand("bar")
+		public void bar(@CliOption(key = "option1")
+		String option1) {
+
+		}
+	}
+
+	public static class StringCompletions implements Converter<String> {
+
+		private final List<String> completions;
+
+		public StringCompletions(List<String> completions) {
+			this.completions = completions;
+		}
+
+		public boolean supports(Class<?> type, String optionContext) {
+			return type == String.class;
+		}
+
+		public String convertFromText(String value, Class<?> targetType, String optionContext) {
+			return value;
+		}
+
+		public boolean getAllPossibleValues(List<Completion> completions, Class<?> targetType, String existingData,
+				String optionContext, MethodTarget target) {
+			for (String s : this.completions) {
+				completions.add(new Completion(s));
+			}
+			return false;
+		}
+
+	}
+
+}

src/test/java/org/springframework/shell/core/TokenizerTests.java

@@ -132,7 +132,7 @@ public class TokenizerTests {
 		expected.put("foo", "bar bazz");
 		expected.put("bizz", "unfinished bizness ");
 		assertEquals(expected, result);
-		assertTrue(tokenizer.lastValueHadQuote());
+		assertTrue(tokenizer.lastValueIsStillBeingTyped());
 	}
 
 	@Test
@@ -143,7 +143,7 @@ public class TokenizerTests {
 		expected.put("foo", "bar bazz");
 		expected.put("bizz", "");
 		assertEquals(expected, result);
-		assertTrue(tokenizer.lastValueHadQuote());
+		assertTrue(tokenizer.lastValueIsStillBeingTyped());
 	}
 
 	@Test