Merge pull request #1765 from ajozwik/0.13

Rollback xml parsing work-around

main/src/main/scala/sbt/EvaluateConfigurations.scala

@@ -93,10 +93,9 @@ object EvaluateConfigurations {
    *
    * @param eval    The evaluating scala compiler instance we use to handle evaluating scala configuration.
    * @param file    The file we've parsed
-   * @param lines   The lines of the configurtion we'd like to evaluate.
    * @param imports The default imports to use in this .sbt configuration.
    *
-   * @return A function which can take an sbt classloader and return the raw types/configuratoin
+   * @return A function which can take an sbt classloader and return the raw types/configuration
    *         which was compiled/parsed for the given file.
    */
   private[sbt] def evaluateSbtFile(eval: Eval, file: File, lines: Seq[String], imports: Seq[String], offset: Int): LazyClassLoaded[LoadedSbtFile] =

main/src/main/scala/sbt/internals/parser/SbtParser.scala

@@ -6,7 +6,6 @@ import java.io.File
 
 import sbt.internals.parser.SbtParser._
 
-import scala.annotation.tailrec
 import scala.reflect.runtime.universe._
 
 private[sbt] object SbtParser {
@@ -14,6 +13,7 @@ private[sbt] object SbtParser {
   val END_OF_LINE = String.valueOf(END_OF_LINE_CHAR)
   private[parser] val NOT_FOUND_INDEX = -1
   private[sbt] val FAKE_FILE = new File("fake")
+  private[parser] val XML_ERROR = "';' expected but 'val' found."
 }
 
 /**
@@ -30,8 +30,6 @@ sealed trait ParsedSbtFileExpressions {
   /** The set of scala tree's for parsed definitions/settings and the underlying string representation.. */
   def settingsTrees: Seq[(String, Tree)]
 
-  /** Represents the changes we had to perform to the sbt file so that XML will parse correctly. */
-  def modifiedContent: String
 }
 
 /**
@@ -57,11 +55,10 @@ private[sbt] case class SbtParser(file: File, lines: Seq[String]) extends Parsed
   //settingsTrees,modifiedContent needed for "session save"
   // TODO - We should look into splitting out "defintiions" vs. "settings" here instead of further string lookups, since we have the
   // parsed trees.
-  val (imports, settings, settingsTrees, modifiedContent) = splitExpressions(file, lines)
+  val (imports, settings, settingsTrees) = splitExpressions(file, lines)
 
-  private def splitExpressions(file: File, lines: Seq[String]): (Seq[(String, Int)], Seq[(String, LineRange)], Seq[(String, Tree)], String) = {
+  private def splitExpressions(file: File, lines: Seq[String]): (Seq[(String, Int)], Seq[(String, LineRange)], Seq[(String, Tree)]) = {
     import sbt.internals.parser.MissingBracketHandler._
-    import sbt.internals.parser.XmlContent._
 
     import scala.compat.Platform.EOL
     import scala.reflect.runtime._
@@ -70,23 +67,35 @@ private[sbt] case class SbtParser(file: File, lines: Seq[String]) extends Parsed
     val mirror = universe.runtimeMirror(this.getClass.getClassLoader)
     val toolbox = mirror.mkToolBox(options = "-Yrangepos")
     val indexedLines = lines.toIndexedSeq
-    val original = indexedLines.mkString(END_OF_LINE)
+    val content = indexedLines.mkString(END_OF_LINE)
-    val modifiedContent = handleXmlContent(original)
     val fileName = file.getAbsolutePath
 
     val parsed =
       try {
-        toolbox.parse(modifiedContent)
+        toolbox.parse(content)
       } catch {
         case e: ToolBoxError =>
           val seq = toolbox.frontEnd.infos.map { i =>
             s"""[$fileName]:${i.pos.line}: ${i.msg}"""
           }
-          throw new MessageOnlyException(
+          val errorMessage = seq.mkString(EOL)
-            s"""======
+
-               |$modifiedContent
+          val error = if (errorMessage.contains(XML_ERROR)) {
-               |======
+            s"""
-               |${seq.mkString(EOL)}""".stripMargin)
+               |$errorMessage
+               |Probably problem with parsing xml group, please add parens or semicolons:
+               |Replace:
+               |val xmlGroup = <a/><b/>
+               |with:
+               |val xmlGroup = (<a/><b/>)
+               |or
+               |val xmlGroup = <a/><b/>;
+               |
+             """.stripMargin
+          } else {
+            errorMessage
+          }
+          throw new MessageOnlyException(error)
       }
     val parsedTrees = parsed match {
       case Block(stmt, expr) =>
@@ -99,8 +108,8 @@ private[sbt] case class SbtParser(file: File, lines: Seq[String]) extends Parsed
     def isBadValDef(t: Tree): Boolean =
       t match {
         case x @ toolbox.u.ValDef(_, _, _, rhs) if rhs != toolbox.u.EmptyTree =>
-          val content = modifiedContent.substring(x.pos.start, x.pos.end)
+          val c = content.substring(x.pos.start, x.pos.end)
-          !(content contains "=")
+          !(c contains "=")
         case _ => false
       }
     parsedTrees.filter(isBadValDef).foreach { badTree =>
@@ -123,7 +132,7 @@ private[sbt] case class SbtParser(file: File, lines: Seq[String]) extends Parsed
     def parseStatementAgain(t: Tree, originalStatement: String): String = {
       val statement = util.Try(toolbox.parse(originalStatement)) match {
         case util.Failure(th) =>
-          val missingText = findMissingText(modifiedContent, t.pos.end, t.pos.line, fileName, th)
+          val missingText = findMissingText(content, t.pos.end, t.pos.line, fileName, th)
           originalStatement + missingText
         case _ =>
           originalStatement
@@ -136,14 +145,14 @@ private[sbt] case class SbtParser(file: File, lines: Seq[String]) extends Parsed
         case NoPosition =>
           None
         case position =>
-          val originalStatement = modifiedContent.substring(position.start, position.end)
+          val originalStatement = content.substring(position.start, position.end)
           val statement = parseStatementAgain(t, originalStatement)
           val numberLines = countLines(statement)
           Some((statement, t, LineRange(position.line - 1, position.line + numberLines)))
       }
     val stmtTreeLineRange = statements flatMap convertStatement
-    val importsLineRange = importsToLineRanges(modifiedContent, imports)
+    val importsLineRange = importsToLineRanges(content, imports)
-    (importsLineRange, stmtTreeLineRange.map { case (stmt, _, lr) => (stmt, lr) }, stmtTreeLineRange.map { case (stmt, tree, _) => (stmt, tree) }, modifiedContent)
+    (importsLineRange, stmtTreeLineRange.map { case (stmt, _, lr) => (stmt, lr) }, stmtTreeLineRange.map { case (stmt, tree, _) => (stmt, tree) })
   }
 
   /**
@@ -232,282 +241,3 @@ private[sbt] object MissingBracketHandler {
     }
   }
 }
-
-/**
- * #ToolBox#parse(String) will fail for xml sequence:
- * <pre>
- * val xml = <div>txt</div>
- * <a>rr</a>
- * </pre>
- * At least brackets have to be added
- * <pre>
- * val xml = (<div>txt</div>
- * <a>rr</a>)
- * </pre>
- */
-private[sbt] object XmlContent {
-
-  private val OPEN_PARENTHESIS = '{'
-
-  private val OPEN_CURLY_BRACKET = '('
-
-  private val DOUBLE_SLASH = "//"
-
-  private val OPEN_BRACKET = s"$OPEN_CURLY_BRACKET"
-
-  private val CLOSE_BRACKET = ")"
-
-  /**
-   *
-   * @param original - file content
-   * @return original content or content with brackets added to xml parts
-   */
-  private[sbt] def handleXmlContent(original: String): String = {
-    val xmlParts = findXmlParts(original)
-    if (xmlParts.isEmpty) {
-      original
-    } else {
-      addExplicitXmlContent(original, xmlParts)
-    }
-  }
-
-  /**
-   * Cut file for normal text - xml - normal text - xml ....
-   * @param content - content
-   * @param ts - import/statements
-   * @return Seq - Right(xml,whiteSpaces) - for xml, Left(statement) - for text
-   */
-  private def splitFile(content: String, ts: Seq[(String, Int, Int)]): Seq[Either[(String), (String, String)]] = {
-    val (statements, lastIndex) = ts.foldLeft((Seq.empty[Either[(String), (String, String)]], 0)) {
-      case ((accSeq, index), (statement, startIndex, endIndex)) =>
-        val toAdd = if (index >= startIndex) {
-          Seq(Right((statement, "")))
-        } else {
-          val s = content.substring(index, startIndex)
-          if (s.trim.isEmpty) {
-            Seq(Right((statement, s)))
-          } else {
-            Seq(Right((statement, "")), Left(s))
-          }
-        }
-        (toAdd ++ accSeq, endIndex)
-    }
-    val endOfFile = content.substring(lastIndex, content.length)
-    val withEndOfFile = if (endOfFile.isEmpty) statements else Left(endOfFile) +: statements
-    withEndOfFile.reverse
-  }
-
-  /**
-   * Cut potential xmls from content
-   * @param content - content
-   * @return sorted by openIndex xml parts
-   */
-  private def findXmlParts(content: String): Seq[(String, Int, Int)] = {
-    val xmlParts = findModifiedOpeningTags(content, 0, Seq.empty) ++ findNotModifiedOpeningTags(content, 0, Seq.empty)
-    val rootXmlParts = removeEmbeddedXmlParts(xmlParts)
-    rootXmlParts.sortBy(z => z._2)
-
-  }
-
-  private def searchForTagName(text: String, startIndex: Int, endIndex: Int) = {
-    val subs = text.substring(startIndex, endIndex)
-    val spaceIndex = subs.indexWhere(c => c.isWhitespace, 1)
-    if (spaceIndex == NOT_FOUND_INDEX) {
-      subs
-    } else {
-      subs.substring(0, spaceIndex)
-    }
-  }
-
-  private val CLOSE_XML_TAG = "/>"
-
-  /**
-   * Modified Opening Tag - <aaa/>
-   * @param offsetIndex - index
-   * @param acc - result
-   * @return Set with tags and positions
-   */
-  @tailrec
-  private def findModifiedOpeningTags(content: String, offsetIndex: Int, acc: Seq[(String, Int, Int)]): Seq[(String, Int, Int)] = {
-    val endIndex = content.indexOf(CLOSE_XML_TAG, offsetIndex)
-    if (endIndex == NOT_FOUND_INDEX) {
-      acc
-    } else {
-      val xmlFragment = findModifiedOpeningTag(content, offsetIndex, endIndex)
-      val newAcc = addOptionToCollection(acc, xmlFragment)
-      findModifiedOpeningTags(content, endIndex + 2, newAcc)
-    }
-  }
-
-  private def findModifiedOpeningTag(content: String, offsetIndex: Int, endIndex: Int): Option[(String, Int, Int)] = {
-    val startIndex = content.substring(offsetIndex, endIndex).lastIndexOf("<")
-    if (startIndex == NOT_FOUND_INDEX) {
-      None
-    } else {
-      val tagName = searchForTagName(content, startIndex + 1 + offsetIndex, endIndex)
-      if (xml.Utility.isName(tagName)) {
-        xmlFragmentOption(content, startIndex + offsetIndex, endIndex + 2)
-      } else {
-        None
-      }
-    }
-
-  }
-
-  private def searchForOpeningIndex(text: String, closeTagStartIndex: Int, tagName: String) = {
-    val subs = text.substring(0, closeTagStartIndex)
-    val index = subs.lastIndexOf(s"<$tagName>")
-    if (index == NOT_FOUND_INDEX) {
-      subs.lastIndexOf(s"<$tagName ")
-    } else {
-      index
-    }
-  }
-
-  /**
-   * Xml like - <aaa>...<aaa/>
-   * @param current - index
-   * @param acc - result
-   * @return Set with tags and positions
-   */
-  @tailrec
-  private def findNotModifiedOpeningTags(content: String, current: Int, acc: Seq[(String, Int, Int)]): Seq[(String, Int, Int)] = {
-    val closeTagStartIndex = content.indexOf("</", current)
-    if (closeTagStartIndex == NOT_FOUND_INDEX) {
-      acc
-    } else {
-      val closeTagEndIndex = content.indexOf(">", closeTagStartIndex)
-      if (closeTagEndIndex == NOT_FOUND_INDEX) {
-        findNotModifiedOpeningTags(content, closeTagStartIndex + 2, acc)
-      } else {
-        val xmlFragment = findNotModifiedOpeningTag(content, closeTagStartIndex, closeTagEndIndex)
-        val newAcc = addOptionToCollection(acc, xmlFragment)
-        findNotModifiedOpeningTags(content, closeTagEndIndex + 1, newAcc)
-      }
-    }
-  }
-
-  private def removeEmbeddedXmlParts(xmlParts: Seq[(String, Int, Int)]) = {
-    def isElementBetween(el: (String, Int, Int), open: Int, close: Int): Boolean = {
-      xmlParts.exists {
-        element =>
-          val (_, openIndex, closeIndex) = element
-          el != element && (open > openIndex) && (close < closeIndex)
-      }
-    }
-    xmlParts.filterNot { el =>
-      val (_, open, close) = el
-      isElementBetween(el, open, close)
-    }
-  }
-
-  /**
-   *
-   * @param content - content
-   * @param xmlParts - xmlParts
-   * @return content with xml with brackets
-   */
-  private def addExplicitXmlContent(content: String, xmlParts: Seq[(String, Int, Int)]): String = {
-    val statements = splitFile(content, xmlParts)
-
-    val (_, seq, lastAdd) = statements.foldLeft[(Option[Either[(String), (String, String)]], Seq[String], Boolean)]((None, Seq.empty[String], false)) {
-      case ((previousOption, acc, add), element) =>
-        val (newAcc, newAdd) = (element, previousOption) match {
-          case (Left(text), _) =>
-            val accWithClose = if (add) {
-              addCloseBracket(acc)
-            } else {
-              acc
-            }
-            (text +: accWithClose, false)
-          case (Right((xml, nonXml)), Some(Left(text))) =>
-            val (accWithOpen, added) = if (areBracketsNecessary(text)) {
-              (OPEN_BRACKET +: acc, true)
-            } else {
-              (acc, false)
-            }
-            (xml +: (nonXml +: accWithOpen), added)
-          case (Right((xml, nonXml)), _) =>
-            (xml +: (nonXml +: acc), add)
-        }
-        (Some(element), newAcc, newAdd)
-    }
-
-    val correctedSeq = if (lastAdd) {
-      addCloseBracket(seq)
-    } else {
-      seq
-    }
-    correctedSeq.reverse.mkString
-  }
-
-  private def addCloseBracket(statements: Seq[String]) = CLOSE_BRACKET +: statements
-
-  /**
-   * Add to head if option is not empty
-   * @param ts - seq
-   * @param option - option
-   * @tparam T - type
-   * @return original seq for None, add to head for Some[T]
-   */
-  private def addOptionToCollection[T](ts: Seq[T], option: Option[T]) = option.fold(ts)(el => el +: ts)
-
-  private def findNotModifiedOpeningTag(content: String, closeTagStartIndex: Int, closeTagEndIndex: Int): Option[(String, Int, Int)] = {
-
-    val tagName = content.substring(closeTagStartIndex + 2, closeTagEndIndex)
-    if (xml.Utility.isName(tagName)) {
-      val openTagIndex = searchForOpeningIndex(content, closeTagStartIndex, tagName)
-      if (openTagIndex == NOT_FOUND_INDEX) {
-        None
-      } else {
-        xmlFragmentOption(content, openTagIndex, closeTagEndIndex + 1)
-      }
-    } else {
-      None
-    }
-  }
-
-  /**
-   * Check, if xmlPart is valid xml. If not - None is returned
-   * @param content - file content
-   * @param openIndex - open index
-   * @param closeIndex - close index
-   * @return Some((String,Int,Int))
-   */
-  private def xmlFragmentOption(content: String, openIndex: Int, closeIndex: Int): Option[(String, Int, Int)] = {
-    val xmlPart = content.substring(openIndex, closeIndex)
-    util.Try(xml.XML.loadString(xmlPart)) match {
-      case util.Success(_)  => Some((xmlPart, openIndex, closeIndex))
-      case util.Failure(th) => None
-    }
-  }
-
-  /**
-   * If xml is in brackets - we do not need to add them
-   * @param statement - statement
-   * @return are brackets necessary?
-   */
-  private def areBracketsNecessary(statement: String): Boolean = {
-    val doubleSlash = statement.indexOf(DOUBLE_SLASH)
-
-    if (doubleSlash != NOT_FOUND_INDEX) {
-      val endOfLine = statement.indexOf(END_OF_LINE, doubleSlash)
-      if (endOfLine == NOT_FOUND_INDEX) {
-        false
-      } else {
-        areBracketsNecessary(statement.substring(endOfLine))
-      }
-    } else {
-      val roundBrackets = statement.lastIndexOf(OPEN_CURLY_BRACKET)
-      val braces = statement.lastIndexOf(OPEN_PARENTHESIS)
-      val max = roundBrackets.max(braces)
-      if (max == NOT_FOUND_INDEX) {
-        true
-      } else {
-        val trimmed = statement.substring(max + 1).trim
-        trimmed.nonEmpty
-      }
-
-    }
-  }
-}

main/src/main/scala/sbt/internals/parser/SbtRefactorings.scala

@@ -27,7 +27,7 @@ private[sbt] object SbtRefactorings {
     val recordedCommands = recordCommands(commands, split)
     val sortedRecordedCommands = recordedCommands.sortBy(_._1)(REVERSE_ORDERING_INT)
 
-    val newContent = replaceFromBottomToTop(split.modifiedContent, sortedRecordedCommands)
+    val newContent = replaceFromBottomToTop(lines.mkString(END_OF_LINE), sortedRecordedCommands)
     (file, newContent.lines.toList)
   }
 

main/src/test/resources/old-format/15.sbt.txt

@@ -3,7 +3,7 @@ name := "play-html-compressor"
 
 scalaVersion := "2.11.1"
 
-val pom = <scm>
+val pom = <xml:group><scm>
     <url>git@github.com:mohiva/play-html-compressor.git</url>
     <connection>scm:git:git@github.com:mohiva/play-html-compressor.git</connection>
   </scm>
@@ -13,7 +13,7 @@ val pom = <scm>
       <name>Christian Kaps</name>
       <url>http://mohiva.com</url>
     </developer>
-  </developers>
+  </developers></xml:group>
 
 publishMavenStyle := true
 

main/src/test/resources/old-format/22.sbt.txt

@@ -22,6 +22,7 @@ parallelExecution in Test := false
 // publishing
 
 pomExtra :=
+  <xml:group>
   <url>http://nbronson.github.com/scala-stm/</url>
   <licenses>
     <license>
@@ -41,6 +42,7 @@ pomExtra :=
       <email>ngbronson@gmail.com</email>
     </developer>
   </developers>
+  </xml:group>
 
 publishMavenStyle := true
 

main/src/test/resources/session-settings-quick/3.sbt.txt

@@ -2,7 +2,7 @@ import sbt._
 
 val scmpom = taskKey[xml.NodeBuffer]("Node buffer")
 
-scmpom := <scm>
+scmpom := <xml:group><scm>
     <url>git@github.com:mohiva/play-html-compressor.git</url>
     <connection>scm:git:git@github.com:mohiva/play-html-compressor.git</connection>
   </scm>
@@ -13,4 +13,4 @@ scmpom := <scm>
       <url>http://mohiva.com</url>
     </developer>
   </developers>
-  <a></a>
+  <a></a></xml:group>

main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set

@@ -1 +1 @@
-scmpom := <a/><b a="rt">OK</b>
+scmpom := <xml:group><a/><b a="rt">OK</b></xml:group>

main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set.result

@@ -2,4 +2,4 @@ import sbt._
 
 val scmpom = taskKey[xml.NodeBuffer]("Node buffer")
 
-scmpom := (<a/><b a="rt">OK</b>)
+scmpom := <xml:group><a/><b a="rt">OK</b></xml:group>

main/src/test/scala/sbt/internals/parser/EmbeddedXmlSpec.scala

@@ -14,7 +14,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec {
                          |
                          |scalaVersion := "2.11.1"
                          |
-                         |val pom = <scm>
+                         |val pom = <xml:group><scm>
                          |<url>git@github.com:mhiva/play-html-compressor.git</url>
                          |<connection>scm:git:git@github.com:mohiva/play-html-compressor.git</connection>
                          |  </scm>
@@ -24,7 +24,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec {
                          |      <name>Christian Kaps</name>
                          |      <url>http://mohiva.com</url>
                          |    </developer>
-                         |  </developers>
+                         |  </developers></xml:group>
                          |$errorLine
                          |
                          |""".stripMargin
@@ -63,7 +63,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec {
         |
         |val pom = "</scm>"
         |
-        |val aaa= <scm><url>git@github.com:mohiva/play-html-compressor.git</url>
+        |val aaa= <xml:group><scm><url>git@github.com:mohiva/play-html-compressor.git</url>
         |   <connection>scm:git:git@github.com:mohiva/play-html-compressor.git</connection>
         |  </scm>
         |  <developers>
@@ -73,15 +73,15 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec {
         |      <url>http://mohiva.com</url>
         |    </developer>
         |  </developers>
-        |  <version>4.0</version>
+        |  <version>4.0</version></xml:group>
         |
         |publishMavenStyle := true
         |
-        |val anotherXml = <a a="r"><bbb>
+        |val anotherXml = <xml:group><a a="r"><bbb>
         |        content</bbb>
         |        <ccc atr="tre" />
         |        <aa/>
-        |          </a>
+        |          </a></xml:group>
         |
         |val tra = "</scm>"
         |
@@ -94,7 +94,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec {
         |
         |val ok = <ccc atr="tre" />
         |
-        |val pom = <scm>
+        |val pom = <xml:group><scm>
         |<url>git@github.com:mhiva/play-html-compressor.git</url>
         |    <connection>scm:git:git@github.com:mohiva/play-html-compressor.git</connection>
         |</scm>
@@ -105,15 +105,15 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec {
         |<url>http://mohiva.com</url>
         |</developer>
         |</developers>
-        |<version>4.0</version>
+        |<version>4.0</version></xml:group>
         |
         |publishMavenStyle := true
         |
-        |val anotherXml = <a a="r"><bbb>
+        |val anotherXml = <xml:group><a a="r"><bbb>
         |        content</bbb>
         |        <ccc atr="tre" />
         |<aa/>
-        | </a>
+        | </a></xml:group>
         |
         | """.stripMargin, "Xml with attributes", false, true),
     (

main/src/test/scala/sbt/internals/parser/ErrorSpec.scala

@@ -39,6 +39,21 @@ class ErrorSpec extends AbstractSpec with ScalaCheck {
         """.stripMargin
       MissingBracketHandler.findMissingText(buildSbt, buildSbt.length, 2, "fake.txt", new MessageOnlyException("fake")) must throwA[MessageOnlyException]
     }
+
+    "handle xml error " in {
+      val buildSbt =
+        """
+          |val a = <a/><b/>
+          |val s = '
+        """.stripMargin
+      SbtParser(SbtParser.FAKE_FILE, buildSbt.lines.toSeq) must throwA[MessageOnlyException].like {
+        case exp =>
+          val message = exp.getMessage
+          println(s"${exp.getMessage}")
+          message must contain(SbtParser.FAKE_FILE.getName)
+      }
+    }
+
   }
 
   private def containsLineNumber(buildSbt: String) = {

main/src/test/scala/sbt/internals/parser/SplitExpressionsFilesTest.scala

@@ -138,9 +138,7 @@ abstract class AbstractSplitExpressionsFilesTest(pathName: String) extends Speci
           println(s"In file: $fileName, new splitter failed. ${ex.toString}")
           ex.printStackTrace()
         case SplitterComparison(util.Success(resultOld), util.Success(resultNew)) =>
-          if (resultOld == resultNew) {
+          if (resultOld != resultNew) {
-            println(s"In file: $fileName, same results (imports, settings): $resultOld")
-          } else {
             println(
               s"""In file: $fileName, results differ:
                  |resultOld: