Don't strip quotes in _sbt_ scripted command arguments

Stripping quotation marks makes it impossible to cleanly test certain
sbt features without resorting to weird hacks. For example:
> set Compile / scalacOptions += "-Xfatal-warnings"
did not work while
> set Compile / scalacOptions += '"-Xfatal-warnings"'
did.

I leave the single quote parser unchanged since single quotes are not
really used in sbt and so there is utility in leaving them as a way to
group arguments that should not be split apart.

This change should only affect the scripted tests in the sbt repo. We
can consider making stripQuotes = false the default for the plugin as
well.

internal/util-scripted/src/main/scala/sbt/internal/scripted/ScriptedTests.scala

@@ -58,8 +58,11 @@ object ScriptedRunnerImpl {
 final class ScriptedTests(
     resourceBaseDirectory: File,
     bufferLog: Boolean,
-    handlersProvider: HandlersProvider
+    handlersProvider: HandlersProvider,
+    stripQuotes: Boolean
 ) {
+  def this(resourceBaseDirectory: File, bufferLog: Boolean, handlersProvider: HandlersProvider) =
+    this(resourceBaseDirectory, bufferLog, handlersProvider, true)
   private val testResources = new Resources(resourceBaseDirectory)
   private val consoleAppender: ConsoleAppender = ConsoleAppender()
 
@@ -127,10 +130,10 @@ final class ScriptedTests(
     }
     val pendingString = if (pending) " [PENDING]" else ""
 
-    def runTest() = {
+    def runTest(): Unit = {
       val run = new ScriptRunner
       val parser = createParser()
-      run(parser.parse(file))
+      run(parser.parse(file, stripQuotes))
     }
     def testFailed(): Unit = {
       if (pending) buffered.clearBuffer() else buffered.stopBuffer()

internal/util-scripted/src/main/scala/sbt/internal/scripted/TestScriptParser.scala

@@ -50,11 +50,22 @@ class TestScriptParser(handlers: Map[Char, StatementHandler]) extends RegexParse
   if (handlers.keys.exists(key => key == '+' || key == '-'))
     sys.error("Start characters cannot be '+' or '-'")
 
+  @deprecated("Use variant that specifies whether to strip quotes or not", "1.4.0")
   def parse(scriptFile: File): List[(StatementHandler, Statement)] =
-    parse(read(scriptFile), Some(scriptFile.getAbsolutePath))
-  def parse(script: String): List[(StatementHandler, Statement)] = parse(script, None)
-  private def parse(script: String, label: Option[String]): List[(StatementHandler, Statement)] = {
-    parseAll(statements, script) match {
+    parse(scriptFile, stripQuotes = true)
+  def parse(scriptFile: File, stripQuotes: Boolean): List[(StatementHandler, Statement)] =
+    parse(read(scriptFile), Some(scriptFile.getAbsolutePath), stripQuotes)
+  @deprecated("Use variant that specifies whether to strip quotes or not", "1.4.0")
+  def parse(script: String): List[(StatementHandler, Statement)] =
+    parse(script, None, stripQuotes = true)
+  def parse(script: String, stripQuotes: Boolean): List[(StatementHandler, Statement)] =
+    parse(script, None, stripQuotes)
+  private def parse(
+      script: String,
+      label: Option[String],
+      stripQuotes: Boolean
+  ): List[(StatementHandler, Statement)] = {
+    parseAll(statements(stripQuotes), script) match {
       case Success(result, next) => result
       case err: NoSuccess => {
         val labelString = label.map("'" + _ + "' ").getOrElse("")
@@ -63,15 +74,21 @@ class TestScriptParser(handlers: Map[Char, StatementHandler]) extends RegexParse
     }
   }
 
+  @deprecated("Use variant that specifies whether to strip quotes or not", "1.4.0")
   lazy val statements = rep1(space ~> statement <~ newline)
+  def statements(stripQuotes: Boolean): Parser[List[(StatementHandler, Statement)]] =
+    rep1(space ~> statement(stripQuotes) <~ newline)
 
-  def statement: Parser[(StatementHandler, Statement)] = {
+  @deprecated("Use variant that specifies whether to strip quotes or not", "1.4.0")
+  def statement: Parser[(StatementHandler, Statement)] = statement(stripQuotes = true)
+  def statement(stripQuotes: Boolean): Parser[(StatementHandler, Statement)] = {
     trait PositionalStatement extends Positional {
       def tuple: (StatementHandler, Statement)
     }
     positioned {
-      val command = (word | err("expected command"))
-      val arguments = rep(space ~> (word | failure("expected argument")))
+      val w = if (stripQuotes) word else rawWord
+      val command = w | err("expected command")
+      val arguments = rep(space ~> w | failure("expected argument"))
       (successParser ~ (space ~> startCharacterParser <~ space) ~! command ~! arguments) ^^ {
         case successExpected ~ start ~ command ~ arguments =>
           new PositionalStatement {
@@ -86,7 +103,9 @@ class TestScriptParser(handlers: Map[Char, StatementHandler]) extends RegexParse
   def space: Parser[String] = """[ \t]*""".r
 
   lazy val word: Parser[String] =
-    ("\'" ~> "[^'\n\r]*".r <~ "\'") | ("\"" ~> "[^\"\n\r]*".r <~ "\"") | WordRegex
+    ("\'" ~> "[^'\n\r]*".r <~ "\'") | "\"" ~> "[^\"\n\r]*".r <~ "\'" | WordRegex
+  private lazy val rawWord: Parser[String] =
+    ("\'" ~> "[^'\n\r]*".r <~ "\'") | "\"[^\"\n\r]*\"".r | WordRegex
 
   def startCharacterParser: Parser[Char] =
     elem("start character", handlers.contains _) |

sbt/src/sbt-test/actions/doc/test

@@ -1,33 +1,33 @@
 -> doc
 
-> 'set sources in (Compile, doc) := { val src = (sources in Compile).value; src.filterNot(_.getName contains "B") }'
+> set sources in (Compile, doc) := { val src = (sources in Compile).value; src.filterNot(_.getName contains "B") }
 
 # hybrid project, only scaladoc run
 > doc
-$ exists "target/api/index.js"
-$ absent "target/api/scala"
-$ absent "target/api/java"
+$ exists target/api/index.js
+$ absent target/api/scala
+$ absent target/api/java
 
-> 'set sources in (Compile, doc) := { val src = (sources in (Compile, doc)).value; src.filterNot(_.getName endsWith ".java") }'
+> set sources in (Compile, doc) := { val src = (sources in (Compile, doc)).value; src.filterNot(_.getName endsWith ".java") }
 
 # compile task is superfluous. Since doc task preceded by compile task has been problematic due to scala
 # compiler's way of handling empty classpath. We have it here to test that our workaround works.
-> ; clean ; compile ; doc
+> clean ; compile ; doc
 
 # pure scala project, only scaladoc at top level
-$ exists "target/api/index.js"
-$ absent "target/api/package-list"
-$ absent "target/api/scala"
-$ absent "target/api/java"
+$ exists target/api/index.js
+$ absent target/api/package-list
+$ absent target/api/scala
+$ absent target/api/java
 
-> 'set sources in (Compile, doc) := { val src = (sources in Compile).value; src.filter(_.getName endsWith ".java") }'
+> set sources in (Compile, doc) := { val src = (sources in Compile).value; src.filter(_.getName endsWith ".java") }
 
-> ; clean ; doc
+> clean ; doc
 
 # pure java project, only javadoc at top level
-$ exists "target/api/index.html"
-$ absent "target/api/index.js"
+$ exists target/api/index.html
+$ absent target/api/index.js
 
 # pending
-# $ absent "target/api/scala"
-# $ absent "target/api/java"
+# $ absent target/api/scala
+# $ absent target/api/java

sbt/src/sbt-test/package/lazy-name/test

@@ -4,19 +4,19 @@
 # much purpose other than checking that the 'set' command
 # re-evaluates the project data.
 
-> 'set name := "lazy-package-name"'
+> set name := "lazy-package-name"
 > set crossPaths := false
 
-> 'set version := "0.1.1"'
+> set version := "0.1.1"
 > package
-$ exists "target/lazy-package-name-0.1.1.jar"
+$ exists target/lazy-package-name-0.1.1.jar
 > clean
 
-> 'set version := "0.1.2"'
+> set version := "0.1.2"
 > package
-$ exists "target/lazy-package-name-0.1.2.jar"
+$ exists target/lazy-package-name-0.1.2.jar
 > clean
 
-> 'set version := "0.1.3"'
+> set version := "0.1.3"
 > package
-$ exists "target/lazy-package-name-0.1.3.jar"
+$ exists target/lazy-package-name-0.1.3.jar

sbt/src/sbt-test/package/manifest/test

@@ -1,4 +1,4 @@
 > package
-$ exists "./target/jar-manifest-test-0.2.jar"
-$ exec java -jar "./target/jar-manifest-test-0.2.jar"
+$ exists ./target/jar-manifest-test-0.2.jar
+$ exec java -jar ./target/jar-manifest-test-0.2.jar
 > run

sbt/src/sbt-test/package/resources/test

@@ -10,7 +10,7 @@
 
 # This should fail because sbt should include the resource in the jar but it won't have the right
 # directory structure
--$ exec java -jar "./target/main-resources-test-0.1.jar"
+-$ exec java -jar ./target/main-resources-test-0.1.jar
 
 # Give the resource the right directory structure
 $ mkdir src/main/resources/jartest
@@ -27,4 +27,4 @@ $ delete src/main/resources/main_resource_test
 > package
 
 # This should succeed because sbt should include the resource in the jar with the right directory structure
-$ exec java -jar "./target/main-resources-test-0.1.jar"
+$ exec java -jar ./target/main-resources-test-0.1.jar

sbt/src/sbt-test/run/fork/test

@@ -1,18 +1,18 @@
-> run "fork"
+> run fork
 $ exists flag
 $ delete flag
 
-$ mkdir "forked"
+$ mkdir forked
 > set fork := true
-> 'set baseDirectory in run := baseDirectory(_ / "forked").value'
+> set baseDirectory in run := baseDirectory(_ / "forked").value
 
-> run "forked"
+> run forked
 $ exists forked/flag
 $ absent flag
 $ delete forked/flag
 
-> 'set envVars += ("flag.name" -> "env.flag")'
-> run "forked"
+> set envVars += ("flag.name" -> "env.flag")
+> run forked
 $ exists forked/env.flag
 $ absent flag
 $ absent forked/flag

sbt/src/sbt-test/tests/arguments/test

@@ -23,4 +23,6 @@ $ delete success3
 
 $ touch failure3
 -> test:test
-$ delete failure3
+$ delete failure3
+
+> set Compile / scalacOptions += "-Xfatal-warnings"

scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala

@@ -443,7 +443,7 @@ final class ScriptedTests(
     catching(classOf[TestException]).withApply(testFailed).andFinally(log.clear).apply {
       preScriptedHook(testDirectory)
       val parser = new TestScriptParser(handlers)
-      val handlersAndStatements = parser.parse(file)
+      val handlersAndStatements = parser.parse(file, stripQuotes = false)
       runner.apply(handlersAndStatements, states)
 
       // Handle successful tests