From de313bbdde866abc07577f89a0077b1de898bf34 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 4 Mar 2021 15:50:39 +0100 Subject: [PATCH] Never set a default for sourcePositionMappers --- build.sbt | 2 + main/src/main/scala/sbt/Defaults.scala | 57 ++++++++++++------ .../sbt-test/reporter/source-mapper/build.sbt | 59 +++++++++---------- .../project/FakePrintWriter.scala | 6 ++ .../source-mapper/src/main/scala/Foo.scala | 4 ++ sbt/src/sbt-test/reporter/source-mapper/test | 12 +++- 6 files changed, 90 insertions(+), 50 deletions(-) create mode 100644 sbt/src/sbt-test/reporter/source-mapper/project/FakePrintWriter.scala diff --git a/build.sbt b/build.sbt index 91e95cacf..ba934905d 100644 --- a/build.sbt +++ b/build.sbt @@ -1033,6 +1033,8 @@ lazy val mainProj = (project in file("main")) // internal impl exclude[IncompatibleSignatureProblem]("sbt.internal.Act.configIdent"), exclude[IncompatibleSignatureProblem]("sbt.internal.Act.taskAxis"), + // private[sbt] method, used to call the correct sourcePositionMapper + exclude[DirectMissingMethodProblem]("sbt.Defaults.foldMappers"), ) ) .configure( diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index e4d4143f3..af19b4a03 100644 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -422,12 +422,7 @@ object Defaults extends BuildCommon { ) }, fileConverter := MappedFileConverter(rootPaths.value, allowMachinePath.value), - sourcePositionMappers ++= { - val fc = fileConverter.value - if (reportAbsolutePath.value) { - List(toAbsoluteSourceMapper(fc) _) - } else Nil - }, + sourcePositionMappers := Nil, // Never set a default sourcePositionMapper, see #6352! Whatever you are trying to solve, do it in the foldMappers method. // The virtual file value cache needs to be global or sbt will run out of direct byte buffer memory. classpathDefinesClassCache := VirtualFileValueCache.definesClassCache(fileConverter.value), fullServerHandlers := { @@ -474,7 +469,7 @@ object Defaults extends BuildCommon { }, ) - private[sbt] def toAbsoluteSourceMapper(fc: FileConverter)(pos: Position): Option[Position] = { + private[sbt] def toAbsoluteSource(fc: FileConverter)(pos: Position): Position = { def isValid(path: String): Boolean = { Try(Paths.get(path)).map(_ => true).getOrElse(false) } @@ -503,9 +498,21 @@ object Defaults extends BuildCommon { override def sourcePath(): Optional[String] = Optional.of(path) override def sourceFile(): Optional[File] = pos.sourceFile() + + override def startOffset(): Optional[Integer] = pos.startOffset() + + override def endOffset(): Optional[Integer] = pos.endOffset() + + override def startLine(): Optional[Integer] = pos.startLine() + + override def startColumn(): Optional[Integer] = pos.startColumn() + + override def endLine(): Optional[Integer] = pos.endLine() + + override def endColumn(): Optional[Integer] = pos.endColumn() } } - .orElse(Some(pos)) + .getOrElse(pos) } // csrCacheDirectory is scoped to ThisBuild to allow customization. @@ -2365,7 +2372,9 @@ object Defaults extends BuildCommon { scalacOptions.value.toArray, javacOptions.value.toArray, maxErrors.value, - f1(foldMappers(sourcePositionMappers.value)), + f1( + foldMappers(sourcePositionMappers.value, reportAbsolutePath.value, fileConverter.value) + ), compileOrder.value, None.toOptional: Optional[NioPath], Some(fileConverter.value).toOptional, @@ -2377,7 +2386,7 @@ object Defaults extends BuildCommon { new ManagedLoggedReporter( maxErrors.value, streams.value.log, - foldMappers(sourcePositionMappers.value) + foldMappers(sourcePositionMappers.value, reportAbsolutePath.value, fileConverter.value) ) }, compileInputs := { @@ -2394,15 +2403,25 @@ object Defaults extends BuildCommon { ) } - private[sbt] def foldMappers[A](mappers: Seq[A => Option[A]]) = - mappers.foldRight({ p: A => - p + private[sbt] def foldMappers( + mappers: Seq[Position => Option[Position]], + reportAbsolutePath: Boolean, + fc: FileConverter + ) = { + def withAbsoluteSource(p: Position): Position = + if (reportAbsolutePath) toAbsoluteSource(fc)(p) else p + + mappers.foldRight({ p: Position => + withAbsoluteSource(p) // Fallback if sourcePositionMappers is empty }) { - (mapper, mappers) => - { p: A => - mapper(p).getOrElse(mappers(p)) + (mapper, previousPosition) => + { p: Position => + // To each mapper we pass the position with the absolute source (only if reportAbsolutePath = true of course) + mapper(withAbsoluteSource(p)).getOrElse(previousPosition(p)) } } + } + private[sbt] def none[A]: Option[A] = (None: Option[A]) private[sbt] def jnone[A]: Optional[A] = none[A].toOptional def compileAnalysisSettings: Seq[Setting[_]] = Seq( @@ -2429,7 +2448,11 @@ object Defaults extends BuildCommon { val problems = analysis.infos.allInfos.values .flatMap(i => i.getReportedProblems ++ i.getUnreportedProblems) - val reporter = new ManagedLoggedReporter(max, streams.value.log, foldMappers(spms)) + val reporter = new ManagedLoggedReporter( + max, + streams.value.log, + foldMappers(spms, reportAbsolutePath.value, fileConverter.value) + ) problems.foreach(p => reporter.log(p)) } diff --git a/sbt/src/sbt-test/reporter/source-mapper/build.sbt b/sbt/src/sbt-test/reporter/source-mapper/build.sbt index 3ca4753ed..a6be7bed5 100644 --- a/sbt/src/sbt-test/reporter/source-mapper/build.sbt +++ b/sbt/src/sbt-test/reporter/source-mapper/build.sbt @@ -1,46 +1,41 @@ -import java.util.Optional -import xsbti.Position +import sbt.internal.util.ConsoleAppender + +extraAppenders := { s => Seq(ConsoleAppender(FakePrintWriter)) } + +val assertEmptySourcePositionMappers = taskKey[Unit]("checks that sourcePositionMappers is empty") val assertAbsolutePathConversion = taskKey[Unit]("checks source mappers convert to absolute path") -val assertHandleFakePos = taskKey[Unit]("checks source mappers handle fake position") +val assertVirtualFile = taskKey[Unit]("checks source mappers handle virtual files") + +val resetMessages = taskKey[Unit]("empties the messages list") + +assertEmptySourcePositionMappers := { + assert { + sourcePositionMappers.value.isEmpty + } +} assertAbsolutePathConversion := { - val converter = fileConverter.value val source = (Compile/sources).value.head - val position = newPosition(converter.toVirtualFile(source.toPath).id, source) - val mappedPos = sourcePositionMappers.value - .foldLeft(Option(position)) { - case (pos, mapper) => pos.flatMap(mapper) - } assert { - mappedPos.get.sourcePath.asScala.contains(source.getAbsolutePath) + FakePrintWriter.messages.exists(_.contains(s"${source.getAbsolutePath}:3:15: comparing values of types Int and String using `==` will always yield false")) + } + assert { + FakePrintWriter.messages.forall(!_.contains("${BASE}")) } } -assertHandleFakePos := { - val position = newPosition("", new File("")) - val mappedPos = sourcePositionMappers.value - .foldLeft(Option(position)) { - case (pos, mapper) => pos.flatMap(mapper) - } +assertVirtualFile := { + val source = (Compile/sources).value.head assert { - mappedPos.get.sourcePath.asScala.get.contains("") + FakePrintWriter.messages.exists(_.contains("${BASE}/src/main/scala/Foo.scala:3:15: comparing values of types Int and String using `==` will always yield false")) + } + assert { + FakePrintWriter.messages.forall(!_.contains(source.getAbsolutePath)) } } -def newPosition(path: String, file: File): Position = new Position { - override def line(): Optional[Integer] = Optional.empty() - - override def lineContent() = "" - - override def offset(): Optional[Integer] = Optional.empty() - - override def pointer(): Optional[Integer] = Optional.empty() - - override def pointerSpace(): Optional[String] = Optional.empty() - - override def sourcePath(): Optional[String] = Optional.of(path) - - override def sourceFile(): Optional[File] = Optional.of(file) -} \ No newline at end of file +resetMessages := { + FakePrintWriter.resetMessages +} diff --git a/sbt/src/sbt-test/reporter/source-mapper/project/FakePrintWriter.scala b/sbt/src/sbt-test/reporter/source-mapper/project/FakePrintWriter.scala new file mode 100644 index 000000000..14d3bcdba --- /dev/null +++ b/sbt/src/sbt-test/reporter/source-mapper/project/FakePrintWriter.scala @@ -0,0 +1,6 @@ +object FakePrintWriter extends java.io.PrintWriter("fake-print-writer") { + @volatile var messages = List.empty[String] + def resetMessages = messages = List.empty[String] + override def println(x: String): Unit = messages = x :: messages + override def print(x: String): Unit = messages = x :: messages +} diff --git a/sbt/src/sbt-test/reporter/source-mapper/src/main/scala/Foo.scala b/sbt/src/sbt-test/reporter/source-mapper/src/main/scala/Foo.scala index e69de29bb..fd36a2e06 100644 --- a/sbt/src/sbt-test/reporter/source-mapper/src/main/scala/Foo.scala +++ b/sbt/src/sbt-test/reporter/source-mapper/src/main/scala/Foo.scala @@ -0,0 +1,4 @@ +object Foo { + // Will cause a warning + val bar = 1 == "" +} diff --git a/sbt/src/sbt-test/reporter/source-mapper/test b/sbt/src/sbt-test/reporter/source-mapper/test index ced01bedc..a0f3e7889 100644 --- a/sbt/src/sbt-test/reporter/source-mapper/test +++ b/sbt/src/sbt-test/reporter/source-mapper/test @@ -1,2 +1,12 @@ +> assertEmptySourcePositionMappers + +> printWarnings > assertAbsolutePathConversion -> assertHandleFakePos \ No newline at end of file + +> resetMessages + +> set reportAbsolutePath := false +> printWarnings +> assertVirtualFile + +> resetMessages