Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-publish diagnostics for warnings #712

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .drone.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ pipeline:
- /drone/.coursier
- /drone/.sbt
- /drone/.dodo
- ./frontend/src/test/resources/cross-test-build-0.6/bloop-config
- ./frontend/src/test/resources/cross-test-build-0.6/target/generation-cache-file
- ./frontend/src/test/resources/cross-test-build-1.0/bloop-config
- ./frontend/src/test/resources/cross-test-build-1.0/target/generation-cache-file
volumes:
- /cache:/cache
cache_key: [ DRONE_REPO_OWNER, DRONE_REPO_NAME, DRONE_BRANCH ]
Expand Down Expand Up @@ -196,6 +200,10 @@ pipeline:
- /drone/.coursier
- /drone/.sbt
- /drone/.dodo
- ./frontend/src/test/resources/cross-test-build-0.6/bloop-config
- ./frontend/src/test/resources/cross-test-build-0.6/target/generation-cache-file
- ./frontend/src/test/resources/cross-test-build-1.0/bloop-config
- ./frontend/src/test/resources/cross-test-build-1.0/target/generation-cache-file
volumes:
- /cache:/cache
cache_key: [ DRONE_REPO_OWNER, DRONE_REPO_NAME, DRONE_BRANCH ]
25 changes: 24 additions & 1 deletion backend/src/main/scala/bloop/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ object Compiler {
}
}

def warningsFromPreviousRuns(
previous: CompileAnalysis,
current: CompileAnalysis
): List[xsbti.Problem] = {
import scala.collection.JavaConverters._
val previousSourceInfos = previous.readSourceInfos().getAllSourceInfos.asScala.toMap
val currentSourceInfos = current.readSourceInfos().getAllSourceInfos.asScala.toMap
val eligibleSourceInfos =
previousSourceInfos.filterKeys(f => !currentSourceInfos.contains(f)).values
eligibleSourceInfos.flatMap { i =>
i.getReportedProblems.filter(_.severity() == xsbti.Severity.Warn)
}.toList
}

