From d559afee9dae78bfa9481c9f84995ab5c58bb78c Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Wed, 3 Jul 2024 18:21:52 +0200 Subject: [PATCH 1/5] Obtain xsbti.Position#pointer information from a Java Diagnostic instance --- .../internal/inc/javac/DiagnosticsReporter.scala | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala index d4d5a1607..9b12c2431 100644 --- a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala +++ b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala @@ -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], @@ -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}" @@ -200,6 +200,7 @@ object DiagnosticsReporter { def endPosition: Option[Long] = checkNoPos(d.getEndPosition) val line: Optional[Integer] = o2jo(checkNoPos(d.getLineNumber) map (_.toInt)) + val pointer: Optional[Integer] = o2jo(checkNoPos(d.getColumnNumber) 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)) @@ -241,11 +242,20 @@ object DiagnosticsReporter { case _ => noPositionInfo } + val pointerSpace = pointer.map[String] { p => + lineContent.toList.take(p.intValue()).map { + case '\t' => '\t' + case _ => ' ' + }.mkString + } + new PositionImpl( sourcePath, line, lineContent, offset, + pointer, + pointerSpace, startOffset, endOffset, startLine, From 193e53911af6bc4ed07a090a977fa3c47624f1cb Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Wed, 3 Jul 2024 18:46:49 +0200 Subject: [PATCH 2/5] Obtain xsbti.Position#pointer information from javac error and warning messages --- .../internal/inc/javac/JavaErrorParser.scala | 44 ++++++++++++------- .../inc/javac/javaErrorParserSpec.scala | 4 +- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala index 8f344636f..cb8c86058 100644 --- a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala +++ b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala @@ -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. */ @@ -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), @@ -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 @@ -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), @@ -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 @@ -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, "")) } diff --git a/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala b/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala index c7412ce49..abdbb4137 100644 --- a/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala +++ b/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala @@ -80,8 +80,8 @@ 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 + problems(0).position.pointer.isPresent shouldBe true + problems(0).position.pointer.get shouldBe 23 } def parseMultipleErrors() = { From d61f77f070344aca024a163fd4efaed68be6adb3 Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Wed, 3 Jul 2024 20:05:00 +0200 Subject: [PATCH 3/5] Add more assertions for the positions parsed from error messages --- .../inc/javac/javaErrorParserSpec.scala | 70 ++++++++++++++++++- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala b/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala index abdbb4137..5e4d8244f 100644 --- a/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala +++ b/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/javaErrorParserSpec.scala @@ -80,8 +80,17 @@ class JavaErrorParserSpec extends UnitSpec with Diagrams { val logger = ConsoleLogger() val problems = parser.parseProblems(sampleErrorPosition, logger) assert(problems.size == 1) - problems(0).position.pointer.isPresent shouldBe true - problems(0).position.pointer.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() = { @@ -89,6 +98,61 @@ class JavaErrorParserSpec extends UnitSpec with Diagrams { 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() = { @@ -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); From 437fc525f728cb801a3929922f098b39466c0d88 Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Wed, 3 Jul 2024 21:00:25 +0200 Subject: [PATCH 4/5] Convert the 1-based column number to 0-based for xsbti.Position#pointer - Add column number assertions in JavaCompilerSpec. --- .../inc/javac/DiagnosticsReporter.scala | 3 +- .../internal/inc/javac/JavaCompilerSpec.scala | 53 ++++++++++++++----- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala index 9b12c2431..e78938d29 100644 --- a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala +++ b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala @@ -200,7 +200,8 @@ object DiagnosticsReporter { def endPosition: Option[Long] = checkNoPos(d.getEndPosition) val line: Optional[Integer] = o2jo(checkNoPos(d.getLineNumber) map (_.toInt)) - val pointer: Optional[Integer] = o2jo(checkNoPos(d.getColumnNumber) 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)) diff --git a/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavaCompilerSpec.scala b/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavaCompilerSpec.scala index a4138c18d..d468f8b3e 100644 --- a/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavaCompilerSpec.scala +++ b/internal/zinc-compile-core/src/test/scala/sbt/internal/inc/javac/JavaCompilerSpec.scala @@ -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 => @@ -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 } @@ -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] @@ -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 ) } From f51114b83cd5607750f55006ded040bc36714f71 Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Wed, 3 Jul 2024 23:29:12 +0200 Subject: [PATCH 5/5] Avoid converting the line content string to a list and back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Ferreira --- .../scala/sbt/internal/inc/javac/DiagnosticsReporter.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala index e78938d29..dd1b34fb2 100644 --- a/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala +++ b/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/DiagnosticsReporter.scala @@ -244,10 +244,10 @@ object DiagnosticsReporter { } val pointerSpace = pointer.map[String] { p => - lineContent.toList.take(p.intValue()).map { + lineContent.take(p.intValue()).map { case '\t' => '\t' case _ => ' ' - }.mkString + } } new PositionImpl(