Never set a default for sourcePositionMappers

This commit is contained in:
Matthias Kurz 2021-03-04 15:50:39 +01:00
parent 33e2bfe281
commit de313bbdde
No known key found for this signature in database
GPG Key ID: 0B4AAA92F1117EF5
6 changed files with 90 additions and 50 deletions

View File

@ -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(

View File

@ -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))
}

View File

@ -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("<macro>", new File("<macro>"))
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("<macro>")
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)
}
resetMessages := {
FakePrintWriter.resetMessages
}

View File

@ -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
}

View File

@ -0,0 +1,4 @@
object Foo {
// Will cause a warning
val bar = 1 == ""
}

View File

@ -1,2 +1,12 @@
> assertEmptySourcePositionMappers
> printWarnings
> assertAbsolutePathConversion
> assertHandleFakePos
> resetMessages
> set reportAbsolutePath := false
> printWarnings
> assertVirtualFile
> resetMessages