def compile(compileInputs: CompileInputs): Task[Result] = {
val classesDir = compileInputs.classesDir.toFile
def getInputs(compilers: Compilers): Inputs = {
Expand Down Expand Up @@ -137,13 +151,22 @@ object Compiler {
val compilers = compileInputs.compilerCache.get(scalaInstance)
val inputs = getInputs(compilers)

// We don't want nanosecond granularity, we're happy with milliseconds
// We don't need nanosecond granularity, we're happy with milliseconds
def elapsed: Long = ((System.nanoTime() - start).toDouble / 1e6).toLong

import scala.util.{Success, Failure}
val logger = compileInputs.reporter.logger
val previousAnalysis = InterfaceUtil.toOption(compileInputs.previousResult.analysis())
BloopZincCompiler.compile(inputs, compileInputs.mode, logger).materialize.map {
case Success(result) =>
// Report warnings that occurred in previous compilation cycles
previousAnalysis.foreach { previous =>
warningsFromPreviousRuns(previous, result.analysis()).foreach { p =>
// Note that buffered warnings are not added back to the current analysis on purpose
compileInputs.reporter.log(p)
}
}

val res = PreviousResult.of(Optional.of(result.analysis()), Optional.of(result.setup()))
Result.Success(compileInputs.reporter, res, elapsed)
case Failure(f: StopPipelining) => Result.Blocked(f.failedProjectNames)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,16 @@ object BloopIncremental {
} yield callback.get
}

try incremental.entrypoint(initialInvClasses, initialInvSources, setOfSources, binaryChanges, lookup, previous, doCompile, classfileManager, 1)
/* Normal Zinc happens to add the source infos of the previous result to the infos
* of the new previous result. In constrast, we desire to only have the source infos
* of those files that we have indeed compiled so that we can know from the outside
* to which extent a new compilation overlaps with a previous compilation. This is
* important whenever we want to know which warnings were not reported in the new
* compilation but should be reported given that they are still present in the codebase.
*/
val previousWithNoSourceInfos =
previous.copy(infos = previous.infos -- previous.infos.allInfos.keys)
try incremental.entrypoint(initialInvClasses, initialInvSources, setOfSources, binaryChanges, lookup, previousWithNoSourceInfos, doCompile, classfileManager, 1)
catch { case e: Throwable => classfileManager.complete(false); throw e }
}

Expand Down
3 changes: 2 additions & 1 deletion frontend/src/main/scala/bloop/bsp/BloopBspServices.scala
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,9 @@ final class BloopBspServices(
}

def reportError(p: Project, problems: List[Problem], elapsedMs: Long): String = {
// Don't show warnings in this "final report", we're handling them in the reporter
val count = bloop.reporter.Problem.count(problems)
s"${p.name} [${elapsedMs}ms] (errors ${count.errors}, warnings ${count.warnings})"
s"${p.name} [${elapsedMs}ms] (errors ${count.errors})"
}

Interpreter.execute(action, Task.now(state)).map { newState =>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/test/resources/cross-test-build-0.6/build.sbt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
bloopExportJarClassifiers in Global := Some(Set("sources"))
bloopConfigDir in Global := baseDirectory.value / "bloop-config"

import sbtcrossproject.{crossProject, CrossType}
Expand Down
36 changes: 25 additions & 11 deletions frontend/src/test/scala/bloop/bsp/BspProtocolSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,25 @@ class BspProtocolSpec {
case Left(error) => Left(error)
case Right(sources) =>
val fetchedSources = sources.items.flatMap(i => i.sources.map(_.value))

val existsScalaLibraryJar =
fetchedSources.exists(_.endsWith("scala-library-2.11.12-sources.jar"))
Assert.assertTrue(
s"The Scala library could not be found in $fetchedSources",
existsScalaLibraryJar
)

val expectedSources = BuildLoader
.loadSynchronously(configDir, logger.underlying)
.flatMap(_.sources.map(s => bsp.Uri(s.underlying.toUri).value))
val msg = s"Expected != Fetched, $expectedSources != $fetchedSources"
val same = expectedSources.sorted.sameElements(fetchedSources.sorted)
Right(Assert.assertTrue(msg, same))

val missingSourcesInFetched = expectedSources.filterNot(fetchedSources.contains(_))
Assert.assertTrue(
s"Missing source $missingSourcesInFetched in $fetchedSources",
missingSourcesInFetched.isEmpty
)

Right(())
}
}
}
Expand Down Expand Up @@ -294,16 +307,15 @@ class BspProtocolSpec {
deleteNewFile()
compileMainProject.map {
case Left(e) => Left(e)
case Right(result) => Right(result)
/*
// Uncomment whenever we receive a report on the third no-op
case Right(result) => Right(())
// Uncomment whenever we implement bspv2 and can upgrade to reset diagnostics
/*
if (receivedReports == 3) Right(result)
else {
Left(
Response.internalError(s"Expected 3, got ${receivedReports} reports")
)
}
*/
}*/
}
}
}
Expand Down Expand Up @@ -341,7 +353,9 @@ class BspProtocolSpec {
)
} else if (isMainProject(report.target) && receivedReports == 2) {
// This is the last compilation which should be successful
// TODO(jvican): Test the initial warning is buffered and shown to the client
receivedReports += 1
Assert.assertEquals(s"Warnings in $MainProject != 1", 1, report.warnings)
Assert.assertEquals(s"Errors in $MainProject != 0", 0, report.errors)
()
} else {
Assert.fail(s"Unexpected compilation report: $report")
Expand All @@ -363,8 +377,8 @@ class BspProtocolSpec {
"[diagnostic] local val in method main is never used Range(Position(5,8),Position(5,8))"
val warnings = logger.underlying.getMessagesAt(Some("warn"))
Assert.assertTrue(
s"Expected $expectedWarning, obtained $warnings.",
warnings == List(expectedWarning)
s"Expected two times $expectedWarning, obtained $warnings.",
warnings == List(expectedWarning, expectedWarning)
)

// The syntax error has to be present as a diagnostic, not a normal log error
Expand Down