Skip to content

Commit

Permalink
Merge pull request #1373 from vasilmkd/javac-column
Browse files Browse the repository at this point in the history 
Parse column/position information from javac error messages
  • Loading branch information
@eed3si9n
eed3si9n authored Jul 7, 2024
2 parents e0ef153 + f51114b commit 15b317b
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 35 deletions.
15 changes: 13 additions & 2 deletions internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ object DiagnosticsReporter {
override val line: Optional[Integer],
override val lineContent: String,
override val offset: Optional[Integer],
override val pointer: Optional[Integer],
override val pointerSpace: Optional[String],
override val startOffset: Optional[Integer],
override val endOffset: Optional[Integer],
override val startLine: Optional[Integer],
Expand All @@ -81,8 +83,6 @@ object DiagnosticsReporter {
) extends xsbti.Position {
override val sourcePath: Optional[String] = o2jo(sourceUri)
override val sourceFile: Optional[File] = o2jo(sourceUri.map(new File(_)))
override val pointer: Optional[Integer] = o2jo(Option.empty[Integer])
override val pointerSpace: Optional[String] = o2jo(Option.empty[String])

override def toString: String =
if (sourceUri.isDefined) s"${sourceUri.get}:${if (line.isPresent) line.get else -1}"
Expand Down Expand Up @@ -200,6 +200,8 @@ object DiagnosticsReporter {
def endPosition: Option[Long] = checkNoPos(d.getEndPosition)

val line: Optional[Integer] = o2jo(checkNoPos(d.getLineNumber) map (_.toInt))
// column number is 1-based, xsbti.Position#pointer is 0-based.
val pointer: Optional[Integer] = o2jo(checkNoPos(d.getColumnNumber - 1) map (_.toInt))
val offset: Optional[Integer] = o2jo(checkNoPos(d.getPosition) map (_.toInt))
val startOffset: Optional[Integer] = o2jo(startPosition map (_.toInt))
val endOffset: Optional[Integer] = o2jo(endPosition map (_.toInt))
Expand Down Expand Up @@ -241,11 +243,20 @@ object DiagnosticsReporter {
case _ => noPositionInfo
}

val pointerSpace = pointer.map[String] { p =>
lineContent.take(p.intValue()).map {
case '\t' => '\t'
case _ => ' '
}
}

new PositionImpl(
sourcePath,
line,
lineContent,
offset,
pointer,
pointerSpace,
startOffset,
endOffset,
startLine,
Expand Down
44 changes: 27 additions & 17 deletions internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,21 @@ import sbt.util.InterfaceUtil.o2jo
import xsbti.{ Problem, Severity, Position }

/** A wrapper around xsbti.Position so we can pass in Java input. */
final case class JavaPosition(_sourceFilePath: String, _line: Int, _contents: String, _offset: Int)
extends Position {
final case class JavaPosition(
_sourceFilePath: String,
_line: Int,
_contents: String,
_pointer: Int,
_pointerSpace: String
) extends Position {
def line: Optional[Integer] = o2jo(Some(_line))
def lineContent: String = _contents
def offset: Optional[Integer] = o2jo(Some(_offset))
def pointer: Optional[Integer] = o2jo(None)
def pointerSpace: Optional[String] = o2jo(None)
def offset: Optional[Integer] = o2jo(None)
def pointer: Optional[Integer] = o2jo(Some(_pointer))
def pointerSpace: Optional[String] = o2jo(Option(_pointerSpace))
def sourcePath: Optional[String] = o2jo(Option(_sourceFilePath))
def sourceFile: Optional[File] = o2jo(Option(new File(_sourceFilePath)))
override def toString = s"${_sourceFilePath}:${_line}:${_offset}"
override def toString = s"${_sourceFilePath}:${_line}:${_pointer}"
}

/** A position which has no information, because there is none. */
Expand Down Expand Up @@ -181,6 +186,11 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
}
fileLineMessage ~ (allUntilCaret ~ '^' ~ restOfLine).? ~ (nonPathLines.?) ^^ {
case (file, line, msg) ~ contentsOpt ~ ind =>
val (pointer, pointerSpace) = contentsOpt match {
case Some(contents ~ _ ~ _) => getPointer(contents)
case _ => (0, "")
}

new JavaProblem(
new JavaPosition(
findFileSource(file),
Expand All @@ -190,10 +200,8 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
case _ => ""
}) + ind
.getOrElse(""), // TODO - Actually parse caret position out of here.
(contentsOpt match {
case Some(contents ~ _ ~ _) => getOffset(contents)
case _ => 0
})
pointer,
pointerSpace
),
Severity.Error,
msg
Expand All @@ -208,6 +216,11 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
}
fileLineMessage ~ (allUntilCaret ~ '^' ~ restOfLine).? ~ (nonPathLines.?) ^^ {
case (file, line, msg) ~ contentsOpt ~ ind =>
val (pointer, pointerSpace) = contentsOpt match {
case Some(contents ~ _ ~ _) => getPointer(contents)
case _ => (0, "")
}

new JavaProblem(
new JavaPosition(
findFileSource(file),
Expand All @@ -216,10 +229,8 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
case Some(contents ~ _ ~ r) => contents + '^' + r
case _ => ""
}) + ind.getOrElse(""),
(contentsOpt match {
case Some(contents ~ _ ~ _) => getOffset(contents)
case _ => 0
})
pointer,
pointerSpace
),
Severity.Warn,
msg
Expand Down Expand Up @@ -292,7 +303,6 @@ class JavaErrorParser(relativeDir: File = new File(new File(".").getAbsolutePath
Seq.empty
}

private def getOffset(contents: String): Int =
contents.linesIterator.toList.lastOption map (_.length) getOrElse 0

private def getPointer(contents: String): (Int, String) =
contents.linesIterator.toList.lastOption.map(line => (line.length, line)).getOrElse((0, ""))
}
53 changes: 40 additions & 13 deletions internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavaCompilerSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,17 @@ class JavaCompilerSpec extends UnitSpec with Diagrams {
case _ => 5
}
})
val importWarn = warnOnLine(lineno = 12, lineContent = Some("java.rmi.RMISecurityException"))
val enclosingError = errorOnLine(lineno = 25, message = Some("not an enclosing class: C.D"))
val importWarn =
warnOnLine(lineno = 12, colno = 15, lineContent = Some("java.rmi.RMISecurityException"))
val enclosingError =
errorOnLine(lineno = 25, colno = 9, message = Some("not an enclosing class: C.D"))
val beAnExpectedError =
List(
importWarn,
errorOnLine(14),
errorOnLine(15),
warnOnLine(18),
errorOnLine(14, 7),
errorOnLine(15, 11),
warnOnLine(18, 18),
warnOnLine(18, 14), // could be expected depending on Java version
enclosingError
) reduce (_ or _)
problems foreach { p =>
Expand All @@ -140,7 +143,7 @@ class JavaCompilerSpec extends UnitSpec with Diagrams {
}
})
assert(problems.size == 2)
val beAnExpectedError = List(errorOnLine(14), errorOnLine(15)) reduce (_ or _)
val beAnExpectedError = List(errorOnLine(14, 7), errorOnLine(15, 11)) reduce (_ or _)
problems foreach { p =>
p should beAnExpectedError
}
Expand Down Expand Up @@ -192,20 +195,43 @@ class JavaCompilerSpec extends UnitSpec with Diagrams {
lineNumberCheck && messageCheck
}

