Skip to content

Commit

Permalink
Merge pull request #2274 from ckipp01/no-op
Browse files Browse the repository at this point in the history 
refactor: don't send task start/end for no-op compilations
  • Loading branch information
@ckipp01
ckipp01 authored Feb 19, 2024
2 parents d96ba1a + 08fbfac commit 3eeab02
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 152 deletions.
39 changes: 10 additions & 29 deletions frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,7 @@ final class BspProjectReporter(
): Unit = {
val problemsInPreviousAnalysisPerFile = Reporter.groupProblemsByFile(previousSuccessfulProblems)

def mockNoOpCompileEventsAndEnd: CompilationEvent.EndCompilation = {
// When no-op, we keep reporting the start and the end of compilation for consistency
val startMsg = s"Start no-op compilation for ${project.name}"
logger.publishCompilationStart(
CompilationEvent.StartCompilation(project.name, project.bspUri, startMsg, taskId)
)

def recheckProblems = {
recentlyReportProblemsPerFile.foreach {
case (sourceFile, problemsPerFile) if reportAllPreviousProblems =>
reportAllProblems(sourceFile, problemsPerFile)
Expand All @@ -326,30 +320,17 @@ final class BspProjectReporter(
logger.noDiagnostic(CompilationEvent.NoDiagnostic(project.bspUri, sourceFile))
}
}

val liftedProblems = allProblems.toIterator.map(super.liftFatalWarning(_)).toList
CompilationEvent.EndCompilation(
project.name,
project.bspUri,
taskId,
liftedProblems,
code,
isNoOp = true,
isLastCycle = true,
clientClassesDir,
clientAnalysisOut
)
}

endEvent = Some(
if (cycleCount.get == 0) mockNoOpCompileEventsAndEnd
else {
// Great, let's report the pending end incremental cycle as the last one
val inputs =
CycleInputs(true, problemsInPreviousAnalysisPerFile, clientClassesDir, clientAnalysisOut)
processEndPreviousCycle(inputs, Some(code))
}
)
endEvent = if (cycleCount.get == 0) {
recheckProblems
None
} else {
// Great, let's report the pending end incremental cycle as the last one
val inputs =
CycleInputs(true, problemsInPreviousAnalysisPerFile, clientClassesDir, clientAnalysisOut)
Some(processEndPreviousCycle(inputs, Some(code)))
}

// Clear the state of files with problems at the end of compilation
clearedFilesForClient.clear()
Expand Down
41 changes: 17 additions & 24 deletions frontend/src/test/scala/bloop/DeduplicationSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {

assertNoDiff(
firstCompiledState.lastDiagnostics(build.userProject),
"""#1: task start 2
"""#1: task start 1
| -> Msg: Compiling user (2 Scala sources)
| -> Data kind: compile-task
|#1: task finish 2
|#1: task finish 1
| -> errors 0, warnings 0
| -> Msg: Compiled 'user'
| -> Data kind: compile-report
Expand Down Expand Up @@ -153,14 +153,7 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {

assertNoDiff(
firstCompiledState.lastDiagnostics(build.userProject),
"""#2: task start 4
| -> Msg: Start no-op compilation for user
| -> Data kind: compile-task
|#2: task finish 4
| -> errors 0, warnings 0
| -> Msg: Compiled 'user'
| -> Data kind: compile-report
""".stripMargin
"" // expect None here since it's a no-op which turns into ""
)

// Same check as before because no-op should not show any more input
Expand Down Expand Up @@ -233,10 +226,10 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {

assertNoDiff(
firstCompiledState.lastDiagnostics(build.userProject),
"""#1: task start 2
"""#1: task start 1
| -> Msg: Compiling user (2 Scala sources)
| -> Data kind: compile-task
|#1: task finish 2
|#1: task finish 1
| -> errors 0, warnings 0
| -> Msg: Compiled 'user'
| -> Data kind: compile-report
Expand Down Expand Up @@ -326,10 +319,10 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {

assertNoDiff(
secondCompiledState.lastDiagnostics(build.userProject),
"""#2: task start 4
"""#2: task start 2
| -> Msg: Compiling user (1 Scala source)
| -> Data kind: compile-task
|#2: task finish 4
|#2: task finish 2
| -> errors 0, warnings 0
| -> Msg: Compiled 'user'
| -> Data kind: compile-report
Expand Down Expand Up @@ -412,10 +405,10 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {
// First compilation should not report warning
assertNoDiff(
firstCompiledState.lastDiagnostics(build.userProject),
"""#1: task start 2
"""#1: task start 1
| -> Msg: Compiling user (2 Scala sources)
| -> Data kind: compile-task
|#1: task finish 2
|#1: task finish 1
| -> errors 0, warnings 0
| -> Msg: Compiled 'user'
| -> Data kind: compile-report
Expand Down Expand Up @@ -563,13 +556,13 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {
// We reproduce the same streaming side effects during compilation
assertNoDiff(
firstCompiledState.lastDiagnostics(`B`),
"""#1: task start 2
"""#1: task start 1
| -> Msg: Compiling b (1 Scala source)
| -> Data kind: compile-task
|#1: b/src/B.scala
| -> List(Diagnostic(Range(Position(2,28),Position(2,28)),Some(Error),Some(_),Some(_),type mismatch; found : Int required: String,None,None,Some({"actions":[]})))
| -> reset = true
|#1: task finish 2
|#1: task finish 1
| -> errors 1, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
Expand Down Expand Up @@ -644,13 +637,13 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {

assertNoDiff(
secondBspState.lastDiagnostics(`B`),
"""#2: task start 4
"""#2: task start 2
| -> Msg: Compiling b (1 Scala source)
| -> Data kind: compile-task
|#2: b/src/B.scala
| -> List(Diagnostic(Range(Position(2,28),Position(2,28)),Some(Error),Some(_),Some(_),type mismatch; found : Int required: String,None,None,Some({"actions":[]})))
| -> reset = true
|#2: task finish 4
|#2: task finish 2
| -> errors 1, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
Expand Down Expand Up @@ -744,13 +737,13 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {

assertNoDiff(
thirdBspState.lastDiagnostics(`B`),
"""#3: task start 6
"""#3: task start 3
| -> Msg: Compiling b (1 Scala source)
| -> Data kind: compile-task
|#3: b/src/B.scala
| -> List()
| -> reset = true
|#3: task finish 6
|#3: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
Expand Down Expand Up @@ -894,10 +887,10 @@ object DeduplicationSpec extends bloop.bsp.BspBaseSuite {
assertNoDiff(
firstCompiledState.lastDiagnostics(`B`),
s"""
|#1: task start 2
|#1: task start 1
| -> Msg: Compiling b (6 Scala sources)
| -> Data kind: compile-task
|#1: task finish 2
|#1: task finish 1
| -> errors 0, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
Expand Down
79 changes: 23 additions & 56 deletions frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,7 @@ class BspCompileSpec(
assertSameExternalClassesDirs(compiledState, secondCompiledState, projects)
assertNoDiff(
secondCompiledState.lastDiagnostics(`A`),
"""#2: task start 2
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#2: task finish 2
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report""".stripMargin
"" // no-op
)
}
}
Expand Down Expand Up @@ -576,17 +570,10 @@ class BspCompileSpec(
assertSameExternalClassesDirs(thirdCompiledState, compiledState, projects)
assertNoDiff(
thirdCompiledState.lastDiagnostics(`A`),
"""#3: task start 3
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#3: a/src/A.scala
"""#3: a/src/A.scala
| -> List()
| -> reset = true
|#3: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
""".stripMargin
""".stripMargin // no-op so it only gives the compile-report
)

writeFile(`A`.srcFor("/A.scala"), Sources.`A3.scala`)
Expand All @@ -603,7 +590,7 @@ class BspCompileSpec(
assertSameExternalClassesDirs(fourthCompiledState, compiledState, projects)
assertNoDiff(
fourthCompiledState.lastDiagnostics(`A`),
"""#4: task start 4
"""#4: task start 3
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#4: a/src/A.scala
Expand All @@ -612,7 +599,7 @@ class BspCompileSpec(
|#4: a/src/A.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = false
|#4: task finish 4
|#4: task finish 3
| -> errors 1, warnings 1
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -626,13 +613,13 @@ class BspCompileSpec(
assertDifferentExternalClassesDirs(fifthCompiledState, compiledState, projects)
assertNoDiff(
fifthCompiledState.lastDiagnostics(`A`),
"""#5: task start 5
"""#5: task start 4
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#5: a/src/A.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#5: task finish 5
|#5: task finish 4
| -> errors 0, warnings 1
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -652,17 +639,17 @@ class BspCompileSpec(
assertSameExternalClassesDirs(sixthCompiledState, fifthCompiledState, projects)
assertNoDiff(
sixthCompiledState.lastDiagnostics(`A`),
"""#6: task start 6
"""#6: task start 5
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#6: a/src/A.scala
| -> List()
| -> reset = true
|#6: task finish 6
|#6: task finish 5
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|#6: task start 6
|#6: task start 5
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#6: a/src/A.scala
Expand All @@ -671,7 +658,7 @@ class BspCompileSpec(
|#6: a/src/A.scala
| -> List(Diagnostic(Range(Position(1,0),Position(3,1)),Some(Error),Some(_),Some(_),object creation impossible, since value y in trait Base of type Int is not defined,None,None,Some({"actions":[]})))
| -> reset = false
|#6: task finish 6
|#6: task finish 5
| -> errors 1, warnings 1
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -686,13 +673,13 @@ class BspCompileSpec(

assertNoDiff(
seventhCompiledState.lastDiagnostics(`A`),
"""#7: task start 7
"""#7: task start 6
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#7: a/src/A.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#7: task finish 7
|#7: task finish 6
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand Down Expand Up @@ -743,18 +730,12 @@ class BspCompileSpec(
assertSameExternalClassesDirs(compiledState.toTestState, secondCliCompiledState, `A`)

// BSP publishes warnings even if it's a no-op
// however it skips the task start/end
assertNoDiff(
compiledState.lastDiagnostics(`A`),
"""#1: task start 1
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#1: a/src/main/scala/App.scala
"""#1: a/src/main/scala/App.scala
| -> List(Diagnostic(Range(Position(2,4),Position(2,4)),Some(Warning),Some(_),Some(_),a pure expression does nothing in statement position,None,None,Some({"actions":[]})))
| -> reset = true
|#1: task finish 1
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report""".stripMargin
| -> reset = true""".stripMargin
)
}
}
Expand Down Expand Up @@ -972,16 +953,9 @@ class BspCompileSpec(

assertNoDiff(
thirdCompiledState.lastDiagnostics(`A`),
"""#3: task start 3
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#3: a/src/main/scala/Bar.scala
"""#3: a/src/main/scala/Bar.scala
| -> List()
| -> reset = true
|#3: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report""".stripMargin
| -> reset = true""".stripMargin
)
}
}
Expand Down Expand Up @@ -1083,14 +1057,7 @@ class BspCompileSpec(

assertNoDiff(
compiledState.lastDiagnostics(`B`),
"""|#2: task start 4
| -> Msg: Start no-op compilation for b
| -> Data kind: compile-task
|#2: task finish 4
| -> errors 0, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
|""".stripMargin
"" // no-op
)

writeFile(`A`.srcFor("/Foo.scala"), Sources.`Foo3.scala`)
Expand All @@ -1100,7 +1067,7 @@ class BspCompileSpec(
assertValidCompilationState(thirdCompiledState, projects)
assertNoDiff(
thirdCompiledState.lastDiagnostics(`A`),
"""|#3: task start 5
"""|#3: task start 4
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#3: a/src/Foo.scala
Expand All @@ -1109,7 +1076,7 @@ class BspCompileSpec(
|#3: a/src/Foo.scala
| -> List(Diagnostic(Range(Position(1,0),Position(1,7)),Some(Error),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = false
|#3: task finish 5
|#3: task finish 4
| -> errors 2, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -1118,13 +1085,13 @@ class BspCompileSpec(

assertNoDiff(
compiledState.lastDiagnostics(`B`),
"""|#3: task start 6
"""|#3: task start 5
| -> Msg: Compiling b (1 Scala source)
| -> Data kind: compile-task
|#3: b/src/Buzz.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,7)),Some(Error),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#3: task finish 6
|#3: task finish 5
| -> errors 1, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
Expand Down
Loading

0 comments on commit 3eeab02

Please sign in to comment.