From 2fe26403e7c20d4ca0662c2b7adea4b0eb86f78f Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 13 Jun 2019 14:28:11 -0700 Subject: [PATCH 1/5] Add test for using switch version in multi command I noticed that in 1.3.0-RC2, the following commands worked: ++2.11.12 ;test ;++2.11.12;test but ++2.11.12; test did not. This issue is fixed in the next commit. --- sbt/src/sbt-test/actions/cross-multi-parser/build.sbt | 1 + .../cross-multi-parser/src/main/scala/cross/Build.scala | 3 +++ sbt/src/sbt-test/actions/cross-multi-parser/test | 5 +++++ 3 files changed, 9 insertions(+) create mode 100644 sbt/src/sbt-test/actions/cross-multi-parser/build.sbt create mode 100644 sbt/src/sbt-test/actions/cross-multi-parser/src/main/scala/cross/Build.scala create mode 100644 sbt/src/sbt-test/actions/cross-multi-parser/test diff --git a/sbt/src/sbt-test/actions/cross-multi-parser/build.sbt b/sbt/src/sbt-test/actions/cross-multi-parser/build.sbt new file mode 100644 index 000000000..1d47987b5 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-multi-parser/build.sbt @@ -0,0 +1 @@ +crossScalaVersions := Seq[String]("2.11.12", "2.12.8") diff --git a/sbt/src/sbt-test/actions/cross-multi-parser/src/main/scala/cross/Build.scala b/sbt/src/sbt-test/actions/cross-multi-parser/src/main/scala/cross/Build.scala new file mode 100644 index 000000000..e6dcf2377 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-multi-parser/src/main/scala/cross/Build.scala @@ -0,0 +1,3 @@ +package cross + +object Build \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/cross-multi-parser/test b/sbt/src/sbt-test/actions/cross-multi-parser/test new file mode 100644 index 000000000..df8b309d2 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-multi-parser/test @@ -0,0 +1,5 @@ +> ++2.11.12; compile + +> ++ 2.12.8 ; compile; + +> ++ 2.12.8 ; compile From ccfc3d7bc78b5543bcb75def1511f47cf8e13b1e Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 13 Jun 2019 16:31:35 -0700 Subject: [PATCH 2/5] 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' --- .../src/main/scala/sbt/BasicCommands.scala | 21 ++++++++++++++----- main-command/src/main/scala/sbt/Command.scala | 3 +-- main-command/src/main/scala/sbt/State.scala | 3 ++- main/src/main/scala/sbt/Main.scala | 1 - .../sbt-test/actions/multi-command/build.sbt | 2 ++ .../actions/multi-command/project/Build.scala | 8 +++++++ sbt/src/sbt-test/actions/multi-command/test | 7 +++++++ 7 files changed, 36 insertions(+), 9 deletions(-) diff --git a/main-command/src/main/scala/sbt/BasicCommands.scala b/main-command/src/main/scala/sbt/BasicCommands.scala index daea8ed99..841244227 100644 --- a/main-command/src/main/scala/sbt/BasicCommands.scala +++ 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) + } } } diff --git a/main-command/src/main/scala/sbt/Command.scala b/main-command/src/main/scala/sbt/Command.scala index ea616cb6e..89ba7a375 100644 --- a/main-command/src/main/scala/sbt/Command.scala +++ b/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 diff --git a/main-command/src/main/scala/sbt/State.scala b/main-command/src/main/scala/sbt/State.scala index 62a0920a1..af81a96de 100644 --- a/main-command/src/main/scala/sbt/State.scala +++ b/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 { diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index c883c42dd..f36fc1322 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -226,7 +226,6 @@ object BuiltinCommands { export, boot, initialize, - BasicCommands.multi, act, continuous, clearCaches, diff --git a/sbt/src/sbt-test/actions/multi-command/build.sbt b/sbt/src/sbt-test/actions/multi-command/build.sbt index 0aff2d364..bff6d9d45 100644 --- a/sbt/src/sbt-test/actions/multi-command/build.sbt +++ b/sbt/src/sbt-test/actions/multi-command/build.sbt @@ -12,3 +12,5 @@ taskThatFails := { throw new IllegalArgumentException("") () } + +checkInput := checkInputImpl.evaluated \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/multi-command/project/Build.scala b/sbt/src/sbt-test/actions/multi-command/project/Build.scala index 5190dc191..06176933e 100644 --- a/sbt/src/sbt-test/actions/multi-command/project/Build.scala +++ b/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")) + } } diff --git a/sbt/src/sbt-test/actions/multi-command/test b/sbt/src/sbt-test/actions/multi-command/test index 8bc828c03..f2626b4d6 100644 --- a/sbt/src/sbt-test/actions/multi-command/test +++ b/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 From 4c814752fb502d7d6d825e1371314af7f0517f53 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 13 Jun 2019 23:48:51 -0700 Subject: [PATCH 3/5] Support braces in multi command parser We run into issues if we naively split the command input on ';' and treat each part as a separate command unless the ';' is inside of a string because it is also valid to have ';'s inside of braced expressions, e.g. `set foo := { val x = 1; x + 1 }`. There was no parser for expressions enclosed in braces. I add one that should parse any expression wrapped in braces so long as each opening brace is matched by a closing brace. The parser returns the original expression. This allows the multi parser to ignore ';' inside of '{...}'. I had to rework the scripted tests to individually run 'reload' and 'setUpScripted' because the new parser rejects setUpScripted because it isn't a valid command until reload has run. --- .../sbt/internal/util/complete/Parsers.scala | 24 +++++++++ .../src/main/scala/sbt/BasicCommands.scala | 51 ++++++++++--------- main-command/src/main/scala/sbt/State.scala | 21 +++++++- .../src/test/scala/sbt/MultiParserSpec.scala | 39 ++++++++++++-- .../sbt-test/actions/multi-command/build.sbt | 6 ++- sbt/src/sbt-test/actions/multi-command/test | 8 +++ .../sbt/scriptedtest/ScriptedTests.scala | 26 ++++++---- 7 files changed, 132 insertions(+), 43 deletions(-) diff --git a/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala b/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala index eb68fa679..037abccc8 100644 --- a/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala +++ b/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala @@ -219,6 +219,30 @@ trait Parsers { (DQuoteChar ~> (NotDQuoteBackslashClass | EscapeSequence).+.string <~ DQuoteChar | (DQuoteChar ~ DQuoteChar) ^^^ "") + /** + * Parses a brace enclosed string and, if each opening brace is matched with a closing brace, + * it returns the entire string including the braces. + * + * @param open the opening character, e.g. '{' + * @param close the closing character, e.g. '}' + * @return a parser for the brace encloosed string. + */ + private[sbt] def braces(open: Char, close: Char): Parser[String] = { + val notDelim = charClass(c => c != open && c != close).*.string + def impl(): Parser[String] = { + (open ~ (notDelim ~ close).?).flatMap { + case (l, Some((content, r))) => Parser.success(l + content + r) + case (l, None) => + ((notDelim ~ impl()).map { + case (leftPrefix, nestedBraces) => leftPrefix + nestedBraces + }.+ ~ notDelim ~ close).map { + case ((nested, suffix), r) => l + nested.mkString + suffix + r + } + } + } + impl() + } + /** * Parses a single escape sequence into the represented Char. * Escapes start with a backslash and are followed by `u` for a [[UnicodeEscape]] or by `b`, `t`, `n`, `f`, `r`, `"`, `'`, `\` for standard escapes. diff --git a/main-command/src/main/scala/sbt/BasicCommands.scala b/main-command/src/main/scala/sbt/BasicCommands.scala index 841244227..3a01ac7b2 100644 --- a/main-command/src/main/scala/sbt/BasicCommands.scala +++ b/main-command/src/main/scala/sbt/BasicCommands.scala @@ -155,29 +155,33 @@ object BasicCommands { private[sbt] def multiParserImpl(state: Option[State]): Parser[List[String]] = { val nonSemi = charClass(_ != ';', "not ';'") - val semi = token(';' ~> OptSpace) - val nonQuote = charClass(_ != '"', label = "not '\"'") - val cmdPart = token( - ((nonSemi & nonQuote).map(_.toString) | StringEscapable.map(c => s""""$c"""")).+, - hide = const(true) - ) - 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) + val semi = token(OptSpace ~> ';' ~> OptSpace) + val nonDelim = charClass(c => c != '"' && c != '{' && c != '}', label = "not '\"', '{', '}'") + val cmdPart = OptSpace ~> matched( + token( + (nonSemi & nonDelim).map(_.toString) | StringEscapable | braces('{', '}'), + hide = const(true) + ).+ + ) <~ OptSpace + val strictParser: Option[Parser[String]] = + state.map(s => OptSpace ~> matched(s.nonMultiParsers) <~ OptSpace) + val parser = strictParser.map(sp => sp & cmdPart).getOrElse(cmdPart) + /* + * There are two cases that need to be handled separately: + * 1) There are multiple commands separated by at least one semicolon with an optional + * leading semicolon. + * 2) There is a leading semicolon, but only on one command + * These have to be handled separately because the performance degrades badly if the first + * case is implemented with the following parser: + * (semi.? ~> ((combinedParser <~ semi).* ~ combinedParser <~ semi.?) + */ + (semi.? ~> (parser <~ semi).+ ~ (parser <~ semi.?).?).flatMap { + case (prefix, last) => + (prefix ++ last).toList.map(_.trim).filter(_.nonEmpty) match { + case Nil => Parser.failure("No commands were parsed") + case cmds => Parser.success(cmds) } - } + } | semi ~> parser.map(_.trim :: Nil) <~ semi.? } def multiParser(s: State): Parser[List[String]] = multiParserImpl(Some(s)) @@ -440,8 +444,7 @@ object BasicCommands { def aliasBody(name: String, value: String)(state: State): Parser[() => State] = { val aliasRemoved = removeAlias(state, name) // apply the alias value to the commands of `state` except for the alias to avoid recursion (#933) - val partiallyApplied = - Parser(Command.combine(aliasRemoved.definedCommands)(aliasRemoved))(value) + val partiallyApplied = Parser(aliasRemoved.combinedParser)(value) val arg = matched(partiallyApplied & (success(()) | (SpaceClass ~ any.*))) // by scheduling the expanded alias instead of directly executing, // we get errors on the expanded string (#598) diff --git a/main-command/src/main/scala/sbt/State.scala b/main-command/src/main/scala/sbt/State.scala index af81a96de..d83d76b1c 100644 --- a/main-command/src/main/scala/sbt/State.scala +++ b/main-command/src/main/scala/sbt/State.scala @@ -41,8 +41,25 @@ final case class State( currentCommand: Option[Exec], next: State.Next ) extends Identity { - private[sbt] lazy val nonMultiParsers = Command.combine(definedCommands)(this) - lazy val combinedParser = (BasicCommands.multiApplied(this) | nonMultiParsers).failOnException + /* + * The `~` and `alias` commands effectively run other commands so they need to be run before + * the multi parser. For example, if the user runs `~foo;bar` and the multi parser runs before + * the `~` parser, then then it will be parsed as two commands `~foo` and `bar`. By running + * the high priority commands before the multi parser, we ensure that the high priority commands + * parse the full command input. Any other command that runs other commands would likely need + * to be added to this list but at the time of writing this comment {~, alias} are the two + * commands that we know need this special treatment. + * + * TODO: add a structured way of indicating that a command needs to run before the multi parser. + */ + private[this] val highPriorityCommands = Set("~", "alias") + private[this] lazy val (highPriority, regularPriority) = + definedCommands.partition(_.nameOption.exists(highPriorityCommands)) + private[this] lazy val highPriorityParser = Command.combine(highPriority)(this) + private[this] lazy val lowPriorityParser = Command.combine(regularPriority)(this) + private[sbt] lazy val nonMultiParsers = highPriorityParser | lowPriorityParser + lazy val combinedParser = + highPriorityParser | BasicCommands.multiApplied(this) | lowPriorityParser def source: Option[CommandSource] = currentCommand match { diff --git a/main-command/src/test/scala/sbt/MultiParserSpec.scala b/main-command/src/test/scala/sbt/MultiParserSpec.scala index cb2962399..4ade2ddc0 100644 --- a/main-command/src/test/scala/sbt/MultiParserSpec.scala +++ b/main-command/src/test/scala/sbt/MultiParserSpec.scala @@ -59,12 +59,41 @@ class MultiParserSpec extends FlatSpec with Matchers { it should "not parse single commands without leading ';'" in { "foo".parseEither shouldBe Left("Expected ';'\nfoo\n ^") "foo bar baz".parseEither shouldBe Left("Expected ';'\nfoo bar baz\n ^") - "foo bar baz;".parseEither shouldBe - Left("Expected not ';'\nExpected '\"'\nfoo bar baz;\n ^") - "foo;".parseEither shouldBe Left("Expected not ';'\nExpected '\"'\nfoo;\n ^") + } + it should "not parse empty commands" in { + assert(";;;".parseEither.isLeft) + assert("; ; ;".parseEither.isLeft) } it should "parse commands with trailing semi-colon" in { - "foo;bar;".parse shouldBe Seq("foo", "bar") - "foo; bar ;".parse shouldBe Seq("foo", "bar") + assert("foo;bar;".parse == Seq("foo", "bar")) + assert("foo; bar ;".parse == Seq("foo", "bar")) + } + val consecutive = "{ { val x = 1}; { val x = 2 } }" + val oneBrace = "set foo := { val x = 1; x + 1 }" + val twoBrace = "set foo := { val x = { val y = 2; y + 2 }; x + 1 }" + val threeBrace = "set foo := { val x = { val y = 2; { val z = 3; y + 2 } }; x + 1 }" + val doubleBrace = "set foo := { val x = { val y = 2; y + 2 }; { x + 1 } }" + val tripleBrace = "set foo := { val x = { val y = 2; y + 2 }; val y = { x + 1 }; { z + y } }" + val emptyBraces = "{{{{}}}}" + it should "parse commands with braces" in { + assert(s"$consecutive;".parse == consecutive :: Nil) + assert(s"$oneBrace;".parse == oneBrace :: Nil) + assert(s"$twoBrace;".parse == twoBrace :: Nil) + assert(s"$threeBrace;".parse == threeBrace :: Nil) + assert(s"$doubleBrace;".parse == doubleBrace :: Nil) + assert(s"$tripleBrace;".parse == tripleBrace :: Nil) + assert(s"$emptyBraces;".parse == emptyBraces :: Nil) + } + it should "parse multiple commands with braces" in { + s"compile; $consecutive".parse shouldBe "compile" :: consecutive :: Nil + s"compile; $consecutive ; test".parse shouldBe "compile" :: consecutive :: "test" :: Nil + } + it should "not parse unclosed braces" in { + val extraRight = "{ { val x = 1}}{ val x = 2 } }" + assert(s"compile; $extraRight".parseEither.isLeft) + val extraLeft = "{{{ val x = 1}{ val x = 2 } }" + assert(s"compile; $extraLeft".parseEither.isLeft) + val unmatchedEmptyBraces = "{{{{}}}" + assert(s"compile; $unmatchedEmptyBraces".parseEither.isLeft) } } diff --git a/sbt/src/sbt-test/actions/multi-command/build.sbt b/sbt/src/sbt-test/actions/multi-command/build.sbt index bff6d9d45..5db2dc44f 100644 --- a/sbt/src/sbt-test/actions/multi-command/build.sbt +++ b/sbt/src/sbt-test/actions/multi-command/build.sbt @@ -13,4 +13,8 @@ taskThatFails := { () } -checkInput := checkInputImpl.evaluated \ No newline at end of file +checkInput := checkInputImpl.evaluated + +val dynamicTask = taskKey[Unit]("dynamic input task") + +dynamicTask := { println("not yet et") } diff --git a/sbt/src/sbt-test/actions/multi-command/test b/sbt/src/sbt-test/actions/multi-command/test index f2626b4d6..0e0e84d40 100644 --- a/sbt/src/sbt-test/actions/multi-command/test +++ b/sbt/src/sbt-test/actions/multi-command/test @@ -1,3 +1,11 @@ +> ;set dynamicTask := { println("1"); println("2") }; dynamicTask + +-> ; set dynamicTask := { throw new IllegalStateException("fail") }; dynamicTask + +> set dynamicTask := { println("1"); println("2") }; dynamicTask + +-> set dynamicTask := { throw new IllegalStateException("fail") }; dynamicTask + > ; setStringValue baz > ; checkStringValue baz diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala index 03d871e56..cf077a326 100644 --- a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala +++ b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala @@ -183,9 +183,11 @@ final class ScriptedTests( s"$group/$name" match { case "actions/add-alias" => LauncherBased // sbt/Package$ case "actions/cross-multiproject" => LauncherBased // tbd - case "actions/external-doc" => LauncherBased // sbt/Package$ - case "actions/input-task" => LauncherBased // sbt/Package$ - case "actions/input-task-dyn" => LauncherBased // sbt/Package$ + case "actions/cross-multi-parser" => + LauncherBased // java.lang.ClassNotFoundException: javax.tools.DiagnosticListener when run with java 11 and an old sbt launcher + case "actions/external-doc" => LauncherBased // sbt/Package$ + case "actions/input-task" => LauncherBased // sbt/Package$ + case "actions/input-task-dyn" => LauncherBased // sbt/Package$ case gn if gn.startsWith("classloader-cache/") => LauncherBased // This should be tested using launcher case "compiler-project/dotty-compiler-plugin" => LauncherBased // sbt/Package$ @@ -324,18 +326,20 @@ final class ScriptedTests( IO.write(tempTestDir / "project" / "InstrumentScripted.scala", pluginImplementation) def sbtHandlerError = sys error "Missing sbt handler. Scripted is misconfigured." val sbtHandler = handlers.getOrElse('>', sbtHandlerError) - val commandsToRun = ";reload;setUpScripted" - val statement = Statement(commandsToRun, Nil, successExpected = true, line = -1) // Run reload inside the hook to reuse error handling for pending tests val wrapHook = (file: File) => { preHook(file) - try runner.processStatement(sbtHandler, statement, states) - catch { - case t: Throwable => - val newMsg = "Reload for scripted batch execution failed." - throw new TestException(statement, newMsg, t) - } + Seq("reload", "setUpScripted") + .map(Statement(_, Nil, successExpected = true, line = -1)) + .foreach { statement => + try runner.processStatement(sbtHandler, statement, states) + catch { + case t: Throwable => + val newMsg = "Reload for scripted batch execution failed." + throw new TestException(statement, newMsg, t) + } + } } commonRunTest(label, tempTestDir, wrapHook, handlers, runner, states, buffer) From ff16d76ad3c60e7617502f182adf19706dfd6a36 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Fri, 14 Jun 2019 11:45:00 -0700 Subject: [PATCH 4/5] Remove matchers from MultiParserSpec We've been trying to move away from the wordy dsl and stick with bare assertions. --- .../src/test/scala/sbt/MultiParserSpec.scala | 67 ++++++++++--------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/main-command/src/test/scala/sbt/MultiParserSpec.scala b/main-command/src/test/scala/sbt/MultiParserSpec.scala index 4ade2ddc0..1beb4059d 100644 --- a/main-command/src/test/scala/sbt/MultiParserSpec.scala +++ b/main-command/src/test/scala/sbt/MultiParserSpec.scala @@ -7,7 +7,7 @@ package sbt -import org.scalatest.{ FlatSpec, Matchers } +import org.scalatest.FlatSpec import sbt.internal.util.complete.Parser object MultiParserSpec { @@ -18,47 +18,54 @@ object MultiParserSpec { } } import sbt.MultiParserSpec._ -class MultiParserSpec extends FlatSpec with Matchers { +class MultiParserSpec extends FlatSpec { "parsing" should "parse single commands" in { - ";foo".parse shouldBe Seq("foo") - "; foo".parse shouldBe Seq("foo") + assert(";foo".parse == Seq("foo")) + assert("; foo".parse == Seq("foo")) } it should "parse multiple commands" in { - ";foo;bar".parse shouldBe Seq("foo", "bar") + assert(";foo;bar".parse == Seq("foo", "bar")) } it should "parse single command with leading spaces" in { - "; foo".parse shouldBe Seq("foo") + assert("; foo".parse == Seq("foo")) } it should "parse multiple commands with leading spaces" in { - "; foo;bar".parse shouldBe Seq("foo", "bar") - "; foo; bar".parse shouldBe Seq("foo", "bar") - ";foo; bar".parse shouldBe Seq("foo", "bar") + assert("; foo;bar".parse == Seq("foo", "bar")) + assert("; foo; bar".parse == Seq("foo", "bar")) + assert(";foo; bar".parse == Seq("foo", "bar")) + assert("; foo ; bar ; baz".parse == Seq("foo", "bar", "baz")) } it should "parse command with string literal" in { - "; foo \"barbaz\"".parse shouldBe Seq("foo \"barbaz\"") - "; foo \"bar;baz\"".parse shouldBe Seq("foo \"bar;baz\"") - "; foo \"barbaz\"; bar".parse shouldBe Seq("foo \"barbaz\"", "bar") - "; foo \"barbaz\"; bar \"blah\"".parse shouldBe Seq("foo \"barbaz\"", "bar \"blah\"") - "; foo \"bar;baz\"; bar".parse shouldBe Seq("foo \"bar;baz\"", "bar") - "; foo \"bar;baz\"; bar \"buzz\"".parse shouldBe Seq("foo \"bar;baz\"", "bar \"buzz\"") - "; foo \"bar;baz\"; bar \"buzz;two\"".parse shouldBe Seq("foo \"bar;baz\"", "bar \"buzz;two\"") - """; foo "bar;\"baz\""; bar""".parse shouldBe Seq("""foo "bar;\"baz\""""", "bar") - """; setStringValue "foo;bar"; checkStringValue "foo;bar"""".parse shouldBe - Seq("""setStringValue "foo;bar"""", """checkStringValue "foo;bar"""") + assert("; foo \"barbaz\"".parse == Seq("foo \"barbaz\"")) + assert("; foo \"bar;baz\"".parse == Seq("foo \"bar;baz\"")) + assert("; foo \"barbaz\"; bar".parse == Seq("foo \"barbaz\"", "bar")) + assert("; foo \"barbaz\"; bar \"blah\"".parse == Seq("foo \"barbaz\"", "bar \"blah\"")) + assert("; foo \"bar;baz\"; bar".parse == Seq("foo \"bar;baz\"", "bar")) + assert("; foo \"bar;baz\"; bar \"buzz\"".parse == Seq("foo \"bar;baz\"", "bar \"buzz\"")) + assert( + "; foo \"bar;baz\"; bar \"buzz;two\"".parse == Seq("foo \"bar;baz\"", "bar \"buzz;two\"") + ) + assert("""; foo "bar;\"baz\""; bar""".parse == Seq("""foo "bar;\"baz\""""", "bar")) + assert( + """; setStringValue "foo;bar"; checkStringValue "foo;bar"""".parse == + Seq("""setStringValue "foo;bar"""", """checkStringValue "foo;bar"""") + ) } it should "parse commands without leading ';'" in { - "setStringValue foo; setStringValue bar".parse shouldBe Seq( - "setStringValue foo", - "setStringValue bar" + assert( + "setStringValue foo; setStringValue bar".parse == Seq( + "setStringValue foo", + "setStringValue bar" + ) ) - "foo; bar".parse shouldBe Seq("foo", "bar") - "foo bar ;bar".parse shouldBe Seq("foo bar", "bar") - "foo \"a;b\"; bar".parse shouldBe Seq("foo \"a;b\"", "bar") - " foo ; bar \"b;c\"".parse shouldBe Seq("foo", "bar \"b;c\"") + assert("foo; bar".parse == Seq("foo", "bar")) + assert("foo bar ;bar".parse == Seq("foo bar", "bar")) + assert("foo \"a;b\"; bar".parse == Seq("foo \"a;b\"", "bar")) + assert(" foo ; bar \"b;c\"".parse == Seq("foo", "bar \"b;c\"")) } it should "not parse single commands without leading ';'" in { - "foo".parseEither shouldBe Left("Expected ';'\nfoo\n ^") - "foo bar baz".parseEither shouldBe Left("Expected ';'\nfoo bar baz\n ^") + assert("foo".parseEither == Left("Expected ';'\nfoo\n ^")) + assert("foo bar baz".parseEither == Left("Expected ';'\nfoo bar baz\n ^")) } it should "not parse empty commands" in { assert(";;;".parseEither.isLeft) @@ -85,8 +92,8 @@ class MultiParserSpec extends FlatSpec with Matchers { assert(s"$emptyBraces;".parse == emptyBraces :: Nil) } it should "parse multiple commands with braces" in { - s"compile; $consecutive".parse shouldBe "compile" :: consecutive :: Nil - s"compile; $consecutive ; test".parse shouldBe "compile" :: consecutive :: "test" :: Nil + assert(s"compile; $consecutive".parse == "compile" :: consecutive :: Nil) + assert(s"compile; $consecutive ; test".parse == "compile" :: consecutive :: "test" :: Nil) } it should "not parse unclosed braces" in { val extraRight = "{ { val x = 1}}{ val x = 2 } }" From 30a16d1e10f1014cb7a32c0219bdc6b233a9fb3d Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Fri, 14 Jun 2019 12:46:44 -0700 Subject: [PATCH 5/5] Update Continuous to directly use multi parser It didn't really make sense for Continuous to use the other command parser and then reparse the results. I was able to slightly simplify things by using the multi parser directly. --- .../main/scala/sbt/internal/Continuous.scala | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala index 4b70ccd83..35bd50f20 100644 --- a/main/src/main/scala/sbt/internal/Continuous.scala +++ b/main/src/main/scala/sbt/internal/Continuous.scala @@ -17,7 +17,6 @@ import sbt.BasicCommandStrings.{ continuousBriefHelp, continuousDetail } -import sbt.BasicCommands.otherCommandParser import sbt.Def._ import sbt.Keys._ import sbt.Scope.Global @@ -105,8 +104,8 @@ private[sbt] object Continuous extends DeprecatedContinuous { */ private[sbt] def continuous: Command = Command(ContinuousExecutePrefix, continuousBriefHelp, continuousDetail)(continuousParser) { - case (s, (initialCount, command)) => - runToTermination(s, command, initialCount, isCommand = true) + case (s, (initialCount, commands)) => + runToTermination(s, commands, initialCount, isCommand = true) } /** @@ -117,9 +116,9 @@ private[sbt] object Continuous extends DeprecatedContinuous { */ private[sbt] def continuousTask: Def.Initialize[InputTask[StateTransform]] = Def.inputTask { - val (initialCount, command) = continuousParser.parsed + val (initialCount, commands) = continuousParser.parsed new StateTransform( - runToTermination(state.value, command, initialCount, isCommand = false) + runToTermination(state.value, commands, initialCount, isCommand = false) ) } @@ -137,15 +136,20 @@ private[sbt] object Continuous extends DeprecatedContinuous { 10000 ) - private[this] val continuousParser: State => Parser[(Int, String)] = { + private[this] val continuousParser: State => Parser[(Int, Seq[String])] = { def toInt(s: String): Int = Try(s.toInt).getOrElse(0) // This allows us to re-enter the watch with the previous count. val digitParser: Parser[Int] = (Parsers.Space.* ~> matched(Parsers.Digit.+) <~ Parsers.Space.*).map(toInt) state => - val ocp = otherCommandParser(state) - (digitParser.? ~ ocp).map { case (i, s) => (i.getOrElse(0), s) } + val ocp = BasicCommands.multiParserImpl(Some(state)) | + BasicCommands.otherCommandParser(state).map(_ :: Nil) + (digitParser.? ~ ocp).flatMap { + case (i, cmds) if cmds.exists(_.nonEmpty) => + Parser.success((i.getOrElse(0), cmds.filter(_.nonEmpty))) + case (_, cmds) => Parser.failure("Couldn't parse any commands") + } } /** @@ -202,8 +206,8 @@ private[sbt] object Continuous extends DeprecatedContinuous { .getOrElse(throw exception) } - private[sbt] def setup[R](state: State, command: String)( - f: (Seq[String], State, Seq[(String, State, () => Boolean)], Seq[String]) => R + private[sbt] def setup[R](state: State, commands: Seq[String])( + f: (State, Seq[(String, State, () => Boolean)], Seq[String]) => R ): R = { // First set up the state so that we can capture whether or not a task completed successfully // or if it threw an Exception (we lose the actual exception, but that should still be printed @@ -253,14 +257,6 @@ private[sbt] object Continuous extends DeprecatedContinuous { ) } - // We support multiple commands in watch, so it's necessary to run the command string through - // the multi parser. - val trimmed = command.trim - val commands = Parser.parse(trimmed, BasicCommands.multiParserImpl(Some(s))) match { - case Left(_) => trimmed :: Nil - case Right(c) => c - } - // Convert the command strings to runnable tasks, which are represented by // () => Try[Boolean]. val taskParser = s.combinedParser @@ -274,7 +270,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { case Left(c) => (i :+ c, v) } } - f(commands, s, valid, invalid) + f(s, valid, invalid) } private[this] def withCharBufferedStdIn[R](f: InputStream => R): R = { @@ -319,7 +315,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { private[sbt] def runToTermination( state: State, - command: String, + commands: Seq[String], count: Int, isCommand: Boolean ): State = withCharBufferedStdIn { in => @@ -341,7 +337,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { stateWithRepo.put(persistentFileStampCache, fileStampCache) else stateWithRepo ) - setup(fullState, command) { (commands, s, valid, invalid) => + setup(fullState, commands) { (s, valid, invalid) => EvaluateTask.withStreams(extracted.structure, s)(_.use(streams in Global) { streams => implicit val logger: Logger = streams.log if (invalid.isEmpty) { @@ -369,7 +365,8 @@ private[sbt] object Continuous extends DeprecatedContinuous { e.throwable.getStackTrace.foreach(e => logger.error(e.toString)) case _ => } - callbacks.onTermination(terminationAction, command, currentCount.get(), state) + val fullCommand = commands.mkString("; ") + callbacks.onTermination(terminationAction, fullCommand, currentCount.get(), state) } finally { callbacks.onExit() }