More improvements to the way CLI handles comments in multi-line statements

ksql-cli/src/main/java/io/confluent/ksql/cli/console/CommentStripper.java

@@ -29,37 +29,84 @@ private CommentStripper() {
   }
 
   static String strip(final String line) {
+    return new Parser(line).parse();
+  }
+
+  private static final class Parser {
+    private final String line;
+    private int pos;
+
+    private Parser(final String line) {
+      this.line = line;
+      this.pos = 0;
+    }
+
+    private String parse() {
+      final String firstPart = strip();
+      if (done()) {
+        // Early out if contains no comments part way through:
+        return firstPart;
+      }
 
-    char lastChar = NONE;
-    char lastQuote = NONE;
-    for (int i = 0; i != line.length(); ++i) {
-      final char c = line.charAt(i);
-      switch (c) {
-        case '`':
-        case '\'':
-        case '"':
-          if (lastQuote == c) {
-            // Matching pair:
-            lastQuote = NONE;
-          } else if (lastQuote == NONE) {
-            lastQuote = c;
-          }
-          break;
-
-        case '-':
-          if (lastChar == '-' && lastQuote == NONE) {
-            // Found unquoted comment marker:
-            return line.substring(0, i - 1).trim();
-          }
-          break;
-
-        default:
-          break;
+      final StringBuilder builder = new StringBuilder(line.length());
+      builder.append(firstPart);
+      while (!done()) {
+        builder.append(strip());
       }
+      return builder.toString();
+    }
 
-      lastChar = c;
+    private boolean done() {
+      return pos == line.length();
     }
 
-    return line;
+    private String strip() {
+      final int start = pos;
+      char lastChar = NONE;
+      char lastQuote = NONE;
+      for (; pos != line.length(); ++pos) {
+        final char c = line.charAt(pos);
+
+        switch (c) {
+          case '`':
+          case '\'':
+          case '"':
+            if (lastQuote == c) {
+              // Matching pair:
+              lastQuote = NONE;
+            } else if (lastQuote == NONE) {
+              lastQuote = c;
+            }
+            break;
+
+          case '-':
+            if (lastChar == '-' && lastQuote == NONE) {
+              // Found unquoted comment marker:
+              return trimComment(start);
+            }
+            break;
+
+          default:
+            break;
+        }
+
+        lastChar = c;
+      }
+
+      return line.substring(start);
+    }
+
+    private String trimComment(final int partStart) {
+      final String part = line.substring(partStart, pos - 1).trim();
+
+      final int newLine = line.indexOf(System.lineSeparator(), pos + 1);
+      if (newLine == -1) {
+        pos = line.length();
+      } else {
+        pos = newLine;
+      }
+
+      return part;
+    }
   }
 }

ksql-cli/src/main/java/io/confluent/ksql/cli/console/JLineReader.java

@@ -68,9 +68,7 @@ public Iterable<? extends History.Entry> getHistory() {
 
   @Override
   public String readLine() {
-    final String line = lineReader
-        .readLine(prompt)
-        .replace("\n", " ");
+    final String line = lineReader.readLine(prompt);
 
     addToHistory(line);
     return line;

ksql-cli/src/main/java/io/confluent/ksql/cli/console/KsqlLineParser.java

@@ -44,10 +44,6 @@ final class KsqlLineParser implements Parser {
   public ParsedLine parse(final String line, final int cursor, final ParseContext context) {
     final ParsedLine parsed = delegate.parse(line, cursor, context);
 
-    if (cliCmdPredicate.test(line)) {
-      return parsed;
-    }
-
     if (context != ParseContext.ACCEPT_LINE) {
       return parsed;
     }
@@ -57,6 +53,10 @@ public ParsedLine parse(final String line, final int cursor, final ParseContext
       return parsed;
     }
 
+    if (cliCmdPredicate.test(bare)) {
+      return parsed;
+    }
+
     if (!bare.endsWith(TERMINATION_CHAR)) {
       throw new EOFError(-1, -1, "Missing termination char", "termination char");
     }

ksql-cli/src/test/java/io/confluent/ksql/cli/console/CommentStripperTest.java

@@ -106,6 +106,7 @@ public void shouldCorrectHandleEscapedSingleQuotes() {
     assertThat(CommentStripper.strip(line), is("'this isn''t a comment -- the first quote isn''t closed'"));
     assertThat(CommentStripper.strip(line2), is("'''this isn''t a comment -- the first quote isn''t closed'"));
   }
+
   @Test
   public void shouldCorrectHandleEscapedDoubleQuotes() {
     // Given:
@@ -116,4 +117,24 @@ public void shouldCorrectHandleEscapedDoubleQuotes() {
     assertThat(CommentStripper.strip(line), is("\"this isn''t a comment -- the first quote isn''t closed\""));
     assertThat(CommentStripper.strip(line2), is("\"\"\"this isn''t a comment -- the first quote isn''t closed\""));
   }
+
+  @Test
+  public void shouldHandleMultiLine() {
+    // Given:
+    final String line = "some multi-line\n"
+        + "statement";
+
+    // Then:
+    assertThat(CommentStripper.strip(line), is("some multi-line\nstatement"));
+  }
+
+  @Test
+  public void shouldTerminateCommentAtNewLine() {
+    // Given:
+    final String line = "some multi-line -- this is a comment\n"
+        + "statement";
+
+    // Then:
+    assertThat(CommentStripper.strip(line), is("some multi-line\nstatement"));
+  }
 }

ksql-cli/src/test/java/io/confluent/ksql/cli/console/JLineReaderTest.java

@@ -194,7 +194,7 @@ public void shouldHandleMultiLineWithoutContinuationChar() throws Exception {
     final List<String> commands = readAllLines(reader);
 
     // Then:
-    assertThat(commands, contains("select * from foo;"));
+    assertThat(commands, contains("select *\nfrom foo;"));
   }
 
   @Test
@@ -209,22 +209,50 @@ public void shouldHandleMultiLineWithOpenQuotes() throws Exception {
     final List<String> commands = readAllLines(reader);
 
     // Then:
-    assertThat(commands, contains("select * 'string that ends in termination char; ' from foo;"));
+    assertThat(commands, contains("select * 'string that ends in termination char;\n' from foo;"));
   }
 
   @Test
   public void shouldHandleMultiLineWithComments() throws Exception {
     // Given:
     final JLineReader reader = createReaderForInput(
-        "select * '-- not a comment\n"
-            + "' from foo; -- some comment\n"
+        "-- first inline comment\n"
+            + "select * '-- not comment\n"
+            + "' -- second inline comment\n"
+            + "from foo; -- third inline comment\n"
+            + "-- forth inline comment\n"
+    );
+
+    // When:
+    final List<String> commands = readAllLines(reader);
+
+    // Then:
+    assertThat(commands, contains(
+        "-- first inline comment",
+        "select * '-- not comment\n' -- second inline comment\nfrom foo; -- third inline comment",
+        "-- forth inline comment"
+    ));
+  }
+
+  @Test
+  public void shouldHandleCliCommandsWithInlineComments() throws Exception {
+    // Given:
+    when(cliLinePredicate.test("Exit")).thenReturn(true);
+    final JLineReader reader = createReaderForInput(
+        "-- first inline comment\n"
+            + "Exit -- second inline comment\n"
+            + " -- third inline comment\n"
     );
 
     // When:
     final List<String> commands = readAllLines(reader);
 
     // Then:
-    assertThat(commands, contains("select * '-- not a comment ' from foo; -- some comment"));
+    assertThat(commands, contains(
+        "-- first inline comment",
+        "Exit -- second inline comment",
+        "-- third inline comment"
+    ));
   }
 
   @SuppressWarnings("InfiniteLoopStatement")

ksql-parser/src/test/java/io/confluent/ksql/parser/KsqlParserTest.java

@@ -1211,4 +1211,29 @@ public void shouldParseBracketedComment() {
     assertThat(statements, hasSize(1));
     assertThat(statements.get(0).getStatement(), is(instanceOf(ListStreams.class)));
   }
+
+  @Test
+  public void shouldParseMultiLineWithInlineComments() {
+    final String statementString =
+        "SHOW -- inline comment\n"
+        + "STREAMS;";
+
+    final List<PreparedStatement<?>> statements =  KSQL_PARSER.buildAst(statementString, metaStore);
+
+    assertThat(statements, hasSize(1));
+    assertThat(statements.get(0).getStatement(), is(instanceOf(ListStreams.class)));
+  }
+
+  @Test
+  public void shouldParseMultiLineWithInlineBracketedComments() {
+    final String statementString =
+        "SHOW /* inline\n"
+            + "comment */\n"
+            + "STREAMS;";
+
+    final List<PreparedStatement<?>> statements =  KSQL_PARSER.buildAst(statementString, metaStore);
+
+    assertThat(statements, hasSize(1));
+    assertThat(statements.get(0).getStatement(), is(instanceOf(ListStreams.class)));
+  }
 }