Validate commands in multiparser

It was reported in https://github.com/sbt/sbt/issues/4808 that compared
to 1.2.8, sbt 1.3.0-RC2 will truncate the command args of an input task
that contains semicolons. This is actually intentional, but not
completely robust. For sbt >= 1.3.0, we are making ';' syntactically
meaningful. This means that it always represents a command separator
_unless_ it is inside of a quoted string. To enforce this, the multi parser
will effectively split the input on ';', it will then validate that each
command that it extracted is valid. If not, it throws an exception. If
the input is not a multi command, then parsing fails with a normal
failure.

I removed the multi command from the state's defined commands and reworked
State.combinedParser to explicitly first try multi parsing and fall back
to the regular combined parser if it is a regular command. If the multi
parser throws an uncaught exception, parsing fails even if the regular
parser could have successfully parsed the command. The reason is so that
we do not ever allow the user to evaluate, say 'run a;b'. Otherwise the
behavior would be inconsitent when the user runs 'compile; run a;b'

main-command/src/main/scala/sbt/BasicCommands.scala

@@ -161,11 +161,22 @@ object BasicCommands {
       ((nonSemi & nonQuote).map(_.toString) | StringEscapable.map(c => s""""$c"""")).+,
       hide = const(true)
     )
-    def commandParser = state.map(s => (s.combinedParser & cmdPart) | cmdPart).getOrElse(cmdPart)
-    val part = semi.flatMap(_ => matched(commandParser) <~ token(OptSpace)).map(_.trim)
-    (cmdPart.? ~ part.+ <~ semi.?).map {
-      case (Some(h), t) => h.mkString.trim +: t.toList
-      case (_, t)       => t.toList
+    lazy val combinedParser =
+      state.map(s => (s.nonMultiParsers & cmdPart) | cmdPart).getOrElse(cmdPart)
+    val part = semi.flatMap(_ => matched(combinedParser) <~ token(OptSpace))
+    (matched(cmdPart).? ~ part.+ <~ semi.?).map {
+      case (h, t) =>
+        val commands = (h ++ t).toList.map(_.trim)
+        commands.collect { case c if Parser.parse(c, combinedParser).isLeft => c } match {
+          case Nil     => commands
+          case invalid =>
+            /*
+             * This is to prevent the user from running something like 'run a;b'. Instead, they
+             * must do 'run "a;b"' if they wish to have semicolongs in the task input.
+             */
+            val msg = s"Couldn't parse commands: ${invalid.mkString("'", "', '", "'")}"
+            throw new IllegalArgumentException(msg)
+        }
     }
   }
 

main-command/src/main/scala/sbt/Command.scala

@@ -178,8 +178,7 @@ object Command {
     )
 
   def process(command: String, state: State): State = {
-    val parser = combine(state.definedCommands)
-    parse(command, parser(state)) match {
+    parse(command, state.combinedParser) match {
       case Right(s) => s() // apply command.  command side effects happen here
       case Left(errMsg) =>
         state.log error errMsg

main-command/src/main/scala/sbt/State.scala

@@ -41,7 +41,8 @@ final case class State(
     currentCommand: Option[Exec],
     next: State.Next
 ) extends Identity {
-  lazy val combinedParser = Command.combine(definedCommands)(this)
+  private[sbt] lazy val nonMultiParsers = Command.combine(definedCommands)(this)
+  lazy val combinedParser = (BasicCommands.multiApplied(this) | nonMultiParsers).failOnException
 
   def source: Option[CommandSource] =
     currentCommand match {

main/src/main/scala/sbt/Main.scala

@@ -226,7 +226,6 @@ object BuiltinCommands {
       export,
       boot,
       initialize,
-      BasicCommands.multi,
       act,
       continuous,
       clearCaches,

sbt/src/sbt-test/actions/multi-command/build.sbt

@@ -12,3 +12,5 @@ taskThatFails := {
   throw new IllegalArgumentException("")
   ()
 }
+
+checkInput := checkInputImpl.evaluated

sbt/src/sbt-test/actions/multi-command/project/Build.scala

@@ -1,11 +1,14 @@
 import sbt._
 
+import sbt.internal.util.complete.Parser._
+
 object Build {
   private[this] var string: String = ""
   private[this] val stringFile = file("string.txt")
   val setStringValue = inputKey[Unit]("set a global string to a value")
   val checkStringValue = inputKey[Unit]("check the value of a global")
   val taskThatFails = taskKey[Unit]("this should fail")
+  val checkInput = inputKey[Unit]("this should extract arguments that are semicolon delimited")
   def setStringValueImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask {
     string = Def.spaceDelimited().parsed.mkString(" ").trim
     IO.write(stringFile, string)
@@ -15,4 +18,9 @@ object Build {
     assert(string == actual)
     assert(IO.read(stringFile) == string)
   }
+
+  def checkInputImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask {
+    val actual = (charClass(_ != ';').+ <~ ';'.?).map(_.mkString.trim).+.parsed
+    assert(actual == Seq("foo"))
+  }
 }

sbt/src/sbt-test/actions/multi-command/test

@@ -19,3 +19,10 @@
 -> setStringValue foo; taskThatFails; setStringValue bar
 
 > checkStringValue foo
+
+# this fails even though the checkInput parser would parse the input into Seq("foo", "bar")
+-> checkInput foo; bar
+
+> checkInput foo
+
+> compile; checkInput foo