From e8e02979674d4a5882fadb2df8ce802d703b448f Mon Sep 17 00:00:00 2001 From: "andrzej.jozwik@gmail.com" Date: Mon, 8 Dec 2014 23:35:17 +0100 Subject: [PATCH] Remove work-around for parsing xml by scala parser --- .../scala/sbt/EvaluateConfigurations.scala | 3 +- .../sbt/internals/parser/SbtParser.scala | 328 ++---------------- .../internals/parser/SbtRefactorings.scala | 2 +- main/src/test/resources/old-format/15.sbt.txt | 4 +- main/src/test/resources/old-format/22.sbt.txt | 2 + .../session-settings-quick/3.sbt.txt | 4 +- .../session-settings-quick/3.sbt.txt_1/1.set | 2 +- .../3.sbt.txt_1/1.set.result | 2 +- .../internals/parser/EmbeddedXmlSpec.scala | 20 +- .../sbt/internals/parser/ErrorSpec.scala | 15 + .../parser/SplitExpressionsFilesTest.scala | 4 +- 11 files changed, 65 insertions(+), 321 deletions(-) diff --git a/main/src/main/scala/sbt/EvaluateConfigurations.scala b/main/src/main/scala/sbt/EvaluateConfigurations.scala index 80b3f8d89..315cd7487 100644 --- a/main/src/main/scala/sbt/EvaluateConfigurations.scala +++ b/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] = diff --git a/main/src/main/scala/sbt/internals/parser/SbtParser.scala b/main/src/main/scala/sbt/internals/parser/SbtParser.scala index c80721f3f..8512cc33b 100644 --- a/main/src/main/scala/sbt/internals/parser/SbtParser.scala +++ b/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 modifiedContent = handleXmlContent(original) + val content = indexedLines.mkString(END_OF_LINE) 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( - s"""====== - |$modifiedContent - |====== - |${seq.mkString(EOL)}""".stripMargin) + val errorMessage = seq.mkString(EOL) + + val error = if (errorMessage.contains(XML_ERROR)) { + s""" + |$errorMessage + |Probably problem with parsing xml group, please add parens or semicolons: + |Replace: + |val xmlGroup = + |with: + |val xmlGroup = () + |or + |val xmlGroup = ; + | + """.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) - !(content contains "=") + val c = content.substring(x.pos.start, x.pos.end) + !(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) - (importsLineRange, stmtTreeLineRange.map { case (stmt, _, lr) => (stmt, lr) }, stmtTreeLineRange.map { case (stmt, tree, _) => (stmt, tree) }, modifiedContent) + val importsLineRange = importsToLineRanges(content, imports) + (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: - * 
- * val xml = 
txt

- * rr
- *
- * At least brackets have to be added - * 
- * val xml = (
txt

- * rr)
- *
- */ -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 - - * @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 - ... - * @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("", 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 - } - - } - } -} \ No newline at end of file diff --git a/main/src/main/scala/sbt/internals/parser/SbtRefactorings.scala b/main/src/main/scala/sbt/internals/parser/SbtRefactorings.scala index dde9643cc..f04a64829 100644 --- a/main/src/main/scala/sbt/internals/parser/SbtRefactorings.scala +++ b/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) } diff --git a/main/src/test/resources/old-format/15.sbt.txt b/main/src/test/resources/old-format/15.sbt.txt index dce8e063b..9feb4f5a4 100644 --- a/main/src/test/resources/old-format/15.sbt.txt +++ b/main/src/test/resources/old-format/15.sbt.txt @@ -3,7 +3,7 @@ name := "play-html-compressor" scalaVersion := "2.11.1" -val pom = +val pom = git@github.com:mohiva/play-html-compressor.git scm:git:git@github.com:mohiva/play-html-compressor.git @@ -13,7 +13,7 @@ val pom = Christian Kaps http://mohiva.com - + publishMavenStyle := true diff --git a/main/src/test/resources/old-format/22.sbt.txt b/main/src/test/resources/old-format/22.sbt.txt index 4b6e131ae..2372c91de 100644 --- a/main/src/test/resources/old-format/22.sbt.txt +++ b/main/src/test/resources/old-format/22.sbt.txt @@ -22,6 +22,7 @@ parallelExecution in Test := false // publishing pomExtra := + http://nbronson.github.com/scala-stm/ @@ -41,6 +42,7 @@ pomExtra := ngbronson@gmail.com + publishMavenStyle := true diff --git a/main/src/test/resources/session-settings-quick/3.sbt.txt b/main/src/test/resources/session-settings-quick/3.sbt.txt index 6db83e79c..8a36f7432 100644 --- a/main/src/test/resources/session-settings-quick/3.sbt.txt +++ b/main/src/test/resources/session-settings-quick/3.sbt.txt @@ -2,7 +2,7 @@ import sbt._ val scmpom = taskKey[xml.NodeBuffer]("Node buffer") -scmpom := +scmpom := git@github.com:mohiva/play-html-compressor.git scm:git:git@github.com:mohiva/play-html-compressor.git @@ -13,4 +13,4 @@ scmpom := http://mohiva.com - \ No newline at end of file + \ No newline at end of file diff --git a/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set b/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set index 3e2aa8d48..9e7c59124 100644 --- a/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set +++ b/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set @@ -1 +1 @@ -scmpom := OK \ No newline at end of file +scmpom := OK \ No newline at end of file diff --git a/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set.result b/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set.result index c992f0b5a..2eab3cd5a 100644 --- a/main/src/test/resources/session-settings-quick/3.sbt.txt_1/1.set.result +++ b/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 := (OK) \ No newline at end of file +scmpom := OK \ No newline at end of file diff --git a/main/src/test/scala/sbt/internals/parser/EmbeddedXmlSpec.scala b/main/src/test/scala/sbt/internals/parser/EmbeddedXmlSpec.scala index 200a13e7d..2504ad01b 100644 --- a/main/src/test/scala/sbt/internals/parser/EmbeddedXmlSpec.scala +++ b/main/src/test/scala/sbt/internals/parser/EmbeddedXmlSpec.scala @@ -14,7 +14,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec { | |scalaVersion := "2.11.1" | - |val pom = + |val pom = |git@github.com:mhiva/play-html-compressor.git |scm:git:git@github.com:mohiva/play-html-compressor.git | @@ -24,7 +24,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec { | Christian Kaps | http://mohiva.com | - | + | |$errorLine | |""".stripMargin @@ -63,7 +63,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec { | |val pom = "" | - |val aaa= git@github.com:mohiva/play-html-compressor.git + |val aaa= git@github.com:mohiva/play-html-compressor.git | scm:git:git@github.com:mohiva/play-html-compressor.git | | @@ -73,15 +73,15 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec { | http://mohiva.com | | - | 4.0 + | 4.0 | |publishMavenStyle := true | - |val anotherXml = + |val anotherXml = | content | | - | + | | |val tra = "" | @@ -94,7 +94,7 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec { | |val ok = | - |val pom = + |val pom = |git@github.com:mhiva/play-html-compressor.git | scm:git:git@github.com:mohiva/play-html-compressor.git | @@ -105,15 +105,15 @@ class EmbeddedXmlSpec extends CheckIfParsedSpec { |http://mohiva.com | | - |4.0 + |4.0 | |publishMavenStyle := true | - |val anotherXml = + |val anotherXml = | content | | - | + | | | """.stripMargin, "Xml with attributes", false, true), ( diff --git a/main/src/test/scala/sbt/internals/parser/ErrorSpec.scala b/main/src/test/scala/sbt/internals/parser/ErrorSpec.scala index 25cd348f4..9f9e10d9f 100644 --- a/main/src/test/scala/sbt/internals/parser/ErrorSpec.scala +++ b/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 = + |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) = { diff --git a/main/src/test/scala/sbt/internals/parser/SplitExpressionsFilesTest.scala b/main/src/test/scala/sbt/internals/parser/SplitExpressionsFilesTest.scala index 327dbc7e2..224bb3b74 100644 --- a/main/src/test/scala/sbt/internals/parser/SplitExpressionsFilesTest.scala +++ b/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) { - println(s"In file: $fileName, same results (imports, settings): $resultOld") - } else { + if (resultOld != resultNew) { println( s"""In file: $fileName, results differ: |resultOld: