From 4c86117b797174eb39cf5c249cec32f68053b290 Mon Sep 17 00:00:00 2001 From: fkorotkov Date: Wed, 15 Jul 2015 14:49:12 -0400 Subject: [PATCH 1/6] Line content from diagnostic classes if available --- build.sbt | 2 +- .../compiler/javac/DiagnosticsReporter.scala | 41 ++++++++++++++----- .../sbt/compiler/javac/JavaCompilerSpec.scala | 27 ++++++++---- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/build.sbt b/build.sbt index 1569ec94e..6d4dde762 100644 --- a/build.sbt +++ b/build.sbt @@ -573,7 +573,7 @@ lazy val safeUnitTests = taskKey[Unit]("Known working tests (for both 2.10 and 2 lazy val safeProjects: ScopeFilter = ScopeFilter( inProjects(mainSettingsProj, mainProj, ivyProj, completeProj, actionsProj, classpathProj, collectionProj, compileIncrementalProj, - logProj, runProj, stdTaskProj), + logProj, runProj, stdTaskProj, compilerProj), inConfigurations(Test) ) lazy val otherUnitTests = taskKey[Unit]("Unit test other projects") diff --git a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala index 20ecb48b7..d7abef74c 100644 --- a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala +++ b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala @@ -69,18 +69,37 @@ final class DiagnosticsReporter(reporter: Reporter) extends DiagnosticListener[J def startPosition: Option[Long] = checkNoPos(d.getStartPosition) def endPosition: Option[Long] = checkNoPos(d.getEndPosition) override val offset: Maybe[Integer] = Logger.o2m(checkNoPos(d.getPosition) map { x => new Integer(x.toInt) }) - // TODO - Is this pulling contents of the line correctly? - // Would be ok to just return null if this version of the JDK doesn't support grabbing - // source lines? - override def lineContent: String = - Option(d.getSource) match { - case Some(source: JavaFileObject) => - (Option(source.getCharContent(true)), startPosition, endPosition) match { - case (Some(cc), Some(start), Some(end)) => cc.subSequence(start.toInt, end.toInt).toString - case _ => "" + override def lineContent: String = { + def getDiagnosticLine: Option[String] = + try { + // See com.sun.tools.javac.api.ClientCodeWrapper.DiagnosticSourceUnwrapper + val diagnostic = d.getClass.getField("d").get(d) + // See com.sun.tools.javac.util.JCDiagnostic#getDiagnosticSource + val getDiagnosticSourceMethod = diagnostic.getClass.getDeclaredMethod("getDiagnosticSource") + Option(getDiagnosticSourceMethod.invoke(diagnostic)) match { + case Some(diagnosticSource) => + // See com.sun.tools.javac.util.DiagnosticSource + val getLineMethod = diagnosticSource.getClass.getMethod("getLine", Integer.TYPE) + Option(getLineMethod.invoke(diagnosticSource, line.get())).map(_.toString) + case _ => None } - case _ => "" - } + } catch { + case ignored: NoSuchMethodException => None + case ignored: NoSuchFieldException => None + } + + def getExpression: String = + Option(d.getSource) match { + case Some(source: JavaFileObject) => + (Option(source.getCharContent(true)), startPosition, endPosition) match { + case (Some(cc), Some(start), Some(end)) => cc.subSequence(start.toInt, end.toInt).toString + case _ => "" + } + case _ => "" + } + + getDiagnosticLine.getOrElse(getExpression) + } private val sourceUri = fixSource(d.getSource) override val sourcePath = Logger.o2m(sourceUri) override val sourceFile = Logger.o2m(sourceUri.map(new File(_))) diff --git a/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala b/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala index 4eef68b22..a2bd1141c 100644 --- a/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala +++ b/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala @@ -60,7 +60,8 @@ object JavaCompilerSpec extends Specification { val (result, problems) = compile(compiler, Seq(knownSampleErrorFile), Seq("-deprecation")) val errored = result must beFalse val foundErrorAndWarning = problems must haveSize(5) - val hasKnownErrors = problems.toSeq must contain(errorOnLine(1), warnOnLine(7)) + val importWarn = warnOnLine(lineno = 1, lineContent = Some("import java.rmi.RMISecurityException;")) + val hasKnownErrors = problems.toSeq must contain(importWarn, errorOnLine(3), warnOnLine(7)) errored and foundErrorAndWarning and hasKnownErrors } @@ -107,20 +108,28 @@ object JavaCompilerSpec extends Specification { leftCompiled and rightCompiled and apisExpectedMatch } - def lineMatches(p: Problem, lineno: Int): Boolean = - p.position.line.isDefined && (p.position.line.get == lineno) + def lineMatches(p: Problem, lineno: Int, lineContent: Option[String] = None): Boolean = { + def lineContentCheck = + lineContent match { + case Some(content) => content.equalsIgnoreCase(p.position.lineContent()) + case _ => true + } + def lineNumberCheck = p.position.line.isDefined && (p.position.line.get == lineno) + lineNumberCheck && lineContentCheck + } + def isError(p: Problem): Boolean = p.severity == Severity.Error def isWarn(p: Problem): Boolean = p.severity == Severity.Warn - def errorOnLine(lineno: Int) = + def errorOnLine(lineno: Int, lineContent: Option[String] = None) = beLike[Problem]({ - case p if lineMatches(p, lineno) && isError(p) => ok - case _ => ko + case p if lineMatches(p, lineno, lineContent) && isError(p) => ok + case _ => ko }) - def warnOnLine(lineno: Int) = + def warnOnLine(lineno: Int, lineContent: Option[String] = None) = beLike[Problem]({ - case p if lineMatches(p, lineno) && isWarn(p) => ok - case _ => ko + case p if lineMatches(p, lineno, lineContent) && isWarn(p) => ok + case _ => ko }) def forkSameAsLocal = { From a6f83dda97d83bffbcaafd46bc19ca2cbf213854 Mon Sep 17 00:00:00 2001 From: fkorotkov Date: Wed, 15 Jul 2015 16:07:05 -0400 Subject: [PATCH 2/6] added a note --- .../proper-line-content-from-local-javac.markdown | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 notes/0.13.9/proper-line-content-from-local-javac.markdown diff --git a/notes/0.13.9/proper-line-content-from-local-javac.markdown b/notes/0.13.9/proper-line-content-from-local-javac.markdown new file mode 100644 index 000000000..2a92fd262 --- /dev/null +++ b/notes/0.13.9/proper-line-content-from-local-javac.markdown @@ -0,0 +1,10 @@ + + [@fkorotkov]: http://github.com/fkorotkov + +### Fixes with compatibility implications + +### Improvements + +Use diagnostic classes to get lines contents in local java compiler. + +### Bug fixes From c73f513016a8c2be94decfda8ea83b6e1b95601d Mon Sep 17 00:00:00 2001 From: fkorotkov Date: Thu, 16 Jul 2015 15:49:06 -0400 Subject: [PATCH 3/6] review comments --- .../main/scala/sbt/compiler/javac/DiagnosticsReporter.scala | 3 +-- .../src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala | 2 +- .../proper-line-content-from-local-javac.markdown | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) rename notes/{0.13.9 => 0.13.10}/proper-line-content-from-local-javac.markdown (79%) diff --git a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala index d7abef74c..702460885 100644 --- a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala +++ b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala @@ -84,8 +84,7 @@ final class DiagnosticsReporter(reporter: Reporter) extends DiagnosticListener[J case _ => None } } catch { - case ignored: NoSuchMethodException => None - case ignored: NoSuchFieldException => None + case ignored: ReflectiveOperationException => None } def getExpression: String = diff --git a/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala b/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala index a2bd1141c..93ad96382 100644 --- a/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala +++ b/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala @@ -114,7 +114,7 @@ object JavaCompilerSpec extends Specification { case Some(content) => content.equalsIgnoreCase(p.position.lineContent()) case _ => true } - def lineNumberCheck = p.position.line.isDefined && (p.position.line.get == lineno) + def lineNumberCheck = p.position.line.exists(_ == lineno) lineNumberCheck && lineContentCheck } diff --git a/notes/0.13.9/proper-line-content-from-local-javac.markdown b/notes/0.13.10/proper-line-content-from-local-javac.markdown similarity index 79% rename from notes/0.13.9/proper-line-content-from-local-javac.markdown rename to notes/0.13.10/proper-line-content-from-local-javac.markdown index 2a92fd262..5203e28c6 100644 --- a/notes/0.13.9/proper-line-content-from-local-javac.markdown +++ b/notes/0.13.10/proper-line-content-from-local-javac.markdown @@ -1,5 +1,6 @@ [@fkorotkov]: http://github.com/fkorotkov + [#2108]: https://github.com/sbt/sbt/pull/2108 ### Fixes with compatibility implications From 5258b4c0d348487a429a5f7553ed1ea961ba8d3c Mon Sep 17 00:00:00 2001 From: fkorotkov Date: Thu, 16 Jul 2015 16:33:04 -0400 Subject: [PATCH 4/6] reverted check --- .../src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala b/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala index 93ad96382..a2bd1141c 100644 --- a/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala +++ b/compile/src/test/scala/sbt/compiler/javac/JavaCompilerSpec.scala @@ -114,7 +114,7 @@ object JavaCompilerSpec extends Specification { case Some(content) => content.equalsIgnoreCase(p.position.lineContent()) case _ => true } - def lineNumberCheck = p.position.line.exists(_ == lineno) + def lineNumberCheck = p.position.line.isDefined && (p.position.line.get == lineno) lineNumberCheck && lineContentCheck } From 93ae6b2db07e6ea7356e88bfd4864b376881e913 Mon Sep 17 00:00:00 2001 From: fkorotkov Date: Thu, 16 Jul 2015 17:34:33 -0400 Subject: [PATCH 5/6] catch Throwable --- .../src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala index 702460885..2160620d6 100644 --- a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala +++ b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala @@ -84,7 +84,7 @@ final class DiagnosticsReporter(reporter: Reporter) extends DiagnosticListener[J case _ => None } } catch { - case ignored: ReflectiveOperationException => None + case ignored: Throwable => None } def getExpression: String = From e4f27ee2ca252bf28640a41c255e6f8ab626a252 Mon Sep 17 00:00:00 2001 From: fkorotkov Date: Thu, 16 Jul 2015 18:09:47 -0400 Subject: [PATCH 6/6] added a todo --- .../src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala index 2160620d6..aec47137e 100644 --- a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala +++ b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala @@ -84,6 +84,7 @@ final class DiagnosticsReporter(reporter: Reporter) extends DiagnosticListener[J case _ => None } } catch { + // TODO - catch ReflectiveOperationException once sbt is migrated to JDK7 case ignored: Throwable => None }