Properly escape or quote values when doing completion

Fixes #54

spring-shell2-core/src/main/java/org/springframework/shell2/CompletingParsedLine.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2017 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.shell2;
+
+import org.jline.reader.ParsedLine;
+
+/**
+ * An extension of {@link ParsedLine} that, being aware of the quoting and escaping rules
+ * of the {@link org.jline.reader.Parser} that produced it, knows if and how a completion candidate
+ * should be escaped/quoted.
+ *
+ * @author Eric Bottard
+ */
+@FunctionalInterface
+public interface CompletingParsedLine {
+
+	public CharSequence emit(CharSequence candidate);
+}

spring-shell2-core/src/main/java/org/springframework/shell2/CompletionContext.java

@@ -32,7 +32,12 @@ public class CompletionContext {
 
 	private final int position;
 
-
+	/**
+	 *
+	 * @param words words in the buffer, excluding words for the command name
+	 * @param wordIndex the index of the word the cursor is in
+	 * @param position the position inside the current word where the cursor is
+	 */
 	public CompletionContext(List<String> words, int wordIndex, int position) {
 		this.words = words;
 		this.wordIndex = wordIndex;
@@ -63,7 +68,7 @@ public class CompletionContext {
 	 * Return the whole word the cursor is in, or {@code null} if the cursor is past the last word.
 	 */
 	public String currentWord() {
-		return wordIndex < words.size() ? words.get(wordIndex) : null;
+		return wordIndex >= 0 && wordIndex < words.size() ? words.get(wordIndex) : null;
 	}
 
 	public String currentWordUpToCursor() {

spring-shell2-core/src/main/java/org/springframework/shell2/ExtendedDefaultParser.java

@@ -0,0 +1,322 @@
+/*
+ * Copyright 2017 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.shell2;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+import java.util.function.Predicate;
+
+import org.jline.reader.EOFError;
+import org.jline.reader.ParsedLine;
+import org.jline.reader.Parser;
+
+/**
+ * Shameful copy-paste of JLine's {@link org.jline.reader.impl.DefaultParser} which
+ * creates {@link CompletingParsedLine}.
+ *
+ * @author Original JLine author
+ * @author Eric Bottard
+ */
+public class ExtendedDefaultParser implements Parser {
+
+	private char[] quoteChars = { '\'', '"' };
+
+	private char[] escapeChars = { '\\' };
+
+	private boolean eofOnUnclosedQuote;
+
+	private boolean eofOnEscapedNewLine;
+
+	public void setQuoteChars(final char[] chars) {
+		this.quoteChars = chars;
+	}
+
+	public char[] getQuoteChars() {
+		return this.quoteChars;
+	}
+
+	public void setEscapeChars(final char[] chars) {
+		this.escapeChars = chars;
+	}
+
+	public char[] getEscapeChars() {
+		return this.escapeChars;
+	}
+
+	public void setEofOnUnclosedQuote(boolean eofOnUnclosedQuote) {
+		this.eofOnUnclosedQuote = eofOnUnclosedQuote;
+	}
+
+	public boolean isEofOnUnclosedQuote() {
+		return eofOnUnclosedQuote;
+	}
+
+	public void setEofOnEscapedNewLine(boolean eofOnEscapedNewLine) {
+		this.eofOnEscapedNewLine = eofOnEscapedNewLine;
+	}
+
+	public boolean isEofOnEscapedNewLine() {
+		return eofOnEscapedNewLine;
+	}
+
+	public ParsedLine parse(final String line, final int cursor, ParseContext context) {
+		List<String> words = new LinkedList<>();
+		StringBuilder current = new StringBuilder();
+		int wordCursor = -1;
+		int wordIndex = -1;
+		int quoteStart = -1;
+
+		for (int i = 0; (line != null) && (i < line.length()); i++) {
+			// once we reach the cursor, set the
+			// position of the selected index
+			if (i == cursor) {
+				wordIndex = words.size();
+				// the position in the current argument is just the
+				// length of the current argument
+				wordCursor = current.length();
+			}
+
+			if (quoteStart < 0 && isQuoteChar(line, i)) {
+				// Start a quote block
+				quoteStart = i;
+			}
+			else if (quoteStart >= 0) {
+				// In a quote block
+				if (line.charAt(quoteStart) == line.charAt(i) && !isEscaped(line, i)) {
+					// End the block; arg could be empty, but that's fine
+					words.add(current.toString());
+					current.setLength(0);
+					quoteStart = -1;
+				}
+				else if (!isEscapeChar(line, i)) {
+					// Take the next character
+					current.append(line.charAt(i));
+				}
+			}
+			else {
+				// Not in a quote block
+				if (isDelimiter(line, i)) {
+					if (current.length() > 0) {
+						words.add(current.toString());
+						current.setLength(0); // reset the arg
+					}
+				}
+				else if (!isEscapeChar(line, i)) {
+					current.append(line.charAt(i));
+				}
+			}
+		}
+
+		if (current.length() > 0 || cursor == line.length()) {
+			words.add(current.toString());
+		}
+
+		if (cursor == line.length()) {
+			wordIndex = words.size() - 1;
+			wordCursor = words.get(words.size() - 1).length();
+		}
+
+		if (eofOnEscapedNewLine && isEscapeChar(line, line.length() - 1)) {
+			throw new EOFError(-1, -1, "Escaped new line", "newline");
+		}
+		if (eofOnUnclosedQuote && quoteStart >= 0 && context != ParseContext.COMPLETE) {
+			throw new EOFError(-1, -1, "Missing closing quote", line.charAt(quoteStart) == '\'' ? "quote" : "dquote");
+		}
+
+		String openingQuote = quoteStart >= 0 ? line.substring(quoteStart, quoteStart + 1) : null;
+		return new ExtendedArgumentList(line, words, wordIndex, wordCursor, cursor, openingQuote);
+	}
+
+	/**
+	 * Returns true if the specified character is a whitespace parameter. Check to ensure
+	 * that the character is not escaped by any of {@link #getQuoteChars}, and is not
+	 * escaped by ant of the {@link #getEscapeChars}, and returns true from
+	 * {@link #isDelimiterChar}.
+	 *
+	 * @param buffer The complete command buffer
+	 * @param pos The index of the character in the buffer
+	 * @return True if the character should be a delimiter
+	 */
+	public boolean isDelimiter(final CharSequence buffer, final int pos) {
+		return !isQuoted(buffer, pos) && !isEscaped(buffer, pos) && isDelimiterChar(buffer, pos);
+	}
+
+	public boolean isQuoted(final CharSequence buffer, final int pos) {
+		return false;
+	}
+
+	public boolean isQuoteChar(final CharSequence buffer, final int pos) {
+		if (pos < 0) {
+			return false;
+		}
+
+		for (int i = 0; (quoteChars != null) && (i < quoteChars.length); i++) {
+			if (buffer.charAt(pos) == quoteChars[i]) {
+				return !isEscaped(buffer, pos);
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * Check if this character is a valid escape char (i.e. one that has not been escaped)
+	 */
+	public boolean isEscapeChar(final CharSequence buffer, final int pos) {
+		if (pos < 0) {
+			return false;
+		}
+
+		for (int i = 0; (escapeChars != null) && (i < escapeChars.length); i++) {
+			if (buffer.charAt(pos) == escapeChars[i]) {
+				return !isEscaped(buffer, pos); // escape escape
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * Check if a character is escaped (i.e. if the previous character is an escape)
+	 *
+	 * @param buffer the buffer to check in
+	 * @param pos the position of the character to check
+	 * @return true if the character at the specified position in the given buffer is an
+	 * escape character and the character immediately preceding it is not an escape
+	 * character.
+	 */
+	public boolean isEscaped(final CharSequence buffer, final int pos) {
+		if (pos <= 0) {
+			return false;
+		}
+
+		return isEscapeChar(buffer, pos - 1);
+	}
+
+	/**
+	 * Returns true if the character at the specified position if a delimiter. This method
+	 * will only be called if the character is not enclosed in any of the
+	 * {@link #getQuoteChars}, and is not escaped by ant of the {@link #getEscapeChars}.
+	 * To perform escaping manually, override {@link #isDelimiter} instead.
+	 */
+	public boolean isDelimiterChar(CharSequence buffer, int pos) {
+		return Character.isWhitespace(buffer.charAt(pos));
+	}
+
+	/**
+	 * The result of a delimited buffer.
+	 *
+	 * @author <a href="mailto:mwp1@cornell.edu">Marc Prud'hommeaux</a>
+	 */
+	public class ExtendedArgumentList implements ParsedLine, CompletingParsedLine {
+		private final String line;
+
+		private final List<String> words;
+
+		private final int wordIndex;
+
+		private final int wordCursor;
+
+		private final int cursor;
+
+		private final String openingQuote;
+
+		public ExtendedArgumentList(final String line, final List<String> words, final int wordIndex,
+				final int wordCursor, final int cursor, final String openingQuote) {
+			this.line = line;
+			this.words = Collections.unmodifiableList(Objects.requireNonNull(words));
+			this.wordIndex = wordIndex;
+			this.wordCursor = wordCursor;
+			this.cursor = cursor;
+			this.openingQuote = openingQuote;
+		}
+
+		public int wordIndex() {
+			return this.wordIndex;
+		}
+
+		public String word() {
+			// TODO: word() should always be contained in words()
+			if ((wordIndex < 0) || (wordIndex >= words.size())) {
+				return "";
+			}
+			return words.get(wordIndex);
+		}
+
+		public int wordCursor() {
+			return this.wordCursor;
+		}
+
+		public List<String> words() {
+			return this.words;
+		}
+
+		public int cursor() {
+			return this.cursor;
+		}
+
+		public String line() {
+			return line;
+		}
+
+		@Override
+		public CharSequence emit(CharSequence candidate) {
+			StringBuilder sb = new StringBuilder(candidate);
+			Predicate<Integer> needToBeEscaped;
+			// Completion is protected by an opening quote:
+			// Delimiters (spaces) don't need to be escaped, nor do other quotes, but everything else does.
+			// Also, close the quote at the end
+			if (openingQuote != null) {
+				needToBeEscaped = i -> isRawEscapeChar(sb.charAt(i)) || String.valueOf(sb.charAt(i)).equals(openingQuote);
+			} // No quote protection, need to escape everything: delimiter chars (spaces), quote chars
+			// and escapes themselves
+			else {
+				needToBeEscaped = i -> isDelimiterChar(sb, i) || isRawEscapeChar(sb.charAt(i)) || isRawQuoteChar(sb.charAt(i));
+			}
+			for (int i = 0; i < sb.length(); i++) {
+				if (needToBeEscaped.test(i)) {
+					sb.insert(i++, escapeChars[0]);
+				}
+			}
+			if (openingQuote != null) {
+				sb.append(openingQuote);
+			}
+			return sb;
+		}
+	}
+
+	private boolean isRawEscapeChar(char key) {
+		for (char e : escapeChars) {
+			if (e == key) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private boolean isRawQuoteChar(char key) {
+		for (char e : quoteChars) {
+			if (e == key) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+}

spring-shell2-core/src/main/java/org/springframework/shell2/JLineShell.java

@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import javax.annotation.PostConstruct;
@@ -96,7 +97,7 @@ public class JLineShell implements Shell {
 			methodTargets.putAll(resolver.resolve());
 		}
 
-		DefaultParser parser = new DefaultParser();
+		ExtendedDefaultParser parser = new ExtendedDefaultParser();
 		parser.setEofOnUnclosedQuote(true);
 		parser.setEofOnEscapedNewLine(true);
 
@@ -143,27 +144,15 @@ public class JLineShell implements Shell {
 					continue;
 				}
 			}
-			String separator = "";
-			StringBuilder candidateCommand = new StringBuilder();
-			MethodTarget methodTarget = null;
-			int c = 0;
-			int wordsUsedForCommandKey = 0;
+
+			String line = lineReader.getParsedLine().line();
+			String command = findLongestCommand(line);
+
 			List<String> words = lineReader.getParsedLine().words();
-			words = sanitizeInput(words);
-
-			for (String word : words) {
-				c++;
-				candidateCommand.append(separator).append(word);
-				MethodTarget t = methodTargets.get(candidateCommand.toString());
-				if (t != null) {
-					methodTarget = t;
-					wordsUsedForCommandKey = c;
-				}
-				separator = " ";
-			}
-
-			if (methodTarget != null) {
-				List<String> wordsForArgs = words.subList(wordsUsedForCommandKey, words.size());
+			if (command != null) {
+				int wordsUsedForCommandKey = command.split(" ").length;
+				MethodTarget methodTarget = methodTargets.get(command);
+				List<String> wordsForArgs = sanitizeInput(words.subList(wordsUsedForCommandKey, words.size()));
 				Method method = methodTarget.getMethod();
 
 				Object result = null;
@@ -180,7 +169,7 @@ public class JLineShell implements Shell {
 
 			}
 			else {
-				System.out.println("No command found for " + words);
+				System.out.println("No command found for " + sanitizeInput(words));
 			}
 		}
 	}
@@ -252,24 +241,23 @@ public class JLineShell implements Shell {
 		public void complete(LineReader reader, ParsedLine line, List<Candidate> candidates) {
 			String prefix = reader.getBuffer().upToCursor();
 
-			String best = methodTargets.keySet().stream()
-					.filter(c -> prefix.startsWith(c))
-					.reduce("", (c1, c2) -> c1.length() > c2.length() ? c1 : c2);
-			if (best.equals("")) { // no command found
-				List<Candidate> result = commandsStartingWith(prefix);
-				candidates.addAll(result);
+			// Find the longest match for a command name with words in the buffer
+			String best = findLongestCommand(prefix);
+			if (best == null) { // no command found
+				candidates.addAll(commandsStartingWith(prefix));
 				return;
-			} // trying to complete args for command <best>,
+			} // if we're here, we're either trying to complete args for command <best> (will fall through)
 			// or trying to complete command whose name starts with <best> (which also happens to be a command)
 			else if (prefix.equals(best)) {
 				candidates.addAll(commandsStartingWith(best));
-				return;
 			} // valid command (<best>) followed by a suffix (but not necessarily [<space> args*])
 			else if (!prefix.startsWith(best + " ")) {
 				// must be an invalid command, can't do anything
 				return;
 			}
 
+			CompletingParsedLine cpl = (line instanceof CompletingParsedLine) ? ((CompletingParsedLine) line) : t -> t;
+
 			// Try to complete arguments
 			MethodTarget methodTarget = methodTargets.get(best);
 			List<String> words = line.words();
@@ -286,9 +274,9 @@ public class JLineShell implements Shell {
 				resolver.complete(methodParameter, context)
 						.stream()
 						.map(completion -> new Candidate(
-										completion.value(),
+										cpl.emit(completion.value()).toString(),
 										completion.displayText(),
-										"Comp for parameter " + resolver.describe(methodParameter).toString(),
+										"Value for parameter " + resolver.describe(methodParameter).toString(),
 										resolver.describe(methodParameter).help(),
 										null, null, true)
 						)
@@ -307,4 +295,16 @@ public class JLineShell implements Shell {
 			return new Candidate(command, command, "Available commands", methodTarget.getHelp(), null, null, true);
 		}
 	}
+
+	/**
+	 * Returns the longest command that can be matched as first word(s) in the given buffer.
+	 *
+	 * @return a valid command name, or {@literal null} if none matched
+	 */
+	private String findLongestCommand(String prefix) {
+		String result = methodTargets.keySet().stream()
+			.filter(prefix::startsWith)
+			.reduce("", (c1, c2) -> c1.length() > c2.length() ? c1 : c2);
+		return "".equals(result) ? null : result;
+	}
 }

spring-shell2-samples/src/main/java/org/springframework/shell2/samples/standard/Commands.java

@@ -18,10 +18,17 @@ package org.springframework.shell2.samples.standard;
 
 import java.lang.annotation.ElementType;
 import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
 
+import org.springframework.core.MethodParameter;
+import org.springframework.shell2.CompletionContext;
+import org.springframework.shell2.CompletionProposal;
 import org.springframework.shell2.standard.ShellComponent;
 import org.springframework.shell2.standard.ShellMethod;
 import org.springframework.shell2.standard.ShellOption;
+import org.springframework.shell2.standard.ValueProviderSupport;
+import org.springframework.stereotype.Component;
 
 /**
  * Example commands for the Shell 2 Standard resolver.
@@ -31,6 +38,11 @@ import org.springframework.shell2.standard.ShellOption;
 @ShellComponent("")
 public class Commands {
 
+	@ShellMethod(help = "a command whose name looks the same as another one")
+	public void helpMeOut() {
+		System.out.println("You can go");
+	}
+
 	@ShellMethod(help = "it's cool")
 	public void foo(String bar) {
 
@@ -41,9 +53,9 @@ public class Commands {
 		System.out.println("You passed " + force);
 	}
 
-	@ShellMethod(help = "something else")
-	public void somethingElse() {
-
+	@ShellMethod(help = "test completion of special values")
+	public void quote(@ShellOption(valueProvider = FunnyValuesProvider.class) String text) {
+		System.out.println("You said " + text);
 	}
 
 	@ShellMethod(help = "add stuff")
@@ -62,3 +74,23 @@ public class Commands {
 	}
 }
 
+/**
+ * A {@link org.springframework.shell2.standard.ValueProvider} that emits values with special characters
+ * (quotes, escapes, <em>etc.</em>)
+ *
+ * @author Eric Bottard
+ */
+@Component
+class FunnyValuesProvider extends ValueProviderSupport {
+
+	private final static String[] VALUES = new String[] {
+		"hello world",
+		"I'm quoting \"The Daily Mail\"",
+		"10 \\ 3 = 3"
+	};
+
+	@Override
+	public List<CompletionProposal> complete(MethodParameter parameter, CompletionContext completionContext, String[] hints) {
+		return Arrays.stream(VALUES).map(CompletionProposal::new).collect(Collectors.toList());
+	}
+}