def lineMatches(p: Problem, lineno: Int, lineContent: Option[String] = None): Boolean = {
def lineMatches(
p: Problem,
lineno: Int,
colno: Int,
lineContent: Option[String] = None
): Boolean = {
def lineContentCheck = lineContent forall (content => p.position.lineContent contains content)
def lineNumberCheck = p.position.line.isPresent && (p.position.line.get == lineno)
lineNumberCheck && lineContentCheck
def columnCheck = {
val res = p.position.pointer.isPresent && (p.position.pointer.get == colno)
if (!res) {
println(s"got ${p.position().pointer().get}, expected $colno, problem $p")
}
res
}
lineNumberCheck && columnCheck && lineContentCheck
}

def errorOnLine(lineno: Int, message: Option[String] = None, lineContent: Option[String] = None) =
problemOnLine(lineno, Severity.Error, message, lineContent)
def errorOnLine(
lineno: Int,
colno: Int,
message: Option[String] = None,
lineContent: Option[String] = None
) =
problemOnLine(lineno, colno, Severity.Error, message, lineContent)

def warnOnLine(lineno: Int, message: Option[String] = None, lineContent: Option[String] = None) =
problemOnLine(lineno, Severity.Warn, message, lineContent)
def warnOnLine(
lineno: Int,
colno: Int,
message: Option[String] = None,
lineContent: Option[String] = None
) =
problemOnLine(lineno, colno, Severity.Warn, message, lineContent)

private def problemOnLine(
lineno: Int,
colno: Int,
severity: Severity,
message: Option[String],
lineContent: Option[String]
Expand All @@ -218,9 +244,10 @@ class JavaCompilerSpec extends UnitSpec with Diagrams {
messageMatches(p, lineno, message) && lineMatches(
p,
lineno,
colno,
lineContent
) && p.severity == severity,
s"Expected $problemType on line $lineno$msg$content, but found $p",
s"Expected $problemType on line $lineno, column $colno$msg$content, but found $p",
"Problem matched: " + p
)
}
Expand Down
70 changes: 67 additions & 3 deletions internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,79 @@ class JavaErrorParserSpec extends UnitSpec with Diagrams {
val logger = ConsoleLogger()
val problems = parser.parseProblems(sampleErrorPosition, logger)
assert(problems.size == 1)
problems(0).position.offset.isPresent shouldBe true
problems(0).position.offset.get shouldBe 23
val position = problems.head.position

position.sourcePath.isPresent shouldBe true
position.sourcePath.get shouldBe "/home/me/projects/sample/src/main/java/A.java"
position.line.isPresent shouldBe true
position.line.get shouldBe 6
position.lineContent should include("System.out.println(foobar);")
position.pointer.isPresent shouldBe true
position.pointer.get shouldBe 23
position.pointerSpace.isPresent shouldBe true
position.pointerSpace.get shouldBe (" " * 23)
}

def parseMultipleErrors() = {
val parser = new JavaErrorParser()
val logger = ConsoleLogger()
val problems = parser.parseProblems(sampleMultipleErrors, logger)
assert(problems.size == 5)

val position1 = problems(0).position
position1.sourcePath.isPresent shouldBe true
position1.sourcePath.get shouldBe "/home/foo/sbt/internal/inc/javac/test1.java"
position1.line.isPresent shouldBe true
position1.line.get shouldBe 3
position1.lineContent should include("public class Test {")
position1.pointer.isPresent shouldBe true
position1.pointer.get shouldBe 7
position1.pointerSpace.isPresent shouldBe true
position1.pointerSpace.get shouldBe (" " * 7)

val position2 = problems(1).position
position2.sourcePath.isPresent shouldBe true
position2.sourcePath.get shouldBe "/home/foo/sbt/internal/inc/javac/test1.java"
position2.line.isPresent shouldBe true
position2.line.get shouldBe 1
position2.lineContent should include("import java.rmi.RMISecurityException;")
position2.pointer.isPresent shouldBe true
position2.pointer.get shouldBe 15
position2.pointerSpace.isPresent shouldBe true
position2.pointerSpace.get shouldBe (" " * 15)

val position3 = problems(2).position
position3.sourcePath.isPresent shouldBe true
position3.sourcePath.get shouldBe "/home/foo/sbt/internal/inc/javac/test1.java"
position3.line.isPresent shouldBe true
position3.line.get shouldBe 4
position3.lineContent should include("public NotFound foo() { return 5; }")
position3.pointer.isPresent shouldBe true
position3.pointer.get shouldBe 11
position3.pointerSpace.isPresent shouldBe true
position3.pointerSpace.get shouldBe (" " * 11)

val position4 = problems(3).position
position4.sourcePath.isPresent shouldBe true
position4.sourcePath.get shouldBe "/home/foo/sbt/internal/inc/javac/test1.java"
position4.line.isPresent shouldBe true
position4.line.get shouldBe 7
position4.lineContent should include("""throw new RMISecurityException("O NOES");""")
position4.pointer.isPresent shouldBe true
position4.pointer.get shouldBe 18
position4.pointerSpace.isPresent shouldBe true
position4.pointerSpace.get shouldBe (" " * 18)

val position5 = problems(4).position
position5.sourcePath.isPresent shouldBe true
position5.sourcePath.get shouldBe "/home/foo/sbt/internal/inc/javac/test1.java"
position5.line.isPresent shouldBe true
position5.line.get shouldBe 7
position5.lineContent should include("""throw new RMISecurityException("O NOES");""")
position5.pointer.isPresent shouldBe true
position5.pointer.get shouldBe 14
position5.pointerSpace.isPresent shouldBe true
position5.pointerSpace.get shouldBe (" " * 14)
}

def parseMultipleErrors2() = {
Expand Down Expand Up @@ -144,7 +208,7 @@ class JavaErrorParserSpec extends UnitSpec with Diagrams {

def sampleErrorPosition =
"""
|A.java:6: cannot find symbol
|/home/me/projects/sample/src/main/java/A.java:6: cannot find symbol
|symbol : variable foobar
|location: class A
| System.out.println(foobar);
Expand Down

0 comments on commit 15b317b

Please sign in to comment.