Merge pull request #557 from eed3si9n/wip/error

Improve Javac error parsing

internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala

@@ -83,6 +83,33 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
     }) ^^ {
       case xs => xs.mkString("\n")
     }
+  val nonPathLine: Parser[String] = {
+    val nonPathLine0 = new Parser[String] {
+      def isStopChar(c: Char): Boolean = c == '\n' || c == '\r'
+
+      def apply(in: Input) = {
+        val source = in.source
+        val offset = in.offset
+        var i = offset
+        while (i < source.length && !isStopChar(source.charAt(i))) {
+          i += 1
+        }
+        val line = source.subSequence(offset, i).toString
+        if ((line.startsWith("/") || line.contains("\\")) && line.contains(".java"))
+          Failure("Path found", in)
+        else if (i == offset) Failure("Empty", in)
+        else Success(line, in.drop(i - offset))
+      }
+    }
+    nonPathLine0 ~ """[\r]?[\n]?""".r ^^ {
+      case msg ~ endline => msg + endline
+    }
+  }
+  val nonPathLines: Parser[String] = {
+    rep(nonPathLine) ^^ {
+      case lines => lines.mkString("")
+    }
+  }
 
   // Parses ALL characters until an expected character is met.
   def allUntilChar(c: Char): Parser[String] = allUntilChars(Array(c))
@@ -148,15 +175,21 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
     val fileLineMessage = fileAndLineNo ~ SEMICOLON ~ restOfLine ^^ {
       case (file, line) ~ _ ~ msg => (file, line, msg)
     }
