Skip to content

Commit

Permalink
Merge pull request #2539 from tgodzik/support-werror
Browse files Browse the repository at this point in the history 
bugfix: Treat -Werror the same as -Xfatal-warnings
  • Loading branch information
@tgodzik
tgodzik authored Dec 20, 2024
2 parents a1823e5 + 6e4c93f commit e89180a
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 166 deletions.
6 changes: 4 additions & 2 deletions backend/src/main/scala/bloop/Compiler.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ object Compiler {
}
}

private def isFatalWarningOpt(opt: String) = opt == "-Xfatal-warnings" || opt == "-Werror"

def compile(
compileInputs: CompileInputs,
isBestEffortMode: Boolean,
Expand Down Expand Up @@ -295,7 +297,7 @@ object Compiler {
}

val isFatalWarningsEnabled: Boolean =
compileInputs.scalacOptions.exists(_ == "-Xfatal-warnings")
compileInputs.scalacOptions.exists(isFatalWarningOpt)
def getInputs(compilers: Compilers): Inputs = {
val options = getCompilationOptions(compileInputs, logger, newClassesDir)
val setup = getSetup(compileInputs)
Expand Down Expand Up @@ -925,7 +927,7 @@ object Compiler {
inputs.scalaInstance.version
)

val optionsWithoutFatalWarnings = scalacOptions.filter(_ != "-Xfatal-warnings")
val optionsWithoutFatalWarnings = scalacOptions.filterNot(isFatalWarningOpt)
val areFatalWarningsEnabled = scalacOptions.length != optionsWithoutFatalWarnings.length

// Enable fatal warnings in the reporter if they are enabled in the build
Expand Down
81 changes: 41 additions & 40 deletions frontend/src/test/scala/bloop/BaseCompileSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1231,52 +1231,53 @@ abstract class BaseCompileSpec extends bloop.testing.BaseSuite {
}
}

test("support -Xfatal-warnings internal implementation") {
TestUtil.withinWorkspace { workspace =>
object Sources {
val `Foo.scala` =
"""/Foo.scala
|import Predef.assert
|class Foo
for (fatalOpt <- List("-Xfatal-warnings", "-Werror"))
test(s"support $fatalOpt internal implementation") {
TestUtil.withinWorkspace { workspace =>
object Sources {
val `Foo.scala` =
"""/Foo.scala
|import Predef.assert
|class Foo
""".stripMargin
val `Bar.scala` =
"""/Bar.scala
|class Bar
val `Bar.scala` =
"""/Bar.scala
|class Bar
""".stripMargin
val `Baz.scala` =
"""/Baz.scala
|class Baz
val `Baz.scala` =
"""/Baz.scala
|class Baz
""".stripMargin
}
}

val logger = new RecordingLogger(ansiCodesSupported = false)
val sources = List(Sources.`Bar.scala`, Sources.`Foo.scala`, Sources.`Baz.scala`)
val options = List("-Ywarn-unused", "-Xfatal-warnings")
val `A` = TestProject(workspace, "a", sources, scalacOptions = options)
val projects = List(`A`)
val state = loadState(workspace, projects, logger)
val compiledState = state.compile(`A`)
assertExitStatus(compiledState, ExitStatus.CompilationError)
// Despite error, compilation of project should be valid
assertValidCompilationState(compiledState, projects)
val logger = new RecordingLogger(ansiCodesSupported = false)
val sources = List(Sources.`Bar.scala`, Sources.`Foo.scala`, Sources.`Baz.scala`)
val options = List("-Ywarn-unused", fatalOpt)
val `A` = TestProject(workspace, "a", sources, scalacOptions = options)
val projects = List(`A`)
val state = loadState(workspace, projects, logger)
val compiledState = state.compile(`A`)
assertExitStatus(compiledState, ExitStatus.CompilationError)
// Despite error, compilation of project should be valid
assertValidCompilationState(compiledState, projects)

val targetFoo = TestUtil.universalPath("a/src/Foo.scala")
assertNoDiff(
logger.renderErrors(exceptContaining = "Failed to compile"),
s"""|[E1] ${targetFoo}:1:15
| Unused import
| L1: import Predef.assert
| ^^^^^^^
|""".stripMargin
)
assertDiagnosticsResult(
compiledState.getLastResultFor(`A`),
errors = 0,
warnings = 1,
expectFatalWarnings = true
)
val targetFoo = TestUtil.universalPath("a/src/Foo.scala")
assertNoDiff(
logger.renderErrors(exceptContaining = "Failed to compile"),
s"""|[E1] ${targetFoo}:1:15
| Unused import
| L1: import Predef.assert
| ^^^^^^^
|""".stripMargin
)
assertDiagnosticsResult(
compiledState.getLastResultFor(`A`),
errors = 0,
warnings = 1,
expectFatalWarnings = true
)
}
}
}

test("scalac -Xfatal-warnings should not set java fatal warnings as errors") {
TestUtil.withinWorkspace { workspace =>
Expand Down
249 changes: 125 additions & 124 deletions frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1109,144 +1109,145 @@ class BspCompileSpec(
}
}

test("support -Xfatal-warnings internal implementation") {
TestUtil.withinWorkspace { workspace =>
object Sources {
val `Foo.scala` =
"""/Foo.scala
|import Predef.assert
|class Foo
for (fatalOpt <- List("-Xfatal-warnings", "-Werror"))
test(s"support $fatalOpt internal implementation") {
TestUtil.withinWorkspace { workspace =>
object Sources {
val `Foo.scala` =
"""/Foo.scala
|import Predef.assert
|class Foo
""".stripMargin
val `Bar.scala` =
"""/Bar.scala
|class Bar
val `Bar.scala` =
"""/Bar.scala
|class Bar
""".stripMargin
val `Baz.scala` =
"""/Baz.scala
|class Baz
val `Baz.scala` =
"""/Baz.scala
|class Baz
""".stripMargin
val `Foo2.scala` =
"""/Foo.scala
|class Foo
val `Foo2.scala` =
"""/Foo.scala
|class Foo
""".stripMargin
val `Foo3.scala` =
"""/Foo.scala
|import Predef.assert
|import Predef.Manifest
|class Foo
val `Foo3.scala` =
"""/Foo.scala
|import Predef.assert
|import Predef.Manifest
|class Foo
""".stripMargin
val `Buzz.scala` =
"""/Buzz.scala
|class Buzz
val `Buzz.scala` =
"""/Buzz.scala
|class Buzz
""".stripMargin
val `Buzz2.scala` =
"""/Buzz.scala
|import Predef.assert
|class Buzz
val `Buzz2.scala` =
"""/Buzz.scala
|import Predef.assert
|class Buzz
""".stripMargin
}
}

val bspLogger = new RecordingLogger(ansiCodesSupported = false)
val sourcesA = List(Sources.`Bar.scala`, Sources.`Foo.scala`, Sources.`Baz.scala`)
val sourcesB = List(Sources.`Buzz.scala`)
val options = List("-Ywarn-unused", "-Xfatal-warnings")
val `A` = TestProject(workspace, "a", sourcesA, scalacOptions = options)
val `B` = TestProject(workspace, "b", sourcesB, List(`A`), scalacOptions = options)
val projects = List(`A`, `B`)
loadBspState(workspace, projects, bspLogger) { state =>
val compiledState = state.compile(`B`)
assertExitStatus(compiledState, ExitStatus.CompilationError)
assertValidCompilationState(compiledState, projects)
assertNoDiff(
compiledState.lastDiagnostics(`A`),
"""|#1: task start 1
| -> Msg: Compiling a (3 Scala sources)
| -> Data kind: compile-task
|#1: a/src/Foo.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,7)),Some(Error),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#1: task finish 1
| -> errors 1, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)
val bspLogger = new RecordingLogger(ansiCodesSupported = false)
val sourcesA = List(Sources.`Bar.scala`, Sources.`Foo.scala`, Sources.`Baz.scala`)
val sourcesB = List(Sources.`Buzz.scala`)
val options = List("-Ywarn-unused", fatalOpt)
val `A` = TestProject(workspace, "a", sourcesA, scalacOptions = options)
val `B` = TestProject(workspace, "b", sourcesB, List(`A`), scalacOptions = options)
val projects = List(`A`, `B`)
loadBspState(workspace, projects, bspLogger) { state =>
val compiledState = state.compile(`B`)
assertExitStatus(compiledState, ExitStatus.CompilationError)
assertValidCompilationState(compiledState, projects)
assertNoDiff(
compiledState.lastDiagnostics(`A`),
"""|#1: task start 1
| -> Msg: Compiling a (3 Scala sources)
| -> Data kind: compile-task
|#1: a/src/Foo.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,7)),Some(Error),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#1: task finish 1
| -> errors 1, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)

assertNoDiff(
compiledState.lastDiagnostics(`B`),
"""|#1: task start 2
| -> Msg: Compiling b (1 Scala source)
| -> Data kind: compile-task
|#1: task finish 2
| -> errors 0, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
|""".stripMargin
)
assertNoDiff(
compiledState.lastDiagnostics(`B`),
"""|#1: task start 2
| -> Msg: Compiling b (1 Scala source)
| -> Data kind: compile-task
|#1: task finish 2
| -> errors 0, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
|""".stripMargin
)

writeFile(`A`.srcFor("/Foo.scala"), Sources.`Foo2.scala`)
val secondCompiledState = compiledState.compile(`B`)
assertExitStatus(secondCompiledState, ExitStatus.Ok)
assertValidCompilationState(secondCompiledState, projects)
assertNoDiff(
secondCompiledState.lastDiagnostics(`A`),
"""|#2: task start 3
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#2: a/src/Foo.scala
| -> List()
| -> reset = true
|#2: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)
writeFile(`A`.srcFor("/Foo.scala"), Sources.`Foo2.scala`)
val secondCompiledState = compiledState.compile(`B`)
assertExitStatus(secondCompiledState, ExitStatus.Ok)
assertValidCompilationState(secondCompiledState, projects)
assertNoDiff(
secondCompiledState.lastDiagnostics(`A`),
"""|#2: task start 3
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#2: a/src/Foo.scala
| -> List()
| -> reset = true
|#2: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)

assertNoDiff(
compiledState.lastDiagnostics(`B`),
"" // no-op
)
assertNoDiff(
compiledState.lastDiagnostics(`B`),
"" // no-op
)

writeFile(`A`.srcFor("/Foo.scala"), Sources.`Foo3.scala`)
writeFile(`B`.srcFor("/Buzz.scala"), Sources.`Buzz2.scala`)
val thirdCompiledState = secondCompiledState.compile(`B`)
assertExitStatus(thirdCompiledState, ExitStatus.CompilationError)
assertValidCompilationState(thirdCompiledState, projects)
assertNoDiff(
thirdCompiledState.lastDiagnostics(`A`),
"""|#3: task start 4
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#3: a/src/Foo.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,7)),Some(Error),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#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 4
| -> errors 2, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)
writeFile(`A`.srcFor("/Foo.scala"), Sources.`Foo3.scala`)
writeFile(`B`.srcFor("/Buzz.scala"), Sources.`Buzz2.scala`)
val thirdCompiledState = secondCompiledState.compile(`B`)
assertExitStatus(thirdCompiledState, ExitStatus.CompilationError)
assertValidCompilationState(thirdCompiledState, projects)
assertNoDiff(
thirdCompiledState.lastDiagnostics(`A`),
"""|#3: task start 4
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#3: a/src/Foo.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,7)),Some(Error),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#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 4
| -> errors 2, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)

assertNoDiff(
compiledState.lastDiagnostics(`B`),
"""|#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 5
| -> errors 1, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
|""".stripMargin
)
assertNoDiff(
compiledState.lastDiagnostics(`B`),
"""|#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 5
| -> errors 1, warnings 0
| -> Msg: Compiled 'b'
| -> Data kind: compile-report
|""".stripMargin
)
}
}
}
}

}

0 comments on commit e89180a

Please sign in to comment.