-    fileLineMessage ~ allUntilCaret ~ restOfLine ~ (allIndented.?) ^^ {
-      case (file, line, msg) ~ contents ~ r ~ ind =>
+    fileLineMessage ~ (allUntilCaret ~ '^' ~ restOfLine).? ~ (nonPathLines.?) ^^ {
+      case (file, line, msg) ~ contentsOpt ~ ind =>
         new JavaProblem(
           new JavaPosition(
             findFileSource(file),
             line,
-            contents + '^' + r + ind
+            (contentsOpt match {
+              case Some(contents ~ _ ~ r) => contents + '^' + r
+              case _                      => ""
+            }) + ind
               .getOrElse(""), // TODO - Actually parse caret position out of here.
-            getOffset(contents)
+            (contentsOpt match {
+              case Some(contents ~ _ ~ _) => getOffset(contents)
+              case _                      => 0
+            })
           ),
           Severity.Error,
           msg
@@ -169,14 +202,20 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
     val fileLineMessage = fileAndLineNo ~ SEMICOLON ~ WARNING ~ SEMICOLON ~ restOfLine ^^ {
       case (file, line) ~ _ ~ _ ~ _ ~ msg => (file, line, msg)
     }
-    fileLineMessage ~ allUntilCaret ~ restOfLine ~ (allIndented.?) ^^ {
-      case (file, line, msg) ~ contents ~ r ~ ind =>
+    fileLineMessage ~ (allUntilCaret ~ '^' ~ restOfLine).? ~ (nonPathLines.?) ^^ {
+      case (file, line, msg) ~ contentsOpt ~ ind =>
         new JavaProblem(
           new JavaPosition(
             findFileSource(file),
             line,
-            contents + "^" + r + ind.getOrElse(""),
-            getOffset(contents)
+            (contentsOpt match {
+              case Some(contents ~ _ ~ r) => contents + '^' + r
+              case _                      => ""
+            }) + ind.getOrElse(""),
+            (contentsOpt match {
+              case Some(contents ~ _ ~ _) => getOffset(contents)
+              case _                      => 0
+            })
           ),
           Severity.Warn,
           msg
@@ -201,9 +240,15 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
         )
     }
 
+  val outputSumamry: Parser[Unit] =
+    """(\s*)(\d+) (\w+)""".r ~ restOfLine ^^ {
+      case a ~ b =>
+        ()
+    }
+
   val potentialProblem: Parser[Problem] = warningMessage | errorMessage | noteMessage | javacError
 
-  val javacOutput: Parser[Seq[Problem]] = rep(potentialProblem)
+  val javacOutput: Parser[Seq[Problem]] = rep(potentialProblem) <~ opt(outputSumamry)
 
   /**
    * Example:

internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavaCompilerSpec.scala

@@ -13,8 +13,9 @@ import xsbti.compile.{ IncToolOptions, IncToolOptionsUtil, JavaTools => XJavaToo
 import sbt.io.IO
 import sbt.util.LogExchange
 import org.scalatest.matchers._
+import org.scalatest.DiagrammedAssertions
 
-class JavaCompilerSpec extends UnitSpec {
+class JavaCompilerSpec extends UnitSpec with DiagrammedAssertions {
 
   "Compiling a java file with local javac" should "compile a java file" in works(local)
   it should "issue errors and warnings" in findsErrors(local)
@@ -37,9 +38,11 @@ class JavaCompilerSpec extends UnitSpec {
 
   def docWorks(compiler: XJavaTools) = IO.withTemporaryDirectory { out =>
     val (result, _) = doc(compiler, Seq(knownSampleGoodFile), Seq("-d", out.getAbsolutePath))
-    result shouldBe true
-    new File(out, "index.html").exists shouldBe true
-    new File(out, "good.html").exists shouldBe true
+    assert(result)
+    val idx = new File(out, "index.html")
+    assert(idx.exists)
+    val good = new File(out, "good.html")
+    assert(good.exists)
   }
 
   def works(compiler: XJavaTools, forked: Boolean = false) = IO.withTemporaryDirectory { out =>
@@ -54,35 +57,39 @@ class JavaCompilerSpec extends UnitSpec {
         .withUseCustomizedFileManager(true)
     )
 
-    result shouldBe true
+    assert(result)
     val classfile = new File(out, "good.class")
-    classfile.exists shouldBe true
-    classfileManager.generatedClasses shouldEqual (if (forked) Set() else Set(classfile))
+    assert(classfile.exists)
+    assert(classfileManager.generatedClasses == (if (forked) Set() else Set(classfile)))
 
     val cl = new URLClassLoader(Array(out.toURI.toURL))
     val clazzz = cl.loadClass("good")
     val mthd = clazzz.getDeclaredMethod("test")
-    mthd.invoke(null) shouldBe "Hello"
+    assert(mthd.invoke(null) == "Hello")
   }
 
   def findsErrors(compiler: XJavaTools) = {
     val (result, problems) = compile(compiler, Seq(knownSampleErrorFile), Seq("-deprecation"))
-    result shouldBe false
-    problems should have size 6
+    assert(result == false)
+    assert(problems.size == 6)
     val importWarn = warnOnLine(lineno = 1, lineContent = Some("java.rmi.RMISecurityException"))
     val enclosingError = errorOnLine(lineno = 14, message = Some("not an enclosing class: C.D"))
     val beAnExpectedError =
       List(importWarn, errorOnLine(3), errorOnLine(4), warnOnLine(7), enclosingError) reduce (_ or _)
-    problems foreach (_ should beAnExpectedError)
+    problems foreach { p =>
+      p should beAnExpectedError
+    }
   }
 
   def findsDocErrors(compiler: XJavaTools) = IO.withTemporaryDirectory { out =>
     val (result, problems) =
       doc(compiler, Seq(knownSampleErrorFile), Seq("-d", out.getAbsolutePath))
-    result shouldBe true
-    problems should have size 2
+    assert(result)
+    assert(problems.size == 2)
     val beAnExpectedError = List(errorOnLine(3), errorOnLine(4)) reduce (_ or _)
-    problems foreach (_ should beAnExpectedError)
+    problems foreach { p =>
+      p should beAnExpectedError
+    }
   }
 
   /**
@@ -109,7 +116,7 @@ class JavaCompilerSpec extends UnitSpec {
 
         // then compile it
         val (result, _) = compile(local, Seq(input), Seq("-d", out.getAbsolutePath))
-        result shouldBe true
+        assert(result)
         val clazzz = new URLClassLoader(Array(out.toURI.toURL)).loadClass("hasstaticfinal")
         ClassToAPI(Seq(clazzz))
       }
@@ -118,8 +125,8 @@ class JavaCompilerSpec extends UnitSpec {
     // values match
     val leftAPI = compileWithPrimitive(leftType, left)
     val rightAPI = compileWithPrimitive(rightType, right)
-    leftAPI.size shouldBe rightAPI.size
-    ((leftAPI, rightAPI).zipped forall SameAPI.apply) shouldBe (left == right)
+    assert(leftAPI.size == rightAPI.size)
+    assert(((leftAPI, rightAPI).zipped forall SameAPI.apply) == (left == right))
     ()
   }
 
@@ -168,19 +175,19 @@ class JavaCompilerSpec extends UnitSpec {
   def forkSameAsLocal() = {
     val (fresult, fproblems) = compile(forked, Seq(knownSampleErrorFile), Seq("-deprecation"))
     val (lresult, lproblems) = compile(local, Seq(knownSampleErrorFile), Seq("-deprecation"))
-    fresult shouldBe lresult
+    assert(fresult == lresult)
 
     (fproblems zip lproblems) foreach {
       case (f, l) =>
         // TODO - We should check to see if the levenshtein distance of the messages is close...
         if (f.position.sourcePath.isPresent)
-          f.position.sourcePath.get shouldBe l.position.sourcePath.get
-        else l.position.sourcePath.isPresent shouldBe false
+          assert(f.position.sourcePath.get == l.position.sourcePath.get)
+        else assert(!l.position.sourcePath.isPresent)
 
-        if (f.position.line.isPresent) f.position.line.get shouldBe l.position.line.get
-        else l.position.line.isPresent shouldBe false
+        if (f.position.line.isPresent) assert(f.position.line.get == l.position.line.get)
+        else assert(!l.position.line.isPresent)
 
-        f.severity shouldBe l.severity
+        assert(f.severity == l.severity)
     }
   }
 

internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavacProcessLoggerSpec.scala

@@ -18,13 +18,13 @@ class JavaProcessLoggerSpec extends UnitSpec {
     val javacLogger = new JavacLogger(errorLogger, reporter, cwd = new File("."))
     javacLogger.err(
       Seq(
-        """Test.java:4: cannot find symbol
+        """/home/someone/Test.java:4: cannot find symbol
                           |symbol  : method baz()
                           |location: class Foo
                           |return baz();
                           |       ^
                           |""",
-        """Test.java:8: warning: [deprecation] RMISecurityException(java.lang.String) in java.rmi.RMISecurityException has been deprecated
+        """/home/someone/Test.java:8: warning: [deprecation] RMISecurityException(java.lang.String) in java.rmi.RMISecurityException has been deprecated
                                   |throw new java.rmi.RMISecurityException("O NOES");
                                   |^
                                   |"""
@@ -41,14 +41,14 @@ class JavaProcessLoggerSpec extends UnitSpec {
     val reporter = new CollectingReporter()
     val errorLogger = new CollectingLogger()
     val javacLogger = new JavacLogger(errorLogger, reporter, cwd = new File("."))
-    javacLogger.err("""Test.java:4: cannot find symbol
+    javacLogger.err("""/home/someone/Test.java:4: cannot find symbol
                           |symbol  : method baz()
                           |location: class Foo
                           |return baz();
                           |       ^
                           |""")
     javacLogger.err(
-      """Test.java:8: warning: [deprecation] RMISecurityException(java.lang.String) in java.rmi.RMISecurityException has been deprecated
+      """/home/someone/Test.java:8: warning: [deprecation] RMISecurityException(java.lang.String) in java.rmi.RMISecurityException has been deprecated
                                   |throw new java.rmi.RMISecurityException("O NOES");
                                   |^
                                   |""")

internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala

@@ -3,33 +3,35 @@ package internal
 package inc
 package javac
 
+import org.scalatest.DiagrammedAssertions
 import sbt.util.Logger
+import sbt.internal.util.ConsoleLogger
 
-class JavaErrorParserSpec extends UnitSpec {
+class JavaErrorParserSpec extends UnitSpec with DiagrammedAssertions {
 
-  "The JavaErrorParser" should "be able to parse linux errors" in parseSampleLinux()
+  "The JavaErrorParser" should "be able to parse Linux errors" in parseSampleLinux()
   it should "be able to parse windows file names" in parseWindowsFile()
   it should "be able to parse windows errors" in parseSampleWindows()
   it should "be able to parse javac errors" in parseSampleJavac()
   it should "register the position of errors" in parseErrorPosition()
   it should "be able to parse multiple errors" in parseMultipleErrors()
+  it should "be able to parse multiple errors without carrets or indentation" in parseMultipleErrors2()
 
   def parseSampleLinux() = {
     val parser = new JavaErrorParser()
-    val logger = Logger.Null
+    val logger = ConsoleLogger()
     val problems = parser.parseProblems(sampleLinuxMessage, logger)
 
-    problems should have size (1)
-    problems(0).position.sourcePath.get shouldBe ("/home/me/projects/sample/src/main/Test.java")
-
+    assert(problems.size == 1)
+    assert(problems(0).position.sourcePath.get == ("/home/me/projects/sample/src/main/Test.java"))
   }
 
   def parseSampleWindows() = {
     val parser = new JavaErrorParser()
-    val logger = Logger.Null
+    val logger = ConsoleLogger()
     val problems = parser.parseProblems(sampleWindowsMessage, logger)
 
-    problems should have size (1)
+    assert(problems.size == 1)
     problems(0).position.sourcePath.get shouldBe (windowsFile)
 
   }
@@ -47,35 +49,62 @@ class JavaErrorParserSpec extends UnitSpec {
 
   def parseSampleJavac() = {
     val parser = new JavaErrorParser()
-    val logger = Logger.Null
+    val logger = ConsoleLogger()
     val problems = parser.parseProblems(sampleJavacMessage, logger)
-    problems should have size (1)
+    assert(problems.size == 1)
     problems(0).message shouldBe (sampleJavacMessage)
   }
 
   def parseErrorPosition() = {
     val parser = new JavaErrorParser()
-    val logger = Logger.Null
+    val logger = ConsoleLogger()
     val problems = parser.parseProblems(sampleErrorPosition, logger)
-    problems should have size (1)
+    assert(problems.size == 1)
     problems(0).position.offset.isPresent shouldBe true
     problems(0).position.offset.get shouldBe 23
   }
 
   def parseMultipleErrors() = {
     val parser = new JavaErrorParser()
-    val logger = Logger.Null
+    val logger = ConsoleLogger()
     val problems = parser.parseProblems(sampleMultipleErrors, logger)
-    problems should have size (5)
+    assert(problems.size == 5)
+  }
+
+  def parseMultipleErrors2() = {
+    val parser = new JavaErrorParser()
+    val logger = ConsoleLogger()
+    val problems = parser.parseProblems(sampleLinuxMessage2, logger)
+
+    assert(problems.size == 3)
+    assert(problems(0).position.sourcePath.get == ("/home/me/projects/sample/src/main/Test.java"))
   }
 
   def sampleLinuxMessage =
     """
-      |/home/me/projects/sample/src/main/Test.java:4: cannot find symbol
-      |symbol  : method baz()
-      |location: class Foo
-      |return baz();
-    """.stripMargin
+      |/home/me/projects/sample/src/main/Test.java:18: error: cannot find symbol
+      |          baz();
+      |          ^
+      |  symbol:   method baz()
+      |  location: class AbstractActorRef
+      |1 error.
+      |""".stripMargin
+
+  def sampleLinuxMessage2 =
+    """
+      |/home/me/projects/sample/src/main/Test.java:100:1: cannot find symbol
+      |symbol: method isBar()
+      |location: variable x of type com.example.List
+      |if (x.isBar()) {
+      |/home/me/projects/sample/src/main/Test.java:200:1: cannot find symbol
+      |symbol: method isBar()
+      |location: variable x of type com.example.List
+      |} else if (x.isBar()) {
+      |/home/me/projects/sample/src/main/Test.java:300:1: cannot find symbol
+      |symbol: method isBar()
+      |location: variable x of type com.example.List
+      |foo.baz(runtime, x.isBar());
+      |""".stripMargin
 
   def sampleWindowsMessage =
     s"""
@@ -117,5 +146,6 @@ class JavaErrorParserSpec extends UnitSpec {
        |/home/foo/sbt/internal/inc/javac/test1.java:7: warning: [deprecation] RMISecurityException(String) in RMISecurityException has been deprecated
        |        throw new RMISecurityException("O NOES");
        |              ^
+       |5 errors.
        |""".stripMargin
 }